Commit Graph

4 Commits

Author SHA1 Message Date
Nick Gerleman
c60050d0cb Fixup hack for flex line size calculation (#1380)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1380

X-link: https://github.com/facebook/react-native/pull/39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on https://github.com/facebook/yoga/pull/1188

Reviewed By: javache

Differential Revision: D49260049

fbshipit-source-id: 218552c5ff938668b9f257df7a1493e13ded4d0d
2023-09-14 20:26:31 -07:00
Eric Rozell
ca4cf852aa Update watir to 7.2.0
Summary: I was not able to run `ruby gentest.rb` from Yoga. Updating watir seems to fix the issue.

Reviewed By: shwanton, NickGerleman

Differential Revision: D46687292

fbshipit-source-id: 9922996144aa23fc7fa2f0dcb372367121689598
2023-06-13 12:40:09 -07:00
Nick Gerleman
c2a0ccf0d4 Add tests for row-gap and column-gap
Summary:
This adds the fixtures from https://github.com/facebook/yoga/pull/1116 and generates tests.

This adds a good amount of coverage, but I plan to follow up with a diff adding a bit more, e.g. for interactions with flex direction of column when we should no-op, etc. I also discovered the current fixtures do not allow testing shorthand props like "gap" without changes.

This also updates the `webdrivers` gem to respond to a break with chromedriver on m1 macs from 4 days ago https://github.com/titusfortner/webdrivers/pull/239.

Reviewed By: yungsters

Differential Revision: D39922413

fbshipit-source-id: dfc7bda894be8dfcb24e25c19a4df0b09a72ce7e
2022-10-13 08:18:49 -07:00
Nick Gerleman
fd180de774 Fix Generation of Tests from Fixtures
Summary:
https://github.com/facebook/yoga/pull/1116 added a change to the test generator "gentests.rb" to support a newer version of chromedriver, along with a change to the enum generator (not touched in this diff) to produce code consistent with the current tests, which seem to have been manually edited since last generation.

I had trouble running the test generator locally, because it relies on unversioned third-party dependencies, whose APIs change. Looking at source history, it seems like each time someone wants to run the script, they end up updating its syntax to match whatever versions they pull in.

This change adds a Gemfile and lock so that that the version of "watir" is locked, and so that we will also automatically pull in a consistent "chomedriver" version via the "webdrivers" gem. It includes the updates from the PR to be consistent with already output tests, and I have also updated the copyright header generation to no longer create lint warnings on newly generated tests (some of the previous ones were fixed manually it looks like).

The test generator would still produce bodies which would fail clang-format, and were manually edited (causing generation to emit new lint warnings), so I updated the generator to suppress clang-format in the body of the generated files.

Three tests, around the interaction of minimum dimensions and flexible children produce different results in Chrome now compared to when the tests were added, so running `gentests.rb` creates tests which break UTs. This doesn't seem like any sort of rounding, or device specific difference, so I have disabled these tests for now. While digging around, it does look like Chrome periodically will fix bugs in its own layout implementation which cause differences, like https://bugs.chromium.org/p/chromium/issues/detail?id=927066

Reviewed By: rozele, Andrey-Mishanin

Differential Revision: D39907416

fbshipit-source-id: f88714ff038b42f935901783452df25eabb6ebb1
2022-09-29 22:25:24 -07:00