Skip to content
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

Try a generic undo handler #4959

Closed

Conversation

youknowriad
Copy link
Contributor

The undo state shouldn’t be related to components at all and maybe have something like this:

  • Compare the redux dispatch action triggering a change with the previous dispatched action. If the two actions are similar: which could mean, they’re both UPDATE_ATTRIBUTES of the same block and they both update the same attribute and the only difference between these two attributes is a single alphanumeric character (not space etc...), override the previous undo state.

  • If the actions are different, create a new undo state.

This would be very generic

@youknowriad youknowriad added the [Status] In Progress Tracking issues with work in progress label Feb 8, 2018
@youknowriad youknowriad self-assigned this Feb 8, 2018
@youknowriad youknowriad requested review from aduth and ellatrix February 8, 2018 18:04
@youknowriad youknowriad force-pushed the try/generic-undo-handler branch from 21d490f to b5764cd Compare February 8, 2018 18:13
partialRight( withHistory, {
resetTypes: [ 'SETUP_NEW_POST', 'SETUP_EDITOR' ],
shouldOverwriteState: ( previous, next ) => {
if ( previous.type !== next.type || previous.type !== 'UPDATE_BLOCK_ATTRIBUTES' ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could generalise this more? Say e.g. someone changes the post format to "link", but then immediate changes it back to "standard", I think we should avoid to create an undo level as well. Similarly we could have input fields outside block scope, which would still add records for each character change?

return false;
}

return areValuesEquivalent( previous.attributes[ previousAttributes[ 0 ] ], next.attributes[ nextAttributes[ 0 ] ] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

areValuesEquivalent sounds like an expensive calculation to execute on every action (an thus keystroke).

@youknowriad
Copy link
Contributor Author

Superseded by #4956

@youknowriad youknowriad deleted the try/generic-undo-handler branch February 13, 2018 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants