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

Inverted FlatList displays activity indicator at the bottom #17553

Open
quadsurf opened this issue Jan 11, 2018 · 43 comments
Open

Inverted FlatList displays activity indicator at the bottom #17553

quadsurf opened this issue Jan 11, 2018 · 43 comments
Labels
Bug Component: FlatList Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Type: Enhancement A new feature or enhancement of an existing feature.

Comments

@quadsurf
Copy link

quadsurf commented Jan 11, 2018

Is this a bug report?

yes

Have you read the Contributing Guidelines?

yes

Environment

Environment: all newer environments
Packages: any packages
Target Platform: any platform
Version: 0.53.0 and all other versions of RN that support FlatList with the inverted attribute

Steps to Reproduce

  1. return <FlatList {...props} inverted data={[...]} refreshing={this.state.isLoading} onRefresh={this.someListUpdatingFunction} />
    (just add the 'inverted' attribute to any RN <FlatList/> to reproduce, then try pulling down to refresh, then try pulling up to refresh)

Expected Behavior

onRefresh should allow users to refresh using the most common way that users have been trained to refresh data over the years, which is to "pull down to refresh" and see an ActivityIndicator spinning above the FlatList, never below it... this should be the default even when inverted={true}

Actual Behavior

user has to pull UP to refresh (with Activity Indicator at the bottom) ... literal vs intuitive: when taken "literally", i can agree that this is expected behavior since it is after all "inverted" ... however, when taken "intuitively", it's not so expected, especially for users... I don't think app users have ever been trained to pull up to refresh data, except for maybe in the "OfferUp" chat app!

Most Applicable Use Case

  • a chat app, where users pull down to load more chat history that's appended above, similar to how it's done in iOS Messages app
  • UIs like terminals, event logs, chat, etc... where it's common to insert new content from the bottom and load old content when the user scrolls to the top or pulls down to refresh

Demo

https://snack.expo.io/S1UOyWgSM
(to reproduce, just add the 'inverted' attribute to any RN <FlatList/> component using v0.53.0 or any version of RN that supports FlatList)

@quadsurf quadsurf changed the title pull down to refresh does not work on inverted FlatList pull down to refresh does not work on "INVERTED" FlatList Jan 11, 2018
@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like you may not be using the latest version of React Native, v0.53.0, released on January 2018. Can you make sure this issue can still be reproduced in the latest version?

I am going to close this, but please feel free to open a new issue if you are able to confirm that this is still a problem in v0.53.0 or newer.

How to ContributeWhat to Expect from Maintainers

@react-native-bot react-native-bot added Ran Commands One of our bots successfully processed a command. Stale There has been a lack of activity on this issue and it may be closed soon. labels Feb 24, 2018
@hramos
Copy link
Contributor

hramos commented Feb 24, 2018

Looks like this may have been closed too eagerly -- the bot didn't see if v53 or newer was used in your issue description.

I'd still argue it's unclear to mention "newer versions" without being explicit, so I'll leave this closed. Please do re-open and make sure to include a specific version.

@Draccan
Copy link

Draccan commented Feb 26, 2018

Hi, I have reproduced the same problem on V53!

@hramos hramos reopened this Feb 27, 2018
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 27, 2018
@hramos hramos changed the title pull down to refresh does not work on "INVERTED" FlatList Inverted FlatList displays activity indicator at the bottom Mar 5, 2018
@hramos
Copy link
Contributor

hramos commented Mar 5, 2018

This issue is straddling the line between bug report and feature request. I'll therefore mark it as a Good first issue for someone to tackle if they would like to get their feet wet with making contributions to React Native.

@hramos hramos added the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label Mar 5, 2018
@quadsurf
Copy link
Author

quadsurf commented Mar 5, 2018

@hramos thank you for keeping this alive! I may be able to tackle this in a few weeks, but rather than changing the current behavior, I would just provide other developers with the option to choose for themselves which behavior they prefer between [pull up to refresh] or [pull down to refresh], while in inverted mode of course.

@hramos hramos added the Lists label Mar 8, 2018
@amloelxer
Copy link

@hramos I would love to work on this as my first issue, but is there a guide for building the React-Native source for iOS? All I see are the steps for Android.

@hramos
Copy link
Contributor

hramos commented Mar 14, 2018

I don't think we have a guide for that, but building from source for iOS may not be necessary if the fix only involves JavaScript.

Open RNTester/RNTester.xcodeproj in Xcode and select the RNTester target, and any of the iPhone simulators:
simulator screen shot - iphone x - 2018-03-14 at 11 58 09

Then open the Flat List Example:

simulator screen shot - iphone x - 2018-03-14 at 11 58 09

Confirm everything works as expected.

You may need to follow the instructions for installing your fork of React Native: http://facebook.github.io/react-native/docs/android-building-from-source.html#1-installing-the-fork

Then you can proceed to add your changes to Libraries/Lists/FlatList.js (or SectionList or VirtualizedList, as needed) in your fork, and re-run the Flat List Example to verify.

Thanks for letting me know these instructions need to be expanded! Let me know how it goes and we can work on improving the docs.

@amloelxer
Copy link

@hramos will do! Thanks for the info

@himanshuchauhan
Copy link

If this has not been fixed .I would like to start working on this. Can someone confirm?

@amloelxer
Copy link

@himanshuchauhan feel free. I spent an hour or two looking at it, but it seemed to me I couldn't do it with just Javascript, and then work and life happened. Curious to your solution if you end up doing it!

@himanshuchauhan
Copy link

I have been trying to fix this . But I am having issues running the RNTester/RNTester.xcodeproj .
I am getting this -

no script url provided make sure packager is running or you have embedded a js bundle

@briank621
Copy link

If anybody else isn't, I'll be taking a look at this

@briank621
Copy link

I don't think this can be solved with a purely Javascript implementation. The inverted attribute causes a scale transform of -1 on the ScrollView (which is then undone by each cell Item inside to make the cell appear upright). Since the activity indicator would normally appear at the top, it now appears below.

The activity indicator is being generated by the RefreshControl component attached to the ScrollView. A fix that I have in mind is changing the location of the frame of the RefreshControl, as discussed here, but I haven't had success doing that. From there we would have to change how RefreshControl triggers a refresh (which normally occurs when scrollY: 0), so that it happens when scrollY matches the height of the page.

Somebody else can take another try at this. There might be a simpler approach, but the main issue is from the scale transform of -1.

@endiny
Copy link

endiny commented May 6, 2018

Is it really a bug at all? Inverted could mean an inverted refresh either

@amloelxer
Copy link

@briank621 I thought the exact the same thing. I didn't spend a ton of time looking at it, but remember at the time I didn't think it could be done solely in JS without doing some hackey things. I'll try to take a peak again and maybe build React-Native from source when I have time.

@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like your issue may be missing some necessary information. Can you run react-native info and edit your issue to include these results under the Environment section?

Thank you for your contributions.

@giantss
Copy link

giantss commented May 24, 2018

@quadsurf @hramos I am now also making a chat interface, I need to do a chat history load, really need this refresh reversal function.

@kiranjd
Copy link

kiranjd commented Jul 4, 2019

Instruction for installing the forked version in this comment is broken. Could someone direct me where I can find it?

@fabOnReact
Copy link
Contributor

fabOnReact commented Apr 17, 2020

users have been trained to refresh data over the years, which is to "pull down to refresh" and see an ActivityIndicator spinning above the FlatList, never below it.

In WhatsApp Inverted FlatList (image on the right) Pull Down displays the older chat history.

In WhatsApp Inverted FlatList (image on the right) Pull Up triggers the refresh:

I agree that the ActivityIndicator should show on top and never on the bottom and I will try to implement this solution, please let me know if I misunderstood the requirement. Thanks a lot.

@fabOnReact
Copy link
Contributor

fabOnReact commented Apr 21, 2020

RefreshControl prop progressViewOffset works only on Android #10718.

progressViewOffset allows to change the position of the RefreshControl in a FlatList on Android.

export default function App() {
  const [refreshing, setRefreshing] = useState(false);  
  const screenHeight = Dimensions.get('window').height;
  const onRefresh = () => {
    setRefreshing(true)
  }

  return (
    <FlatList
      data={POSTS}
      renderItem={({ item }) => <Item data={item} />}
      keyExtractor={item => item.id}
      refreshControl={<RefreshControl progressViewOffset={screenHeight - 100} refreshing={refreshing} onRefresh={onRefresh} />}
      inverted
    />
  );
}

You can try the following snack on an Android device. The RefreshControl will show on top of the screen in an inverted FlatList and as explained in #10718 it does not work on iOS, I will work on another issue as I focus on Android.

iOS developer should read the pull requests and discussions related to #10718 to find a solution. Thanks a lot 😃

@elicwhite
Copy link
Member

I don't have any context on progressViewOffset but it is interesting that that exists.

If that existed on iOS does that feel like a more proper solution to this problem?

I'd like to avoid adding yet-another-prop to FlatList in #28857 if there is an existing prop that is intended to enable this behavior.

@fabOnReact
Copy link
Contributor

If that existed on iOS does that feel like a more proper solution to this problem?

Yes, but it does not work on iOS with the changes to RCTScrollView from 1d22f8f, as Pull Request #11356 never made it to master.

iOS Developer should check the diff and comments from RCTRefreshControl 11356 and publish a new pull request.

Currently I am working on a pull request for #18266 (comment) and CircleCI, so I would avoid working with iOS. Thanks a lot 😃 I wish you a good weekend 😃

@awalmubarak
Copy link

awalmubarak commented May 9, 2020

If that existed on iOS does that feel like a more proper solution to this problem?

I thought the same but it only works on android and not iOS.

Also when i use progressViewOffset,on android, It just moves the refresh control up but you still have to swipe from bottom to top in order to trigger refreshing instead of dragging from top to bottom.

@fabOnReact
Copy link
Contributor

changes included in 1d22f8f broke progressViewOffset on iOS

this breaks setting negative offsets, I'm using this with contentInset + RefreshControl to make sure it is positioned properly under a overlapping header. This could probably be solved by supporting progressViewOffset on iOS, not sure if there are other use cases for negative offsets.

#11356 was meant to fix iOS, but never made it to master

I'll be happy to implement changes for Android if they will be merged in master.

@sabinayakc
Copy link

Any update on this issue?

@indira-lima
Copy link

Just ran into this issue in RN 0.62.2. I believe @ChenLi0830 's workaround isn't the best solution due the Nested VirtualizedList warning ('VirtualizedLists should never be nested inside plain ScrollViews')

@utkarsh-dixit
Copy link
Contributor

Is some working on this? If not, I'd like to work on this.

@acrazing
Copy link

Aha, I solved this problem with the following code:

        <FlatList
          ref={this.messageList}
          style={styles.list}
          contentContainerStyle={styles.listContent}
          data={messages}
          renderItem={renderItem}
          inverted={true}
          keyExtractor={(item) => item.id.toString()}
          onEndReached={this.fetchMoreMessages}
          ListFooterComponent={<ActivityIndicator />}
          ListFooterComponentStyle={{ flexGrow: 1, paddingTop: 20 }}
        />

@icecapp
Copy link

icecapp commented Nov 1, 2021

IMO refresh is, as its name indicates, intended for fresh, new data. It doesn't make sense to use this for past data.
UX-wise, users today are accustomed to infinite scroll when scrolling in the past. Not pull-to-refresh.
Notice e.g. most chats when reaching the tail of the list use infinite scroll.
Also if the list and everything in it are inverted, I think it would be really counter-intuitive to not invert the RefreshControl by default, since lists load and stay scrolled at the head, which is never understood the be "the beginning", but rather, "most recent".
For use cases where this is still necessary, IMO this should really not interfere with inverted but rather as an additional prop, e.g. akin to invertedStickyHeaders in SectionList, you'd get 'invertedRefresh`.

@RominHalltari
Copy link

@hramos I think @icecapp's argument makes sense. I would like to try to tackle this but I don't know in which direction I should go at this point. Should I introduce the invertedRefresh prop and its functionality?

@AbdelhamidLarachi
Copy link

i solved it by using
contentContainerStyle={{ flex: 1, justifyContent: 'flex-end' }} instead of inverted. This will keep displaying your messages from the bottom and keep your RefreshControl on top.

<FlatList
refreshControl={<RefreshControl refreshing={refreshing} onRefresh={onRefresh}/>}
contentContainerStyle={{ flex: 1, justifyContent: 'flex-end' }}
renderItem={Message}
data={loadedMessages ? getConversation(conversation, conversations) : MessagesLoaderMockData}
style={styles.MessagesFlatList} />

@marco-hic
Copy link

i solved it by using contentContainerStyle={{ flex: 1, justifyContent: 'flex-end' }} instead of inverted. This will keep displaying your messages from the bottom and keep your RefreshControl on top.

<FlatList
refreshControl={<RefreshControl refreshing={refreshing} onRefresh={onRefresh}/>}
contentContainerStyle={{ flex: 1, justifyContent: 'flex-end' }}
renderItem={Message}
data={loadedMessages ? getConversation(conversation, conversations) : MessagesLoaderMockData}
style={styles.MessagesFlatList} />

I've used flexGrow instead of flex, flex blocks scrolling :(

@MariuzM
Copy link

MariuzM commented Nov 14, 2023

Damn its 4 years and still no native solution from facebook...

@arjun1194
Copy link

What is the status of this issue ? labels added mention it as a good-first-issue and help-wanted is it available to be picked up ?

@fabOnReact
Copy link
Contributor

#17553 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Component: FlatList Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.