[lexical-playground] Table Hover Action Buttons #6355
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
size-limit report 📦
|
ivailop7
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
Sahejkm and
potatowagon
as code owners
packages/lexical-playground/src/plugins/TableHoverActionsPlugin/index.tsx
Outdated
Show resolved
Hide resolved
packages/lexical-playground/src/plugins/TableHoverActionsPlugin/index.tsx
Outdated
Show resolved
Hide resolved
| return () => { | ||
| setShownRow(false); | ||
| setShownColumn(false); | ||
| debouncedOnMouseMove.cancel(); |
There was a problem hiding this comment.
[Opinionated] debounce is fine, I would've chosen throttle
There was a problem hiding this comment.
Picked it because it's the same way it's done in the CodeActionsPlugin. Where can I see a usage of throttle, which one would offer a more performant approach?
| if (target && target instanceof HTMLElement) { | ||
| const tableDOMNode = target.closest<HTMLElement>( | ||
| 'td.PlaygroundEditorTheme__tableCell, th.PlaygroundEditorTheme__tableCell', | ||
| ); | ||
|
|
||
| const isOutside = !( | ||
| tableDOMNode || | ||
| target.closest<HTMLElement>( | ||
| 'button.PlaygroundEditorTheme__tableAddRows', | ||
| ) || | ||
| target.closest<HTMLElement>( | ||
| 'button.PlaygroundEditorTheme__tableAddColumns', | ||
| ) || | ||
| target.closest<HTMLElement>('div.TableCellResizer__resizer') | ||
| ); | ||
|
|
||
| return {isOutside, tableDOMNode}; |
There was a problem hiding this comment.
I would've considered using getNearestNodeFromDOMNode, nothing beats the performance of your implementation but this method will already do the editor checks for you and will work regardless of the markup.
There was a problem hiding this comment.
The limitation with using getNearest here is that the div for the table border and resizer 'div.TableCellResizer__resizer' is not a node, (this was causing for the buttons to disappear when you hover on the borders between the table cells) also because getNearestNodeFromDOMNode needs to run inside editor.update, yet it does only read, I don't want to run an extra update reconciliation check, forced to collaborators when you just move your mouse around the table.
packages/lexical-playground/src/plugins/TableHoverActionsPlugin/index.tsx
Outdated
Show resolved
Hide resolved
| if (insertRow) { | ||
| $insertTableRow__EXPERIMENTAL(); | ||
| setShownRow(false); | ||
| } else { | ||
| $insertTableColumn__EXPERIMENTAL(); | ||
| setShownColumn(false); | ||
| } |
There was a problem hiding this comment.
No strong opinions. Your version is easy to read
| if (insertRow) { | |
| $insertTableRow__EXPERIMENTAL(); | |
| setShownRow(false); | |
| } else { | |
| $insertTableColumn__EXPERIMENTAL(); | |
| setShownColumn(false); | |
| } | |
| $insertTableColumn__EXPERIMENTAL(); | |
| setShownColumn(insertRow); |
There was a problem hiding this comment.
This doesn't work since it's different methods it's setShownRow and setShownColumn, not setShownColumn at all times.
yeah, hovering over the bottom right cell, should show both, but right now only does add row, as a higher priority, running on the assumption that 'add row' would be much more popular than 'add column' in a everyday use. It's good enough IMO in the current state, but open for people polishing the nits. |
|
@zurfyx for a second look post addressing comments |
Co-authored-by: Ivaylo Pavlov <ipavlov@Ivaylos-MacBook-Pro.local>
Recreate the Experimental Table Hover Action Buttons for the Classic Table as a standalone playground plugin.
Clean up some of the remnants from the original implementation that was deleted a year ago.
Screen.Recording.2024-06-30.at.23.35.38.mov