Skip to content

ListView implementation #260

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

Closed
necolas opened this issue Nov 23, 2016 · 17 comments
Closed

ListView implementation #260

necolas opened this issue Nov 23, 2016 · 17 comments

Comments

@necolas
Copy link
Owner

necolas commented Nov 23, 2016

Catch-all issue for ListView. Leave comments here if you want to report issues or have PRs that improve it.

What is the current behavior?

ListView is partially implemented and missing many features. It has been reported that this component has performance issues on native platforms, making it unlikely that the core of the RN implementation will perform well on web. I'm not actively working on ListView, nor do I plan to work on it given that it doesn't currently help us solve our problems on Web.

There's also an experimental ListView replacement being built within RN, much like the replacement for Navigator.

@gethinwebster
Copy link
Contributor

@necolas, do you have any details on the experimental ListView in RN? On a quick search I wasn't able to find any references to this, and would be interested to find out more, we're currently looking at options for a RN and RNW shared ListView component.

@necolas
Copy link
Owner Author

necolas commented Nov 23, 2016

It's called WindowedListView - https://github.com/facebook/react-native/blob/master/Libraries/Experimental/WindowedListView.js

@mkdotcom
Copy link

mkdotcom commented Dec 19, 2016

New listview is very much faster, nice 👍
But I still have this big layout issue :
#234

@gethinwebster
Copy link
Contributor

@mkdotcom I've done a quick bit of testing, and applying flexBasis: 0 to the style of the ListView seems to fix it. I've come across similar issues from time to time with other components (and fixed it in the same way), and am not sure whether this is a systematic difference between react-native and react-native-web, or something more subtle than that - I seem to remember that setting flexBasis: 0 across the board caused even more issues.

Anyway - hopefully that helps a little, here's a pen with the fix applied:

https://codepen.io/gethinwebster/pen/PbgjYw

@mkdotcom
Copy link

@gethinwebster nice, thanks !

@MarkMurphy
Copy link

Is ListView supposed to only render enough rows to fill the viewport of the component + the scrollRenderAheadDistance? (or something along those lines) If so, it doesn't seem to do that...I tried adding a couple thousand rows and it seems to render all of them at once even though it only takes 20 or so to fill the viewport.

@MarkMurphy
Copy link

MarkMurphy commented Jan 23, 2017

Also, getting this error/warning while using ListView:

Warning: Unknown props `dataSource`, `renderRow`, `initialListSize`, `pageSize`, `scrollRenderAheadDistance`, `onEndReachedThreshold`, `stickyHeaderIndices`, `onKeyboardWillShow`, `onKeyboardWillHide`, `onKeyboardDidShow`, `onKeyboardDidHide` on <div> tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop
    in div (created by View)
    in View (created by ScrollViewBase)
    in ScrollViewBase (created by ScrollView)
    in ScrollView (created by ListView)
    in ListView (created by ListViewDemo)

Using [email protected]

@gethinwebster
Copy link
Contributor

@MarkMurphy the ListView implementation is the same as the core React Native one - initialListSize rows are rendered first, and then when scrolling past the point determined by scrollRenderAheadDistance it will add pageSize additional rows.

https://facebook.github.io/react-native/docs/listview.html

@MarkMurphy
Copy link

@gethinwebster That doesn't seem to be the case. It seems no matter what I try, all rows are rendered to the DOM. Here's the code I'm trying this with: https://gist.github.com/MarkMurphy/310e7801b497381854b6ea713eb4e42f

@gethinwebster
Copy link
Contributor

@MarkMurphy If you move the fixed height to the ListView itself, then this should work as expected - it doesn't try to look up the view tree to see how much space is available, but relies on the ListView component itself (which has built-in scrolling capability) having a constrained height.

@RangerMauve
Copy link

The docs for listview seem to be kind of lacking

@MarkMurphy
Copy link

@gethinwebster

If you move the fixed height to the ListView itself, then this should work as expected - it doesn't try to look up the view tree to see how much space is available, but relies on the ListView component itself (which has built-in scrolling capability) having a constrained height.

Still didn't work for me but then I discovered that not using StyleSheet.create and just passing a raw style object got it working. I also noticed that without using StyleSheet, the render time is significantly faster i.e 1 or 2 seconds to render vs. 30 seconds when using StyleSheet. That seems like a bug...

@necolas
Copy link
Owner Author

necolas commented Jan 24, 2017

ListView isn't backed by a scroll view recycler, so it's going to render rows even if they are off screen.

That seems like a bug

Please can you create a reduced test case and create an issue?

@MarkMurphy
Copy link

@necolas I'm guessing the StyleSheet issue must have been because I was using a div or something instead of a View or other react-native-web component because this demo seems to work fine. It's just weird that I got errors using a div in the demo but not locally when I first had the issue.

@MarkMurphy
Copy link

MarkMurphy commented Jan 25, 2017

Just to reiterate on the other issue I was having, the ListView shouldn't be passing it's own props down to the ScrollView. To get around that in the mean time I had to specify my own renderScrollComponent method:

class ContactList extends Component {
  // ...
  renderScrollComponent = (props = {}) => {
    const scrollViewProps = {}

    for (let prop in ScrollView.propTypes) {
      if (props.hasOwnProperty(prop)) {
        scrollViewProps[prop] = props[prop]
      }
    }

    return <ScrollView {...scrollViewProps} />
  }

  render() {
    return (
      <ListView
        dataSource={this.state.dataSource}
        renderScrollComponent={this.renderScrollComponent}
        // ...
      />
    )
  }
}

@necolas
Copy link
Owner Author

necolas commented Jan 26, 2017

the ListView shouldn't be passing it's own props down to the ScrollView

Yeah good call, I've noticed this issue in the storybook example too.

@necolas
Copy link
Owner Author

necolas commented Apr 14, 2017

Closing this as no one is working on ListView and RN has replacements #388

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants