Add support for gap, column-gap, and row-gap #1116

jacobp100 · 2021-12-20T19:01:38Z

Only supports point values - percents don't work, and shouldn't be exposed as part of the API (similar to the current situation with borders).

I've added tests, and they all work.

I'm not 100% sure I've handled the bindings all correctly. I did run enums.py - but this created white a lot of extra changes too.

Let me know if there's anything I've missed.

Fixes #812

intergalacticspacehighway · 2021-12-21T07:24:46Z

Great work 👏 I have been trying a similar approach but got stuck with setting child's cross-axis offset. Glad to see this solution. I've added a PR for a fix. I'll be testing it more as I get time.

Flex gap fixes

intergalacticspacehighway · 2021-12-21T11:42:08Z

This looks good! Can you create a PR on this branch with yoga changes, I'll update the React native bindings to match with this version. It'll make it easier to test stuff and get feedback on this PR. AFAIKS React native has Yoga here. Would be cool to figure out if there's an automated way to get a specific Yoga version in React native.

jacobp100 · 2021-12-21T16:46:15Z

I’m away from my laptop until the new year now. If you want I can give you write access to my fork if that makes things easier!

intergalacticspacehighway · 2021-12-21T17:20:38Z

Cool. no worries. I'll figure it out. Happy holidays! 🥳

jacobp100 · 2021-12-22T10:27:31Z

You too! Feel free to ping me if you need me to sanity check anything or are stuck with anything!

intergalacticspacehighway · 2021-12-23T04:51:20Z

yoga/Yoga.cpp

            collectedFlexItemsValues.itemsOnLine;
-        leadingMainDim = betweenMainDim / 2;
+        betweenMainDim += leadingMainDim * 2;


could be a nit pick - this can also be represented as below

case YGJustifySpaceAround: { const float betweenMainDimWithoutGap = collectedFlexItemsValues.remainingFreeSpace / collectedFlexItemsValues.itemsOnLine; // Space on the edges is half of the space between elements leadingMainDim = betweenMainDimWithoutGap / 2; betweenMainDim += betweenMainDimWithoutGap; break; }

need to check if cost of multiplication > cost of defining a scope and a variable.

Deja vu! I actually did it like this originally - then I thought this path is used a lot less than the flex start, and the extra variable could affect the entire function.

Let me know what you think anyway. I’m not adverse to changing it

As a thought, we could do,

leadingMainDim = collectedFlexItemsValues.remainingFreeSpace / collectedFlexItemsValues.itemsOnLine; betweenMainDim += leadingMainDim; leadingMainDim *= 0.5;

yeah, this might be good as it prevents additional multiplication, but we can wait for someone with c++ performance experience to review it 😅

jaraujocds · 2022-03-21T19:35:03Z

We need this 🙏🏻

davbrito · 2022-03-25T19:22:34Z

Any update on this?

erisvaldojunior · 2022-04-18T21:29:39Z

Any update on this?

kvbalib · 2022-04-21T10:07:27Z

+1!

gmantovani97 · 2022-05-25T15:20:24Z

Someone approve it please.

wood1986 · 2022-06-09T20:24:32Z

any update?

jacobp100 · 2022-06-09T20:33:17Z

jacobp100 · 2022-06-09T20:35:50Z

For real though, I get notified for every comment. I don't have any updates.

ShivamJoker · 2022-07-03T11:56:08Z

Common Facebook devs, its been more than 6 months and you guys just can't review & click that approve button

etasdemir · 2022-07-17T17:54:31Z

Still waiting for approvement

NickGerleman · 2022-08-01T22:53:25Z

Hey folks! I want to apologize for the lack of engagement on this issue.

Yoga has been without a full-time maintainer. PRs like this show that the community is still finding important issues. I'm going to import this PR, and try to find a reviewer.

There is a spiritual successor to Yoga, that has been successfully adopted by other teams at Meta. We have been looking at whether it can help provide a path to enable new capabilities and support for layout in react-native.

facebook-github-bot · 2022-08-01T23:01:41Z

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

intergalacticspacehighway · 2022-08-02T04:21:56Z

Thanks @NickGerleman

There is a spiritual successor to Yoga, that has been successfully adopted by other teams at Meta.

Cool! would be awesome to learn more about this and If there are any plans to open source the implementation :D

jacobp100 · 2022-08-02T08:41:59Z

@NickGerleman Thanks so much for taking this on! If you have any questions, me and @intergalacticspacehighway will be here to help

NickGerleman · 2022-09-27T18:21:20Z

I left a comment in react-native-community/discussions-and-proposals, that we've decided not to go forward with flexlayout yet in react-native. This reopens the possibility to add new capabilities to Yoga, without the same concern of giving something to the community to then take it back.

The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification.

There are a couple of more superficial issues. E.g. formatting, missing methods on the node to set/get gap in Java/C# (where the generated tests use that), or calling a function YGFlexDirectionToString instead of YGGapToString. I'm working to fix those up at the moment, but we should be able to merge this soon 🙂.

Summary: facebook/yoga#1116 adds a new enum. The enum generator is out of date with copyright header, and some codemods, but it also looks like there were manual changes, types added, etc since generation. I fixed up the script to incorporate generating the changes folks made manually, and also added an enum that was previously only added manually to the C ABI. Changelog: [General][Fixed] - Fixup Yoga Enum Generator Reviewed By: yungsters Differential Revision: D39922252 fbshipit-source-id: b678fa9a43a896873d8c434745bdaf3f16fd991f

Summary: This adds the YGGutter enum, used to choose between row/column gap variants (row-gap, column-gap, gap). This used later in changes from facebook/yoga#1116, in the APIs which deal with setting gap on style on yoga node. Note the original PR called this `YGGap`, but this ending up leading to a couple public method signatures that could appear ambiguous: 1. `SetGap(YGGap gap, float gapLength)`: Enums like `YGAlign` are the vaues for an `align` prop. `YGGap` controls the variant of the gap (like `YGEdge` does for left/right/top/bottom variants). So the enum reads as if it is the `gapValue`, and it looks like we have two of the same parameter. 2. `SetGap(YGGap gapDirection, float gap)`: This is misleading, because the direction gaps flow is the cross-axis of flex-direction. 3. `GetGap(YGGap gap)`: `gap` is the variant, but looks like an out param. The [CSS Box Alignment](https://www.w3.org/TR/css-align-3/#column-row-gap) spec refers to these gaps as "Gutters", which removes the ambiguity. Changelog: [General][Added] - Add YGGutter Enum Reviewed By: yungsters Differential Revision: D39922412 fbshipit-source-id: 4b0baf800fecb3d03560a4267c7fb4c4330fd39e

Summary: #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

Summary: #1116 adds a new enum. The enum generator is out of date with copyright header, and some codemods, but it also looks like there were manual changes, types added, etc since generation. I fixed up the script to incorporate generating the changes folks made manually, and also added an enum that was previously only added manually to the C ABI. Changelog: [General][Fixed] - Fixup Yoga Enum Generator Reviewed By: yungsters Differential Revision: D39922252 fbshipit-source-id: b678fa9a43a896873d8c434745bdaf3f16fd991f

Summary: This adds the YGGutter enum, used to choose between row/column gap variants (row-gap, column-gap, gap). This used later in changes from #1116, in the APIs which deal with setting gap on style on yoga node. Note the original PR called this `YGGap`, but this ending up leading to a couple public method signatures that could appear ambiguous: 1. `SetGap(YGGap gap, float gapLength)`: Enums like `YGAlign` are the vaues for an `align` prop. `YGGap` controls the variant of the gap (like `YGEdge` does for left/right/top/bottom variants). So the enum reads as if it is the `gapValue`, and it looks like we have two of the same parameter. 2. `SetGap(YGGap gapDirection, float gap)`: This is misleading, because the direction gaps flow is the cross-axis of flex-direction. 3. `GetGap(YGGap gap)`: `gap` is the variant, but looks like an out param. The [CSS Box Alignment](https://www.w3.org/TR/css-align-3/#column-row-gap) spec refers to these gaps as "Gutters", which removes the ambiguity. Changelog: [General][Added] - Add YGGutter Enum Reviewed By: yungsters Differential Revision: D39922412 fbshipit-source-id: 4b0baf800fecb3d03560a4267c7fb4c4330fd39e

Summary: facebook/yoga#1116 adds a new enum. The enum generator is out of date with copyright header, and some codemods, but it also looks like there were manual changes, types added, etc since generation. I fixed up the script to incorporate generating the changes folks made manually, and also added an enum that was previously only added manually to the C ABI. Changelog: [General][Fixed] - Fixup Yoga Enum Generator Reviewed By: yungsters Differential Revision: D39922252 fbshipit-source-id: b678fa9a43a896873d8c434745bdaf3f16fd991f

Summary: This adds the YGGutter enum, used to choose between row/column gap variants (row-gap, column-gap, gap). This used later in changes from facebook/yoga#1116, in the APIs which deal with setting gap on style on yoga node. Note the original PR called this `YGGap`, but this ending up leading to a couple public method signatures that could appear ambiguous: 1. `SetGap(YGGap gap, float gapLength)`: Enums like `YGAlign` are the vaues for an `align` prop. `YGGap` controls the variant of the gap (like `YGEdge` does for left/right/top/bottom variants). So the enum reads as if it is the `gapValue`, and it looks like we have two of the same parameter. 2. `SetGap(YGGap gapDirection, float gap)`: This is misleading, because the direction gaps flow is the cross-axis of flex-direction. 3. `GetGap(YGGap gap)`: `gap` is the variant, but looks like an out param. The [CSS Box Alignment](https://www.w3.org/TR/css-align-3/#column-row-gap) spec refers to these gaps as "Gutters", which removes the ambiguity. Changelog: [General][Added] - Add YGGutter Enum Reviewed By: yungsters Differential Revision: D39922412 fbshipit-source-id: 4b0baf800fecb3d03560a4267c7fb4c4330fd39e

Summary: This extracts the core changes from #1116, to support gap/row-gap/column-gap, mostly identical, apart from the rename of gaps -> gutters. The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification. Changelog: [Genral][Added] - Implement gap/row-gap/column-gap (within the yoga C ABI) Reviewed By: javache Differential Revision: D39922410 fbshipit-source-id: 5850f22032169028bd8383b49dd240b335c11d3d

Summary: This adds the fixtures from #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 titusfortner/webdrivers#239. Reviewed By: yungsters Differential Revision: D39922413 fbshipit-source-id: dfc7bda894be8dfcb24e25c19a4df0b09a72ce7e

Summary: This extracts the core changes from facebook/yoga#1116, to support gap/row-gap/column-gap, mostly identical, apart from the rename of gaps -> gutters. The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification. Changelog: [Genral][Added] - Implement gap/row-gap/column-gap (within the yoga C ABI) Reviewed By: javache Differential Revision: D39922410 fbshipit-source-id: 5850f22032169028bd8383b49dd240b335c11d3d

NickGerleman · 2022-10-13T16:52:55Z

Good news! All of the changes from this PR should now be merged.

The change should be synced to React Native already, and facebook/react-native#34360 tracks the PR exposing the capability.

I am planning to release this change along with the other recently merged commits in a new version of Yoga. This will be published to Maven Central and Cocoapods. The latter needs a little bit of infra work before this happens.

There was a delay here that I don't this is acceptable. I think we should be better positioned now to accept Yoga changes going forward.

Summary: This extracts the core changes from facebook/yoga#1116, to support gap/row-gap/column-gap, mostly identical, apart from the rename of gaps -> gutters. The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification. Changelog: [Genral][Added] - Implement gap/row-gap/column-gap (within the yoga C ABI) Reviewed By: javache Differential Revision: D39922410 fbshipit-source-id: 5850f22032169028bd8383b49dd240b335c11d3d

Summary: This PR adds React native binding for facebook/yoga#1116 ## Changelog [General] [Added] - Flex gap yoga bindings  Pull Request resolved: #34974 Test Plan: Run rn tester app and go to view example. You'll find a flex gap example. Example location - `packages/rn-tester/js/examples/View/ViewExample.js` ### Tested on - [x] iOS Fabric - [x] iOS non-fabric - [x] Android Fabric - [x] Android non-fabric To test on non-fabric Android, I just switched these booleans. Let me know if there's anything else I might have missed. <img width="674" alt="Screenshot 2022-10-14 at 3 30 48 AM" src="https://user-images.githubusercontent.com/23293248/195718971-7aee4e7e-dbf0-4452-9d47-7925919c61dc.png"> Reviewed By: mdvacca Differential Revision: D40527474 Pulled By: NickGerleman fbshipit-source-id: 81c2c97c76f58fad3bb40fb378aaf8b6ebd30d63

Summary: facebook/yoga#1116 adds a new enum. The enum generator is out of date with copyright header, and some codemods, but it also looks like there were manual changes, types added, etc since generation. I fixed up the script to incorporate generating the changes folks made manually, and also added an enum that was previously only added manually to the C ABI. Changelog: [General][Fixed] - Fixup Yoga Enum Generator Reviewed By: yungsters Differential Revision: D39922252 fbshipit-source-id: b678fa9a43a896873d8c434745bdaf3f16fd991f

Summary: This adds the YGGutter enum, used to choose between row/column gap variants (row-gap, column-gap, gap). This used later in changes from facebook/yoga#1116, in the APIs which deal with setting gap on style on yoga node. Note the original PR called this `YGGap`, but this ending up leading to a couple public method signatures that could appear ambiguous: 1. `SetGap(YGGap gap, float gapLength)`: Enums like `YGAlign` are the vaues for an `align` prop. `YGGap` controls the variant of the gap (like `YGEdge` does for left/right/top/bottom variants). So the enum reads as if it is the `gapValue`, and it looks like we have two of the same parameter. 2. `SetGap(YGGap gapDirection, float gap)`: This is misleading, because the direction gaps flow is the cross-axis of flex-direction. 3. `GetGap(YGGap gap)`: `gap` is the variant, but looks like an out param. The [CSS Box Alignment](https://www.w3.org/TR/css-align-3/#column-row-gap) spec refers to these gaps as "Gutters", which removes the ambiguity. Changelog: [General][Added] - Add YGGutter Enum Reviewed By: yungsters Differential Revision: D39922412 fbshipit-source-id: 4b0baf800fecb3d03560a4267c7fb4c4330fd39e

Summary: This extracts the core changes from facebook/yoga#1116, to support gap/row-gap/column-gap, mostly identical, apart from the rename of gaps -> gutters. The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification. Changelog: [Genral][Added] - Implement gap/row-gap/column-gap (within the yoga C ABI) Reviewed By: javache Differential Revision: D39922410 fbshipit-source-id: 5850f22032169028bd8383b49dd240b335c11d3d

Summary: This PR adds React native binding for facebook/yoga#1116 ## Changelog [General] [Added] - Flex gap yoga bindings  Pull Request resolved: facebook#34974 Test Plan: Run rn tester app and go to view example. You'll find a flex gap example. Example location - `packages/rn-tester/js/examples/View/ViewExample.js` ### Tested on - [x] iOS Fabric - [x] iOS non-fabric - [x] Android Fabric - [x] Android non-fabric To test on non-fabric Android, I just switched these booleans. Let me know if there's anything else I might have missed. <img width="674" alt="Screenshot 2022-10-14 at 3 30 48 AM" src="https://user-images.githubusercontent.com/23293248/195718971-7aee4e7e-dbf0-4452-9d47-7925919c61dc.png"> Reviewed By: mdvacca Differential Revision: D40527474 Pulled By: NickGerleman fbshipit-source-id: 81c2c97c76f58fad3bb40fb378aaf8b6ebd30d63

facebook-github-bot added the CLA Signed label Dec 20, 2021

jacobp100 mentioned this pull request Dec 20, 2021

Gap/row-gap/column/gap properties #812

Closed

1 task

Add support for gap, column-gap, and row-gap properties

812d4de

jacobp100 force-pushed the gap branch from 908195b to 812d4de Compare December 20, 2021 19:11

intergalacticspacehighway added 2 commits December 21, 2021 12:12

fix: align items stretch and flex-end

c92c8fe

fix: align items stretch and flex-end

a52740b

Merge pull request #2 from intergalacticspacehighway/flex-gap-fixes

0428c95

Flex gap fixes

intergalacticspacehighway reviewed Dec 23, 2021

View reviewed changes

intergalacticspacehighway mentioned this pull request Dec 23, 2021

Flex gap facebook/react-native#32766

Closed

mlecoq mentioned this pull request Jan 4, 2022

gap property not working (React Native) styled-components/styled-components#3628

Closed

handsomematt mentioned this pull request Mar 29, 2022

CSS flex gap support Facepunch/sbox-issues#1713

Closed

intergalacticspacehighway mentioned this pull request Aug 6, 2022

feat: flex gap yoga bindings facebook/react-native#34360

Closed

EvanBacon mentioned this pull request Aug 12, 2022

RFC: React DOM for Native (reduce API fragmentation) react-native-community/discussions-and-proposals#496

Open

escaton mentioned this pull request Aug 15, 2022

[w3c] ☂️ Web Styles (Part 1) umbrella issue facebook/react-native#34425

Open

74 tasks

process0 mentioned this pull request Aug 24, 2022

Add gap property to StyleSheet API diegomura/react-pdf#1522

Closed

NickGerleman added the Algorithm label Oct 4, 2022

NickGerleman closed this Oct 13, 2022

intergalacticspacehighway mentioned this pull request Oct 13, 2022

feat: flex gap bindings facebook/react-native#34974

Closed

4 tasks

transitive-bullshit mentioned this pull request Oct 19, 2022

Style issues / feedback vercel/satori#217

Open

nicoburns mentioned this pull request Nov 22, 2022

Implement "gap" property (part of the spec for flex layout) DioxusLabs/taffy#66

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for gap, column-gap, and row-gap #1116

Add support for gap, column-gap, and row-gap #1116

jacobp100 commented Dec 20, 2021 •

edited

Loading

intergalacticspacehighway commented Dec 21, 2021

intergalacticspacehighway commented Dec 21, 2021

jacobp100 commented Dec 21, 2021

intergalacticspacehighway commented Dec 21, 2021

jacobp100 commented Dec 22, 2021

intergalacticspacehighway Dec 23, 2021

jacobp100 Dec 23, 2021

jacobp100 Dec 23, 2021

jacobp100 Dec 23, 2021

intergalacticspacehighway Dec 23, 2021

jaraujocds commented Mar 21, 2022

davbrito commented Mar 25, 2022

erisvaldojunior commented Apr 18, 2022

kvbalib commented Apr 21, 2022

gmantovani97 commented May 25, 2022

wood1986 commented Jun 9, 2022

jacobp100 commented Jun 9, 2022

jacobp100 commented Jun 9, 2022

ShivamJoker commented Jul 3, 2022

etasdemir commented Jul 17, 2022

NickGerleman commented Aug 1, 2022 •

edited

Loading

facebook-github-bot commented Aug 1, 2022

intergalacticspacehighway commented Aug 2, 2022

jacobp100 commented Aug 2, 2022

NickGerleman commented Sep 27, 2022 •

edited

Loading

NickGerleman commented Oct 13, 2022

Add support for gap, column-gap, and row-gap #1116

Add support for gap, column-gap, and row-gap #1116

Conversation

jacobp100 commented Dec 20, 2021 • edited Loading

intergalacticspacehighway commented Dec 21, 2021

intergalacticspacehighway commented Dec 21, 2021

jacobp100 commented Dec 21, 2021

intergalacticspacehighway commented Dec 21, 2021

jacobp100 commented Dec 22, 2021

intergalacticspacehighway Dec 23, 2021

Choose a reason for hiding this comment

jacobp100 Dec 23, 2021

Choose a reason for hiding this comment

jacobp100 Dec 23, 2021

Choose a reason for hiding this comment

jacobp100 Dec 23, 2021

Choose a reason for hiding this comment

intergalacticspacehighway Dec 23, 2021

Choose a reason for hiding this comment

jaraujocds commented Mar 21, 2022

davbrito commented Mar 25, 2022

erisvaldojunior commented Apr 18, 2022

kvbalib commented Apr 21, 2022

gmantovani97 commented May 25, 2022

wood1986 commented Jun 9, 2022

jacobp100 commented Jun 9, 2022

jacobp100 commented Jun 9, 2022

ShivamJoker commented Jul 3, 2022

etasdemir commented Jul 17, 2022

NickGerleman commented Aug 1, 2022 • edited Loading

facebook-github-bot commented Aug 1, 2022

intergalacticspacehighway commented Aug 2, 2022

jacobp100 commented Aug 2, 2022

NickGerleman commented Sep 27, 2022 • edited Loading

NickGerleman commented Oct 13, 2022

jacobp100 commented Dec 20, 2021 •

edited

Loading

NickGerleman commented Aug 1, 2022 •

edited

Loading

NickGerleman commented Sep 27, 2022 •

edited

Loading