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

Fixing initialPage on Android #24

Merged
merged 5 commits into from Oct 5, 2017
Merged

Fixing initialPage on Android #24

merged 5 commits into from Oct 5, 2017

Conversation

vbuch
Copy link
Contributor

@vbuch vbuch commented Oct 4, 2017

Changes:

  • Use getItemLayout;
  • Immediate scroll is done after interactions;
  • FlatList is directly called when scrolling and the recordInteraction() method is called;
  • The initialScrollIndex prop of FlatList is passed;
  • Lifecycle methods are followed by event methods, then come all our methods, then rendering;

Mostly work on #20

Valery Buchinsky added 4 commits October 4, 2017 15:39
Remove console.log
Remove extraData as it is not used
Remove initialNumToRender and let it be passed through flatListProps if needed
@smolleyes
Copy link

hello

same problem for me if i just replace my viewerPage/index.js, still not loading initialPage correctly

@vbuch
Copy link
Contributor Author

vbuch commented Oct 5, 2017

Those changes did the trick for me. Occasionally, the first image shows up and in an instant the initialPage one does. But that's only when I already have the first image cached. @smolleyes could you try and console.log() in the runAfterInteractions() so we know you don't have a blocking interaction?

@davidpaulsson
Copy link

@vbuch Yeah, problem is that runAfterInteractions() isn't being called, just like you suspect. If i patch this lib's InteractionManager with our internally used patch (lol import InteractionManager from '../../../../../src/components/InteractionManager';) it seems to work. We're doing some shenanigans like this → facebook/react-native#8624 (comment)

@davidpaulsson
Copy link

davidpaulsson commented Oct 5, 2017

Would it be a terrible idea to remove the runAfterInteractions call and just allow the scroller to scroll if (immediate) { /**/ }?

@davidpaulsson
Copy link

davidpaulsson commented Oct 5, 2017

I just want to add that yes; it does actually work on Android now (although the flickering when the 1st image shows for an instant is ugly). BUT the runAfterInteractions prevents iOS from functioning properly (at least for me. I have no idea what animations is blocking the InteractionManager, probably some transition animation from NavigationExperimental).

edit: Also noticed not that although removing the runAfterInteraction call here https://github.com/novanor/react-native-image-gallery/blob/ac8c74f8976e881514bd800f9a0596c37b3d8a40/src/libraries/ViewPager/index.js#L196 doesn't help with iOS, it breaks Android again. So back to square one.

(I'm seriously thinking of forking this PR and using my own patched version of InteractionManager I mentioned above.)

@vbuch
Copy link
Contributor Author

vbuch commented Oct 5, 2017

If InteractionsManager is blocked, you have a problem to solve that will either way cause troubles in the future. You have to look at it for your project.
The flickering could get fixed if we don't initially scroll to 0 but leave it where it is. Currently we tell the flat list to render page 5 for example, then we scroll to offset 0 and after interactions we're back to the offset of page 5. I'll give it a try and write back here if I have something.

@vbuch
Copy link
Contributor Author

vbuch commented Oct 5, 2017

I'm glad to say that the flickering is now gone. Also we have two function calls less now (see my previous comment). @davidpaulsson @smolleyes please try it with my latest commit. It's pretty close to perfect.
There is one "but". It is a tiny one. When you go to page 5 and want to swipe left, the image on position 4 is not yet rendered. This is FlatList's behavior. I'll see if I can do anything about it. I doubt that thou. Still, as I said, I think it's pretty clos to perfect now.

@smolleyes
Copy link

hello

initialPage seems working now with the lastest fix, bit i sometimes i have an error:

TaskQueue Error with task : undefined is not an object with this3.refs['innerFlastList'].scrollToOffset

thanks :)

@vbuch
Copy link
Contributor Author

vbuch commented Oct 5, 2017

Hm. Haven't seen it. Could you try and tell me what you are doing as interactions so that I can try and reproduce and eventually fix this.

@smolleyes
Copy link

i will if it come back, may be my fault

@davidpaulsson
Copy link

Confirm the flickering is indeed gone! Great work! :) Thanks for your time.

@smolleyes
Copy link

smolleyes commented Oct 5, 2017

seems ok for me, thanks ! :) will wait for merge now :p

@Exilz
Copy link
Contributor

Exilz commented Oct 5, 2017

This works on my LG G3 and Samsung Galaxy S3, thanks for the great work guys.

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.

None yet

4 participants