Add errata "PercentAbsoluteOmitsPadding" #1290
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This can be marked in fixtures to skip a test without commenting it out. We add one more usage of this. The same functionality existed (unused) before for `experiments`, which I changed to `data-experiments`. Formatting of JS tests changed to be closer to what Prettier would output, and to remove usage of `Yoga.UNDEFINED` which doesn't existi and just resolves to `undefined` (this is converted to NaN by the wrapper layer). Differential Revision: D45723003 fbshipit-source-id: 192c0f7584069017000f5cfa30726539a681e302
Summary: Outputs tests as TypeScript, along with using/testing the new form of enums imported directly from the package. We need to change how we are telling Jest which variant to run, so that tests can import enums from "yoga-layout" and have it resolve to the entrypoint which has a binary which has already been built. Differential Revision: D45723545 fbshipit-source-id: 300accf064a9efaf145204a1023803b003758cb7
Summary: This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. I also fixed a reversal of `height` and `width` in one place compared to the original PR, and enabled a significantly more edge-casey fix without the errata. Differential Revision: D45763574 fbshipit-source-id: 00471f861dc6022a3f3f4842b90b6a4237b0917d
|
This pull request was exported from Phabricator. Differential Revision: D45763574 |
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Differential Revision: https://internalfb.com/D45763574 fbshipit-source-id: 787acafe038f6988a2fc6603243789a67e913529
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Differential Revision: D45763574 fbshipit-source-id: 4431a112bec22921d9a0e842701d87c876798f2d
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Differential Revision: https://internalfb.com/D45763574 fbshipit-source-id: 5eb2f3bf551a8765e384a19b8df77a13628529bd
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: Pull Request resolved: facebook#37376 X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Reviewed By: yungsters Differential Revision: D45763574 fbshipit-source-id: 4c1257cb0babf391d9327365e8e1ca56623d4286
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Differential Revision: https://internalfb.com/D45763574 fbshipit-source-id: 1ec48dbc521c259f45cee03635e02efaae5b40bd
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: Pull Request resolved: facebook#37376 X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Reviewed By: yungsters Differential Revision: D45763574 fbshipit-source-id: 2811aeba3c24621557f294eee7f4bfb170e9e5ab
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Differential Revision: https://internalfb.com/D45763574 fbshipit-source-id: 6658adb99ad997518b17b5070a5939a664239384
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
May 11, 2023
Summary: Pull Request resolved: facebook#37376 X-link: facebook/yoga#1290 This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See facebook/yoga#850 for more details of the backing issue. I have so far left this behind a `YGExperimentalFeature`, because it will change the dimensions of existing views which use absolute percentages with parent padding. This change moves it from a `YGExperimentalFeature` to `YGErrata`. This means the conformant path is enabled by default, but users of `YGErrataClassic` and `YGErrataAll` will stay on the previous behavior. Right now the name is `PercentAbsoluteOmitsPadding` as something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas. Reviewed By: yungsters Differential Revision: D45763574 fbshipit-source-id: a5e17e2efd5478a236cf585f2bf67c6b823539e3
jacobp100
reviewed
May 12, 2023
| child | ||
| ->getLeadingMargin( | ||
| mainAxis, | ||
| node->getLayout().measuredDimensions[dim[mainAxis]]) |
There was a problem hiding this comment.
Minor - but we could pull this out into mainAxisDim since it's used twice
|
Could we mark the test cases that are affected by this flag with a data attribute or similar (even if it doesn't actually do anything yet)? Seems that it would be useful to build up a test suite that explicitly tests the quirk modes. I guess ideally we would also include assertion values for those tests too (seeing as we can't just generate the quirky values using Chrome). Also, TypeScript tests seem to include unrelated changes to formatting / variable naming... |
Let me take a look, the original PR made just added a test for the new case, but will see if I can add one for the old.
This is a quirk of how changes made in Phabriactor are exported to GitHub. Each "commit" here is separately merged/managed internally, but depends on the previous commits. I would recommend looking only at the latest commit, and using the others as context (they had separate reviews). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This change default-enables a bugfix to Yoga's handling of absolute-positioned children sized using percentages. Yoga previously measured percentage of the box without padding, where the W3C spec, and browser behavior verified by test, use the padding box instead. See #850 for more details of the backing issue.
I have so far left this behind a
YGExperimentalFeature, because it will change the dimensions of existing views which use absolute percentages with parent padding.This change moves it from a
YGExperimentalFeaturetoYGErrata. This means the conformant path is enabled by default, but users ofYGErrataClassicandYGErrataAllwill stay on the previous behavior.Right now the name is
PercentAbsoluteOmitsPaddingas something that tries to explain the issue in lay-terms and be a little less long than the old one, but I am open to other ideas.I also fixed a reversal of
heightandwidthin one place compared to the original PR, and enabled a significantly more edge-casey fix without the errata.Differential Revision: D45763574