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

[ScrollView] Add support for nested scrollable views. #8024

Closed
alloy opened this issue Jun 9, 2016 · 47 comments
Closed

[ScrollView] Add support for nested scrollable views. #8024

alloy opened this issue Jun 9, 2016 · 47 comments
Labels
Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@alloy
Copy link
Contributor

alloy commented Jun 9, 2016

For Discussion

In larger complex view hierarchies it may happen that a ScrollView-based component is nested inside another ScrollView.

Currently a nested ScrollView won’t receive scroll events, only the outer parent ScrollView will. In order to receive them, you would have to push scroll event callbacks down each child component of the parent ScrollView and all of those would have to send them down to their children, just in case a component somewhere down the tree needs to receive such events.

The developer and user of that component shouldn’t need to jump through hoops to have ScrollView callbacks work, but should be able to write isolated components that are unaware of the presence of other ScrollView instances in the view hierarchy and have React Native deal with the details of dispatching events to all components that require them.

Examples

Contrived

Multiple ListView instances as part of a larger scrolling view:

<ScrollView>
  <Header />
  <ListView />
  <ListView />
  <Footer />
</ScrollView>

Often the advice is to add views that are supposed to be above or below a ListView as a header or footer of the ListView, but that doesn’t solve the case where both ListView instance have to scroll in tandem as part of the parent ScrollView.

Real-world

My real world example is a ‘tab view’ component that displays a ScrollView-based component that needs to paginate.

You can run the example app that’s contained in the repo to see it for yourself, or in our production app that is currently in the store, but here’s a short demonstration of the ‘WORKS’ grid component being nested inside the top-level ScrollView and still paginating onEndReached as expected:

nested scrollview

Proof of Concept

For my app I wrote some 🐒-patches that do the following:

  1. When a RCTScrollView is added to a view hierarchy, it registers for a RCTScrollEvent notification.
  2. When a RCTScrollView receives a native scroll callback it first does its own JS scroll event dispatching and afterwards it posts a RCTScrollEvent notification for any child RCTScrollView instances to pick-up.
  3. The child RCTScrollView instance creates a new RCTScrollEvent object that offsets the scrollview’s contentOffset to reflect its location inside the parent RCTScrollView.
  4. The child RCTScrollView then dispatches the JS scroll event as normal and the component doesn’t know any better than that it has scrolled as normal.

Notes:

  • Please don’t judge my implementation as anything other than a POC of the high-level functionality.
  • I used NSNotificationCenter because there’s a potential 1-to-many relationship. I haven’t noticed any discernible lag in delivery, but there’s also no guarantee. A possible alternative would be to use a proxy object as the UIScrollView delegate that can dispatch events to multiple delegates (example).
  • While I have not yet used the callbacks for a ListView component in the current version of our app, I have tested and verified that the onEndReached callback gets triggered, thus I see no reason why the removeClippedSubviews optimisation wouldn’t work either. At least that’s my goal, as I will need this soon.

Discussion

  • I’m looking for suggestions as to how this should be implemented for it to be acceptable in React Native.
  • When I discussed this with @javache offline he suggested abstracting the scroll position/event part from ScrollView into a Scrollable container. That container could then create the ScrollView if not inside a parent ScrollView.
  • I want to spend the time to write the iOS implementation.
  • I currently do not target Android and thus don’t feel like I’m the right person to implement it for that platform at the moment, at least not from the get-go.

/cc @javache @nicklockwood

@lucasfeliciano
Copy link
Contributor

👍 Beautiful issue.
Good job

@charpeni
Copy link
Contributor

@facebook-github-bot label Good First Task

@ghost ghost added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Ran Commands One of our bots successfully processed a command. labels Jun 10, 2016
@grabbou
Copy link
Contributor

grabbou commented Jun 13, 2016

I don't think that's a good first task @charpeni ?

@javache javache removed the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label Jun 14, 2016
@javache
Copy link
Member

javache commented Jun 14, 2016

I agree, this will require some more significant changes in how we create scrollviews and dispatch scroll events.

@shaneosullivan
Copy link
Contributor

I'm running into this same issue. In iOS it seems to work as expected for me, but in Android the inner ScrollView never receives horizontal pan gestures if it is contained in a ScrollView that can also scroll horizontally, e.g. a zoomed image in a gallery

@adbl
Copy link
Contributor

adbl commented Oct 24, 2016

@alloy just ran into this, any progress on the issue?

@alloy
Copy link
Contributor Author

alloy commented Oct 28, 2016

The provided monkey-patch still works perfect in production with latest RN. I haven’t had much spare time to discuss and cleanup this patch until last week when we shipped a new major version of our app.

@javache Can we catch-up sometime soon to talk it through?

@ilyadoroshin
Copy link

ilyadoroshin commented Jan 19, 2017

Guys, adding TouchableWithoutOpacity inside nested ScrollView worked like a charm for me on android:
Checkout my answer on SO (couldn't make the markup right in here..)
Maybe it's not the case though...

@AlanFoster
Copy link
Contributor

AlanFoster commented Mar 13, 2017

With 0.43.0-rc.2 I'm running into the same problem when attempting to wrap a ScrollView around the new FlatList component; the onEndReached event won't triggered as expected

@cstafie
Copy link

cstafie commented Apr 3, 2017

@ilyadoroshin TouchableWithoutFeedback component worked for me on iOS.

@andrerfneves
Copy link

Has anyone other than @alloy found a workaround? Even if it's a monkeypatch approach, I am trying to resolve this.

@alloy
Copy link
Contributor Author

alloy commented Apr 18, 2017

@ilyadoroshin @restlessCalm FYI The issue described in the linked SO post is about a completely different scenario than what this ticket is about.

@alloy
Copy link
Contributor Author

alloy commented Apr 18, 2017

@AlanFoster @andrerfneves Did you try my monkey patches in latest RN and/or with FlatList? I haven’t upgrade to the latest yet, but the internal scrollview API hasn't’t changed much over the months, so should probably be ok.

@andrerfneves
Copy link

@alloy how would you go about using such monekypatches? Not entirely familiar with iOS development. My issue is actually the same but on the Android platform. Any thoughts?

@alloy
Copy link
Contributor Author

alloy commented Apr 18, 2017

Ahh I see. No sorry I have no input on Android right now, only iOS. As to how to integrate these monkey patches, you can take a look at our OSS project they reside in.

@jenskuhrjorgensen
Copy link

jenskuhrjorgensen commented Jul 20, 2017

Any solution to nested scroll views on Android?
I have a vertical SectionList inside a vertical ScrollView with RefreshControl. When the bottom of the outer ScrollView is reached I can scroll down in the inner SectionList. But I can never scroll up in the inner SectionList because of the outer ScrollView's RefreshControl.

@mferranti
Copy link

mferranti commented Sep 12, 2017

Same here. Any workaround? i have something like this:

<ScrollView>
  <FlatList onEndReach={this.fetchNewPage} />
</ScrollView>

@hemantasapkota
Copy link

hemantasapkota commented Oct 5, 2017

Greetings,

For anyone looking for a workaround for android, I have a solution that was tested successfully.

The core of the fix is to use NestedScollView instead of ScrollView. There's a library that already does this - https://github.com/mohtada-h/react-native-nested-scrollview, however it's not being maintained to support the latest RN.

I've created a patch that can be applied directly from the root of a react native project. You can find the patch file here:

https://raw.githubusercontent.com/hemantasapkota/react-native-nested-scrollview/master/root.patch

Summary of the fix:

npm install react-native-nested-scrollview --save
# From the root of your RN project
wget https://github.com/hemantasapkota/react-native-nested-scrollview/blob/master/root.patch
# Apply the patch
patch -p0 < root.patch | 2>/dev/null

Final step would be to use NestedScrollView instead of ScrollView in your source project.

@dviluk
Copy link

dviluk commented Oct 17, 2017

@hemantasapkota hello dude, i get this error with your solution:

react-native-nested-scrollview\android\src\main\java\com\mohtada\nestedscrollview\ReactNestedScrollViewManager.j
ava:37: error: ReactNestedScrollViewManager is not abstract and does not override abstract method flashScrollIndicators(ReactNestedScrollView) in ScrollComman
dHandler
public class ReactNestedScrollViewManager

package.js

	"dependencies": {
		"react": "16.0.0-beta.5",
		"react-native": "0.49.3",
		"react-native-html-parser": "^0.0.5",
		"react-native-loading-spinner-overlay": "^0.5.2",
		"react-native-navigation": "^1.1.236",
		"react-native-nested-scrollview": "^0.0.3",
		"react-native-platform-touchable": "^1.1.1",
		"react-native-scrollable-tab-view": "^0.8.0",
		"react-native-smart-loading-spinner-overlay": "^1.0.2"
	},

@ZionChang
Copy link

<FlatList
  data = {[{}, {}, {}]}
  horizontal
  pagingEnabled
  keyExtractor = {(item, index) => index}
  renderItem = {({item, index}) => {
    if (index == 0) {
      return (
        <View style = {{width}}>
          <View style = {styles.header}>
            <Text>{`Page: ${index}`}</Text>
          </View> 
          <ScrollView
            horizontal
            pagingEnabled
          >
            <View style = {styles.header}>
              <Text>ChildPage: 1</Text>
            </View>
            <View style = {styles.header}>
              <Text>ChildPage: 2</Text>
            </View>
          </ScrollView>
        </View>
      )
    } else {
      return (
        <View style = {styles.header}>
          <Text>{`Page: ${index}`}</Text>
        </View>
      )
    }
  }}
/>

When scrollview is embedded in a item of flatlist, it works in iOS, but not working in android.

Any help? thanks

@hramos hramos added Type: Discussion Long running discussion. and removed Ran Commands One of our bots successfully processed a command. labels Mar 5, 2018
@rekha110254
Copy link

@jenskuhrjorgensen i'm experiencing the same issue!! Did you find any solution?? :(

@jenskuhrjorgensen
Copy link

@rekha110254 Nope - I was so lucky that the design changed instead! 🍀 😄

@rekha110254
Copy link

@chetankotkar i'm experiencing same issue!! Did u find any solution? :/

@wamry
Copy link

wamry commented Apr 12, 2018

@ashrithks ur method is buggy, it doesnt work after u scroll down and then go up. tried it on a real device.

@b1acKr0se
Copy link

Anyone has the solution to this?

I have to use nested FlatList inside ScrollView, but onEndReached keeps firing without reaching the end...

@alz10
Copy link

alz10 commented May 30, 2018

Mine was ScrollView is rendered inside FlatList. The ScrollView scrolling effect doesnt work anymore but the FlatList is working fine.

Any solution for this?

@alz10

This comment has been minimized.

@ButuzGOL
Copy link

I have found this nestedScrollEnabled but seems like its not working
https://facebook.github.io/react-native/docs/scrollview#nestedscrollenabled
https://snack.expo.io/HJsi9gDmQ
?

@ivanzotov
Copy link
Contributor

nestedScrollEnabled works, first of all make sure your RN version is >= 0.56.0, because this property was introduced in 0.56.0. Add nestedScrollEnabled for inner/child ScrollView component.

@ZeroCool00

This comment has been minimized.

@kelset
Copy link
Contributor

kelset commented Sep 19, 2018

As pointed out in earlier comments, now nested scrolling is possible on both platforms via nestedScrollEnabled (which needs to be explicitly set for Android to use it, and as Ivan pointed out you need to have it set in the inner list).

@kelset kelset closed this as completed Sep 19, 2018
@alloy
Copy link
Contributor Author

alloy commented Sep 19, 2018

@kelset The OP is not about being able to scroll in nested scrollviews, though, that’s just something that many people have been discussing here but is off-topic.

@kelset
Copy link
Contributor

kelset commented Sep 20, 2018

Hey @alloy thanks for the clarification, but then I'm confused by:

The developer and user of that component shouldn’t need to jump through hoops to have ScrollView callbacks work, but should be able to write isolated components that are unaware of the presence of other ScrollView instances in the view hierarchy and have React Native deal with the details of dispatching events to all components that require them.

I mean, it seems to me that nestedScrollEnabled obtains that 🤔
Or there is a key "under the surface" difference that I'm missing? Was it more about reaching something closer to this?

he suggested abstracting the scroll position/event part from ScrollView into a Scrollable container

Anyway I'll reopen :)

@kelset kelset reopened this Sep 20, 2018
@alloy
Copy link
Contributor Author

alloy commented Sep 20, 2018

Yeah it’s a nuanced issue 😄 It’s about having scrollviews inside a scrollview, which would disable the nested scrollviews from scrolling independently but still receive scroll events as if there were scrolling their own content. If you look at the GIF in the OP, you’ll see that per tab there’s only 1 large scrolling view, but there are actually some nested scrollviews in that hierarchy.

@cpojer
Copy link
Contributor

cpojer commented Mar 19, 2019

I discussed this issue with @alloy in person. This is a valid proposal and definitely a change we'd like to have in React Native but over the course of almost three years nobody has put themselves forward to actually implement it and we haven't had a need for this at Facebook either, hence I'm going to close this issue.

However, if you feel passionate about adding the right APIs without breaking existing code, please feel free to keep discussing the concrete implementation here and start sending PRs that gets us closer to the goal :)

@cpojer cpojer closed this as completed Mar 19, 2019
@nishaQueppelin
Copy link

Try this, It worked for me
<ScrollView {...parentProps}>
<ScrollView {...childProps} nestedScrollEnabled={true}>

@facebook facebook locked as resolved and limited conversation to collaborators Mar 20, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

No branches or pull requests