bpmn-io / diagram-js

A toolbox for displaying and modifying diagrams on the web.
MIT License
1.68k stars 420 forks source link

Hotkeys do Not work on non-English Keyboard Layout #928

Open Vovamzur opened 3 weeks ago

Vovamzur commented 3 weeks ago

Describe the Bug

The problem relates to detecting key events, particularly when relying on the event.key property, which returns different values depending on the active keyboard layout. We should fix it in https://github.com/bpmn-io/diagram-js/blob/develop/lib/features/keyboard/KeyboardUtil.js by relying on event.code === 'KeyC' instead for example I believe

Steps to Reproduce

  1. Set your keyboard layout to Ukrainian.
  2. Try to copy/paste elements with the Ctrl+c/Ctrl+v
barmac commented 3 weeks ago

Thanks for your report. This is a known issue for which we have done some research in https://github.com/camunda/camunda-modeler/issues/4080 This all boils down to a need for localisation of the shortcuts. For some languages, the shortcuts rely on the physical location of the key (for them event.code will be an excellent choice), but for others it is the exact key which matters. An example of the latter is the German QWERTZ layout where letter Z is used to trigger undo even though it's located right below the digits.

barmac commented 3 weeks ago

So right now we support German-style layout well, but seem to miss out for Ukrainian, Russian, and perhaps more.

However, on MacOS, Ukrainian layout works just fine with the existing shortcuts. So it may be platform-dependent. What OS do you use?

Vovamzur commented 3 weeks ago

It is reproduced as on Windows as on Ubuntu 22.04. I found this commit https://github.com/bpmn-io/diagram-js/commit/c63a452d13616ad1a73e96f7f10204d9e5869bd1 . Before that change, it worked fine. At least we could just add Ukrainian letters to KEYS_* arrays if there are issues with German

barmac commented 2 weeks ago

Thanks for notice. The thing is that even if we add Ukrainian letters, that does not solve the problem of other languages. So we should probably use the physical location, and only support special cases like German separately.

Vovamzur commented 2 weeks ago

Hello. Looking forward to the fix. How can I help speed it up? Thanks!