Skip to content

Conversation

@vafada
Copy link

@vafada vafada commented Sep 17, 2016

There is a bug in ListView in iOS if the renderHeader returns an empty array.

The section header doesn't become sticky, instead the first row in the section becomes the sticky.

Here's the sample test app in React Native Playground: https://rnplay.org/apps/M-h9IQ

React Native version: 0.33

@ghost
Copy link

ghost commented Sep 17, 2016

By analyzing the blame information on this pull request, we identified @nathanajah and @mkonicek to be potential reviewers.

@ghost
Copy link

ghost commented Sep 17, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@ghost
Copy link

ghost commented Sep 17, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 17, 2016
@ghost
Copy link

ghost commented Sep 21, 2016

@vafada updated the pull request - view changes

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 21, 2016
@janicduplessis
Copy link
Contributor

I don't think you are supposed to return an array from renderHeader. I'd rather make it an error than support this.

@vafada
Copy link
Author

vafada commented Sep 26, 2016

@janicduplessis

An error or warning would be nice. The problem is you can return an array from renderHeader and it render properly on both iOS and Android. I had to backtrack on my commit to figure out that the empty array is causing the problem with scrolling in iOS

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 26, 2016
@facebook-github-bot
Copy link
Contributor

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @nathanajah as a potential reviewer. Could you take a look please or cc someone with more context?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 23, 2016
@hramos
Copy link
Contributor

hramos commented Nov 11, 2016

@vafada sorry for the delay here. Is this still an issue on master?

@lacker
Copy link
Contributor

lacker commented Dec 1, 2016

I think I agree with Janic - you are not supposed to return an array, so it's more of an error or warnable-offense that it kinda works. So the right thing to do is more like have a warning or just suck up the breaking change and disable that, and I wouldn't be surprised if we do that in our upcoming flowtype cleanup. So I think we should not go this direction, and I am going to close this pull request. Thanks for pointing this out and for making a good faith effort to improve the codebase here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants