[lexical-playground][lexical-table] Bugfix: Fix the overlapping issue of table cell height resizer indicator#8571
Conversation
…resizer indicator when table has many columns
|
Hi @dondomiedungca! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
…icator-overlapping-issue
Review: Table Cell Height Resizer Overlap FixReviewed by: Navi (AI review assistant for @potatowagon) SummaryThis PR fixes the overlapping issue of the table cell height resizer indicator by computing the width relative to the editor's content-editable boundary rather than using the table width directly. What I Verified
ConcernThe selector Verdict✅ Safe to approve — small, focused fix with clear intent. The playground-specific selector is the only minor concern but is appropriate for the file's context. |
…icator-overlapping-issue
| const editorWidth = | ||
| contentEditableElement?.getBoundingClientRect().right ?? 0; | ||
|
|
||
| styles[draggingDirection].width = `${editorWidth - tableRect.left}px`; |
There was a problem hiding this comment.
| styles[draggingDirection].width = `${editorWidth - tableRect.left}px`; | |
| styles[draggingDirection].width = `${Math.min(tableRect.width, editorWidth - tableRect.left)}px`; |
I don't think the intention here is to always size it to the end of the editor.
| const contentEditableElement = document.querySelector( | ||
| '.ContentEditable__root', | ||
| ); |
There was a problem hiding this comment.
| const contentEditableElement = document.querySelector( | |
| '.ContentEditable__root', | |
| ); | |
| const contentEditableElement = editor.getRootElement(); |
potatowagon
left a comment
There was a problem hiding this comment.
Review — Table Cell Height Resizer Indicator Fix
Assessment: Has minor concern
What I verified:
-
Bug fix logic: The fix changes the horizontal resize indicator width calculation from using
tableRect.width(which only covers the table element) to spanning from the table's left edge to the editor's right edge. This prevents the indicator from overlapping/underlapping when the table doesn't fill the full editor width. -
CI status: All checks pass (limited CI run — CLA and Vercel only, no full test suite triggered).
Concern:
The fix uses document.querySelector('.ContentEditable__root') which is a hardcoded class name selector. This:
- Couples the plugin to a specific CSS class name that's only guaranteed in the playground
- Won't work for consumers using custom class names or different root element selectors
- A more robust approach would be to use the editor's root element via
editor.getRootElement()
This is a playground-only plugin change so it's lower risk, but the selector coupling is fragile. Consider using editor.getRootElement()?.getBoundingClientRect().right instead.
— via Navi on behalf of potatowagon
Description
When the editor has table with many columns and the user will resize the table cell height, the indicator is overlapping from the editor container.
Before
After