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-playground] Fix table hover actions button position #7011

Merged
merged 1 commit into from
Jan 4, 2025

Conversation

ivailop7
Copy link
Collaborator

@ivailop7 ivailop7 commented Jan 1, 2025

After introducing the getDOMSlot usage, the render coordinates for the table hover actions had to be adjusted.

Before:

Screen.Recording.2025-01-01.at.14.17.56.mov

After:

Screen.Recording.2025-01-01.at.14.17.30.mov

Copy link

vercel bot commented Jan 1, 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 1, 2025 2:21pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 1, 2025 2:21pm

@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 1, 2025
@ivailop7 ivailop7 added the extended-tests Run extended e2e tests on a PR label Jan 1, 2025
Copy link

github-actions bot commented Jan 1, 2025

size-limit report 📦

Path Size
lexical - cjs 31.21 KB (0%)
lexical - esm 31.1 KB (0%)
@lexical/rich-text - cjs 40.31 KB (0%)
@lexical/rich-text - esm 33.01 KB (0%)
@lexical/plain-text - cjs 38.85 KB (0%)
@lexical/plain-text - esm 30.22 KB (0%)
@lexical/react - cjs 42.13 KB (0%)
@lexical/react - esm 34.26 KB (0%)

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.

Not a complete fix because there's no scroll event handling (looking forward to #6923 so all of these things can be fixed). A visible button will remain in a fixed position while the table scrolls behind it. Also a bit of a hack because it doesn't use the theme, but it is playground-only code so that's a bit par for the course.

@@ -115,6 +115,19 @@ function TableHoverActionsContainer({
height: tableElemHeight,
} = (tableDOMElement as HTMLTableElement).getBoundingClientRect();

// Adjust for using the scrollable table container
const parentElement = (tableDOMElement as HTMLTableElement)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A more robust check here would be to compare tableDOMElement to parentElement = editor.getElementByKey(table.getKey()) - if they are not identical, then the latter is the outermost wrapping node. This is already computed above just not saved to its own variable.

@etrepum etrepum added this pull request to the merge queue Jan 4, 2025
Merged via the queue into facebook:main with commit 8100d6d Jan 4, 2025
44 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.

3 participants