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

Update removedAt for already deleted tree and text nodes when performing range deletion operation #821

Open
raararaara opened this issue May 24, 2024 · 2 comments · May be fixed by #909
Open
Assignees

Comments

@raararaara
Copy link
Contributor

Description:
During the process of performing a range deletion operation, if there are nodes within the range that have already been removed, the removedAt field should be updated accordingly.

Locally, there is no need to update removedAt for nodes that are already marked as removed. However, when considering remote scenarios, it is necessary to update removedAt. Currently, the logic does not update removedAt for either treeNode or textNode.

In more detail, when collecting nodes within a range, the logic for treeNode filters out nodes that are already marked as removed(see tokensBetween() and children()), while textNode performs a similar role through the canDelete().

What you expected to happen:
The removedAt field should be updated for nodes that have already been deleted when performing a range deletion operation.

How to reproduce it (as minimally and precisely as possible):
This issue works normally in the current spec, but will recur when undo/redo is added in the future.

Anything else we need to know?:
N/A

Environment:

  • Operating system: N/A
  • Browser and version: N/A
  • Yorkie version (use yorkie version): N/A
  • Yorkie JS SDK version: v0.4.20
@minai621
Copy link

I'm interested in this issue, Can I try it?

@blurfx
Copy link
Member

blurfx commented Jul 18, 2024

Sure :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

3 participants