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

[Backdrop] Render children inside div #17115

Merged
merged 3 commits into from
Aug 23, 2019

Conversation

dominictwlee
Copy link
Contributor

fixes #17084

@@ -9,6 +9,9 @@ export const styles = {
root: {
zIndex: -1,
position: 'fixed',
display: 'flex',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not used Grid as a container?

Copy link
Member

Choose a reason for hiding this comment

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

The idea is to come with a better default. I would expect more people to want to center the child.

Copy link
Contributor

@behnammodi behnammodi Aug 22, 2019

Choose a reason for hiding this comment

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

Do you agree with me?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think what @oliviertassinari meant was that a reasonable default would be to just have a center aligned child. In that case, I don't think Grid is necessary with this use case.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly, it's even better if the usage of the Grid is not required.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mui-pr-bot
Copy link

mui-pr-bot commented Aug 22, 2019

Details of bundle changes.

Comparing: 343ae8e...1fad821

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.03% 🔺 +0.01% 🔺 329,350 329,435 89,946 89,955
@material-ui/core/Paper 0.00% 0.00% 68,774 68,774 20,490 20,490
@material-ui/core/Paper.esm 0.00% 0.00% 62,148 62,148 19,215 19,215
@material-ui/core/Popper 0.00% 0.00% 28,468 28,468 10,177 10,177
@material-ui/core/Textarea 0.00% 0.00% 5,094 5,094 2,137 2,137
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,614 1,614
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,386 16,386 5,829 5,829
@material-ui/core/useMediaQuery 0.00% 0.00% 2,541 2,541 1,058 1,058
@material-ui/lab 0.00% 0.00% 153,314 153,314 46,716 46,716
@material-ui/styles 0.00% 0.00% 51,492 51,492 15,307 15,307
@material-ui/system 0.00% 0.00% 15,658 15,658 4,370 4,370
Button 0.00% 0.00% 78,778 78,778 24,072 24,072
Modal 0.00% 0.00% 14,346 14,346 5,010 5,010
Portal 0.00% 0.00% 2,907 2,907 1,319 1,319
Rating 0.00% 0.00% 70,245 70,245 21,940 21,940
Slider 0.00% 0.00% 74,483 74,483 23,065 23,065
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 53,510 53,510 13,854 13,854
docs.main +0.01% 🔺 +0.02% 🔺 597,188 597,273 190,715 190,757
packages/material-ui/build/umd/material-ui.production.min.js +0.03% 🔺 +0.01% 🔺 300,265 300,350 86,390 86,395

Generated by 🚫 dangerJS against 1fad821

@oliviertassinari oliviertassinari added component: backdrop This is the name of the generic UI component, not the React module! new feature New feature or request labels Aug 22, 2019
@oliviertassinari
Copy link
Member

@dominictwlee Thanks

RohanM added a commit to conversation/ui that referenced this pull request Nov 13, 2020
RohanM added a commit to conversation/ui that referenced this pull request Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: backdrop This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BackDrop component does not render children
4 participants