mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.87k stars 32.26k forks source link

[Select] De-Selecting causes multiple select to jump to top #37895

Open supidupicoder2 opened 1 year ago

supidupicoder2 commented 1 year ago

Duplicates

Latest version

Steps to reproduce ๐Ÿ•น

Link to live example: This happens on the official MUI Website (as of 10. July 2023): https://mui.com/material-ui/react-select/ (or alternatively, stackblitz: https://stackblitz.com/edit/react-gtavad?file=demo.tsx)

Steps:

  1. Go to the official MUI Website: https://mui.com/material-ui/react-select/
  2. Scroll down to "Multiple select"
  3. In the "Default" Example, click on "Name", scroll to the very bottom of the popup
  4. Click and Select "Kelly Snyder"
  5. Deselect "Kelly Snyder"

Gif: chrome_e5IlyPWFdF

Current behavior ๐Ÿ˜ฏ

The scrolling position of the popup will jump to the top of the Select popup.

If more than one item was selected and one item gets deselected, the scroll will not jump to the very top, but will focus one of the other selected items instead.

Expected behavior ๐Ÿค”

The scrolling position of the popup should stay where it is.

Context ๐Ÿ”ฆ

I want to be able to deselect something in a Select Component with the multiple prop, without the scrolling position of the popup jumping around.

There was a very similar issue back in 2021: https://github.com/mui/material-ui/issues/19245

Some other issue: This behaviour can be circumvented as described here: https://stackoverflow.com/questions/72955537/material-ui-select-jumps-to-the-top-of-the-page

But this will break other things: -) Keyboard not working properly anymore -) When opening the selection and there are already things selected, the popup should already focus on those things, not start at the very top, but this will be broken with the workaround.

Your environment ๐ŸŒŽ

npx @mui/envinfo ``` System: OS: Windows 10 10.0.19044 Binaries: Node: 14.18.0 - C:\Program Files\nodejs\node.EXE Yarn: Not Found npm: 8.8.0 - C:\Program Files\nodejs\npm.CMD Browsers: Chrome: Not Found Edge: Spartan (44.19041.1266.0), Chromium (114.0.1823.67) npmPackages: @emotion/react: ^11.10.5 => 11.10.5 @emotion/styled: ^11.10.5 => 11.10.5 @mui/base: 5.0.0-alpha.114 @mui/core-downloads-tracker: 5.11.5 @mui/icons-material: ^5.10.9 => 5.11.0 @mui/lab: ^5.0.0-alpha.106 => 5.0.0-alpha.116 @mui/material: ^5.10.12 => 5.11.5 @mui/private-theming: 5.11.2 @mui/styled-engine: 5.11.0 @mui/styles: ^5.10.10 => 5.11.2 @mui/system: 5.11.5 @mui/types: 7.2.3 @mui/utils: 5.13.1 @mui/x-data-grid: ^5.17.10 => 5.17.26 @mui/x-date-pickers: ^5.0.7 => 5.0.14 @types/react: ^18.0.24 => 18.0.26 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^4.9.5 => 4.9.5 ```
michaldudak commented 1 year ago

Thanks for reporting this. It seems we've got a bug here. Would you like to contribute to the project by investigating it?

gitstart commented 1 year ago

@michaldudak @supidupicoder2 I would like to pick this up?

evankennedy commented 5 months ago

I investigated this a bit. It doesn't seem like it's the Select, but rather the Menu at fault. It's related to Menu.autoFocus specifically on the selectedMenu variant. I'm happy to submit a fix, but I want to verify the intended functionality first.

How it currently works is that it will focus either the first selected menu item, or the first item in the list if none are selected. As a user selects or deselects items, this focus element is re-evaluated and the focus changes, causing scroll. If a user deselects all items, it will then put focus back on the first menu item, causing it to scroll back to the top.

To fix this, I can think of a couple solutions:

  1. Only automatically focus items when the menu first opens, then don't forcibly change a user's focus
  2. Keep the same functionality except if none are selected. In this case, don't auto-select the first menu item

The first one feels more proper to me personally, but it's also a larger change in how the component functions.

I found a second issue I can also fix where the select will auto-select the last item, rather than the first as the docs suggest.