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 wrong min-width percentage calculation for child #1015

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

woehrl01
Copy link
Contributor

This PR fixes #872 by using the correct parent values instead of the grandparents.

Unfortunately that fix reveals a bug which causes two existing tests to fail, which are false negatives.

  • percentage_flex_basis_cross_min_height
  • percentage_flex_basis_main_min_width

The underlying bug is a misbehavior from the specs (https://www.w3.org/TR/css-flexbox-1/#algo-main-item).

When determining the flex base size, the item’s min and max main sizes are ignored (no clamping occurs).

Sadly the current algorithm takes the clamping into account. As the generated tests also have changed based on using the latest (83) chrome, it also looks that this has been a previous bug in chrome, too.

Despite the fact this PR is still valid, it just reveals another existing bug. But as Yoga does not apply to the web basis specs all together I suggest having this fix in place instead of using the wrong values for calculation. As the underlying bug (including min values for flex basis) is more predictable.

I also moved one of the tests to a different file to not get overridden by the generator.

@SidharthGuglani I highly appreciate your opinion on that one.
@emilsjolander I know it's a long time, maybe you have an idea here, too. Perhaps I also have an oversight here.

@devsnek
Copy link

devsnek commented May 21, 2021

Any chance of this being merged?

@NickGerleman
Copy link
Contributor

NickGerleman commented Oct 4, 2022

Ooh this explains the test failures I was seeing.

I do think we will want to fix this, but behind a flag. For React Native, we are planning on doing a push for layout conformance, where we know there will be enough changes to require apps to migrate themselves.

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

Successfully merging this pull request may close these issues.

min-width in percent isn't calculated against parent node
4 participants