Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1823
X-link: https://github.com/facebook/react-native/pull/52348
Fixes https://github.com/facebook/yoga/issues/1819
Yoga has a fast path when measuring a node, if it thinks it knows its dimensions ahead of time.
This path has some eroneous logic, to set both axis to owner size, if *either* will evaluate to zero, while having an `YGMeasureModeAtMost`/`FitContent` constraint. This means that if a node is given a zero width, and Yoga later measures with with `FitContent`, its height will become the maximum allowable height, even if it shouldn't be that large.
We can fix this, by only allowing if both axis are this fixed case, instead of just one.
This bug has existed for about a decade (going back to at least D3312496).
Changelog:
[General][Fixed] - Fix possible invalid measurements with width or height is zero pixels
Reviewed By: yungsters
Differential Revision: D76851589
fbshipit-source-id: 6f5a0e6beccc51f591726c9e83e9b90f3350ed0f
Summary:
X-link: https://github.com/facebook/react-native/pull/52073
Pull Request resolved: https://github.com/facebook/yoga/pull/1820
Fixes https://github.com/facebook/yoga/issues/1819
Yoga has a fast path when measuring a node, if it thinks it knows its dimensions ahead of time.
This path has some eroneous logic, to set both axis to owner size, if *either* will evaluate to zero, while having an `YGMeasureModeAtMost`/`FitContent` constraint. This means that if a node is given a zero width, and Yoga later measures with with `FitContent`, its height will become the maximum allowable height, even if it shouldn't be that large.
We can fix this, by only allowing if both axis are this fixed case, instead of just one.
This bug has existed for about a decade (going back to at least D3312496).
Changelog:
[General][Fixed] - Fix possible invalid measurements with width or height is zero pixels
Reviewed By: yungsters
Differential Revision: D76793705
fbshipit-source-id: ea4c00e688912a58c08801e4a14ddf1b293a5d86
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1768
Depending on the flex direction text will either be capped to the measured size or to the longest word, so I added that functionality
Reviewed By: mlord93
Differential Revision: D67106199
fbshipit-source-id: 0b4691768809004043a847f3fc5f7b94e92f1575
Summary:
X-link: https://github.com/facebook/react-native/pull/47973
Gap can be styled using both `points` and `percentages`, but YGNodeStyleGetGap currently returns a float value.
To maintain alignment with the `padding` and `margin` functionalities and allow it to be handled in bridging code, this function has been updated to return YGValue.
Pull Request resolved: https://github.com/facebook/yoga/pull/1753
Reviewed By: joevilches
Differential Revision: D66513236
Pulled By: NickGerleman
fbshipit-source-id: b7110855c037f20780f031f22a945bde4446687d
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/47895
Pull Request resolved: https://github.com/facebook/yoga/pull/1750
These APIs were only added so that we could do TDD as we work on intrinsic sizing functionality. As of right now they do nothing. We are aiming on publishing a new version of Yoga soon so for the time being we are going to back these out so as not to confuse anyone with this new functionality. Ideally we get to a point where we have some temporary experimental header to stage these in but this is a bit time sensitive so just backing out for now
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66332307
fbshipit-source-id: 1d596964e0c893091c541988506e8b80fa6d1957
Summary:
X-link: https://github.com/facebook/react-native/pull/47896
Pull Request resolved: https://github.com/facebook/yoga/pull/1752
These APIs were only added so that we could do TDD as we work on intrinsic sizing functionality. As of right now they do nothing. We are aiming on publishing a new version of Yoga soon so for the time being we are going to back these out so as not to confuse anyone with this new functionality. Ideally we get to a point where we have some temporary experimental header to stage these in but this is a bit time sensitive so just backing out for now
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66332309
fbshipit-source-id: 793f77dad021fa5e57b52c36ae954307636bcbf0
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/46939
Pull Request resolved: https://github.com/facebook/yoga/pull/1722
tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D64002837
fbshipit-source-id: f15fae9fc0103175e1d85850fc9aa68579989fd3
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1721
X-link: https://github.com/facebook/react-native/pull/46938
The private internals of how we store styles needed to change a bit to support 3 new keyword values. Right now the only other keyword that can be stored is `auto`. As a result there isn't much fancy logic to support storing this and its just stored as a specific type inside of `StyleValueHandle`. There are only 3 bits for types (8 values), so it is not sustainable to just stuff every keyword in there. So the change writes the keyword as a value with a new `keyword` `Type`.
I chose not to put `auto` in there even though it is a keyword since it is a hot path, I did not want to regress perf when I did not need to.
I also make a new `StyleSizeValue` class to store size values - so values for `width`, `height`, etc. This way these new keywords are kept specific to sizes and we will not be able to create, for example, a margin: `max-content`.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63927512
fbshipit-source-id: 7285469d37ac4b05226183b56275c77f0c06996c
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:
X-link: https://github.com/facebook/react-native/pull/46984
Pull Request resolved: https://github.com/facebook/yoga/pull/1725
The legacy (wrong) absolute positioning path positions in two places, including work that is definitely always overwritten in the new absolute layout path.
This came up before for position: static, but we didn't clean this up at the time. This code is also now leading display: contents impl being more annoying.
This diff tries to converge to the more spec correct implementation of positioning here, that also only happens in one place.
Previous path would potentially also incorrectly justify when `justify-content` was non-default, but not handled in the previous few cases? We don't have access to the flexLine at this point later, and apart from the existing tests now passing I reused the new correct logic for justification (spec says we should position child as if its the only child in the container https://www.w3.org/TR/css-flexbox-1/#abspos-items).
I added a new, more scoped errata `AbsolutePositionWithoutInsetsExcludesPadding` to preserve some of the legacy behavior that showed as very breaking.
I also did not try removing `AbsolutePercentAgainstInnerSize` which I suspect would be more breaking than this change.
Changelog:
[General][Breaking] - More spec compliant absolute positioning
Reviewed By: joevilches
Differential Revision: D64244949
fbshipit-source-id: ca97570e0de82e8f0424a0912adfd0b05254559e
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/46800
Pull Request resolved: https://github.com/facebook/yoga/pull/1716
Had a mini heart attack thinking I set the default to content box. Wrote this to double check and it passed. Might as well check it in
Technically the default to BoxSizing.h is ContentBox, but in the style we override that. Regardless I switched that around so border box was the default.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63802722
fbshipit-source-id: 49ed29657c964bc12a2bf70988061ab4599267ec
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:
Pull Request resolved: https://github.com/facebook/yoga/pull/1701
X-link: https://github.com/facebook/react-native/pull/46630
I would like to write some tests for box sizing that will drive a lot of my development as I implement content box. To do that, I need this publicly exposed. Obviously not that ideal since this currently does not do anything. Maybe we can name the value in such a way that its clear it is in development?
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63135970
fbshipit-source-id: 7520823bf925364eae45341531e012e80ec92284
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/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:
Pull Request resolved: https://github.com/facebook/yoga/pull/1672
`gentest-validate` checks signedsource for generated tests. D58307002 adds a new directory under `java/tests/com/facebook/yoga/`, which [confuses](https://github.com/facebook/yoga/actions/runs/9653979292/job/26627690870) the current directory traversal logic.
This replaces the traversal with `glob`, and makes us skip files without a signature, instead of special-casing the Java directory.
Reviewed By: yungsters
Differential Revision: D58987293
fbshipit-source-id: a5640f5faee4aa40879c266211c5e736a0b077be
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:
Pull Request resolved: https://github.com/facebook/yoga/pull/1648
Node made a breaking change in a security release for 18/20 where `spawn()` no longer loads `.bat` files by default. 69ffc6d50d. Execute command in shell.
Reviewed By: javache
Differential Revision: D56230965
fbshipit-source-id: 52e9bd8a76664bd07ea25b6355ac54fcb24cbb9a
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1645
Right now we publish Yoga by transforming in-place, and rewriting the entrypoint to point to the generated vanilla JavaScript.
It is nice to include the original source, e.g. so that users can use sourcemaps when debugging, but putting these files on top of each other has been causing problems, like https://github.com/facebook/yoga/issues/1637#issuecomment-2049099690
This changes the packaging step to instead put all the outputs into a "dist" folder, and point the package entrypoints there. We still include original source for sourcemap usage.
Reviewed By: yungsters
Differential Revision: D56093470
fbshipit-source-id: ecd52dddd9040294aae66747cf1fdf48c7f602e7
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: Remove some outdated information and make sure that npmjs shows the builtin TypeScript typings (now that we no longer have clever resolution schemes).
Reviewed By: javache
Differential Revision: D54788636
fbshipit-source-id: 76e7663924189fd68ac62b27730f44213b13ad85
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1599
tsia. Had to add JS api support for set/get direction
Reviewed By: yungsters
Differential Revision: D54778992
fbshipit-source-id: 346152e1d61c80aa524b515e8b30a96fe32fe7c3
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/1558
X-link: https://github.com/facebook/react-native/pull/42318
AbsolutePositioning -> AbsolutePositioningCatchAll
A bit more clear. This errata is for various issues with positioning absolute nodes. There really isn't a clear description as to what specifically this enables/disables, so I just opted to say "catch all" to indicate that this controls various bugs
Reviewed By: NickGerleman
Differential Revision: D52820117
fbshipit-source-id: 80b77832baf65e68e57ca523c418422dd346ef0f
