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: Correct Layout Behavior for Combined align-content and align-items #1742

Conversation

phuccvx12
Copy link
Contributor

This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in Issue #1739.

Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

Copy link

vercel bot commented Nov 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2024 2:27am

@facebook-github-bot
Copy link
Contributor

Hi @phuccvx12!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 14, 2024
@NickGerleman
Copy link
Contributor

Thank you for the PR! I’m away until next week, where I could look closer, but @joevilches or @rozele may be able to look in the meantime. We have some existing coverage around here, and this adds more tests, which is great!

@NickGerleman
Copy link
Contributor

Triggering the CI, it looks like the generated tests might be out of date with the fixture. That usually means it was run on an earlier revision, or more rarely, that the used version of Chrome in CI comes up with different results (Chrome does sometimes change behavior in edge cases to align closer to the spec).

@facebook-github-bot
Copy link
Contributor

@zeyap has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@phuccvx12
Copy link
Contributor Author

Triggering the CI, it looks like the generated tests might be out of date with the fixture. That usually means it was run on an earlier revision, or more rarely, that the used version of Chrome in CI comes up with different results (Chrome does sometimes change behavior in edge cases to align closer to the spec).

The Validate Tests job failed on the CI, but it passed when I ran it both locally and on the GitHub runner. Here’s the link to the run: Validate Tests. Could you please check if this might be an environment-specific issue on the CI? Let me know if any further action is needed.

@joevilches
Copy link
Contributor

@phuccvx12 I ran yarn gentest locally around 7 hours ago and did not see any tests need to change. I also recently added a change that is also failing. Likely an issue with the CI I can take a look

@joevilches
Copy link
Contributor

@phuccvx12 I tried again today and was able to repro the Chrome changes. #1746 should fix this action

zeyap pushed a commit to zeyap/react-native that referenced this pull request Nov 19, 2024
Summary:
This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).

# Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

# Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

# Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

X-link: facebook/yoga#1742

Reviewed By: joevilches

Differential Revision: D65953882

Pulled By: zeyap
@phuccvx12 phuccvx12 force-pushed the fix-align-content-stretch-align-items branch from 0ac9d56 to 2f2fd5b Compare November 20, 2024 02:24
zeyap pushed a commit to zeyap/react-native that referenced this pull request Nov 20, 2024
…ms (facebook#47732)

Summary:

This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).

# Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

# Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

# Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

X-link: facebook/yoga#1742

Reviewed By: joevilches

Differential Revision: D65953882

Pulled By: zeyap
zeyap pushed a commit to zeyap/react-native that referenced this pull request Nov 21, 2024
…ms (facebook#47732)

Summary:

This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).

# Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

# Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

# Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

X-link: facebook/yoga#1742

Reviewed By: joevilches

Differential Revision: D65953882

Pulled By: zeyap
@facebook-github-bot
Copy link
Contributor

@zeyap merged this pull request in 77c9987.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 21, 2024
…ms (#47732)

Summary:
Pull Request resolved: #47732

This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).

# Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

# Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

# Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

X-link: facebook/yoga#1742

Reviewed By: joevilches

Differential Revision: D65953882

Pulled By: zeyap

fbshipit-source-id: 7e12a21b1d442b35c3f3536cad32dc4b82130d15
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Nov 21, 2024
Summary:
X-link: facebook/react-native#47732

This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).

# Changes Made:

Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.

Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.

# Testing:

All existing tests pass successfully.

New test cases confirm that the layout behaves as expected when align-content and align-items are used together.

# Impact:

This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.

X-link: facebook/yoga#1742

Reviewed By: joevilches

Differential Revision: D65953882

Pulled By: zeyap

fbshipit-source-id: 7e12a21b1d442b35c3f3536cad32dc4b82130d15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Merged Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants