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

SF-1665 Clear edit history before chapter embeds are duplicated #1461

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RaymondLuong3
Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 commented Aug 5, 2022

Texts were being corrupted when a user typed in the first section heading in a blank chapter with no introductory texts or headings. This is a side effect of a problem when Quill diffs the delta before and after a change is applied. The diff algorithm that Quill users to compare deltas cannot accurately determine if embeds have changed. Most of the time this is not an issue, and quill simply assumes that embeds have changed, reintroduces them, and deletes the originals.

This change clears the history stack if it finds a chapter embed is being inserted as a result of an undo operation. The user will not be able to undo their edits to the section heading.


This change is Reviewable

@RaymondLuong3 RaymondLuong3 added the will require testing PR should not be merged until testers confirm testing is complete label Aug 5, 2022
@RaymondLuong3 RaymondLuong3 requested a review from Nateowami August 5, 2022 21:26
Copy link
Collaborator Author

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

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

Here are the steps that lead to the bad behaviour:

  1. User enters a word in the first segment after the chapter.
  2. User does ctrl + z to undo the text changes.
  3. The undo op looks like { insert: chapter, insert: blank, delete: 2 }
  4. The text is not fully deleted (the blot that was inserted with the blank was mistakenly considered to have length 2) so only the duplicate chapter is deleted, when it should also have deleted the text.
  5. The text-view-model makes adjustments, deleting the blank since text still exists.
  6. A delta gets added to the undo stack that looks like { insert: chapter, insert: blank }
  7. The user can undo, and corrupt their text.

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @Nateowami)

@Nateowami Nateowami marked this pull request as draft June 22, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will require testing PR should not be merged until testers confirm testing is complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants