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

fix(overlay): don't reset global overlay alignment when width is 100% and there's a maxWidth #17842

Open
wants to merge 1 commit into
base: master
from

Conversation

@crisbeto
Copy link
Member

crisbeto commented Nov 29, 2019

A long time ago we introduced some logic that clears the justifyContent from a global overlay if it's width is set to 100%, in order to ensure that the element is flush against the viewport edge. Some time later we added a maxWidth option, but we never accounted for it which means that if an element is set to be width: 100%; maxWidth: '500px', we'll reset the alignment incorrectly. These changes tweak the logic so it only resets if there is no maxWidth or if it's set to 100%.

Fixes #17841.

… and there's a maxWidth

A long time ago we introduced some logic that clears the `justifyContent` from a global overlay if it's `width` is set to 100%, in order to ensure that the element is flush against the viewport edge. Some time later we added a `maxWidth` option, but we never accounted for it which means that if an element is set to be `width: 100%; maxWidth: '500px'`, we'll reset the alignment incorrectly. These changes tweak the logic so it only resets if there is no `maxWidth` or if it's set to 100%.

Fixes #17841.
* particular dimension, based on the intended size of the element and its maximum size.
*/
function shouldBeFlush(value: string|undefined, maxValue: string|undefined): boolean {
return value === '100%' && (!maxValue || maxValue === '100%');

This comment has been minimized.

Copy link
@jelbourn

jelbourn Dec 3, 2019

Member

Does it make sense to also check 100vh / 100vw?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.