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

fix: fixing custom app rich text dnd [TOL-1742] #1599

Merged
merged 4 commits into from
Feb 2, 2024
Merged

Conversation

aodhagan-cf
Copy link
Contributor

@aodhagan-cf aodhagan-cf commented Feb 1, 2024

image

Error being throw when dragging and dropping with custom rich text editor.

  • Related to this history change & the new dndkit library

Todo

  • Understand why change was introduced in the first place
  • Remove or refactor as required

EDIT

Because we push a new undo as an empty array this condition doesn't short-circuit and the operations doesn't exist on the empty array. I'm not sure why this is only a problem for the App SDK and hasn't been happening the regular web app Rich Text 🤔

const lastOp =
lastBatch && lastBatch.operations[lastBatch.operations.length - 1]

https://github.com/ianstormtaylor/slate/blob/cd93871ae69721f7361e17e8b28feab004a9c855/packages/slate-history/src/with-history.ts#L72C5-L74C73

@@ -48,7 +49,10 @@ export function createDragAndDropPlugin(): PlatePlugin {
if (!dropDisallowed) {
// Move the drop event to a new undo batch mitigating the bug where undo not only moves it back,
// but also undoes a previous action: https://github.com/ianstormtaylor/slate/issues/4694
editor.history.undos.push([]);
(editor as unknown as HistoryEditor).history.undos.push({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

slate-history behind the scenes got refactored to use batch objects with operations instead of an array of arrays

@aodhagan-cf aodhagan-cf marked this pull request as ready for review February 2, 2024 10:21
@aodhagan-cf aodhagan-cf requested a review from a team as a code owner February 2, 2024 10:21
@@ -67,7 +67,7 @@
"is-plain-obj": "^3.0.0",
"react-popper": "^2.3.0",
"slate": "0.94.1",
"slate-history": "0.66.0",
"slate-history": "0.100.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

that's a tiny bump 🙄

Copy link
Contributor

@YvesRijckaert YvesRijckaert left a comment

Choose a reason for hiding this comment

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

LGTM

@aodhagan-cf aodhagan-cf merged commit 43c17ca into master Feb 2, 2024
14 checks passed
@aodhagan-cf aodhagan-cf deleted the fix/TOL-1742 branch February 2, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants