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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Enable window/body based scrolling for all ScrollViews #1472

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enable window/body based scrolling for all ScrollViews #1472

wants to merge 2 commits into from

Conversation

EyMaddis
Copy link

@EyMaddis EyMaddis commented Nov 9, 2019

I am always impressed by this project and today is finally the day to give a little bit back! 馃槂

This PR is the first version of using the full page scrolling with React Native Web apps.
In the past, people could only use <ScrollViews /> with fixed heights wrapped around the full page.

This has multiple restrictions, for example it blocks mobile browsers (both Chrome on Android and iOS) from hiding the navigation bars if the page is scrolled down, blocking a lot of the screen. Generally, this leads to the pages feeling less like proper websites.

My proposed solution allows for any ScrollView component to use a new prop useWindowScrolling={true} to let itself grow freely and use the scroll events of document.body. This also works for VirtualizedLists (including FlatList).

I added a <FlatList Window Scroll> example to show its functionality.

windowscroll-smaller

I am sure that there are more issues to tackle. For example I am unsure wether scroll positions should be normalized such that the position of the <ScrollView /> on the page is used to subtract from the scroll position x/y values.

@EyMaddis
Copy link
Author

EyMaddis commented Nov 9, 2019

The idea is to wrap the whole app in a
<ScrollView useWindowScrolling> or possibly<FlatList useWindowScrolling> if you want to nest other virtualized list.

(Hacky tip for people who want to use nested Flatlists: use A Flatlist without data and put everything in the header element. This way nested Lists will work properly, because simple ScrollViews do not work here)

@MathieuUrstein
Copy link

MathieuUrstein commented Nov 19, 2019 •
edited

I鈥檓 playing with react-native-web and so far, everything works extremely well and I'm in love with the general idea & quality of this library.

I'm facing some issues related to the fact that the web is designed to have the <body> being the main <ScrollView> which is not the case in mobile development.

Here are some of the issues I'm facing without this PR because of the previously stated problem:

  • The Web Browser header won't collapse (as shown in the PR description).

  • The ScrollComponents (ScrollView, FlatList, SectionList, ...) ScrollBars won't take the full window height thus making their content being offset. (This happens where ScrollBars are visible like on a Mac without a trackpad or on Windows. Mobile & MacOs with a trackpad are not subject to this problem because the ScrollBar is floating & hides when inactive). Here is a Snack for demonstration (https://snack.expo.io/@mathieuurstein/react-native-web-window-scroll-issues).
    Screenshot 2019-11-19 at 21 45 22

  • This is all I found for now, but we can expect more of those problems because of the browsers expecting the to scroll the main content.

I haven't looked into this PR code but I think the general idea of an useWindowScroll props on ScrollComponents could be a great idea to fix the previously stated issues.

@EyMaddis
Copy link
Author

EyMaddis commented Nov 19, 2019 •
edited

@MathieuUrstein Did you test the code of the PR or of the general React Native Web version. If you used Expo Snack you unfortunately cannot test this PR (as far as I know).

I'm facing some issues related to the fact that the web is designed to have the being the main which is not the case in mobile development.

This is all I found for now, but we can expect more of those problems because of the browsers expecting the to scroll the main content.

This PR tries to solve exactly that. It disables the default scroll behaviour of the <ScrollView> and allows it to grow and resize the <body>, while using the scroll tracking and event which are compatible with ScrollViews.

Did you use the exact Snack, because this neither uses my PR nor the useWindowScroll prop. If you checkout the PR locally, compile it, set useWindowScroll the page would scroll via the body. However the header and footer would not be sticky.
An easy fix would be to set position: fixed on these elements.

I think that designing for body scrolling will always have some caveats, but is generally an improvement. Body scrolling is not something that people would use who just want to put their mobile app on the web with whatever means but instead tailors to people who want to build a proper web app.

@EyMaddis EyMaddis mentioned this pull request Nov 19, 2019
Closed
@EyMaddis
Copy link
Author

@necolas I rebased the PR on the current master with the new and fancy storybook.
I had to completely remove my example as it does not work in storybook as it itself is not window scrollable.

@MathieuUrstein
Copy link

MathieuUrstein commented Nov 20, 2019 •
edited

@MathieuUrstein Did you test the code of the PR or of the general React Native Web version. If you used Expo Snack you unfortunately cannot test this PR (as far as I know).

No I was demonstrating some issues we currently have with react-native-web that your PR can solve. My comment was targeted to prove that your PR is a great idea :)

P:S: I have edited my previous comment to clarify my point.

@blackbing

This comment has been minimized.

Sign in to view
@EyMaddis EyMaddis requested a review from necolas January 16, 2020 17:05
This was referenced Mar 10, 2020
Closed
Closed
@piranna
Copy link

piranna commented Mar 11, 2020

What about working this feature as a external component? A drop-in replacement for the native ScrollView...

@EyMaddis
Copy link
Author

@piranna My changes are mostly in ScrollViewBase.js. I don't see an easy way to avoid duplicating the whole code and reexporting every ScrollView (incl. FlatList, ListView, ...). This would be a way better fit.

@piranna
Copy link

piranna commented Mar 11, 2020

@piranna My changes are mostly in ScrollViewBase.js. I don't see an easy way to avoid duplicating the whole code and reexporting every ScrollView (incl. FlatList, ListView, ...). This would be a way better fit.

This is a valid solution for me...

@necolas necolas modified the milestone: 0.13 Apr 18, 2020
@pierreis
Copy link

pierreis commented May 6, 2020 •
edited

That sounds like a fantastic feature, thanks! Any way to help?

@EyMaddis - do you have any example to share to test your proposal?

@jfrolich
Copy link
Contributor

jfrolich commented Jul 1, 2020 •
edited

I am actually really interested in this. What is the status of this, does this work with the new responder system?

@EyMaddis
Copy link
Author

Before I invest any more time into supporting this, I would like to know whether this has a chance of getting merged at all. Could you comment on that, @necolas?
Thanks

@blackbing
Copy link

looking forward to this feature

@jfrolich
Copy link
Contributor

jfrolich commented May 7, 2021

I don't think this is necessary anymore in the new version of react-native-web. You can just put components in the root of your document now with the new gesture responder system.

@blackbing
Copy link

@jfrolich do you mean AppRegistery on <body> ?

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

@necolas necolas Awaiting requested review from necolas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@EyMaddis @MathieuUrstein @blackbing @piranna @pierreis @jfrolich @necolas