facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.28k stars 24.35k forks source link

Starting from 0.64.0 `RefreshControl` causes an unwanted visual jump in the list content when the `refreshing` prop goes from `true` to `false` #31461

Open krismeld opened 3 years ago

krismeld commented 3 years ago

Description

Starting from 0.64.0 in iOS on a physical device: When using <RefreshControl> inside <ScrollView> or <FlatList>, the content in the list has an unwanted visual jump down when the refreshing prop goes from true to false - this is not the case in 0.63.4.

https://user-images.githubusercontent.com/1485576/116700722-626f9d00-a9c7-11eb-8ce9-107bf3dca592.mov

React Native version:

System:
    OS: macOS 11.2
    CPU: (8) x64 Apple M1
    Memory: 667.83 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.15.4 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.10 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.1 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 14.4, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
    Android SDK:
      API Levels: 28, 29, 30
      Build Tools: 28.0.3, 29.0.2, 30.0.3
      System Images: android-29 | Google Play Intel x86 Atom, android-30 | Google APIs Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 4.1 AI-201.8743.12.41.7042882
    Xcode: 12.4/12D4e - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_282 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.1 => 17.0.1 
    react-native: 0.64.0 => 0.64.0 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

Install a clean version of 0.64.0 and run the code from the RefreshControl docs https://reactnative.dev/docs/refreshcontrol on a physical device. In the provided videos I have only changed the fake timeout from 2000ms -> 500ms to show the jumping quicker and printed the refreshing state.

Expected Results

This is how it looks in 0.63.4, where no jumping is happening:

https://user-images.githubusercontent.com/1485576/116701050-c1351680-a9c7-11eb-8507-cbc0e8eab36b.mov

Snack, code example, screenshot, or link to a repository:

import React from 'react';
import { RefreshControl, SafeAreaView, ScrollView, StyleSheet, Text } from 'react-native';

const wait = (timeout) => {
  return new Promise(resolve => setTimeout(resolve, timeout));
}

const App = () => {
  const [refreshing, setRefreshing] = React.useState(false);

  const onRefresh = React.useCallback(() => {
    setRefreshing(true);
    wait(500).then(() => setRefreshing(false));
  }, []);

  return (
    <SafeAreaView style={styles.container}>
      <ScrollView
        contentContainerStyle={styles.scrollView}
        refreshControl={
          <RefreshControl
            refreshing={refreshing}
            onRefresh={onRefresh}
          />
        }
      >
        <Text>{refreshing ? 'Refreshing' : 'Done'}</Text>
      </ScrollView>
    </SafeAreaView>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  scrollView: {
    flex: 1,
    backgroundColor: 'pink',
    alignItems: 'center',
    justifyContent: 'center',
  },
});

export default App;
Deain commented 3 years ago

same issue

dandre-hound commented 3 years ago

I opened (and have now closed) a duplicate issue. I had added some notes there that may be worthwhile to pin to this issue. My notes are in that issue #31563.

blastoy commented 3 years ago

@dandre-hound were you able to fix it? I'm also experiencing the same issue.

dandre-hound commented 3 years ago

Nope -- today I was going to look at any RefreshControl-related files and their diffs between 0.63.4 and 0.64.1 to see what changed. Maybe something will stand out.

Edit: nothing immediately stands out in the 0.63.4...0.64.1 diff. I searched the diff for any direct references to refresh control but there is none. Nor are there changes to any of the list view implementations, minus a few seemingly unrelated changes to the Android scrollview implementation. We're not seeing this issue on Android anyway so I ignored these changes. Unsure where to go from here. It may be helpful to follow the logic for when the refreshing prop is changed.

JuanpaG94 commented 3 years ago

Can confirm we're facing this issue after upgrading RN to 0.64.

dandre-hound commented 3 years ago

Has anyone had any luck with this issue on RN 0.65.x?

krismeld commented 3 years ago

@dandre-hound I have not upgraded and tested it yet - have you tried?

dandre-hound commented 3 years ago

@krismeld I built a new RN v0.65-rc.2 app with the same sample code attached above and observed the same behavior, despite the RC change log mentioning a lot of changes to the refresh control component.

krismeld commented 3 years ago

@dandre-hound An update: I am pretty sure I have found the PR where this behaviour was introduced, it happens here:

28236

I have rolled back the code changes made in that PR by hand in my test repo, and the issue is gone. So it looks like some of the changes introduced in that code has caused this issue to appear.

dandre-hound commented 3 years ago

Thanks @krismeld. I asked the developer who opened that PR whether it's safe to exclude this commit. I'm surprised more developers haven't brought this up. Lists are an important mobile component and this is such an awkward and obvious UI issue. I think we are just going to patch-package this and remove the changes from that PR like you suggested, assuming that PR doesn't fix any other critical pull to refresh issues.

safaiyeh commented 3 years ago

Thanks everyone for the discussion & investigation. It would be nice to find a fix that resolves both the large navigation use case and general usages here. cc @yogevbd

yogevbd commented 3 years ago

I'm looking into it.

dandre-hound commented 3 years ago

@yogevbd friendly ping to see if there are any updates here? I see that this 1b0fb9bead was formally added to the v0.65 RC changelog.

danieloi commented 3 years ago

Would love to see this fixed 🙏🏾

mdaj06 commented 2 years ago

@lunaleaps can i have a go at this?

lunaleaps commented 2 years ago

@lunaleaps can i have a go at this?

Yea for sure! No need to ask for permission -- I realize some issues I had created (as part of a project effort re: accessibility) I had wanted to coordinate to avoid folks to step on each other's toes -- but other issues in this repo, please grab as interested! Thanks so much for your help!

oleksandr-dziuban commented 2 years ago

Hello guys,

We have a reproduction on empty RN template for this issue and video recordings in this issue: https://github.com/facebook/react-native/issues/34394

Steps to reproduce Pull to refresh During refreshing, swipe left (right) on refresh control area

Snack, code example, screenshot, or link to a repository link to a repository: https://github.com/wanted-o/RefreshIssue

https://user-images.githubusercontent.com/14195600/184646719-8972aef1-a622-492e-8ce0-5a8f3deac763.mov https://user-images.githubusercontent.com/14195600/184646770-c538de03-987f-405d-abc2-3e5861c99e6c.mov

oleksandr-dziuban commented 2 years ago

Could someone from RN maintainers team take a look? Thanks in advance

ranaavneet commented 2 years ago

Hi Everyone,

This issue still persists on 0.69.5. I am surprised the issue is still there even after more than a year.

Does anyone have a patch for it, until it gets resolved in latest RC

@dandre-hound @mdaj06 @yogevbd @krismeld

nzcodarnoc commented 1 year ago

I've just upgrade to 0.70.5 and I am experiencing this issue.

patrickxunuo commented 1 year ago

0.71.8 still have this issue

LukaBabunadze commented 10 months ago

Guys, just add flex: 1 to the parent and it should work. For example: `<View style={{flex: 1}}>

`

kelset commented 10 months ago

just a quick update, aside from the workaround @LukaBabunadze offered, I was able to (STILL) repro the issue using @oleksandr-dziuban's comment -> https://github.com/facebook/react-native/issues/31461#issuecomment-1217689522

on 0.73.1:

Screenshot 2024-01-03 at 17 33 53
Daavidaviid commented 9 months ago

Guys, just add flex: 1 to the parent and it should work. For example: <View style={{flex: 1}}> <FlatList/> </View>

For me it's still jumpy :'(

tj-mc commented 5 months ago

Edit: Actually, it is not occuring 🤔

Ongoing in 0.73.6