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

[tree view] Cursor navigation interferes with browser shortcut keys #14296

Closed
joshkel opened this issue Aug 22, 2024 · 2 comments · Fixed by #14798
Closed

[tree view] Cursor navigation interferes with browser shortcut keys #14296

joshkel opened this issue Aug 22, 2024 · 2 comments · Fixed by #14798
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@joshkel
Copy link
Contributor

joshkel commented Aug 22, 2024

Steps to reproduce

Link to live example: https://mui.com/x/react-tree-view/simple-tree-view/items/#basics

Steps:

  1. On a Mac, navigate around a bit in the browser to have some history.
  2. End up on https://mui.com/x/react-tree-view/simple-tree-view/items/#basics
  3. Click on the first tree view item ("Data Grid") to set the keyboard focus there.
  4. Press Command-Left or Command-Right (Mac) to try to go forward or backward in the browser history.

Current behavior

The tree view responds to the Left and Right arrow keys to go higher or lower in the tree view, which is useful, but it also intercepts the arrow keys even if Command is pressed, so it interferes with browser history.

Expected behavior

The tree view responds only to plain left/right arrow key presses (no Command-), or the tree view at least doesn't intercept Command-Left and Command-Right.

Context

I noticed that the tree view doesn't respond to the Alt key, so the Alt-Left and Alt-Right browser shortcuts work on Windows, but it allows the Command key (KeyboardEvent.metaKey) for Mac.

https://github.com/mui/mui-x/blob/v7.13.0/packages/x-tree-view/src/internals/plugins/useTreeViewKeyboardNavigation/useTreeViewKeyboardNavigation.ts#L95

Your environment

npx @mui/envinfo
  System:
    OS: macOS 14.6.1
  Binaries:
    Node: 18.20.2 - ~/.nvm/versions/node/v18.20.2/bin/node
    npm: 10.5.0 - ~/.nvm/versions/node/v18.20.2/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 127.0.6533.120
    Edge: 127.0.2651.105
    Safari: 17.6
  npmPackages:
    @emotion/react:  11.13.3
    @emotion/styled:  11.13.0
    @mui/base:  5.0.0-beta.41
    @mui/core-downloads-tracker:  5.16.7
    @mui/icons-material:  5.16.7
    @mui/lab:  5.0.0-alpha.173
    @mui/material:  5.16.7
    @mui/private-theming:  5.16.6
    @mui/styled-engine:  5.16.6
    @mui/system:  5.16.7
    @mui/types:  7.2.15
    @mui/utils:  5.16.6
    @mui/x-date-pickers:  7.13.0
    @mui/x-internals:  7.13.0
    @mui/x-tree-view:  7.13.0
    @types/react: ^18.3.4 => 18.3.4
    react: ^18.3.1 => 18.3.1
    react-dom: ^18.3.1 => 18.3.1
    typescript: ^5.5.4 => 5.5.4```
</details>



**Search keywords**: browser back hotkey, browser shortcut key
@joshkel joshkel added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 22, 2024
@github-actions github-actions bot added the component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! label Aug 22, 2024
@michelengelen
Copy link
Member

Thanks for raising this @joshkel .... I can confirm this bug.

I'll open this up for the community as a good first issue.

Here is a little diff to get this started faster:

diff --git a/packages/x-tree-view/src/internals/plugins/useTreeViewKeyboardNavigation/useTreeViewKeyboardNavigation.ts b/packages/x-tree-view/src/internals/plugins/useTreeViewKeyboardNavigation/useTreeViewKeyboardNavigation.ts
index c00321273..ab09ffd0b 100644
--- a/packages/x-tree-view/src/internals/plugins/useTreeViewKeyboardNavigation/useTreeViewKeyboardNavigation.ts
+++ b/packages/x-tree-view/src/internals/plugins/useTreeViewKeyboardNavigation/useTreeViewKeyboardNavigation.ts
@@ -182,6 +182,10 @@ export const useTreeViewKeyboardNavigation: TreeViewPlugin<
       // If the focused item is expanded, we move the focus to its first child
       // If the focused item is collapsed and has children, we expand it
       case (key === 'ArrowRight' && !isRtl) || (key === 'ArrowLeft' && isRtl): {
+        if (ctrlPressed) {
+          return;
+        }
+
         if (instance.isItemExpanded(itemId)) {
           const nextItemId = getNextNavigableItem(instance, itemId);
           if (nextItemId) {
@@ -199,6 +203,10 @@ export const useTreeViewKeyboardNavigation: TreeViewPlugin<
       // If the focused item is expanded, we collapse it
       // If the focused item is collapsed and has a parent, we move the focus to this parent
       case (key === 'ArrowLeft' && !isRtl) || (key === 'ArrowRight' && isRtl): {
+        if (ctrlPressed) {
+          return;
+        }
+
         if (canToggleItemExpansion(itemId) && instance.isItemExpanded(itemId)) {
           instance.toggleItemExpansion(event, itemId);
           event.preventDefault();

Anyone picking this up should probably add some tests for this as well to prevent regressions! Thanks!

@michelengelen michelengelen changed the title Cursor navigation interferes with browser shortcut keysy [tree view] Cursor navigation interferes with browser shortcut keys Aug 23, 2024
@michelengelen michelengelen added good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 23, 2024
Copy link

github-actions bot commented Oct 2, 2024

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

We value your feedback @joshkel! How was your experience with our support team?
We'd love to hear your thoughts in this brief Support Satisfaction survey. Your insights help us improve!

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: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants