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

Update complex-flexbox.html #108

Merged
merged 1 commit into from Nov 5, 2018

Conversation

Projects
None yet
3 participants
@stefsulzer
Copy link
Contributor

stefsulzer commented Nov 2, 2018

Wrapping behavior of the buttons will now visually match the photograph finished example.

Changed line 58 button's flex value to 1 auto. Equivalent to flex: auto but continues above explicit usage of flex-grow/flex-shrink.

Update complex-flexbox.html
Wrapping behavior of the buttons will now visually match the photograph finished example. 

Changed line 58 button's flex value to 1 auto. Equivalent to flex: auto but continues above explicit usage of flex-grow/flex-shrink.
@rachelandrew

This comment has been minimized.

Copy link
Collaborator

rachelandrew commented Nov 2, 2018

I think flex: 1 is correct here - which would set flex-basis to 0, giving equally sized items. Although I didn't build the example.

I think it would be a good idea not to change the text on the page until the PR has been discussed and merged though as now this is inconsistent.

@chrisdavidmills

This comment has been minimized.

Copy link
Collaborator

chrisdavidmills commented Nov 5, 2018

I think I'd prefer to keep this as flex: 1 — I don't think adding auto makes much difference in this case, and in addition it adds the need to explain what auto is doing in this context. I don't think it gains us much, and it adds some possibility of confusion.

@chrisdavidmills

This comment has been minimized.

Copy link
Collaborator

chrisdavidmills commented Nov 5, 2018

Ah, wait. I just read your revision message on MDN about this making it work in Saf/Chrome. I'm doing a U turn; PR merged ;-)

Thanks for helping out with this.

@chrisdavidmills chrisdavidmills merged commit a31974e into mdn:master Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment