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

[DataGrid] ALT+C keyboard shortcut doesn't work #3658

Closed
cherniavskii opened this issue Jan 18, 2022 · 1 comment · Fixed by #3660
Closed

[DataGrid] ALT+C keyboard shortcut doesn't work #3658

cherniavskii opened this issue Jan 18, 2022 · 1 comment · Fixed by #3660
Assignees
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@cherniavskii
Copy link
Member

cherniavskii commented Jan 18, 2022

Current behavior 😯

Alt+C (Option+C) on selected rows does nothing (at least on MacOS).

Expected behavior 🤔

Alt+C (Option+C) should copy currently selected rows with headers (as per https://mui.com/components/data-grid/accessibility/#selection)

Steps to reproduce 🕹

Steps:

  1. Go to https://codesandbox.io/s/6upyt?file=/demo.tsx
  2. Select any row
  3. Press Alt+C (Option+C)
  4. Paste copied content somewhere to see that clipboard content hasn't changed

Context 🔦

The problem

On Mac, Option is a modifier key that allows to type modified letters (e.g. letters with accent) depending on keyboards layout.
For example, on British keyboard layout Option+c would print ç, and on Ukrainian layout - .

This is how Alt+C shortcut is handled:
https://github.com/mui-org/material-ui-x/blob/85c16c1d240fc5741bdc1fd8bf2325dfe6d59793/packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts#L57-L60

In this case, event.key.toLowerCase() returns ç, not c and condition won't pass.

Proposed solution

For this specific case I would propose to use event.code.
It represents a physical key rather than an actual letter, so event.code would have a value KeyC.
You can use https://w3c.github.io/uievents/tools/key-event-viewer.html for experiments.

It's not ideal though, because:

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.
Source: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code

So the solution would be to check event.code in addition to event.key:

-if (event.key.toLowerCase() !== 'c' || !isModifierKeyPressed) {
+if ((event.key.toLowerCase() !== 'c' && event.code !== 'KeyC') || !isModifierKeyPressed) {
  return;
}

I'm pretty sure this problem doesn't exist on Windows, because left ALT doesn't alter the letter pressed.
Would love to get some feedback on this, especially from people using different keyboard layouts and operating system.

Your environment 🌎

`npx @mui/envinfo`
  MacOS Monterey 12.1
  Browser: Brave, Safari

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

No response

@cherniavskii cherniavskii added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 18, 2022
@DanailH DanailH added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 18, 2022
@cherniavskii cherniavskii self-assigned this Jan 18, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 18, 2022

I can reproduce the bug.

Actually, maybe we could also improve the docs

diff --git a/docs/src/pages/components/data-grid/accessibility/accessibility.md b/docs/src/pages/components/data-grid/accessibility/accessibility.md
index 96cd7b33..f900fc9f 100644
--- a/docs/src/pages/components/data-grid/accessibility/accessibility.md
+++ b/docs/src/pages/components/data-grid/accessibility/accessibility.md
@@ -90,7 +90,10 @@ Use the arrow keys to move the focus.
 ### Key assignment conventions

 The above key assignments are for Windows and Linux.
-On macOS, replace <kbd class="key">CTRL</kbd> with <kbd class="key">⌘ Command</kbd>.
+On macOS:
+
+- replace <kbd class="key">CTRL</kbd> with <kbd class="key">⌘ Command</kbd>.
+- replace <kbd class="key">ALT</kbd> with <kbd class="key">⌥ Option</kbd>.

 ## API

? This was not clear for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants