ProjectVisBug icon indicating copy to clipboard operation
ProjectVisBug copied to clipboard

Add undo and redo history

Open Kitenite opened this issue 1 year ago • 5 comments

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 and CMD+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.

let oldStyles = element.dataset.oldStyles
        ? JSON.parse(element.dataset.oldStyles)
        : {};

      // Save the current style value to the map before updating, only if it doesn't exist
      if (oldStyles[key] === undefined) {
        const oldStyle = element.style[key];
        if (oldStyle !== undefined) {
          // Save only if there's a current value
          oldStyles[key] = oldStyle;
          element.dataset.oldStyles = JSON.stringify(oldStyles); // Serialize and save back to dataset
        }
      }

      // Update the style
      element.style[key] = value;
      // Emit event
      handleEditEvent({
        el: element,
        editType: EditType.STYLE,
        newValue: { [key]: value },
        oldValue: { [key]: oldStyles[key] },
      });

Kitenite avatar Mar 31 '24 16:03 Kitenite

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.

google-cla[bot] avatar Mar 31 '24 16:03 google-cla[bot]

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 😅

argyleink avatar Apr 01 '24 17:04 argyleink

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 :)

Kitenite avatar Apr 01 '24 17:04 Kitenite

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.

argyleink avatar Apr 02 '24 16:04 argyleink

Sounds good, I'll get to cleaning it up

Kitenite avatar Apr 03 '24 14:04 Kitenite