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

[Popover] Fix max height issue in some mobile browsers #11349

Merged
merged 1 commit into from
May 15, 2018
Merged

[Popover] Fix max height issue in some mobile browsers #11349

merged 1 commit into from
May 15, 2018

Conversation

gaborcs
Copy link
Contributor

@gaborcs gaborcs commented May 12, 2018

Viewport height is taller than the visible part of the document in some mobile browsers.
https://nicolas-hoizey.com/2015/02/viewport-height-is-taller-than-the-visible-part-of-the-document-in-some-mobile-browsers.html

Before:
screenshot_20180512-132154

After:
screenshot_20180512-133052

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: Popover The React component. labels May 12, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented May 12, 2018

@gaborcs This change looks good to me. I have been wondering about it too in the past. Shouldn't we update this line too?
https://github.com/mui-org/material-ui/blob/911b0fc2d71aa5d44426c2def0d71cac49559864/packages/material-ui/src/Menu/Menu.js#L26

Unless this CSS can simply be hosted by the Popover instead of the Menu?

@gaborcs
Copy link
Contributor Author

gaborcs commented May 12, 2018

Shouldn't we update this line too?

I guess we should.

Unless this CSS can simply be hosted by the Popover instead of the Menu?

Do you mean popovers should have a max height of calc(100% - 96px) by default? The reasoning for being able to dismiss menus sounds applicable to other popovers as well, so I'd support that change.

@oliviertassinari oliviertassinari changed the base branch from v1-beta to master May 12, 2018 18:19
@oliviertassinari oliviertassinari self-assigned this May 15, 2018
@oliviertassinari oliviertassinari merged commit e08f44f into mui:master May 15, 2018
@oliviertassinari oliviertassinari changed the title [Popover] fix max height issue in some mobile browsers [Popover] Fix max height issue in some mobile browsers May 15, 2018
@oliviertassinari
Copy link
Member

@gaborcs Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Popover The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants