Skip to content

fix Dialog and Drawer scroll lock - #1668

Open
mayank99 wants to merge 2 commits into
mainfrom
mayank/dialog-scrolllock
Open

fix Dialog and Drawer scroll lock#1668
mayank99 wants to merge 2 commits into
mainfrom
mayank/dialog-scrolllock

Conversation

@mayank99

@mayank99 mayank99 commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Changes

This fixes #1656 by adding CSS overflow: clip to the <html> and <body> elements when a Dialog or Drawer is open. This is done using a compound :has selector, similar to one used in ~reset.css.

Normally, this would be handled by the Modal JS. However, as pointed out in #1656 (comment), the scroll container is calculated incorrectly when the container prop is set. To prevent a stray overflow: hidden from being added to the portal container, I've set disableScrollLock to true (for both Dialog and Drawer).

Notes

  • While scroll lock should only happen for modal dialogs, this is not an immediate concern because non-modal dialogs are not currently supported.
  • I would have liked to handle this more generally for Modal based on the value of disableScrollLock, but couldn't find a way. See comment.
  • iOS Safari has some weirdness with overflow: clip. See comment 1 and comment 2.

Testing

Confirmed that the page is not scrollable when a Dialog or Drawer is open.

Also tested that scroll is not over-aggressively blocked when using keepMounted or scroll="body" props on Dialog.

Deploy preview

@mayank99 mayank99 self-assigned this Jul 29, 2026
@mayank99
mayank99 requested a review from a team as a code owner July 29, 2026 16:30
@mayank99
mayank99 requested review from FlyersPh9 and angrycat9000 and removed request for a team July 29, 2026 16:30
@mayank99
mayank99 force-pushed the mayank/dialog-scrolllock branch 3 times, most recently from 32aa79f to 605aad6 Compare July 29, 2026 16:51
@mayank99

mayank99 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Looking into handling this more generally for Modal, so that it also fixes Drawer (#1656 (comment)).

@mayank99
mayank99 marked this pull request as draft July 29, 2026 17:23
angrycat9000
angrycat9000 approved these changes Jul 29, 2026
View reviewed changes

@angrycat9000 angrycat9000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works as expected.

FlyersPh9
FlyersPh9 approved these changes Jul 29, 2026
View reviewed changes

@FlyersPh9 FlyersPh9 left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tested:

@mayank99
mayank99 force-pushed the mayank/dialog-scrolllock branch from 605aad6 to 17e7ea7 Compare July 29, 2026 21:00
@mayank99

mayank99 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Looking into handling this more generally for Modal, so that it also fixes Drawer (#1656 (comment)).

I've looked into this and found it difficult to handle in a more general way. There is no reliable selector that I can use. I would have liked to target a custom class which is conditionally set based on the disableScrollLock prop. I was unable to add this class because Modal does not fully support slotProps, and using slots was breaking the implementation.

In the end, I decided to simply duplicate the CSS for Drawer. (diff)

@mayank99
mayank99 force-pushed the mayank/dialog-scrolllock branch from 17e7ea7 to b4078be Compare July 29, 2026 21:16
@mayank99 mayank99 changed the title fix Dialog scroll lock fix Dialog and Drawer scroll lock Jul 29, 2026
@mayank99
mayank99 marked this pull request as ready for review July 29, 2026 21:17
@mayank99
mayank99 requested a review from GerardasB July 29, 2026 21:17
@mayank99 mayank99 mentioned this pull request Jul 29, 2026
Open
@mayank99

mayank99 commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

iOS Safari: Noticed the address bar still resizes on upwards scroll but I don't think we have any control over that. https://github.com/user-attachments/assets/229d447e-3ed5-4c5d-9170-f18d25a05c68

@FlyersPh9 This is a very interesting finding, thanks for testing.

I think the "modern" way to solve this is by using overscroll-behavior: contain, which now works on overflow: hidden elements (shipped in Chrome 144 and Firefox 150, but not yet implemented in Safari).

Once this becomes widely available, we might be able to replace the current implementation with this:

.MuiModal-root {
  overflow: hidden;
  overscroll-behavior: contain;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FlyersPh9 FlyersPh9 FlyersPh9 approved these changes
@GerardasB GerardasB GerardasB approved these changes
@angrycat9000 angrycat9000 angrycat9000 approved these changes

Assignees

@mayank99 mayank99

Labels

pkg: @stratakit/mui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dialog does not block scroll on the page underneath

4 participants

@mayank99 @FlyersPh9 @GerardasB @angrycat9000