Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lexical-table] Support table alignment #7044

Merged
merged 9 commits into from
Jan 15, 2025
Merged

Conversation

ivailop7
Copy link
Collaborator

Fixes #6909

To differentiate between table alignment and cell text alignment, the table-level alignment logic only kicks in when the selection is the whole exact table. This does introduce the extra step if one wants to align all cell texts to do it in 2 steps, all columns except one and then the last column.

Before:

Screen.Recording.2025-01-12.at.19.57.08.mov

After:

Screen.Recording.2025-01-12.at.19.57.43.mov

Copy link

vercel bot commented Jan 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 10:04pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 10:04pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 12, 2025
Copy link

github-actions bot commented Jan 12, 2025

size-limit report 📦

Path Size
lexical - cjs 29 KB (0%)
lexical - esm 28.84 KB (0%)
@lexical/rich-text - cjs 37.96 KB (0%)
@lexical/rich-text - esm 30.97 KB (0%)
@lexical/plain-text - cjs 36.51 KB (0%)
@lexical/plain-text - esm 28.13 KB (0%)
@lexical/react - cjs 39.84 KB (0%)
@lexical/react - esm 32.2 KB (0%)

@ivailop7
Copy link
Collaborator Author

Rewrote this with classNames vs using 'style', to leave the option for overridability to users, but unsure if ended up any better because I couldn't get the class waterfall to work nicely without !imporant to override the margins. In both cases (style vs classNames) it works correctly with both with and without tableWrapper

@ivailop7
Copy link
Collaborator Author

Thanks @etrepum, implemented the PR comments

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't looked closely at the changes but there are test failures now

@ivailop7
Copy link
Collaborator Author

Haven't looked closely at the changes but there are test failures now

Addressed

@ivailop7 ivailop7 enabled auto-merge January 15, 2025 22:43
@ivailop7
Copy link
Collaborator Author

@etrepum could we wrap this one up please.

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and confirmed that it works as expected in the playground

@ivailop7 ivailop7 added this pull request to the merge queue Jan 15, 2025
Merged via the queue into facebook:main with commit a3ef4f3 Jan 15, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Cannot align tables
3 participants