Open Kitenite opened 3 months ago
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 😅
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.
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.
export type EditEvent = {
createdAt: string;
selector: string;
editType: EditType;
newVal: Record<string, string> | Component | TextVal;
oldVal: Record<string, string> | Component | TextVal;
}
export type TextVal = {
text: string;
}
export enum EditType {
TEXT = "TEXT",
STYLE = "STYLE",
ATTR = "ATTR",
INSERT = "INSERT",
REMOVE = "REMOVE",
}
export interface Component {
selector: string;
parentSelector: string; // Parent selector
index: number; // Index within parent
content: string; // String content of the element (For reversibility)
}
how often is that unique selector used instead of the node as the key?
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.
i imagine margin/padding/font/position adjustments are pretty easy to do next?
Positions are trickier, more on the insert-remove side but the rest are pretty straight forward.
small favor, consider temporarily disabling your formatter before a PR, so it's easier for me to see which lines you actually changed 😅
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.