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

Flicker issue in Android while scrolling #39

Open
Jack-Alfie opened this issue Aug 22, 2020 · 14 comments
Open

Flicker issue in Android while scrolling #39

Jack-Alfie opened this issue Aug 22, 2020 · 14 comments
Labels
status: waiting RN Waiting an upstream fix from react-native type: bug

Comments

@Jack-Alfie
Copy link

Hi, I am using your library its really great. Now I am facing 1 issue in Android when I scroll the events right or left the data is flickering but its working fine in iOS. Kindly find the attachment thank you.
flickering.mov.zip

@pdpino
Copy link
Collaborator

pdpino commented Aug 22, 2020

I've seen this issue.
I think it is due to the eventsGrid scrolling done in WeekView.componentDidUpdate() here. When the user scrolls horizontally, the component re-renders 5 Events components (5 pages), then componentDidMount() is triggered, and scrolls to the correct position. The flickering is caused by this scrolling.

I don't see a simple way to change this, the scroll logic may need to be changed to allow infinite scrolling, without limiting it to 5 pages.

@Jack-Alfie
Copy link
Author

oh Ok thanks for your advice.

@hoangnm hoangnm reopened this Aug 30, 2020
@hoangnm
Copy link
Owner

hoangnm commented Aug 30, 2020

Reopened the issue, let's find a way to improve this.

@pdpino
Copy link
Collaborator

pdpino commented Sep 3, 2020

If the forced scrolling was invisible (i.e. so fast it is not perceived by the user), I think this would be solved. Maybe calling scrollTo at the beginning of componentDidUpdate could help.
I haven't tested this in iOS, but somehow it is invisible there.

@pdpino
Copy link
Collaborator

pdpino commented Sep 3, 2020

If the above cannot be done, my proposal would be enabling infinite horizontal scrolling, opposed to rendering a fixed number of pages. This would be more robust, as pages that were rendered at some point would only be rendered once.

  • At first render, a certain amount of pages should be rendered, for example 2 to the left and 2 to the right, as it is now. I think 1 to the left and 1 to the right is also enough. It could be configured by the user.
  • When the user scrolls, e.g. to the right, new pages should be rendered to the right, without removing the ones from the left. Thus, there is no need to scroll to the correct position in componentDidUpdate. For example:
    • At first, rendered pages are: 0, 1, 2, 3, 4. User is viewing page 2, in position horizontalPosition = 2*pageWidth
    • The user scrolls to the right, moving to page 3 and adding page 5. The user is viewing page 3, and in position 3*pageWidth.
    • Since page 0 is not removed, the user is in the correct position, without the need of an extra scroll
    • If page 0 was removed, the user would need to scroll to 2*pageWidth, causing the flickering. (This is the current behavior)
  • Notice the user may continue scrolling infinitely to the right, without needing the forced scroll
  • To avoid memory issues, the amount of pages rendered at the same time could be limited to a certain number, for example 20. If the number of pages exceeds this, the oldest page from the other side could be dropped. (This number could also be configurable).

However, I'm not sure how to implement infinite scrolling to both sides at the same time without any glitches:

  • If the user scrolls to the left, new pages will be added to the left --> the scrolling to the correct position will be needed. Following the previous example:
    • Currently rendered pages are: 0, 1, 2, 3, 4, 5, the user is viewing at page 3, in position 3*pageWidth.
    • The user scrolls (twice) to page 1, reaching position 1*pageWidth.
    • A new page to the left should be rendered (to keep the additional 2 pages to the left). This would add view -1
    • Thus, now the correct position for the user is 2*pageWidth, needing a forced scroll anyway.

Final thought: infinite scrolling could be implemented to both sides, plus configuring to be either from left or right, so the glitches occur only to one side, depending on the specific use case. For example, in my use case, I only display events in the past, never in the future, hence scrolling to the left would be smooth, but scrolling to the right will flicker (as pages will be prepended). Other use cases may favor the events in the future.

(*) In this scenario, the forced scrolling would occur in two cases: (1) when the user scrolls to the other side; and (2) when the max amount of pages is reached, and pages from the other side start to get dropped

@hoangnm
Copy link
Owner

hoangnm commented Sep 3, 2020

Thanks a lot for your suggestions! I'm trying to implement infinite horizontal scrolling using FlatList, and the problem is like you mentioned, when scrolling to the left, the scroll position is not keeping because the data is prepended to the array. The problem from react-native is not fixed yet, facebook/react-native#25239.

@pdpino
Copy link
Collaborator

pdpino commented Sep 4, 2020

The problem from react-native is not fixed yet, facebook/react-native#25239.

Too bad. Maybe using ScrollView? I'm not sure which one is more appropriate for this case. I can do some research about this when I have some time :)

What do you think about implementing infinite scroll to both sides, but favouring one side (left or right) for now? (i.e. glitches will only appear to one side). I think this could be done with either ScrollView or FlatList. Probably FlatList has more optimizations already built-in.

@hoangnm
Copy link
Owner

hoangnm commented Sep 5, 2020

Let me try with ScrollView to see if it works.

About scrolling to just one side, I think it does not feel natural. I would choose this way as the last option.

@pdpino
Copy link
Collaborator

pdpino commented Sep 5, 2020

Sorry, I think I misexplained before. I meant scrolling to both sides infinitely, but choosing to arrange the pages from left to right (older to newer), or the other way around.

For example, if choosing from left to right, the scrolling to the right will go fine and smooth, but when scrolling to the left the pages will get prepended, and there will be a flicker.

In any case, the user can scroll to both sides infinitely. (I updated the previous comments to explain better 😅)

@Osher671279
Copy link

Hello, i have the same problem here in Android, How can i fix this??

@hoangnm
Copy link
Owner

hoangnm commented Sep 27, 2020

I understand what you meant, @pdpino, that makes sense! Seems we have to accept there will be a flicker on one side. It's the behaviour when I tried switching to infinite scroll, will raise the PR soon!

@hoangnm
Copy link
Owner

hoangnm commented Oct 5, 2020

I just merged this MR, #51, applying infinite horizontal scrolling. The flicker on one side is kind of annoying, but I think this is a better way to handle scrolling. Let see if we can fix the flicker on one side issue...

@pdpino pdpino mentioned this issue Oct 5, 2020
@pdpino
Copy link
Collaborator

pdpino commented Oct 5, 2020

Great! I agree that this is better way.

In #52 I added the prop prependMostRecent, to have the flicker on the most-recent-dates side of the scrolling. Feels kind of hacky, but I think is useful for use cases where there are only past events (like mine 🙂), at least for now

@pdpino
Copy link
Collaborator

pdpino commented Oct 9, 2020

I think the definitive solution is to set the prop maintainVisibleContentPosition on the VirtualizedList (which comes from ScrollView, see prop here).

Sadly, is only implemented for iOS now, see this issue and this PR on the react-native repo.

There is also a quick solution in this comment from the PR. We could implement that or wait for the PR to be merged (and pushed to a new react-native version, I guess)

(I would still merge the #52 with the prependMostRecent hotfix, as implementing the definitive solution may take a while 🙂 )

@pdpino pdpino added the status: waiting RN Waiting an upstream fix from react-native label Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting RN Waiting an upstream fix from react-native type: bug
Projects
None yet
Development

No branches or pull requests

4 participants