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

fix(history): use correct shortcuts for undo/redo #4435

Merged

Conversation

webkadiz
Copy link
Contributor

Please describe your changes

The problem with the shortcuts for undo/redo. the decision is made immediately for z/y.

I will use the abstract Ctrl-Shift-Z to avoid overlapping with tiptap.

I will describe for z:

  • when you press Ctrl-Shift-Z, Mod-Z and Shift-Mod-z catch this event, that is, potentially we have two handlers for the same event, but with different functions, and they are not always executed together
  • If the undo stack is not empty, then only the Mod-Z handler will be captured, which will execute undo by shortcut Ctrl-Shift-Z, we get a completely diametric behavior, the behavior should have been called redo
  • If the undo stack is empty, then Mod-Z will work, saying that it did nothing, and we will run in Shift-Mod-z, finally getting the same redo

Shift-Mod-Z doesn't make sense according to "Modifiers can be given in any order. Shift- (or s-), Alt- (or a-), Ctrl- (or c- or Control-) and Cmd- (or more Meta-) are recognized. For characters that are created by holding shift, the Shift- prefix is implied, and should not be added explicitly" prosemirror.

Only bindings with Shift are used for clarity.

How did you accomplish your changes

The solution is to set the correct commands and remove duplicate handlers

How have you tested your changes

tested locally. with stack length: 0, 1, 5

How can we verify your changes

open demo and see correct undo/redo behavior

Remarks

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Related issues

Fixes #4403

@netlify
Copy link

netlify bot commented Sep 10, 2023

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 520ce79
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/650de78977161000080e4887
😎 Deploy Preview https://deploy-preview-4435--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@janthurau janthurau self-assigned this Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug]: redo doesn't work correctly
3 participants