calband / calchart-redesign

Calchart 4 for the web! 🌐
https://calband.github.io/calchart-redesign/
4 stars 0 forks source link

Undo/Redo system (WIP) #119

Closed rmpowell77 closed 3 years ago

rmpowell77 commented 3 years ago

Adding an undo/redo system.

Description

This closely follows the outline by Anthony Gore: https://vuejsdevelopers.com/2017/11/13/vue-js-vuex-undo-redo/

What we do is we have the undo system implemented as a stack of Store Commit changes. When we do an undo, we go back to an initial state and redo all of the Store commit commands. With CalChart, not every Store commit is a change we want to undo, so we have a filter where we only save certain commits.

Using this change to also clean up several things to make thing easier:

  1. Adding Undo/Redo buttons at the bottom of the screen.
  2. Changing all the commit keywords to Enums so it's easier to characterize, catalog them.
  3. Making sure that the store.commit commands are all "primitive" copy like objects. For instance, when adding a new dot, pass a partial (or a json equivalent) instead of the Dot so that it gets constructed in the state itself.

Have not done the unit tests yet.

Pre-PR checklist

Screenshots/GIFs

[Attach screenshots if making a visible change!]

kmdurand commented 3 years ago

I have a newfound love for this review feature:

Screen Shot 2021-01-02 at 7 55 28 PM
MichaelTamaki commented 3 years ago

Notes from Richard:

Need to look for any changes to the store that are outside of the mutations. For example, in our continuity editor UI:

const continuity: ContEven = this.$store.getters.getContinuity(
  this.dotTypeIndex,
  this.continuityIndex
);
continuity.marchType = marchType;
this.$store.commit("updateDotTypeContinuity", {
  dotTypeIndex: this.dotTypeIndex,
  continuityIndex: this.continuityIndex,
  continuity,
});

Additionally, mutations should not accept references as parameters. This is because it makes it difficult to have undos/redos if the original state gets modified. It should only accept primitives, or at the very least, not a reference to data in the current state.

Testing? Whatever gets you unblocked the fastest.