Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Add undo and redo #10

Merged
merged 5 commits into from
Jun 3, 2020
Merged

Add undo and redo #10

merged 5 commits into from
Jun 3, 2020

Conversation

bigfoodK
Copy link
Collaborator

@bigfoodK bigfoodK commented May 31, 2020

Notice
It saves whole state of redux
This can be performance issue

R and Ctrl+Z for undo
Shift+R and Ctrl+Shift+Z for redo

@namse
Copy link
Contributor

namse commented May 31, 2020

@bigfoodK I have a suggestion about implementation of history.

What about keep state using store.subscribe and restore with that?
Below is psudocode.

const stateStorage: state[] = [];
let stateStorageIndex;
store.subscribe((nextState) => {
  stateStorage.removeElementsAfter(stateStorageIndex);
  stateStorage.push(nextState);
  stateStorageIndex = stateStorage.length - 1;
});

function Undo() {
  const state = stateStorage[stateStorageIndex - 1];
  stateStorageIndex -= 1;
  setState(state);
}

function Redo() {
  const state = stateStorage[stateStorageIndex + 1];
  if (!state) {
    throw new Error('cant redo');
  }
  stateStorageIndex += 1;
  setState(state);
}

Pros : Don't have to set editHistory.push();
Cons : We can't control which action can be rollback or not.
--> Maybe we can put some attributes in .gen file to control it?

@bigfoodK
Copy link
Collaborator Author

bigfoodK commented Jun 1, 2020

Consider points

  1. There is some functions that does multiple dispatch
    https://github.com/team-luda/Milishita-NoteEditor/blob/7415637ff8c14b361d439b2b28098e624daf93e4/web/src/utils/note.ts#L27-L33
    It's because the function dispatch multiple states
    So If You keep state on dispatch or action, You have to becareful not to keep state multiple time

  2. There is one function, In here, It has to keep state, but In there, It should not to keep state
    https://github.com/team-luda/Milishita-NoteEditor/blob/7415637ff8c14b361d439b2b28098e624daf93e4/web/src/runAddLongNoteProcess.ts#L68-L76
    dispatch(BarAction.addNote());
    In adding single note or making start note of long note, This function has to keep state,
    But in adding end note of long note, That should not be.
    If the function keep state in adding end note of long note,
    Undo after adding long note, It goes to long note editting process,
    There is issue, Even if state goes back, We can't edit again, It's because long note adding process is over already.

Solutions

Consider point 1
Just add history.push() to first thing of dispatches
dispatch(someAction.some) <- just add to this
dispatch(somesomeAction.some)
dispatch(somesomesomebarAction.some)

Consider point 2
Just add new action that same function as different name

With these solutions, How about just add editHistory.push() to actions with .genfile
ex) in bar.gen
addNote() -someWord

then, in barActions.ts

function addNote(...) {
  editHistory.push()
  ...
}

like this, Is it look good to you?

@namse
Copy link
Contributor

namse commented Jun 2, 2020

  1. multiple actions : Batching
    You can create Batch Reducer which get types which contains multiple actions.
    Check this description -> Batching actions reduxjs/redux#911 (comment)

  2. Long Note Procedure Issue
    Maybe we have to re-implement long Note adding procedure from async function to redux actions...

@bigfoodK
Copy link
Collaborator Author

bigfoodK commented Jun 3, 2020

Fixed issues.

Thank you for your advice.

@namse namse merged commit d47a2ff into NamseEnt:master Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants