[lexical-table] Feature: Improve logic for pasting table into table#7408
[lexical-table] Feature: Improve logic for pasting table into table#7408
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fe8b281 to
ed4154e
Compare
| * taking into account any spans. If successful, returns the | ||
| * inserted table row node. | ||
| */ | ||
| export function $insertTableRowAtNode__EXPERIMENTAL( |
There was a problem hiding this comment.
(The "experimental" tag for insert row/column follows the existing functions that they are being extracted from. Open qns: Those function were added 2 years ago, so I'm wondering if we could promote them to non-experimental and get rid of the legacy versions.)
There was a problem hiding this comment.
I'd recommend exporting them with the non-experimental names and maintaining the experimental names for some period of time with exports with @deprecated annotations
There was a problem hiding this comment.
In the case for new functions I don't think they should have these experimental names, if a function is experimental it would be better to just document it appropriately (e.g. with @internal if we don't want it to show up in docs). I think this naming convention was a failed experiment and not a convention worth following for new code.
There was a problem hiding this comment.
I've renamed the new functions. For the existing "experimental" insert row/column functions, there are legacy exported functions using the same name (without the __EXPERIMENTAL tag) but with different APIs 😅
There was a problem hiding this comment.
hah! well I guess we need to come up with new names then, would be hard to do a proper deprecation if the call signature changes. Doesn't have to be in the scope of this PR
|
While you're in this part of the code there's another issue with one of these table functions that is probably a simple fix: #7140 |
|
Regarding the paste with cmd+V I think the problem is that the browser needs to have an active selection in order to handle paste. I think the real solution to this problem will be to create a selection in the browser that is promptly removed or otherwise specifically ignored by all of the code that tries to reconcile a change in the browser's selection to a new lexical RangeSelection (e.g. which happens while starting every update unless certain conditions are met, plus the selectionchange listener). This is probably also relevant to NodeSelection or any other custom selection. What may be needed is another hack like BlockCursorElement but for holding the browser's selection while a non-RangeSelection is in use. |
etrepum
left a comment
There was a problem hiding this comment.
I didn't do a careful audit of the code or tests but I did some manual testing and it seems to work much better now. Will approve once the functions are renamed without the EXPERIMENTAL postfix.
ed4154e to
10cfb73
Compare
Fixes #13386 Below I write a clarification to copy and paste into the release note, based on our latest upgrade of Lexical [in v3.29.0](https://github.com/payloadcms/payload/releases/tag/v3.29.0). ## Important This release upgrades the lexical dependency from 0.28.0 to 0.34.0. If you installed lexical manually, update it to 0.34.0. Installing lexical manually is not recommended, as it may break between updates, and our re-exported versions should be used. See the [yellow banner box](https://payloadcms.com/docs/rich-text/custom-features) for details. If you still encounter richtext-lexical errors, do the following, in this order: - Delete node_modules - Delete your lockfile (e.g. pnpm-lock.json) - Reinstall your dependencies (e.g. pnpm install) ### Lexical Breaking Changes The following Lexical releases describe breaking changes. We recommend reading them if you're using Lexical APIs directly (`@payloadcms/richtext-lexical/lexical/*`). - [v.0.33.0](https://github.com/facebook/lexical/releases/tag/v0.33.0) - [v.0.30.0](https://github.com/facebook/lexical/releases/tag/v0.30.0) - [v.0.29.0](https://github.com/facebook/lexical/releases/tag/v0.29.0) ___ TODO: - [x] facebook/lexical#7719 - [x] facebook/lexical#7362 - [x] facebook/lexical#7707 - [x] facebook/lexical#7388 - [x] facebook/lexical#7357 - [x] facebook/lexical#7352 - [x] facebook/lexical#7472 - [x] facebook/lexical#7556 - [x] facebook/lexical#7417 - [x] facebook/lexical#1036 - [x] facebook/lexical#7509 - [x] facebook/lexical#7693 - [x] facebook/lexical#7408 - [x] facebook/lexical#7450 - [x] facebook/lexical#7415 - [x] facebook/lexical#7368 - [x] facebook/lexical#7372 - [x] facebook/lexical#7572 - [x] facebook/lexical#7558 - [x] facebook/lexical#7613 - [x] facebook/lexical#7405 - [x] facebook/lexical#7420 - [x] facebook/lexical#7662 --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211202581885926
Description
Improve how we handle pasting table into table:
Closes #7262
Closes #7263
Test plan
Added E2E tests for table pasting:
npm run test-e2e-chromium Tables.specBefore
table-pasting__before.mov
After
table-pasting__after.mov
Additional notes
The new logic for pasting tables can be summarized as:
To support the new logic, I've added a few additional table utils (in
lexical-tablepackage) that works on TableCellNode(s)—insert row, insert column, merge cells, unmerge cell. The logic for merging cells is taken from theTableActionMenuPlugininlexical-playgroundpackage.Some potential followups: