Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1759
We just have block based tests right now. Intrinsic sizing is commonly used with text so lets add a few there.
Reviewed By: NickGerleman
Differential Revision: D66662940
fbshipit-source-id: f8b91419c89d22d79a91d3bd8c7da70429c827fb
Summary:
X-link: https://github.com/facebook/react-native/pull/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.
Pull Request resolved: https://github.com/facebook/yoga/pull/1742
Reviewed By: joevilches
Differential Revision: D65953882
Pulled By: zeyap
fbshipit-source-id: 7e12a21b1d442b35c3f3536cad32dc4b82130d15
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1746
Chrome made some changes for how overflowed row-reverse containers are laid out which was causing some issues on CI. I updated them here and skipped the new failing tests which we would want to followup on.
For LTR, the differences are seen below
|Before|After|
|--|
|{F1962694149} | {F1962694151}|
The extra space is now extending past the flex start edge vs flex end. RTL is the opposite. NickGerleman had deviated from the spec back in the day to match Chrome and it seems they made the adjustment recently. T208209388 is tracking the followup to align with the spec again. Basically, there is a notion of fallback alignment when certain justification/alignment values cannot actually apply. Right now we are falling back to flex start in all cases but we should fallback to start sometimes.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66138361
fbshipit-source-id: c46d2e9b0cd297069b9cc544e3bded995e4867a6
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1723
Adds gentest support for intrinsic sizing keywords and creates an initial batch of somewhat interesting tests for these keywords on `width`, `height`, `min-width`, `min-height`, `max-width`, `max-height`, and `flex-basis`
Reviewed By: NickGerleman
Differential Revision: D64145117
fbshipit-source-id: 1e3e7214fab062ab6f260cfe7bdfaf3c0aca3bf7
Summary:
X-link: https://github.com/facebook/react-native/pull/47035
This PR adds support for `display: contents` style by effectively skipping nodes with `display: contents` set during layout.
This required changes in the logic related to children traversal - before this PR a node would be always laid out in the context of its direct parent. After this PR that assumption is no longer true - `display: contents` allows nodes to be skipped, i.e.:
```html
<div id="node1">
<div id="node2" style="display: contents;">
<div id="node3" />
</div>
</div>
```
`node3` will be laid out as if it were a child of `node1`.
Because of this, iterating over direct children of a node is no longer correct to achieve the correct layout. This PR introduces `LayoutableChildren::Iterator` which can traverse the subtree of a given node in a way that nodes with `display: contents` are replaced with their concrete children.
A tree like this:
```mermaid
flowchart TD
A((A))
B((B))
C((C))
D((D))
E((E))
F((F))
G((G))
H((H))
I((I))
J((J))
A --> B
A --> C
B --> D
B --> E
C --> F
D --> G
F --> H
G --> I
H --> J
style B fill:https://github.com/facebook/yoga/issues/050
style C fill:https://github.com/facebook/yoga/issues/050
style D fill:https://github.com/facebook/yoga/issues/050
style H fill:https://github.com/facebook/yoga/issues/050
style I fill:https://github.com/facebook/yoga/issues/050
```
would be laid out as if the green nodes (ones with `display: contents`) did not exist. It also changes the logic where children were accessed by index to use the iterator instead as random access would be non-trivial to implement and it's not really necessary - the iteration was always sequential and indices were only used as boundaries.
There's one place where knowledge of layoutable children is required to calculate the gap. An optimization for this is for a node to keep a counter of how many `display: contents` nodes are its children. If there are none, a short path of just returning the size of the children vector can be taken, otherwise it needs to iterate over layoutable children and count them, since the structure may be complex.
One more major change this PR introduces is `cleanupContentsNodesRecursively`. Since nodes with `display: contents` would be entirely skipped during the layout pass, they would keep previous metrics, would be kept as dirty, and, in the case of nested `contents` nodes, would not be cloned, breaking `doesOwn` relation. All of this is handled in the new method which clones `contents` nodes recursively, sets empty layout, and marks them as clean and having a new layout so that it can be used on the React Native side.
Relies on https://github.com/facebook/yoga/pull/1725
Changelog: [Internal]
Pull Request resolved: https://github.com/facebook/yoga/pull/1726
Test Plan: Added tests for `display: contents` based on existing tests for `display: none` and ensured that all the tests were passing.
Reviewed By: joevilches
Differential Revision: D64404340
Pulled By: NickGerleman
fbshipit-source-id: f6f6e9a6fad82873f18c8a0ead58aad897df5d09
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1719
Right now there is no way to test fixtures with `auto` widths, heights, or flex basis - even though we expose those functions. I updated gentest to generate those functions. Notably, position and margin (the other auto-able props) already account for this.
I also created `YGAutoTest.html` to test this. Not really testing the capabilities of `auto` here, just if we can create a test about it.
Reviewed By: NickGerleman
Differential Revision: D64125522
fbshipit-source-id: 291ec82003cf53f99c21943142a63e2ef30402a5
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1715
X-link: https://github.com/facebook/react-native/pull/46799
Content box impl had a bug where we resolved padding % against the same reference length as the dimensions. Padding should always be against containing block's width. This is also true for width, but not for height, which should be against containing block's height.
This just pipes the width into our box sizing functions.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63787577
fbshipit-source-id: e512338770f25b66506cabab5a7cde8f04397ea0
Summary:
X-link: https://github.com/facebook/react-native/pull/46741
Pull Request resolved: https://github.com/facebook/yoga/pull/1711
box sizing is really just a reinterpretation of what length properties (like `width`, `height`, `max-width`, etc) mean. So to implement this I just add the border and padding if we are in content box when we ask for any of these properties. All the math that gets done by the algorithm is still in border box land, and the layout we return is to be interpreted as the border box (this is actually the expected behavior per https://drafts.csswg.org/css-sizing/#box-sizing). This makes this implementation pretty simple actually.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63416833
fbshipit-source-id: fd76132cf51e8a5092129802c3a12ab24023018b
Summary:
X-link: https://github.com/facebook/react-native/pull/46428
Pull Request resolved: https://github.com/facebook/yoga/pull/1696
We do not validate the aspect ratio to ensure it is non zero and non inf in a lot of places. Per the spec, these values should act like auto. There is no auto keyword, but it is the default so I just set the style to a default FloatOptional in this case
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D62473161
fbshipit-source-id: 6857de819538a7a87ce0a652e99f5a49992921ae
Summary:
X-link: https://github.com/facebook/react-native/pull/46216
Regarding [issue](https://github.com/facebook/react-native/issues/45817) with incorrect layout when `left` is set to `auto`. This PR introduces handling `auto` whenever inline or flex position is checked to be defined and it fixes above issue.
Changelog:
[General][Fixed] - Fix handling 'auto' checks in absolute layout
## Tests:
I have run the provided unit tests and everything passes.
Pull Request resolved: https://github.com/facebook/yoga/pull/1689
Reviewed By: cipolleschi
Differential Revision: D61737876
Pulled By: NickGerleman
fbshipit-source-id: 531199a91c5e122b930b49725ea567cbb1d592ce
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1677
We have Clang Tidy warnings enabled internally, that will flag `const YG*Ref` as misleading, as a const pointer to non-const, instead of non-const pointer to const.
Let's remove all the misleading const in existing tests, and generated tests.
Reviewed By: joevilches
Differential Revision: D59335968
fbshipit-source-id: c66af878904ba7900f8ffcd99162968d90f8e5c7
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1671
This diff adds support for intrinsic sizing in generated tests. This is done by importing a testing font called "Ahem" which, as used, has an exact width and height of 10px for each character. Support has been added for C++, Java, and Javascript generated tests.
Reviewed By: NickGerleman
Differential Revision: D58307002
fbshipit-source-id: e1dcc1e03310d35a32e0c70f71994880d8f7de55
Summary:
X-link: https://github.com/facebook/react-native/pull/44792
Pull Request resolved: https://github.com/facebook/yoga/pull/1663
Fixing https://github.com/facebook/yoga/issues/1658. We had a problem where if a child had a different flex direction than its parent, and it also set a position as a percent, it would look at the wrong axis to evaluate the percent. What was happening was we were passing in the container's mainAxis size and crossAxis size to use to evaluate the position size if it was a percent. However, we matched these sizes with the main/cross axis of the child - which is wrong if the flex direction is different.
I changed it so that the function just takes in ownerWidth and ownerHeight then calls isRow to determine which one to use for the main/cross axis position. This reduces the ambiguity quite a bit imo.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D58172416
fbshipit-source-id: eafd8069e03493fc56c41a76879d1ad9b7e9236d
Summary:
X-link: https://github.com/facebook/react-native/pull/44791
Pull Request resolved: https://github.com/facebook/yoga/pull/1662
This should fix https://github.com/facebook/yoga/issues/1657. Rather insidious bug but we had code like
```
// The total padding/border for a given axis does not depend on the direction
// so hardcoding LTR here to avoid piping direction to this function
return node->style().computeInlineStartPaddingAndBorder(
axis, Direction::LTR, widthSize) +
node->style().computeInlineEndPaddingAndBorder(
axis, Direction::LTR, widthSize);
```
That comment is NOT true if someone sets both the physical edge and relative edge. So like paddingLeft and paddingEnd for RTL. This diff simply pipes the direction to that spot to use instead of hardcoding LTR. Every file changed is just to pipe `direction`.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D58169843
fbshipit-source-id: 5b4854dddc019285076bd06955557edf73ef7ec5
Summary:
X-link: https://github.com/facebook/react-native/pull/43752
Gentest tests started failing because Chrome changed behavior of overflowed align-content container. Spec says should fallback to "safe center", which is really just "start", instead of previous "center" behavior. This changes behavior accordingly.
There is one bit where I think we are doing the wrong thing wrt alignment of flex start vs start (which we don't support yet), but couldn't repro a failing chrome test.
Changelog: [Internal]
Reviewed By: joevilches
Differential Revision: D55617689
fbshipit-source-id: 08f23d198c75f2c2f51ccaa8795289e6e4a92cb8
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1593
X-link: https://github.com/facebook/react-native/pull/43417
There was a bug where we did not position absolute nodes correctly if the static node had a different main/cross axis from the containing node. This fixes that. The change is somewhat complicated unfortunately but I tried to add sufficient comments to explain what is happening
Reviewed By: NickGerleman
Differential Revision: D54703955
fbshipit-source-id: 096c643f61d4f9bb3ee6278d675ebd69b57350d7
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1561
Back when I introduced the inline functions that would get the edge according to the writing direction I swapped some instances of `setLayoutPosition` which wrote to the flexStart edge erroneously. We should basically never read from some inline style and write to the flex edge. This changes them all to use the flex values.
Reviewed By: NickGerleman
Differential Revision: D52921401
fbshipit-source-id: 92b74d652018596134c91827806272ed7418ef6c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1560
I added these when I was trying to debug the Facepile break removing the row-reverse errata caused. Yoga is doing the right thing, and the tests pass. We didn't have this specific coverage before, so add it.
Reviewed By: joevilches
Differential Revision: D52909633
fbshipit-source-id: d1e8f55bb534d76bd7dfdc46a1e1cc6f0a3ca211
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1549
X-link: https://github.com/facebook/react-native/pull/42253
This experimental feature is always false, and with the next diff I will be deleting the branch that actually calls into this. Separating this diff out to simplify the review process.
Reviewed By: NickGerleman
Differential Revision: D52705765
fbshipit-source-id: 705f4aa297eae730af9b44753eb01c9dec385dcf
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1530
Yoga has a shortcut where if a min dimension and max dimension are the same, the value acts as a definite length.
I was curious how browsers handled this.
CSS 2.1 said:
> If the containing block's width depends on this element's width, then the resulting layout is undefined
This is superceded in the CSS box sizing spec. https://www.w3.org/TR/css-sizing-3/#sizing-values
> If, in a particular axis, the containing block’s size depends on the box’s size, see the relevant layout module for special rules on how to resolve percentages. Negative values are invalid.
And later:
https://www.w3.org/TR/css-sizing-3/#cyclic-percentage-contribution
> Sometimes the size of a percentage-sized box’s containing block depends on the intrinsic size contribution of the box itself, creating a cyclic dependency. When calculating the intrinsic size contribution of such a box (including any calculations for a content-based automatic minimum size), a percentage value that resolves against a size in the same axis as the intrinsic size contribution (a cyclic percentage size) is resolved specially:
> If the box is non-replaced, then the entire value of any max size property or preferred size property (width/max-width/height/max-height) specified as an expression containing a percentage (such as 10% or calc(10px + 0%)) that is cyclic is treated for the purpose of calculating the box’s intrinsic size contributions only as that property’s initial value. For example, given a box with width: calc(20px + 50%), its max-content contribution is calculated as if its width were auto. (The percentage is honored as usual, however, during the actual sizing of the box itself; see below.)
> Otherwise, the percentage is resolved against the containing block’s size. (The containing block’s size is not re-resolved based on the resulting size of the box; the contents might thus overflow or underflow the containing block).
So, for the purpose of sizing the parent, the child sized using a percentage does not contribute, but we should be sizing children based on that size.
Yoga does not really work like this right now, but gets the answer right answer for half of these tests.
Reviewed By: yungsters
Differential Revision: D52251601
fbshipit-source-id: 4978b90723130283b00e87bbf49795a4d209174c
Summary:
X-link: https://github.com/facebook/react-native/pull/41964
Pull Request resolved: https://github.com/facebook/yoga/pull/1524
D52087013 (#1513) fixed some issues, including where measuring under max-content or fit-content, align-content stretch would consume the entire available cross-dimensions, instead of only sizing to definite dimension, like the spec dicates.
I missed a case, where flexbox considers a container as having a definite cross-size if it is being stretched, even if it doesn't have a definite length.
https://www.w3.org/TR/css-flexbox-1/#definite-sizes
> 3. Once the cross size of a flex line has been determined, items in auto-sized flex containers are also considered definite for the purpose of layout;
> 1. If a single-line flex container has a definite cross size, the outer cross size of any stretched flex items is the flex container’s inner cross size (clamped to the flex item’s min and max cross size) and is considered definite.
We handle `align-items: stretch` of a flex container after cross-size determination by laying out the child under stretch-fit (previously YGMeasureModeExactly) constraint. This checks that case, and sizing the line container to specified cross-dim if we are told to stretch to it.
We could probably afford to merge this a bit with later with what is currently step 9, where we end up redoing some of this same math.
Reviewed By: yungsters
Differential Revision: D52234980
fbshipit-source-id: 475773a352fd01f63a4b21e93a55519726dc0da7
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1513
X-link: https://github.com/facebook/react-native/pull/41916
Fixes https://github.com/facebook/yoga/issues/1300
Fixes https://github.com/facebook/yoga/issues/1008
This fixes a smattering of issues related to both sizing and aligment of multi-line-containers:
1. We were previously incorrectly bounding the size of each flex line to the min/max of the entire container.
2. Per-line leads were sometimes incorrectly contributing to alignment within the line
3. The cross dim size used for multi-line alignment is not correct, or correctly clamped. If the available size comes from a max constraint, that was incorrectly used instead of a definite size, or size of content. Leads were entirely skipped for min constraint.
Need to test how breaking this is, to see if it might need to go behind an errata.
See related PRs:
1. https://github.com/facebook/yoga/pull/1491
2. https://github.com/facebook/yoga/pull/1493
3. https://github.com/facebook/yoga/pull/1013
Changelog:
[General][Fixed] - Fix Yoga sizing and alignment issues with multi-line containers
Reviewed By: joevilches
Differential Revision: D52087013
fbshipit-source-id: 8d95ad17e58c1fec1cceab9756413d0b3bd4cd8f
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1503
This diff makes it so that our driver will sign all of the generated files to help ensure that they are not edited by hand. Next I will add CI to actually verify the signature
Reviewed By: NickGerleman
Differential Revision: D51966201
fbshipit-source-id: f7e3f4fde1c98832212a448b2dcc8e21be0560c4
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1501
Now that we have `gentest-driver.ts` we can delete the ruby gentest. I also regened all of the tests that have a comment with the wrong file name for where it was generated.
Reviewed By: yungsters, NickGerleman
Differential Revision: D51956567
fbshipit-source-id: d389492e54711cf161dff9e649396cc40f1e5073
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1498
The only instance of ruby in this repository is `gentest.rb` used to generate test cases from html fixtures. This is quite annoying as ruby is not the most popular compared to something like Node and it does not integrate into the rest of our stack. I changed this to use Node.js instead. Instead of `watir` we now use `selenium-webdriver`. `watir` is backed by Selenium so I do not expect anything to change.
Next commits will add command line options, clean up gentest.rb and its references, and change the README
allow-large-files
Reviewed By: yungsters, NickGerleman
Differential Revision: D51874433
fbshipit-source-id: ef8588d48aa7f8b720b57df08738bbd01e9e74a3
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1488
These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing
Reviewed By: NickGerleman
Differential Revision: D51404875
fbshipit-source-id: ed10004968b871c1d033640d75138f00afc15968
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1490
X-link: https://github.com/facebook/react-native/pull/41692
In the previous diffs I fixed problems with justifying absolute nodes. The same issues plague aligning so I fixed them in the same way. Added tests that were failing before but now passing
Reviewed By: NickGerleman
Differential Revision: D51404489
fbshipit-source-id: 604495d651eb67cfdcca40df9d8d3a125c5741a8
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1487
X-link: https://github.com/facebook/react-native/pull/41691
The code here was just wrong. I changed it to be the same logic as the Justify:FlexStart case, but with the flex end sides. Then I get the position for the opposite edge since we need to write to flex start side.
Reviewed By: NickGerleman
Differential Revision: D51383792
fbshipit-source-id: 372835a44edff361dbd84dd92ff9f2ec844b9f9c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1489
X-link: https://github.com/facebook/react-native/pull/41690
Centering involves centering the margin box in the content box of the parent, and then getting the distance from the flex start edge of the parent to the child
Reviewed By: NickGerleman
Differential Revision: D51383625
fbshipit-source-id: 6bbbace95689ef39c35303bea4b99505952df457
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1485
X-link: https://github.com/facebook/react-native/pull/41686
The size of the containing block is the size of the padding box of the containing node for absolute nodes. We were looking at `containingNode->getLayout().measuredDimension(Dimension::Width)` which is the border box. So we need to subtract the border from this.
Added a test that was failing before this change as well
Reviewed By: NickGerleman
Differential Revision: D51330526
fbshipit-source-id: adc448dfb71b54f1bbed0d9d61c5553bda4b106c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1482
X-link: https://github.com/facebook/react-native/pull/41685
This is the final step (that I know of) to get the core features of static working. Here we turn on all of the tests and pass down the correct owner size for the call to `calculateLayoutInternal` that is in `layoutAbsoluteChild`
Reviewed By: NickGerleman
Differential Revision: D51293606
fbshipit-source-id: 972259e7ebecb19b55aef2ef866bd7cb57aaf0ca
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1496
Gentest code has a problem where we try to apply a border in our test when the web browser is not actually adding one. This happens when we do something like `border-top: 10px`. This will actually set the style of the border to `initial` which is just `none`, so nothing renders. This is causing at least 1 test to pass when it actually fails.
I changed it so we ignore setting this value if the style is one of these values. I then re-ran the gentest code and excluded the now failing test (which gets fixed in my static stack).
Reviewed By: NickGerleman
Differential Revision: D51831754
fbshipit-source-id: a325e4a42b2d7cd6f19efc6cd5a2445574467fb7
Summary:
X-link: https://github.com/facebook/react-native/pull/41480
Pull Request resolved: https://github.com/facebook/yoga/pull/1469
The previous version of static didn't do anything inside of Yoga. Now that we're making it do something, this changes the default back to relative so that users with no errata set don't see their deafult styles changing.
Reviewed By: joevilches
Differential Revision: D51182955
fbshipit-source-id: c0ea357694e1367fb6786f1907dfff784b19a4bc
Summary:
X-link: https://github.com/facebook/react-native/pull/41293
Pull Request resolved: https://github.com/facebook/yoga/pull/1446
NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right.
Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here.
Reviewed By: NickGerleman
Differential Revision: D50945475
fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
Summary:
`child.offsetLeft/Top` calculates the offset from child to its nearest positioned ancestor, not its direct parent. These are often the same and have not mattered in the past since we have not supported position static. Since are are in the process of supporting that, we would like our tests to be usable so this adjusts the gentest methodology to only speak the same language as Yoga - that is left/top are always relative to direct parents.
It works by using `getBoundingClientRect().left/top` instead. Then we pass that down to children and subtract it from the childs `getBoundingClientRect()` to get the position relative to the parent. Note we have to round the final result as `child.offsetLeft/Top` is rounded.
Reviewed By: NickGerleman
Differential Revision: D51053629
fbshipit-source-id: 8809588d12953565228ae50fdf38197213c46182
Summary:
I was playing around with absolute children and padding and noticed an issue so adding tests to track.
Made a github issue: https://github.com/facebook/yoga/issues/1436
Reviewed By: yungsters
Differential Revision: D50670457
fbshipit-source-id: 4672d1e8b831a0a42509d95e91178944fc0f5c06
