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

auto is not a valid value for min/max height #6509

Merged
merged 2 commits into from Oct 19, 2018

Conversation

@angusmcleod
Copy link
Contributor

commented Oct 18, 2018

Discovered through a related style issue with the Custom Wizard Plugin.

https://meta.discourse.org/t/custom-wizard-plugin/73345/209?u=angus

@discoursebot

This comment has been minimized.

Copy link

commented Oct 18, 2018

You've signed the CLA, angusmcleod. Thank you! This pull request is ready for review.

@discoursebot

This comment has been minimized.

Copy link

commented Oct 19, 2018

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/custom-wizard-plugin/73345/215

@hellekin

This comment has been minimized.

Copy link

commented Oct 19, 2018

@angusmcleod according to https://developer.mozilla.org/en-US/docs/Web/CSS/min-height, auto is a valid value, but none is not -- it's only available to max-height. We're not ready for fit-content which would be optimal... I guess leaving min-height: auto is fine, it's the max-height: none that needs to be implemented.

@SamSaffron

This comment has been minimized.

Copy link
Member

commented Oct 19, 2018

@awesomerobot your call on what to do here.

@angusmcleod

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2018

@hellekin Auto is not valid for max-height, which is the main issue.

I've removed the additional change to min-height.

@awesomerobot awesomerobot merged commit 85ef8e5 into discourse:master Oct 19, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@awesomerobot

This comment has been minimized.

Copy link
Member

commented Oct 19, 2018

Merged. Get that invalid value out of here!

Thanks @angusmcleod

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