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

fallback to max-width and max-height if necessary #3033

Merged
merged 2 commits into from
Sep 25, 2018
Merged

Conversation

antoinerg
Copy link
Contributor

Simple fix to issue #2968

Copy link
Contributor Author

@antoinerg antoinerg left a comment

Choose a reason for hiding this comment

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

It looks like computedStyle.height will never be greater than computedStyle.maxHeight because it's the computed style ie. css logic was applied.

The problem we're solving here is the rare case where a CSS developer didn't specify the height of the container. In that case, newHeight is 0 and the final height ends up with the default value of 450 which is sometimes too large and overflows the div.

src/plots/plots.js Show resolved Hide resolved
@etpinard
Copy link
Contributor

It looks like computedStyle.height will never be greater than computedStyle.maxHeight because it's the computed style ie. css logic was applied.

Thanks for the info!

Let's 💃

@antoinerg antoinerg merged commit 4b52e42 into master Sep 25, 2018
@antoinerg antoinerg deleted the 2968-max-dim branch September 25, 2018 14:01
@antoinerg
Copy link
Contributor Author

Closes #2968

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants