Skip to content

Add errata "PercentAbsoluteOmitsPadding" (#1290) - #37376

Closed
NickGerleman wants to merge 2 commits into
react:mainfrom
NickGerleman:export-D45763574
Closed

NickGerleman wants to merge 2 commits into
react:mainfrom
NickGerleman:export-D45763574

Conversation

@NickGerleman

Copy link
Copy Markdown
Contributor

Summary:
X-link: react/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 react/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

@facebook-github-bot 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 May 11, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@analysis-bot

analysis-bot commented May 11, 2023 •

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,726,941 +278
android hermes armeabi-v7a 8,037,783 +269
android hermes x86 9,214,065 +319
android hermes x86_64 9,068,310 +324
android jsc arm64-v8a 9,291,520 +108
android jsc armeabi-v7a 8,479,783 +97
android jsc x86 9,350,029 +152
android jsc x86_64 9,607,872 +147

Base commit: 40f816d
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

NickGerleman and others added 2 commits May 11, 2023 03:05
Summary:
Adds internal helpers to YGConfig to make bit manipulation more readable. We also expose `hasErrata()` to YGNode beacuse checking that will be a common pattern. We intentionally don't add mutating functions to the node, since current model is to inval a node on commiting whole config.

This is not exposed via the C ABI.

Differential Revision: D45765971

fbshipit-source-id: f8e6ad737d5693cf2036b73bd7a5fa30fc0419d4
Summary:
Pull Request resolved: react#37376

X-link: react/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 react/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
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@cortinico cortinico closed this Oct 23, 2023
@cortinico

Copy link
Copy Markdown
Contributor

Closing as planned changes internally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants