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

[Autocomplete] Make the scrollbar stay in current position on mouse expand #30711

Closed
wants to merge 1 commit into from

Conversation

ajlamarc
Copy link

As shown in this CSB, the scrollbar returns to top on scrolling down the list with mouse.

https://codesandbox.io/s/confident-kare-5n6i6

removing the else-if condition in the following if statement removes the reason for this scrollbar adjustment, the scrollbar will now stay in its current position.

if (listboxNode.scrollHeight > listboxNode.clientHeight && reason !== 'mouse')

@mui-bot
Copy link

mui-bot commented Jan 20, 2022

Details of bundle changes

Generated by 🚫 dangerJS against 1dca529

@danilo-leal danilo-leal changed the title do not scroll to top (stay in current position on mouse expand) [Autocomplete] Make the scrollbar stay in current position on mouse expand Jan 20, 2022
@danilo-leal danilo-leal added component: autocomplete This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base labels Jan 20, 2022
@mnajdova mnajdova requested a review from michaldudak January 26, 2022 10:57
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the scrolling behavior when the highlight moves up. See https://deploy-preview-30711--material-ui.netlify.app/components/autocomplete/#combo-box - when you scroll to the middle then move the highlight up, the highlighted item is not visible.

Also, the code you removed in this PR was added in #19305 to improve scrolling on grouped options.

@siriwatknp
Copy link
Member

👋 The migration PR has been merged.

Please follow these steps to make sure that the contents are all updated. (Sorry for the inconvenience)

  1. pull latest master from upstream to your branch
  2. if your PR has changes on the *.md or demo files, you are likely to encounter conflict because all of them have been moved to the new folder.
    2.1 revert the change on those markdown files you did
    2.2 pull latest master from upstream (you should not see conflict)
    2.3 make the changes again at docs/data/material/*
  3. run yarn docs:api
    • you might see the changes in docs/pages/material/api/* if your PR touches some of the components
    • it is okay if there is no changes

If you are struggle with the steps above, feel free to tag @siriwatknp

@michaldudak michaldudak added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Feb 4, 2022
@mnajdova
Copy link
Member

@ajlamarc please open an issue before creating a pull request, so that we can discuss the solution to the problem first. Thank you!

@mnajdova mnajdova closed this Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants