Support inverted ScrollView on macOS #1264

christophpurrer · 2022-07-20T23:41:14Z

Please select one of the following

I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
I am making a fix / change for the macOS implementation of react-native
I am making a change required for Microsoft usage of react-native

Summary

Allow to render a scrollView's content in inverted order which is especially helpful in messaging applications.

Co-authored-by: Scott Kyle [email protected]

Changelog

[macOS] [Added] - Support inverted ScrollView on macOS

Test Plan

Added RNTester example

Screen.Recording.2022-07-20.at.4.30.22.PM.mov

Default, non-inverted example

Screen.Recording.2022-07-20.at.4.39.55.PM.mov

Libraries/Components/ScrollView/ScrollView.js

Saadnajmi · 2022-07-21T00:22:33Z

The goal of this PR is that instead of doing a scale transformation on the scrollview contents like Flatlist on iOS does, we're instead taking advantage of macOS's flipped coordinate space?

christophpurrer · 2022-07-21T04:55:14Z

@Saadnajmi > This is an internal change we have used at Meta because

We can't rely on -1 scale hacks on macOS because of inverse issues with trackpad/scrollwheel, dragging scrollbars, tracking hovers, etc.

Hence we added 'native' support for inverted views

Libraries/Lists/VirtualizedList.js

React/Views/ScrollView/RCTScrollView.m

packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js

Libraries/Components/ScrollView/ScrollView.js

Saadnajmi

When I test the Flatlist example page (packages/rn-tester/js/examples/FlatList/FlatList-basic.js) with this change, all the elements seem inverted?

Interestingly enough, if I make an edit in a file (causing fast refresh), it fixes itself.

Screen.Recording.2022-07-22.at.11.20.46.AM.mov

christophpurrer · 2022-07-22T19:34:37Z

Good catch! And easy to repro.

It fixes 'itself' when you edit FlatList-basic.js because fast refresh will reset the 'inverted' flag.

I'll look for a proper fix!

christophpurrer · 2022-07-23T07:48:45Z

That latest version should both work with ScrollView and FlatList.
Demo:
https://www.youtube.com/shorts/TtXp9Laf9kQ

Would be great to get some additional verification as well

Saadnajmi · 2022-07-25T23:32:03Z

The latest iteration does indeed pass the "Basic" Flatlist test page, but seems to have no effect on both the Flatlist and SectionList "Inverted" test pages :/

Screen.Recording.2022-07-25.at.4.26.48.PM.mov

Saadnajmi · 2022-08-09T17:57:48Z

This PR looks like one we'd like to merge, but needs a bit more love for all of the test pages / use cases of Flatlist. Thanks for this fix!

ghost · 2022-08-18T00:05:18Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

Saadnajmi · 2022-08-18T00:28:15Z

don't close, bot?

ghost · 2022-08-25T03:03:06Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

Saadnajmi · 2022-08-25T03:03:53Z

Don't close?

christophpurrer · 2022-08-30T06:08:36Z

So it seems this always worked after all, however initially it never supported to change the inverted property on the fly / at runtime?
Is this actually a real usage scenario?

Anyway, supporting that behavior solves the issue in rn-tester as the previous manual resolution

Interestingly enough, if I make an edit in a file (causing fast refresh), it fixes itself.

To make it work I had to piggy-bag on a previous work-around > https://l.workplace.com/l.php?u=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Freact-native-macos%2Fblame%2Fmain%2FLibraries%2FComponents%2FScrollView%2FScrollView.js%23L1191&h=AT3fib22xIV2fmZMhNOJG8QMW7Wyff9Mb7mUrteHJQ1tgW6ZAyCefrhhTUDGNA9TPxzc7B9a7ciemm-px_U13LznEMFnxwbRmLX7ohWZALyeB60HNQDmqLDueA08-dbWqU6_w-w7BPPMb9uKazppMrxFRHfkJno8eM2QpQ

ScrollView sample added

ScrollView.mov

Flatlist inverted

FlatListInverted.mov

Flatlist basic

FlatList basic has a terrible performance so I restricted the max list size to 50 (from 1000)
https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/js/examples/FlatList/FlatList-basic.js#L269

And the incremental list expansion to 10 (from 100)
https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/js/examples/FlatList/FlatList-basic.js#L71
https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/js/examples/FlatList/FlatList-basic.js#L273

FlatListBasic480.mov

React/Views/ScrollView/RCTScrollView.m

Saadnajmi

Switched back to request changes for last comment, it would be bad to break iOS 😅

Allow to render a scrollView's content in inverted order which is especially helpful in messaging applications. We can't rely on -1 scale hacks on macOS because of inverse issues with trackpad/scrollwheel, dragging scrollbars, tracking hovers, etc. Hence we added 'native' support for inverted views

christophpurrer · 2022-09-07T18:26:28Z

I have remove those changes

Saadnajmi

Latest version lost my Flatlist keyboard nav changes, but that's OK I'll mark a followup for that.

Allow to render a scrollView's content in inverted order which is especially helpful in messaging applications. We can't rely on -1 scale hacks on macOS because of inverse issues with trackpad/scrollwheel, dragging scrollbars, tracking hovers, etc. Hence we added 'native' support for inverted views Co-authored-by: Scott Kyle <[email protected]>

christophpurrer requested a review from a team as a code owner July 20, 2022 23:41

harrieshin reviewed Jul 20, 2022

View reviewed changes

Libraries/Components/ScrollView/ScrollView.js Show resolved Hide resolved

Saadnajmi reviewed Jul 21, 2022

View reviewed changes

Libraries/Lists/VirtualizedList.js Outdated Show resolved Hide resolved

Saadnajmi reviewed Jul 21, 2022

View reviewed changes

React/Views/ScrollView/RCTScrollView.m Show resolved Hide resolved

Saadnajmi reviewed Jul 21, 2022

View reviewed changes

packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js Outdated Show resolved Hide resolved

christophpurrer force-pushed the scrollViewInverted branch from 1ed7255 to 22e7794 Compare July 21, 2022 22:08

Saadnajmi self-assigned this Jul 22, 2022

harrieshin reviewed Jul 22, 2022

View reviewed changes

Libraries/Components/ScrollView/ScrollView.js Show resolved Hide resolved

Saadnajmi reviewed Jul 22, 2022

View reviewed changes

christophpurrer force-pushed the scrollViewInverted branch from 22e7794 to 9a6231a Compare July 23, 2022 07:36

christophpurrer force-pushed the scrollViewInverted branch from 9a6231a to a0b0de8 Compare July 23, 2022 07:58

Saadnajmi added the Partner: Facebook label Jul 28, 2022

christophpurrer changed the title ~~Support inverted ScrollView on macOS~~ Support inverted ScrollView on macOS [WIP] Aug 2, 2022

Saadnajmi added the Needs: Author Feedback label Aug 9, 2022

christophpurrer changed the title ~~Support inverted ScrollView on macOS [WIP]~~ [WIP] Support inverted ScrollView on macOS Aug 10, 2022

ghost removed the Needs: Author Feedback label Aug 10, 2022

Saadnajmi added the Needs: Author Feedback label Aug 10, 2022

ghost added the no-recent-activity label Aug 18, 2022

ghost removed the no-recent-activity label Aug 18, 2022

ghost added the no-recent-activity label Aug 25, 2022

ghost removed the no-recent-activity label Aug 25, 2022

christophpurrer force-pushed the scrollViewInverted branch from a0b0de8 to 2856e4c Compare August 30, 2022 06:00

ghost removed the Needs: Author Feedback label Aug 30, 2022

christophpurrer changed the title ~~[WIP] Support inverted ScrollView on macOS~~ Support inverted ScrollView on macOS Aug 30, 2022

christophpurrer force-pushed the scrollViewInverted branch 2 times, most recently from cc8087c to ea07b6f Compare August 30, 2022 06:16

christophpurrer force-pushed the scrollViewInverted branch 2 times, most recently from 270ba00 to 8a052a5 Compare September 7, 2022 03:47

Saadnajmi approved these changes Sep 7, 2022

View reviewed changes

React/Views/ScrollView/RCTScrollView.m Outdated Show resolved Hide resolved

Saadnajmi requested changes Sep 7, 2022

View reviewed changes

ghost added the Needs: Author Feedback label Sep 7, 2022

christophpurrer force-pushed the scrollViewInverted branch from 8a052a5 to 156cdd7 Compare September 7, 2022 18:24

ghost removed the Needs: Author Feedback label Sep 7, 2022

christophpurrer requested a review from Saadnajmi September 7, 2022 18:25

christophpurrer closed this Sep 7, 2022

christophpurrer reopened this Sep 7, 2022

Saadnajmi approved these changes Sep 7, 2022

View reviewed changes

Saadnajmi merged commit 6876a33 into microsoft:main Sep 7, 2022

Saadnajmi mentioned this pull request Sep 7, 2022

Inverted Flatlist has incorrect keyboard handling #1421

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support inverted ScrollView on macOS #1264

Support inverted ScrollView on macOS #1264

christophpurrer commented Jul 20, 2022

Saadnajmi commented Jul 21, 2022

christophpurrer commented Jul 21, 2022

Saadnajmi left a comment •

edited

Loading

christophpurrer commented Jul 22, 2022

christophpurrer commented Jul 23, 2022

Saadnajmi commented Jul 25, 2022

Saadnajmi commented Aug 9, 2022

ghost commented Aug 18, 2022

Saadnajmi commented Aug 18, 2022

ghost commented Aug 25, 2022

Saadnajmi commented Aug 25, 2022

christophpurrer commented Aug 30, 2022 •

edited

Loading

Saadnajmi left a comment

christophpurrer commented Sep 7, 2022

Saadnajmi left a comment

Support inverted ScrollView on macOS #1264

Support inverted ScrollView on macOS #1264

Conversation

christophpurrer commented Jul 20, 2022

Please select one of the following

Summary

Changelog

Test Plan

Saadnajmi commented Jul 21, 2022

christophpurrer commented Jul 21, 2022

Saadnajmi left a comment • edited Loading

Choose a reason for hiding this comment

christophpurrer commented Jul 22, 2022

christophpurrer commented Jul 23, 2022

Saadnajmi commented Jul 25, 2022

Saadnajmi commented Aug 9, 2022

ghost commented Aug 18, 2022

Saadnajmi commented Aug 18, 2022

ghost commented Aug 25, 2022

Saadnajmi commented Aug 25, 2022

christophpurrer commented Aug 30, 2022 • edited Loading

ScrollView sample added

Flatlist inverted

Flatlist basic

Saadnajmi left a comment

Choose a reason for hiding this comment

christophpurrer commented Sep 7, 2022

Saadnajmi left a comment

Choose a reason for hiding this comment

Saadnajmi left a comment •

edited

Loading

christophpurrer commented Aug 30, 2022 •

edited

Loading