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

Fix inverted contentOffset in scroll events in RTL #39031

Closed
wants to merge 2 commits into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
UIScrollView contentOffset is flow-relative, so x is relative to the right edge of the screen. This coordinate space disagrees with layout events, scrollTo coordinates, and other platforms.

This applies the same logic we use for inverting scrollTo coordinates to invert contentOffset in scroll events, in both Paper and Fabric. We then remove the iOS specific workaround we have in VirtualizedList.

Differential Revision: D48379915

Summary:
D29164056 fixed `scrollTo` coordinate space in RTL, but D38499666 regressed it by adding a comparison before the RTL conversion happens.

This makes `scrollTo` no-op if we are scrolling from the beginning to the end of the list, since the end of the list is `x: 0` in cartesian coordinates, and the start of the list is `x: 0` in flow-relative coordinates.

Do coordinate conversion before the early exit check.

Changelog:
[iOS][Fixed] - Fix bad comparison in RCTScrollViewComponentView RTL

Differential Revision: D48378414

fbshipit-source-id: edef543bab9612bb61176a7cda259f18ad6e8c45
Summary:
`UIScrollView` `contentOffset` is flow-relative, so `x` is relative to the right edge of the screen. This coordinate space disagrees with layout events, `scrollTo` coordinates, and other platforms.

This applies the same logic we use for inverting `scrollTo` coordinates to invert `contentOffset` in scroll events, in both Paper and Fabric. We then remove the iOS specific workaround we have in VirtualizedList.

Differential Revision: D48379915

fbshipit-source-id: caa48b99d43298cf51e5d668dd7e7f68fee2ec4e
@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 labels Aug 16, 2023
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,948,034 -134
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,541,500 -2
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 5063871
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 22, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4f8a8ce.

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 Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants