MultiSelect/ListBox: KeyA is 'q' on azerty keeboard

[Et7f3]

Describe the bug

The bug is this line

primereact/components/lib/listbox/ListBox.js

Line 457 in ef05cdd

if (props.multiple && event.code === 'KeyA' && metaKey) {

primereact/components/lib/multiselect/MultiSelect.js

Line 360 in ef05cdd

const value = visibleOptions.filter((option) => isValidOption(option)).map((option) => getOptionValue(option));

Here it is the event object:

Reproducer

Just use any example in the documentation

System Information

N/A

Steps to reproduce the behavior

1. Take physical qwerty keyboard
2. Change keyboard layout to azerty
3. Go to https://primereact.org/listbox/#multiple
4. Click on one item
5. press meta+a (so the physical key with label q on the physical keyboard) and nothing is selected
6. press meta+q (so the physical key with label a on the physical keyboard) all is selected

Expected behavior

The shortcut should execute when the key labeled A is triggered.

[melloware]

@Et7f3 do you have a solution?

[sja-cslab]

@melloware KeyboardEvent.key should be used.

MDN says:

For example, the code returned is "KeyQ" for the Q key on a QWERTY layout keyboard, but the same code value also represents the ' key on Dvorak keyboards and the A key on AZERTY keyboards. That makes it impossible to use the value of code to determine what the name of the key is to users if they're not using an anticipated keyboard layout.

To determine what character corresponds with the key event, use the KeyboardEvent.key property instead.

[melloware]

@sja-cslab can you submit a PR?

[sja-cslab]

If I find time - just back from vacation today so I've to catch 3 weeks of work :/

[Et7f3]

do you have a solution?

Yes I showed the event object that hinted we should use .key and @sja-cslab confirmed it is the right choice. So I let him open a PR (I won't be able to open one before october).

My quickfix is a call to sed that replace KeyA by KeyQ in my node_modules (because my users have azerty layout). I can live with this workaround so no rush in implementing a fix.

[sja-cslab]

@melloware we may need to discuss if we just want to fix the condition

if (props.multiple && event.code === 'KeyA' && metaKey) {

or if we want to change the whole handling in that component. One point we need to keep in mind is, that .key works a bit different than .code in case of the keys itself.

a e.g. is event.key === "a" where shift+a would be event.key === "A"

The KeyboardEvent interface's key read-only property returns the value of the key pressed by the user, taking into consideration the state of modifier keys such as Shift as well as the keyboard locale and layout.

I'm not sure if it's a good or a bad idea if we would combine .key and .metaKey / .ctrlKey
Found an example in MDN which uses combination of both.

[melloware]

Yes we typically try and use event.code when possible and event.key when necessary. Its OK to use event.key in this specific case.