Redo shortcut CTRL+Y does not work on German keyboard layout

[nikku]

Describe the Bug

I cannot use the CTRL+Y shortcut on Windows / Linux anymore to trigger redo.

Steps to Reproduce

1. Open https://demo.bpmn.io/s/start
2. Perform change
3. CTRL+Z + CTRL+Y
4. Observe undo works, while redo does not

Expected Behavior

Redo shortcut works.

Environment

• Browser: Chrome 110
• OS: Linux, Windows
• Library version: 11.3.0

[nikku]

Broke with [email protected] / [email protected].

[nikku]

Root causing this:

• diagram-js < 11 we intercepted both key (actual key) and keyCode / which (physical key). We had a workaround in place for German keyboards and removed it with 11.0.0 (bpmn-io/diagram-js@ff861b0#r100581210).
• What we have now in place is a behavior where physical keys, depending on the keyboard shortcut registration order, overrule actual keys, so both y and KeyZ on German keyboards will trigger undo

A proper solution would likely value actual keys over physical keys:

• We first process bindings against actual keys
• THEN we process bindings against physical keys (in case actual keys did not trigger a change yet)

The alternative:

• Don't bind physical keys at all (only actual keys)
• Potentially make it extensible (i.e. allow to specify additional keys for special keyboard layouts; but even then these can be mapped as actual keys, not physical keys)

[philippfromme]

I can reproduce the issue. Using a German keyboard layout:

German keyboard produces "z", "KeyY" for Z and "y", "KeyZ" for Y
Englisch keyboard produces "y", "KeyY" for Z and "z", "KeyZ" for Y

As a result when pressing Y the undo listener matches.

A proper solution would likely value actual keys over physical keys:

• We first process bindings against actual keys
• THEN we process bindings against physical keys (in case actual keys did not trigger a change yet)

That's what we have to do. Or drop code alltogether.

[philippfromme]

@nikku Do you want to go ahead and prototype the solution or should I give it a try?

[nikku]

@philippfromme I've sketched an alternative:

The alternative:

• Don't bind physical keys at all (only actual keys)
• Potentially make it extensible (i.e. allow to specify additional keys for special keyboard layouts; but even then these can be mapped as actual keys, not physical keys)

If we ever want to support non latin keyboards (where users expect undo to be triggered on the physical KeyY position we'd need to build that, one way or the other). What would be your preference?

[philippfromme]

It sounds like the alternative would be to right approach in the long term so why don't we just implement that instead of wasting time on an intermediate fix?

[nikku]

If I understand you correctly (#1842 (comment)) you second the alternative I sketched.

I#ll go ahead and remove the physical key fix then. We can then (potentially, some day in the future), add support for shortcut customization / physical key positions.

[philippfromme]

I#ll go ahead and remove the physical key fix then. We can then (potentially, some day in the future), add support for shortcut customization / physical key positions.

👍🏻

[nikku]

Closed via 530b61e.