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

Don’t use getItemLayout #47

Closed
wants to merge 2 commits into from

Conversation

infostreams
Copy link
Contributor

@Exilz
Copy link
Contributor

Exilz commented Jan 12, 2018

Shipped in 2.1.5.

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

Was this done because of an issue? Where can I see how to reproduce it? @Exilz @infostreams

@infostreams
Copy link
Contributor Author

Well, it broke whenever you did the following:

  1. Make a Gallery with a window size and an initialNumToRender
<Gallery
   {...props}
    flatListProps={{windowSize: 2, initialNumToRender: 1}}  
  />
  1. Open the gallery, then close the gallery again
  2. Open the gallery on an initialPage that is larger than your windowSize
  3. Broken; your user sees a black screen because the item is never rendered.

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

OK. Thank you. I'll take a look.
The reason for asking is the regression this has created in the lib. There were things that used to work and don't anymore.

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

Well, I have a gallery of 4 items. I passed in windowSize: 2 with initialNumToRender: 1 and I have no issue browsing around and/or using initialPage when using 2.1.4.
We're using:
"react-native": "^0.49.3",
"react-native-image-gallery": "^2.1.4",

@infostreams

@infostreams
Copy link
Contributor Author

Well perhaps I misremember the exact circumstances, but I'm not making up that it is (or was) broken:

facebook/react-native#1831
facebook/react-native#8607
facebook/react-native#13316
facebook/react-native#13202
meliorence/react-native-snap-carousel#238

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

So there was never an issue reported for react-native-image-gallery? But you think the issue was that you needed to scroll for rendering to happen? I also cannot remember "the circumstances" but I remember I've seen this bug while debugging about #20 and it was something that was not a problem of react-native-image-gallery but in my app.

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

@infostreams Here is the discussion on this. #20 (comment) Could you check and try to remember if it is the same issue?

@infostreams
Copy link
Contributor Author

Not entirely sure why you are accusing me of making things up, but yes - one of the most commented on issues in React Native was also showing its ugly head in this repository. I'm sorry that you can't reproduce it, and I'm sorry that you are having issues with this.

I'm not sure if it's the same bug as #20, but the symptoms do look the same.

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

Sorry. I'm not accusing anyone of anything. Just trying to make sure the lib works as expected. I see there is a regression and I am sure that it is fighting an issue that is not an issue with the lib but with the rest of people's apps. FlatList does some thing "afterinteraction". If you have something that blocks the queue, you wn't get a render. So... that's what the real problem is. It's a problem with usage and not with how the lib works.

@vbuch vbuch mentioned this pull request Jan 30, 2018
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

Successfully merging this pull request may close these issues.

3 participants