[iOS] Figure a better EstimatedItemSize for HorizontalList #20022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmarinho
added
platform/ios
area-controls-collectionview
jsuarezruiz
reviewed
Jan 26, 2024
rmarinho
requested review from
Redth
and removed request for
StephaneDelcroix
|
So for my understanding, without your changes, the layout you created for your uitest doesn't work because the layout reserved three 50-width spots and the second item takes up both of them? |
tj-devel709
reviewed
Feb 14, 2024
|
yes @tj-devel709 that's it, when we have just a couple of items, the estimated size (1st item) is smaller than other cells that will be visible it will take that space. |
mattleibow
reviewed
Feb 15, 2024
|
/rebase |
github-actions
bot
force-pushed
the
fix-15815
branch
from
c410e5b to
927129c
Compare
jsuarezruiz
approved these changes
Feb 19, 2024
rmarinho
added a commit
that referenced
this pull request
Feb 22, 2024
* [Samples] Add repro case for issue #15815 * [iOS] Remove dead code * [iOS] Try find a better EstimatedItemSize for horizontal list * [uitest] Add Uitest for CollectionView * Fix test * Pink color
rmarinho
added a commit
that referenced
this pull request
Feb 27, 2024
* [Samples] Add repro case for issue #15815 * [iOS] Remove dead code * [iOS] Try find a better EstimatedItemSize for horizontal list * [uitest] Add Uitest for CollectionView * Fix test * Pink color
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
When we have Items with different width or height users should use
MeasureAllItemsfor that we rely on AutoLayout of CollectionView . This works by setting the EstimatedItemSize to some value other then empty.We use the 1st cell as the prototype to be measured and use as
EstimatedItemSize, but the problem that we see here, is that cell size is the smalles compared with the other cells that haver a wider width. When CollectionView is calculating the ContentSize, it use the EstimatedItemSize and multiplies by the number of items of the section. IF the EstimatedItemSize is very small the ContentSize can hide items in the end of the collection. This is special an issue if we have a couple of items on the List.I can't reproduce this on VerticalList so I m not sure if this is or not a issue with how UICollectionView and EstimatedItemSize. Because the idea is even if we give it a "estimated size" if should call each item to get the real value (and it does) but to does not update the initial calculate ContentSize We have seen other issues with horizontal scroll on UICollectionView so im not sure if this is also a bug related with that or not.
The fix here is simple, try to find the wider cell from the ones that are going to maybe be visible on the screen. That way the EstimatedItemSize will return a value that will fit all the cells.
Still need to write a test.
Issues Fixed
Fixes #15815