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.

Sign up for GitHub

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

Jump to bottom

[ModalManager] Lock body scroll when container is inside shadow DOM #33168

Merged
merged 2 commits into from Jun 22, 2022
Merged

[ModalManager] Lock body scroll when container is inside shadow DOM #33168

merged 2 commits into from Jun 22, 2022

Conversation

jacobweberbowery
Copy link
Contributor

@jacobweberbowery jacobweberbowery commented Jun 16, 2022 •
edited

#17473 (comment)

If you set a modal's container to an element inside a shadow DOM, MUI will try to lock scrolling on the container, rather than document.body.

This locks document.body, only in the case where the modal container's immediate parent is the shadow root. This works for the example in the guide, where the modal container shadowRootElement is a child of shadowContainer.

@jacobweberbowery
if container's parent is shadow root,
bb8a654 
lock document body scroll instead
@jacobweberbowery jacobweberbowery changed the title if container's parent is shadow root, [ModalManager] Lock body scroll when container is inside shadow DOM Jun 16, 2022
@mui-bot
Copy link

mui-bot commented Jun 16, 2022 •
edited

Details of bundle changes

Generated by 🚫 dangerJS against 443a544

@jacobweberbowery jacobweberbowery marked this pull request as ready for review June 16, 2022 01:24
@jacobweberbowery jacobweberbowery mentioned this pull request Jun 16, 2022
Closed
@cherniavskii
Copy link
Member

Before: https://codesandbox.io/s/shadow-dom-forked-xgqijx
After: https://codesandbox.io/s/shadow-dom-forked-192tjn

@cherniavskii cherniavskii added new feature New feature or request component: modal This is the name of the generic UI component, not the React module! labels Jun 20, 2022
mnajdova
mnajdova reviewed Jun 20, 2022
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Great PR description & test 👍 It's a very good first contribution to MUI 👌 Thank you for working on it!

@mnajdova mnajdova merged commit 6b9970e into mui:master Jun 22, 2022
@Jack-Works Jack-Works mentioned this pull request Sep 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnajdova mnajdova mnajdova approved these changes

Assignees
No one assigned
Labels
component: modal This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jacobweberbowery @mui-bot @cherniavskii @mnajdova