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

[Modal] Issue with overflow-y: scroll on <html>

Open nuanyang233 opened this issue 3 years ago • 12 comments

Summary 💡

Add scrollContainer to support custom scroll lock containers.

Examples 🌈

Our product is a browser plugin that enables some features on social platforms. We have implemented some features on Twitter and used dialog. But since twitter implements overflow on the html node.

image

Motivation 🔦

However, scrollLock currently only works on the body node. And developers cannot customize the nodes that require scroll lock.

So I implemented a simple patch.

nuanyang233 avatar Sep 29 '22 10:09 nuanyang233

Could you please provide a simple codesandbox showing the problem so we can reproduce the issue and verify any potential solutions?

michaldudak avatar Oct 14 '22 12:10 michaldudak

Well, I guess I don't know how to provide an example, it's really simple, I just want to use <html> node as a container, like the pull request submitted by Jack works that adds a scrollContainer prop to support custom container.

nuanyang233 avatar Oct 20 '22 15:10 nuanyang233

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

github-actions[bot] avatar Oct 29 '22 15:10 github-actions[bot]

This should already be supported, I did the work in #17972.

oliviertassinari avatar Oct 29 '22 15:10 oliviertassinari

This should already be supported, I did the work in #17972.

I think it didn't solve our's problem. We're already using the mui version that contains #17972 but it didn't help. I opened a PR in #34697

Jack-Works avatar Oct 29 '22 15:10 Jack-Works

A reproduction that demonstrates that it already works: https://codesandbox.io/s/nervous-sutherland-ism1ch?file=/demo.tsx

oliviertassinari avatar Oct 29 '22 15:10 oliviertassinari

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

github-actions[bot] avatar Oct 29 '22 16:10 github-actions[bot]

I suspect that If you have an extension, the container of the portals/modals can't be the <body>, you have to set something that is a bit deeper in the DOM tree. However, the Modal relies on the container to be the <body> to handle the <html> overflow case, hence it breaks. If you can provide a reproduction, it would be great!

Having a new scrollLockContainer prop could make sense. We would only need to make sure that it still makes sense with the need to set the rest of the page inert:

Screenshot 2022-10-29 at 18 22 28

https://www.w3.org/WAI/ARIA/apg/patterns/dialogmodal/

and that it also makes sense to disable the scroll on something that isn't the <body>/<html>, because the alternative fix could be to traverse the tree up to find these elements, instead of using the container and needing scrollLockContainer.


Anyway, I'm leaving this discussion here. I have only engaged here because I was fixing the bot that handles Screenshot 2022-10-29 at 18 27 07 with https://github.com/mui/material-ui/commit/cfb777024fee1fdb0ee020c4cbcf254f5701de09. Handling this is no longer in my scope.

oliviertassinari avatar Oct 29 '22 16:10 oliviertassinari

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

github-actions[bot] avatar Nov 05 '22 17:11 github-actions[bot]

hello! @oliviertassinari please re-open this issue please! I have made a partially reproduction environment here:

https://codesandbox.io/s/blissful-bassi-0bjynp

There are two problems with the current implementation in our environment:

  • The padding-right is not added to body or html.
  • I need overflow: hidden; to be added on the HTML element, instead of the body element (note: you can try to open the Twitter timeline, scroll some pages, and then add overflow: hidden; on the body element, you will find the whole list disappears)

Adding a scrollContainer can solve this problem, thanks!

Jack-Works avatar Nov 21 '22 10:11 Jack-Works

I can reproduce the issue: https://codesandbox.io/s/vigilant-glitter-7i5plg?file=/src/index.tsx

oliviertassinari avatar Nov 21 '22 10:11 oliviertassinari

@oliviertassinari we also need specific styles in the root html that affect overflow behavior.

Our use case is to avoid jumping the content when the content is long enough to add a scrollbar. We have the content of the website to be center aligned with margin-left and -right set to auto.

Our css looks a bit like this:

/* ================== Scrollbars ================== */
/*
 * Always show the scrollbar to allocate the space.
 * Safari will respect this only if in the OS X settings the user
 * has selected "Show scroll bars: Always"
*/
html {
  overflow-y: scroll;
}

/*
 * Chrome, Edge and firefox will support this and when it overflows
 * the gutter will be stable
 * https://caniuse.com/mdn-css_properties_scrollbar-gutter
 */
@supports (scrollbar-gutter: stable) {
  html {
    overflow-y: auto;
    scrollbar-gutter: stable;
  }
}

/*
 * Chrome and Edge will support this and when it overflows
 * the gutter will be auto and the scroll bar will be an overlay
 * https://caniuse.com/css-overflow-overlay
 */
@supports (overflow-y: overlay) {
  html {
    overflow-y: overlay;
    scrollbar-gutter: auto;
  }
}

The issue with this is that when a <Menu /> component sets the body overflow to hidden, the scroll lock feature breaks. If the overflow: hidden was applied to the html element it would solve our issue.

Do you have any suggestions?

(We also have a mui-x license but I think it doesn't apply here)

triglian avatar May 19 '23 14:05 triglian