Apply layout corrections only to consecutive cells #788
Conversation
| // Only apply correction if the next cell is consecutive. | ||
| val isNeighbourConsecutive = cell.index == neighbour.index + 1 |
There was a problem hiding this comment.
Should we apply this logic on iOS, too, or does the problem not manifest there?
There was a problem hiding this comment.
iOS changes are also present in this PR. Check below
ios/Sources/AutoLayoutView.swift
Outdated
| @objc(onBlankAreaEvent) | ||
| var onBlankAreaEvent: RCTDirectEventBlock? |
There was a problem hiding this comment.
@naqvitalha it seems the indentation for the whole file has changed and it's now hard to figure out what has actually changed. Can we keep here only the actual changes to make the review easier?
There was a problem hiding this comment.
Yeah apologies for this. I reformatted because indentations were looking off in github. I'll revert ASAP
naqvitalha
force-pushed
the
blank-item-issue
branch
from
434baf0 to
067a221
Compare
|
Did extensive tophatting. Looks good! |
|
Hey @naqvitalha 👋 I saw this issue on the web platform too, are there any plans to fix it not only for iOS and Android? |
|
@naqvitalha This change broke the flash list usage in my project. Issue which includes some details: #812 Do you have an idea? cc @fortmarek |
|
@AlexanderEggers Replied on the issue |
Description
resolves #600
Issue summary: FlashList sometimes overlaps items when
getItemTypeis used.This issue was reported long ago but we couldn’t identify the root cause. We recently figured out that the issue happens when you have two items of different types next to each other but not in consecutive order from the data perspective.
For example, assume View1, View2, View3, View4, View7, View8 are present in the view hierarchy. From the parent view’s perspective View4 and View7 are next to each other but they’re actually not because we should have View5 and View6 in between. The reason for the two views being missing could be many, one of them being slow rendering due to expensive components and View7 / View8 are already in the view tree because they’re of different type so they might not be getting recycled at the same time.
In the aforementioned case AutoLayout will also think View4 and 7 are neighbours with a gap and it’ll pull 7 up pushing it out of sync with the react layout. This may or may not get fixed based on future onLayout operations. This is why we can see overlaps or gaps.
This PR should resolve the issue by making sure AutoLayout understands whether or not views are consecutive.
Reviewers’ hat-rack 🎩
Try the list sample on a slow device. Scroll down so that you see item 98 and 99 once. Scroll to the top and quickly scroll down. You will notice red items popping in and out of places where they should never be.
Checklist