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

[TreeView] Jumpy focus when content higher than the browser content's height #10234

Closed
idrakimuhamad opened this issue Feb 9, 2022 · 8 comments · Fixed by #12226
Closed

[TreeView] Jumpy focus when content higher than the browser content's height #10234

idrakimuhamad opened this issue Feb 9, 2022 · 8 comments · Fixed by #12226
Assignees
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module!

Comments

@idrakimuhamad
Copy link

idrakimuhamad commented Feb 9, 2022

Current behavior 😯

I'm facing an issue with TreeView and TreeItem, it happened when the TreeView are expanded larger than the browser height. For certain TreeItem, it requires 2 click to expand/collapse the row. But certain will not expand, and keep on jumping out of focus, and you need to place the section at certain area to be able to expand it.

This seems to be the same as #24096 however it still happened in my case.

Currently, I've patched the TreeItem.js to stop the handleFocus from running anything within. I don't think I need the focus anyway so that's the workaround, as there're no way to overwrite the TreeItem's onFocus for now.

Expected behavior 🤔

I expect it to not jump to the top of the page, and just expand the TreeItem

Steps to reproduce 🕹

Steps: I've created a sandbox replicating this issue at the link below.

  1. https://codesandbox.io/s/withered-sun-ph8hb
  2. Tap the section to expand, and keep opening the children section
  3. Monitor the outcome

Context 🔦

No response

Your environment 🌎

`npx @mui/envinfo`
System:
    OS: macOS 11.6
  Binaries:
    Node: 16.13.2 - ~/.nvm/versions/node/v16.13.2/bin/node
    Yarn: 3.0.1 - ~/.nvm/versions/node/v16.13.2/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
  Browsers:
    Edge: 97.0.1072.62
    Firefox: Not Found
    Safari: 14.1.2
  npmPackages:
    @emotion/react: ^11.7.1 => 11.7.1 
    @emotion/styled: ^11.6.0 => 11.6.0 
    @mui/base:  5.0.0-alpha.65 
    @mui/icons-material: ^5.2.5 => 5.3.0 
    @mui/lab: ^5.0.0-alpha.68 => 5.0.0-alpha.68 
    @mui/material: ^5.2.5 => 5.3.0 
    @mui/private-theming:  5.3.0 
    @mui/styled-engine:  5.4.1 
    @mui/styles: ^5.2.3 => 5.3.0 
    @mui/system:  5.4.1 
    @mui/types:  7.1.0 
    @mui/utils:  5.3.0 
    @mui/x-data-grid: ^5.2.1 => 5.2.2 
    @mui/x-data-grid-generator: ^5.2.1 => 5.2.2 
    @mui/x-data-grid-pro: ^5.2.1 => 5.2.2 
    @mui/x-license-pro:  5.2.2 
    @types/react: ^17.0.37 => 17.0.38 
    react-dom: ^17.0.2 => 17.0.2 
    styled-components:  5.3.3 
    typescript: ^4.4.3 => 4.5.3 
@idrakimuhamad idrakimuhamad added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 9, 2022
@idrakimuhamad idrakimuhamad changed the title TreeView jumpy focus when content higher than the browser content's height [TreeView] Jumpy focus when content higher than the browser content's height Feb 9, 2022
@oliviertassinari oliviertassinari added the component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! label Feb 9, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 9, 2022

Yeap, we fixed mui/material-ui#24096 with an API that is not supported by Safari, so we didn't solve the problem in reality. https://caniuse.com/mdn-api_htmlelement_focus_options_preventscroll_parameter

A different bug that I have just noticed, when using the Arrow keys to move the focus, the page doesn't scroll. We might want to revert mui/material-ui#21695.

@michaldudak
Copy link
Member

@idrakimuhamad Does this happen on both Edge and Safari?

@michaldudak michaldudak added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 11, 2022
@idrakimuhamad
Copy link
Author

idrakimuhamad commented Feb 11, 2022

@idrakimuhamad Does this happen on both Edge and Safari?

I haven't tested it before, but I just tried it on Edge -- seems it's not happening on Edge.

@oliviertassinari
Copy link
Member

@idrakimuhamad so if we understand it correctly, your bug report is for Safari v14.1.2?

@idrakimuhamad
Copy link
Author

@idrakimuhamad so if we understand it correctly, your bug report is for Safari v14.1.2?

Yes, for the 2 browsers I have, only happened in Safari.

@joaomarcuslf
Copy link

Any Update on this issue?

I've tested on later versions of Safari(15.4), and it was working, but on Safari 14.1.2.

@eladkariti
Copy link

eladkariti commented Aug 10, 2022

Yeap, we fixed mui/material-ui#24096 with an API that is not supported by Safari, so we didn't solve the problem in reality. https://caniuse.com/mdn-api_htmlelement_focus_options_preventscroll_parameter

A different bug that I have just noticed, when using the Arrow keys to move the focus, the page doesn't scroll. We might want to revert mui/material-ui#21695.

Hi @oliviertassinari, I get the same unexpected behavior regarding the Arrows keys that doesn't scroll the component.

is it a know issue?

thanks

Copy link

⚠️ This issue has been closed.
If you have a similar problem, please open a new issue and provide details about your specific problem.
If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @idrakimuhamad?
Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.

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!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants