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

[Select][material-ui] Don't lock the body scroll and make it non-modal select #17353

Open
ghost opened this issue Sep 7, 2019 · 46 comments
Open
Assignees
Labels
component: menu This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module! new feature New feature or request priority: important This change can make a difference

Comments

@ghost
Copy link

ghost commented Sep 7, 2019

Current Behavior 😯

When I open a Select component vertical scroll bar of the page disappears.
See here
scrn

I don't have a demo to reproduce sorry, but I suppose this is a known issue: #8710, #7239, so maybe someone can help.

The select component is located on an app bar on the screenshot.

I tried setting box-sizing: border-box on App bar, no help.

Expected Behavior 🤔

The scrollbar should not disappear.

Your Environment 🌎

Tech Version
Material-UI v3.1.1
React 16.5
Browser Chrome
@oliviertassinari oliviertassinari added the component: select This is the name of the generic UI component, not the React module! label Sep 7, 2019
@oliviertassinari oliviertassinari changed the title Opening combo box makes vertical scroll bar dissapear [Select] Don't lock the body scroll Sep 7, 2019
@oliviertassinari oliviertassinari added the new feature New feature or request label Sep 7, 2019
@oliviertassinari oliviertassinari added component: menu This is the name of the generic UI component, not the React module! priority: important This change can make a difference labels Oct 6, 2019
@sjsakib
Copy link

sjsakib commented Nov 26, 2019

Any update on this issue? I have a select element inside a hero/landing page. When the select menu opens, scroll gets locked. And that causes body to shrink by about .2px. And that causes the background image to jump a bit. I find this very annoying. Anyone have any suggestions to work around this?

@sjsakib

This comment was marked as resolved.

@oliviertassinari oliviertassinari removed the priority: important This change can make a difference label Dec 1, 2019
@jeronimomora
Copy link

jeronimomora commented Dec 14, 2019

@sjsakib

Quote from eplumecocq for a Select fix

<Select MenuProps={{ disableScrollLock: true }} />

Which means that if you have this problem for a menu, just add disableScrollLock={true} as a prop to Menu

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 14, 2019

@jeronimomora Unfortunately, this isn't enough to solve the problem, the position won't be correct. I would also expect the backdrop to be removed, and the aria inert not to be applied.

@WholemealDrop
Copy link

Following, also experiencing this with Select component.

| Material-UI | v3.9.2 |
| React | v16.9.0 |
| Browser | chrome |

@tactycHQ
Copy link

@sjsakib

Quote from eplumecocq for a Select fix

<Select MenuProps={{ disableScrollLock: true }} />

Which means that if you have this problem for a menu, just add disableScrollLock={true} as a prop to Menu

While this seems to works for the Select component, React throws the following warning. I suspect this is because there is no child Menu compnent under Select. Select only has the Menuiitem components as children.

"Warning: React does not recognize the disableScrollLockprop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercasedisablescrolllock instead. If you accidentally passed it from a parent component, remove it from the DOM element."

@TomPradat
Copy link
Contributor

Any updates ? I also have the problem with version 4.8.2 when displaying drawers, menus. The disableScrollLock works for me on the Menus but this is not a satisfying solution IMO.

@BrentGrammer
Copy link

BrentGrammer commented Jan 12, 2020

I'm having a scroll bar issue with the Select component as well - the scroll bar appears when hovering over it and disappears on mouse leave. It causes a janky visual effect. Setting overflow-y to hidden on all sorts of elements doesn't fix it. Anyone else run into this and solve it?

@TomPradat
Copy link
Contributor

Personnaly i've locked the version on 4.6.x, you can try the disableScrollLock prop trick else

@Dito-Orkodashvili
Copy link

In my case this problem is caused by Tooltip component. It is a bit strange issue.

if I use it like this:
const ActionButton = ({ tooltip }) => ( <Tooltip title={tooltip} > <Button /> </Tooltip> )

it adds padding-right to the body (17px), but if I remove Tooltip from inside the ActionButton component and wrap it from outside like this:
<Tooltip title={tooltip}> <ActionButton /> </Tooltip>
it works fine

@defkev
Copy link

defkev commented Mar 10, 2020

Seeing the same behavior as @Dito-Orkodashvili with a <Button> wrapped by a <Tooltip> toggling a <Menu>

If the box model is close to showing the vertical scrollbar, opening the menu will add the padding-right:17px scrollbar to the body though there is no scrollbar moving the body 17px to the left.

As a workaround set the tooltip's title to nothing when the <Menu> is opened:

<Tooltip title={this.state.menuAnchor ? '' : 'The Tooltip'}>

@material-ui/core 4.9.5

@sedatbasar
Copy link

sedatbasar commented Jan 24, 2023

This is because of the calculation in MUI .
So the issue that MUI try to fix is to disable the scrolling MUI adds "overflow: hidden" to your body element and if your browser shows scrollbar, then it will have a flickering effect. Because while adding the overflow:hidden, the scrollbar will also get hidden and ur page will shift to right. So to prevent this MUI is adding the padding to body element to not have that flickering effect.
For me following workaround is working well.

html {
  overflow: hidden
}

body {
  overflow: auto !important;
  max-height: 100vh;
}

So with this we are forcing our window.innerWidth and document.documentElement.clientWidth to have same value so that MUI dont add that padding while keeping our scrolling behavior. But now scrolling will work on body element so you should use it carefully since it might have unexpected behaviors. (For example if you are using react-router with ScrollRestoration it might broke that)

@AChangXD
Copy link

AChangXD commented Mar 3, 2023

Why can't <Select/> replicate the behavior of <Autocomplete/>? <Autocomplete/>'s dropdown works pefectly fine while scrolling either inside of the dropdown or outside of the page, the dropdown also doesn't stay fixed in place. @oliviertassinari

Edit: Nevermind, saw a different issue about re-writing this. Is there an ETA on this? '' is nice for multiselect but it does not work well on Mobile (The keyboard always pop up no matter what you do).

@krnl0138
Copy link

krnl0138 commented Apr 9, 2023

I've spent 4 hours on this weird behaviour.
This is deeply messed up as @peterlu-hinter correctly summarised.
The issue is being opened for 4 years and its priority is set to 'important'. Is there any progress?
Maybe you could at least point out this quirkiness in the docs?
I believe this may be an expected behaviour for MUI, but not for devs and users.

@sedatbasar 's solution works fine, though. Don't know if there is any major implications out of it.
This way, however, disableScrollLock: true isn't working anymore for me in MenuProps in a TextField component.
Definitely can be misleading in large projects, since I suppose it won't work for anything that is popover-based.

@jovicheng
Copy link

jovicheng commented Aug 3, 2023

If you want the disableScrollLock: true props and don't want the body misalignment with layout,
you can try setting box-sizing: content-box; with body tag, it works for me!

@DiegoAndai
Copy link
Member

DiegoAndai commented Aug 29, 2023

Hi everyone! Thanks for your patience.

We've added a workaround for this use case in #37773, released in version 5.14.7. You can now turn off the Select component's scroll lock and margin threshold. To do so, set the MenuProps properties disableScrollLock to true and marginThreshold to null:

<Select
  MenuProps={{
    // disable scroll lock
    disableScrollLock: true,
    // allow the menu to go outside the window
    marginThreshold: null
  }}
/>

Here's a CodeSandbox example of this.

Doing so on a TextField component with the select prop enabled is also possible. To do so, provide the same MenuProps properties through the SelectProps prop. Here's a CodeSandbox example of this.

I'll close this issue as this workaround enables the scroll behavior when the select is open. If you have any questions, don't hesitate to leave a comment. Feel free to open a new issue if you encounter any problems with the workaround.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 30, 2023

@DiegoAndai your demo looks great, but I think this issue is about having a non-modal select, per #17353 (comment).

In https://codesandbox.io/s/issue-17353-scrollable-text-field-select-forked-8hq5xr, there is still aria-hidden=true on all the elements of the page, and most importantly, can't focus on the input in one click it takes two.

@DiegoAndai
Copy link
Member

@oliviertassinari The original intent of this issue is that when Select opens, it shouldn't lock the page's scroll. We won't make that change in v5 as it's a breaking change, so adding a workaround to achieve it is the next best thing we can do.

Keeping this issue open is not useful IMO. It has a lot of comments and opinions, so it's hard to read and understand the scope. I propose closing it.

I agree that there is value in this issue's comments, like #17353 (comment). The Select component is next in line to be migrated to use Base UI for v6. For that work we should keep these in mind cc: @mj12albert

@DiegoAndai DiegoAndai self-assigned this Sep 1, 2023
@DiegoAndai DiegoAndai changed the title [Select] Don't lock the body scroll [Select][material] Don't lock the body scroll Sep 1, 2023
@mnajdova
Copy link
Member

mnajdova commented Sep 1, 2023

@DiegoAndai your demo looks great, but I think this issue is about having a non-modal select, per #17353 (comment).

In https://codesandbox.io/s/issue-17353-scrollable-text-field-select-forked-8hq5xr, there is still aria-hidden=true on all the elements of the page, and most importantly, can't focus on the input in one click it takes two.

Let's create a new issue about this, seems like it is a bit different than the original issue. @DiegoAndai please include the original comment too. We can generalize the issue to all components that use modals, to allow a modal and non-modal options. I have partially covered this in #38630

@DiegoAndai
Copy link
Member

I created a new issue to track the non-modal select option for v6: #38756.

Do you think we can close this one now, @oliviertassinari?

@oliviertassinari
Copy link
Member

I think that this issue is a subset of #38756 (cover more components). I mean, I fail to understand why people would land here to only ask to allow the scrollbar and not to allow focus on the page too, or keeping any modal behavior. If we close this one, we would lose the 👍, I think it's better open for visibility.

@oliviertassinari oliviertassinari changed the title [Select][material] Don't lock the body scroll [Select][material] Don't lock the body scroll and make it non-modal select Sep 28, 2023
@oliviertassinari oliviertassinari changed the title [Select][material] Don't lock the body scroll and make it non-modal select [Select][material-ui] Don't lock the body scroll and make it non-modal select Sep 28, 2023
@samueltesfayegari
Copy link

added this

body { overflow: auto !important; }

To overcome any CSS or JavaScript preventing the body from scrolling or locking the scroll bar.

But I ended up with the MUI offset of the right margin of the screen, like it fluctuates to the opposite.

I made that to stop the fluctuation from the scrollbar hiding, but MUI still got another offset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module! new feature New feature or request priority: important This change can make a difference
Projects
None yet
Development

No branches or pull requests