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

Implement undo/redo example with tldraw #695

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

devleejb
Copy link
Member

@devleejb devleejb commented Nov 26, 2023

What this PR does / why we need it?

This pull request adds undo/redo functionality to the Tldraw whiteboard example code.

Any background context you want to provide?

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

@devleejb devleejb changed the title feat: Implement undo/redo example with tdlraw Implement undo/redo example with tdlraw Nov 26, 2023
Copy link

codecov bot commented Nov 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4bbdf2e) 69.04% compared to head (f11a332) 69.04%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #695   +/-   ##
=======================================
  Coverage   69.04%   69.04%           
=======================================
  Files          58       58           
  Lines        8787     8787           
  Branches      800      800           
=======================================
  Hits         6067     6067           
  Misses       2459     2459           
  Partials      261      261           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krapie krapie self-requested a review November 26, 2023 08:08
@krapie krapie added the enhancement 🌟 New feature or request label Nov 26, 2023
Copy link
Member

@krapie krapie left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

When I run tldraw example with this changes, I get this error.
Could you test your PR again?

Screenshot 2023-11-26 at 5 25 17 PM

};
}
}
Copy link
Member

Choose a reason for hiding this comment

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

As always, file newline here.

@krapie krapie changed the title Implement undo/redo example with tdlraw Implement undo/redo example with tldraw Nov 30, 2023
@devleejb
Copy link
Member Author

@krapie
Is there any update or fix planned after the bug discovered?
#697

@krapie
Copy link
Member

krapie commented Dec 24, 2023

@devleejb
Not sure there are any updates on this bug.
cc. @hackerwins

@devleejb
Copy link
Member Author

@krapie
I will apply the comment you provided after the issue is resolved.

@hackerwins
Copy link
Member

hackerwins commented Dec 24, 2023

The current Undo/Redo implementation in the JS SDK is designed for objects, but it malfunctions when the garbage collection(GC) is triggered.

Therefore, in the current Undo/Redo test code, the GC functionality is disabled to run the tests:

const doc1 = new Document<TestDoc>(docKey, { disableGC: true });
const doc2 = new Document<TestDoc>(docKey, { disableGC: true });

In the future, there are plans to handle the removal of objects targeted by the GC when they are no longer referenced by the undo/redo stack. For more details, please refer to the following issue:
yorkie-team/yorkie#664

I think it will take some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants