Fix issue where absolute children of row-reverse containers would inset on the wrong side #41293

joevilches · 2023-11-02T20:48:00Z

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

Differential Revision: D50945475

facebook-github-bot · 2023-11-02T20:48:12Z

This pull request was exported from Phabricator. Differential Revision: D50945475

analysis-bot · 2023-11-02T21:18:51Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	17,638,567	+4,256
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,022,320	+4,273
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: d11d5f3
Branch: main

facebook-github-bot · 2023-11-03T19:51:51Z

This pull request was exported from Phabricator. Differential Revision: D50945475

…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 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

…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#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

facebook-github-bot · 2023-11-06T18:09:51Z

This pull request was exported from Phabricator. Differential Revision: D50945475

…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 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

…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#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

facebook-github-bot · 2023-11-06T18:10:58Z

This pull request was exported from Phabricator. Differential Revision: D50945475

…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 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

…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#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

facebook-github-bot · 2023-11-06T22:51:44Z

This pull request was exported from Phabricator. Differential Revision: D50945475

github-actions · 2023-11-06T22:52:46Z

	Fails
🚫	Danger failed to run `dangerfile.js`.

Error ReferenceError

eslint is not defined
ReferenceError: eslint is not defined
    at Object.<anonymous> (dangerfile.js:86:1)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at requireFromString (/home/runner/work/react-native/react-native/node_modules/require-from-string/index.js:28:4)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:157:68
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:52:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:33:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:27:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:23:12)
    at runDangerfileEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:118:132)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:178:38
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:44:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:25:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:19:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:15:12)
    at Object.executeRuntimeEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:143:88)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:100:47
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:34:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:15:53)
    at fulfilled (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:6:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Dangerfile

81| // Warns if the PR is opened against stable, as commits need to be cherry picked and tagged by a release maintainer.
82| // Fails if the PR is opened against anything other than `main` or `-stable`.
83| const isMergeRefMain = danger.github.pr.base.ref === 'main';
84| const isMergeRefStable = danger.github.pr.base.ref.endsWith('-stable');
85| if (!isMergeRefMain && !isMergeRefStable) {
---^
86|   const title = ':exclamation: Base Branch';
87|   const idea =
88|     'The base branch for this PR is something other than `main` or a `-stable` branch. [Are you sure you want to target something other than the `main` branch?](https://reactnative.dev/docs/contributing#pull-requests)';
89|   fail(`${title} - <i>${idea}</i>`);

Generated by 🚫 dangerJS against 91b47c3

…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 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

…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#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

facebook-github-bot · 2023-11-07T00:45:58Z

This pull request was exported from Phabricator. Differential Revision: D50945475

…ok#41208) Summary: X-link: facebook/yoga#1437 Reading through the sizing logic and this seemed a bit redundant/confusing. Lets use the same function we just used for the main axis for the cross axis as well so people do not think its special. Also we will need one less variable. The reason this was done it seems is because we need the leading padding + border elsewhere so this is technically a few less steps but this is cleaner Reviewed By: NickGerleman Differential Revision: D50704177

) Summary: X-link: facebook/yoga#1439 There are so many instances in this code base where we use the double negative of `!yoga::isUndefined(<something>)`. This is not as easy to read since because of this double negative imo. Additionally, sometimes we have really long chains like `!longVariableName.longFunctionName(longArgumentName).isUndefined()` and it is hard to see that this undefined is inverted. This just replaces all instances of inverted `isUndefined()` with `isDefined()` so its easier to read. Reviewed By: NickGerleman Differential Revision: D50705523

…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#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

…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 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

facebook-github-bot · 2023-11-07T01:42:03Z

This pull request was exported from Phabricator. Differential Revision: D50945475

…et on the wrong side Summary: X-link: facebook/react-native#41293 X-link: facebook/yoga#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

…et on the wrong side (#1446) Summary: X-link: facebook/react-native#41293 Pull Request resolved: #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

facebook-github-bot · 2023-11-07T19:08:29Z

This pull request has been merged in 9847bca.

…et on the wrong side (facebook#41293) Summary: Pull Request resolved: facebook#41293 X-link: facebook/yoga#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

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Nov 2, 2023

joevilches force-pushed the export-D50945475 branch from bfb4e1e to 545f13e Compare November 3, 2023 19:51

joevilches force-pushed the export-D50945475 branch from 545f13e to 75faa3d Compare November 6, 2023 18:09

joevilches force-pushed the export-D50945475 branch from 75faa3d to 08f81a2 Compare November 6, 2023 18:10

joevilches force-pushed the export-D50945475 branch from 08f81a2 to 1f5c4a4 Compare November 6, 2023 22:51

joevilches force-pushed the export-D50945475 branch from 1f5c4a4 to 39e86c3 Compare November 7, 2023 00:45

Joe Vilches and others added 3 commits November 6, 2023 17:41

joevilches force-pushed the export-D50945475 branch from 39e86c3 to 91b47c3 Compare November 7, 2023 01:41

facebook-github-bot closed this in 9847bca Nov 7, 2023

facebook-github-bot added the Merged This PR has been merged. label Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue where absolute children of row-reverse containers would inset on the wrong side #41293

Fix issue where absolute children of row-reverse containers would inset on the wrong side #41293

joevilches commented Nov 2, 2023

facebook-github-bot commented Nov 2, 2023

analysis-bot commented Nov 2, 2023 •

edited

Loading

facebook-github-bot commented Nov 3, 2023

facebook-github-bot commented Nov 6, 2023

facebook-github-bot commented Nov 6, 2023

facebook-github-bot commented Nov 6, 2023

github-actions bot commented Nov 6, 2023 •

edited

Loading

facebook-github-bot commented Nov 7, 2023

facebook-github-bot commented Nov 7, 2023

facebook-github-bot commented Nov 7, 2023

Fix issue where absolute children of row-reverse containers would inset on the wrong side #41293

Fix issue where absolute children of row-reverse containers would inset on the wrong side #41293

Conversation

joevilches commented Nov 2, 2023

facebook-github-bot commented Nov 2, 2023

analysis-bot commented Nov 2, 2023 • edited Loading

facebook-github-bot commented Nov 3, 2023

facebook-github-bot commented Nov 6, 2023

facebook-github-bot commented Nov 6, 2023

facebook-github-bot commented Nov 6, 2023

github-actions bot commented Nov 6, 2023 • edited Loading

Error ReferenceError

Dangerfile

facebook-github-bot commented Nov 7, 2023

facebook-github-bot commented Nov 7, 2023

facebook-github-bot commented Nov 7, 2023

analysis-bot commented Nov 2, 2023 •

edited

Loading

github-actions bot commented Nov 6, 2023 •

edited

Loading