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

Bug: Undo hotkey stops working in a specific case #6674

Open
Ulop opened this issue Sep 26, 2024 · 4 comments
Open

Bug: Undo hotkey stops working in a specific case #6674

Ulop opened this issue Sep 26, 2024 · 4 comments

Comments

@Ulop
Copy link

Ulop commented Sep 26, 2024

Lexical version: lexical: 0.17.1

Steps To Reproduce

  1. Switch to a non-Latin keyboard layout (e.g. Greek layout)
  2. Open Lexical editor with a non-empty content (playground is a good variant)
  3. Select all content
  4. Press the Backspace (or Delete) key to clear all content
  5. Press the Ctrl+Z(undo) hotkey

Link to code example: https://playground.lexical.dev/

The current behavior

Nothing happens

The expected behavior

The editor restores the last state from the undoStack field of the history when you press the Ctrl + Z hotkey.

By default, in the LexicalEvents.ts file there are two points that dispatch the UNDO_COMMAND command:

  • onKeyDown listener (by checking whether event.ctrlKey is true and event.key equals to z character)
  • onBeforeInput listener (by checking whether event.inputType equals to historyUndo)

When the content has been cleared as described above (in the Steps To Reproduce section) only the first listener (onKeyDown) is called and never the second one.

For Latin layout, it works correctly because it passes the isUndo function test. But it doesn't work for other layouts, since the event.key value is not the 'z' character.

However, if we modify the content (e.g. type in any character) before step 3 and then execute steps 3-5 everything will work as expected. The onBeforeInput event is called and the previous state is restored.

@fantactuka
Copy link
Contributor

Thanks for reporting @Ulop

I've tried to repro with Greek and Korean layouts and in both cases it worked well, is it the same for you in all browsers, or specific one?

Regarding event handlers: as long as isUndo check passes in onKeyDown it preventsDefault so onBeforeInput won't be called (so it'll avoid double triggering for undo command)

@Ulop
Copy link
Author

Ulop commented Sep 30, 2024

@fantactuka, Thanks for answer.

Maybe it's a browser-dependent issue?

I asked a colleague to check this situation on his MacBook (MacOS 17.6) in Safari - everything worked as expected.

On MacBook in Chrome the editor state is not restored after pressing the undo hotkey.

On Win(10|11) in Chrome\FireFox the situation is similar.

@Ulop
Copy link
Author

Ulop commented Oct 2, 2024

I can assume that I may have found the cause, but I can’t suggest a simple solution.

As I'm mentioned above, Lexical has two point that can dispatch UNDO_COMMAND when the user presses the Ctrl + Z command.

onKeyDown listener will dispatch the undo command only if the user's current keyboard layout contains the z key.

So in other cases we must rely on the onBeforeInput listener.
But with Chrome(and apparentyl with FireFox too) there is some problem.
Input\Content editable element will receive onBeforeInput event with inputType == 'historyUndo' only if that element was previously modified by user input, not by JS.

A simallar issue was closed a comment that is expected behaviour.

More simple example:

  1. Switch to a non-Latin keyboard layout
  2. Open Lexical editor with a non-empty content
  3. Insert Image
  4. Press the undo hotkey

onBeforeInput doesn't called, nothing happened.

@Ulop
Copy link
Author

Ulop commented Oct 3, 2024

Having thought about this problem once again, I came to the conclusion that it is most likely worth solution is changing hotkeys handling in the onKeyDown listener.

I decided to see how VSCode solves the hotkey problem. And as far as I understand, they follow the points from these recomendations and rely on KeyboardEvent::keyCode field.
See extractKeyCode function from keyboardEvent.ts and how EVENT_KEY_CODE_MAP filled.

Similar issues with hotkeys handling:
w3c/uievents#377
w3c/uievents#267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants