material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

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

Open supidupicoder2 opened this issue 2 years ago • 3 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the 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

supidupicoder2 avatar Jul 10 '23 08:07 supidupicoder2

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

michaldudak avatar Jul 11 '23 07:07 michaldudak

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

gitstart avatar Jul 11 '23 11:07 gitstart

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.

evankennedy avatar May 22 '24 23:05 evankennedy