facebook/react-native

[0.19][Android][ListView] onChangeVisibleRows isn't called on Android

bit3725 opened this issue ยท 29 comments

Hi, i found the function for onChangeVisibleRows of ListView would be called on iOS when init or scroll a ListView but not on Android.

I did a quick dig in source code of ListView component and found e.nativeEvent.updatedChildFrames was undefined when passed to _updateVisibleRows function which caused onChangeVisibleRows can't be called on Android.

I checked UIExplorer example also and saw this prop was demonstrated in ListViewPagingExample, but only included in example of iOS version. Does it mean it's not supported on Android currently?

Hey bit3725, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!
nikuz commented

The issue is reproduced for me also. The "onChangeVisibleRows" event is triggered on iOS but don't triggered on Android.

Same here. Also happened on 0.18 btw.

nikuz commented

Adding "renderScrollComponent" to the "ListView" will break the "onChangeVisibleRows" even in iOS as and "onEndReached".

any updates?

Maybe any workaround to get visible rows?
/cc @bit3725 @nikuz @alongubkin

For everyone interested I've created productpain post. Please vote it up.

No updates on this ?

Hi, to anybody looking for a solution to this problem, I wrote one in javascript. It seems to work pretty well, but only for vertical listviews (at the moment), and with no real thought given to additional complexity. I haven't submitted a pull request because I think this should be done natively, and my code is not robust enough yet. However, if you'd like to follow these steps, you can increase the performance of your ListView by a lot and get onChangeVisibleRows to fire.

  • Add this diff to your ListView. Careful to convert offsetX/offsetY to regular offsets, as support for horizontal ListViews has apparently been added since this diff was recorded.
  • At the top of _updateVisibleRows add the following code:
    var updatedFrames = [];
    let ySum = 0;
    for (var i = 0; i < this.props.dataSource._dataBlob.s1.length; i++) {
      const height = (this.props.childHeights[i] || this.props.defaultRowHeight);
      updatedFrames.push({
        index: i,
        x: 0,
        y: ySum,
        width: this.props.defaultRowWidth,
        height
      });
      ySum += height;
    }

This will make sure that you have an array of meta data with which the existing algorithm can calculate changes in visibility.

  • In order to make this work, you need to bind an onLayoutHandler to each item in the list that populates the childHeights array. Something like:
class MyListWrapper extends Component {
  constructor() {
    super()
    this.childHeights = [];
  }
   ....
  renderRow(data, secId, rowId) {
    return <View onLayout={(e) => {this.childHeights[parseInt(rowId)] = e.nativeEvent.layout.height;}}/>
  }
  ...
  render() {
    return <ListView renderRow={this.renderRow} childHeights={this.childHeights} defaultRowHeight={n}/>
  }
}

This could also probably be done within the ListView itself. But I didn't want to go around meddling too much in there. I have not had any problems with rendering since implementing this, and I can confirm that views are indeed recycled as you scroll. I have an image heavy ListView in my app, and now I can confirm that my memory allocation has been decimated since doing this.

I just want to reiterate that this solution is a hack and only to be used when you have little time for anything else. Hope it helps somebody that's in a jam.

Sam

@sampurcell93 Thanks sam! though, I agree with you that should be handled natively

@sampurcell93 thank you so much! Gonna try your workaround with my GitterMobile and will write back my results for anyone interested.

Also, I just realized I'm an idiot and forgot that the x and y values are present in the nativeEvent.layout object. So there is no need for computing those.

@sampurcell93 so what should we and I do? I didn't understand :) I think it would be great if you write the blog post or smth like that about it.

Hey terry, the wrapper code can be modified to look something like this:

class MyListWrapper extends Component {
  constructor() {
    super()
    this.childSizes = [];
  }
   ....
  renderRow(data, secId, rowId) {
    /** Don't get layout.height, just get layout. It already contains {height, x, y, width} so we don't need to compute */
    return <View onLayout={(e) => {this.childSizes[parseInt(rowId)] = e.nativeEvent.layout;}}/>
  }
  ...
  render() {
    return <ListView renderRow={this.renderRow} childSizes={this.childSizes} defaultRowHeight={n}/>
  }
}

And the ListView itself, at the top of _updateVisibleRows():

var updatedFrames = [];
    const horizontal = this.props.horizontal;
    if (this.props.dataSource._dataBlob.s1) {
      for (var i = 0; i < this.props.dataSource._dataBlob.s1.length; i++) {
        if (this.props.childSizes[i]) {
          updatedFrames.push({
            ...this.props.childSizes[i],
            index: i
          })
        } else {
          updatedFrames.push({
            index: i,
            height: horizontal ? 0 : this.props.defaultRowSize,
            width: horizontal ? this.props.defaultRowSize : 0,
            y: horizontal ? 0 : i * this.props.defaultRowSize,
            x: horizontal ? i * this.props.defaultRowSize : 0
          });
        }
      }
    }

Just a reminder that this.props.dataSource._dataBlob.s1 will fail on any list with custom sections / more than one section.

This has helped me deal with the vast majority of simple ListView use cases. I am working on applying it to more complex grid-based views.

Thanks, @sampurcell93, but it doesn't want to work for some reasons. Can you, please, share working example? (:

Because in my case it just calling once child renders with visibility 'true' for all the rows, and doesn't call on scroll (as it should do).

Make sure you comment out this line:

if (!this.props.onChangeVisibleRows) {
  return; // No need to compute visible rows if there is no callback
}

Follow the flow of your code. Ensure that the updatedFrames array is being populated correctly.

Still can't get it works. Also, I've noticed some naming differents between list view component in your diff and official one.
Can you make a gist with working ListView, please?

So, I realized you've used RN 0.8 version of ListView, so now seems to works, but very laggy. For first -- all my rows are visible after render (why?). Second -- updateVisibleRows doesn't calls while scrolling; Third -- it's randomly calling; Fourth -- I have to delete isVsisble part because my rows don't render at all.

/cc @sampurcell93

Hey terry, sorry for the delay. Here is a link to the complete ListView gist.

When calling this listview, pass in an array of height numbers (ie [300, 327, ...etc]). You can also pass in withOptimizations={false} to disable this computation behavior.

Hey, Sam. Thank you so much! Now it works great.

Hey, Sam. Can you make an npm module with your modified ListView? It would be super cool as I'm not using forked react native, also it would be great if you will maintain it.

/cc @sampurcell93

guyca commented

+1

@guyca it'll be better if you vote up productpains post and ask your friends to do the same thing instead of writing a useless comment like +1. Thanks)

I wrote a real android RecyclerView to solve this problem. No memory leaks and slide is very smooth.
react-native-RealRecyclerView

@droidwolf is it working with dynamic rows' height?

No, you need to set up rowHeight property values

Any progress on this issue? Would be nice if it worked ๐Ÿ‘

I've provided a PR on this at #11945

Closing as ListView is deprecated in 0.43. Use FlatList.

Does FlatList support pages? may I do a pagination using FlatList?