Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove row-reverse errata #1547

Closed
wants to merge 1 commit into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
Yoga has an odd behavior, where start/end edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

row-reverse is surprisingly uncommon though:

  1. Litho has <40 usages
  2. RN has ~40 usages in RKJSModules,~30 in arvr/js, ~6 in xplat/archon
  3. CK has ~80 usages
  4. NT has ~40 usages

These components are pretty static, and few enough that we can do this style of refactor, where we just fix everything we find. But there is some risk of missing complonents.

CK accounts for 17/20 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveryed, do I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make start/end in styles always refer to writing direction

Differential Revision: D52698130

Copy link

vercel bot commented Jan 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 11, 2024 7:24pm

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

Summary:

X-link: facebook/react-native#42251

Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages

These components are pretty static, and few enough that we can do this style of refactor, where we just fix everything we find. But there is some risk of missing complonents.

CK accounts for 10/13 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveryed, do I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction

Reviewed By: joevilches

Differential Revision: D52698130
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Jan 11, 2024
Summary:
X-link: facebook/yoga#1547


Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages

These components are pretty static, and few enough that we can do this style of refactor, where we just fix everything we find. But there is some risk of missing complonents.

CK accounts for 10/13 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveryed, do I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction

Reviewed By: joevilches

Differential Revision: D52698130
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 8c3e011.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jan 12, 2024
Summary:
X-link: facebook/yoga#1547

Pull Request resolved: #42251

Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages

There are few enough, mostly simple components, that we can inspect through each of them, looking for signs they will hit the issue (at the potential chance of missing some).

CK accounts for 10/14 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveyed, so I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction

Reviewed By: pentiumao, joevilches

Differential Revision: D52698130

fbshipit-source-id: 2a9ac47e177469f30dc988d916b6c0ad95d53461
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Jan 12, 2024
Summary:
X-link: facebook/yoga#1547

X-link: facebook/react-native#42251

Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages

There are few enough, mostly simple components, that we can inspect through each of them, looking for signs they will hit the issue (at the potential chance of missing some).

CK accounts for 10/14 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveyed, so I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction

Reviewed By: pentiumao, joevilches

Differential Revision: D52698130

fbshipit-source-id: 2a9ac47e177469f30dc988d916b6c0ad95d53461
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants