-
Notifications
You must be signed in to change notification settings - Fork 289
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
Add undo and redo history #617
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
very exciting, thank you for sharing 🙂 there's a lot of your work here that crosses over how my mind had architected it. I hadnt considered deduping tho or debouncing, nice ideas! also, the tests are great, thanks could the pattern handle adding and removing elements? i mean, i bet it could be taught to, but in the current state it looks to deal a lot with nodes and not a node's participation in a tree. was just curious. how often is that unique selector used instead of the node as the key? i imagine margin/padding/font/position adjustments are pretty easy to do next? small favor, consider temporarily disabling your formatter before a PR, so it's easier for me to see which lines you actually changed 😅 |
Yes, this can be extended to inserting, removing and reordering components by storing the parent, index in the parent and the selector itself. This would also work with text content as well. Using something like the Component below. I'm actively working on coming up with a good schema for this atm, I would appreciate your thoughts on this.
I like the selector better since it can be stored across sessions or exported as a string (to reproduce on another browser for example). But a weakmap to the element on the same session works too.
Positions are trickier, more on the insert-remove side but the rest are pretty straight forward.
Yeah sorry I noticed that happening after making the PR but had already messed up everything 😞. Will do next time. Let me know if you think this is worth pursuing and I can find some time to clean up and add more :) |
def worth pursuing 🙂 this would be a very valuable feature, worthy of the effort. happy to help you think things out and be an early adopter while it evolves. |
Sounds good, I'll get to cleaning it up |
Summary
This is a POC for undo redo. Only working for colors but is simple to add for any edit types. Adapting changes from my own internal repo so please excuse some poor formatting.
Testing
I added some unit test but you can also test this by applying a background change then
CMD+Z
to undo andCMD+SHIFT+Z
to redo.Description
The idea is to store reversible events for any edits, then have an
applyEvent
call that can call the reverse event (akin to the command pattern).Possible improvements
One possible improvement to this is to save the old style as a parameter in the element. This way, the old style is always ensured to be the original style. This requires some refactoring of the way styles are applied in Visbug but is how we do it in my project.