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

Hook up history undo/redo input type #16989

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Aug 9, 2019

Description

Fixes woocommerce/woocommerce-blocks#690.

When undoing through the right-click menu or main browser menu, the browser will undo a change and Gutenberg will create an extra undo level. This is not ideal.

This PR intercepts this behaviour for browsers that support the beforeinput event. Unfortunately, there's not much we can do for other browsers (Firefox and Edge)...

What this PR does not do is update the internal browser history/undo stack so that the browser buttons reflect Gutenberg's history/undo stack. Unfortunately, this is not possible today in any browser, or at least not that I'm aware of. There might be some tricks to make sure that both buttons are always enabled, perhaps something to experiment with in a separate PR.

In the future when all browsers support this event, we could event stop binding cmd+z etc, because normally this also triggers the beforeinput event. :) Actually no, because the browser doesn't know if there's past or future records. Not sure if there's any any to "fool" the browser into thinking there is.

Cc @nerrad.

How has this been tested?

Open the demo post. Make some changes.

Use either of these:

Screenshot 2019-08-09 at 18 25 58

Screenshot 2019-08-09 at 18 25 43

Ensure the history buttons in Gutenberg update correctly: the redo button should become active, and the undo button shouldn't reapply the change.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added the [Feature] History History, undo, redo, revisions, autosave. label Aug 9, 2019
@ellatrix ellatrix added the [Type] Bug An existing feature does not function as intended label Aug 9, 2019
@ellatrix
Copy link
Member Author

ellatrix commented Aug 9, 2019

Related: w3c/editing#150

@ellatrix
Copy link
Member Author

ellatrix commented Aug 9, 2019

This is also very interesting. Turned out I did exactly here as what one of the comments is describing. https://discuss.prosemirror.net/t/native-undo-history/1823

@nerrad
Copy link
Contributor

nerrad commented Sep 26, 2019

Sorry for the delay in getting to this.

Rebased on latest master. There was a conflict due to the new <Typewriter> component add. I think I resolved it correctly.

I'm going to test this now and see how it works.

@nerrad
Copy link
Contributor

nerrad commented Sep 26, 2019

So latest results here

It looks like this fix isn't working in Chrome (it may have and a chrome update broke again?). I've also checked Firefox and Safari (interestingly, Safari doesn't have a context menu item for undo within the input).

Based on what you linked to here: w3c/editing#150 - does it sound like we'll need to introduce our own undo/redo stack within inputs if we want this to work reliably across browsers?

Base automatically changed from master to trunk March 1, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] History History, undo, redo, revisions, autosave. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pressing Ctrl+Z behaves different than clicking 'Undo' inside a block search input
2 participants