CollectionView - Recycle DataTemplates when using template selector #12011
Conversation
This file contains 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
Previously we were returning a single view type id for all 'items' in collectionview, which meant if you were using a template selector, and had a variety of returned different DataTemplates, each would share the same recycled item. When a recycled item is reused for a different template, it causes the view for that template to be recreated to be correct for the template selected. This change returns a unique item id per DataTemplate.Type, so that recycled items should always have the correct DataTemplate.Type and not need to be recreated with a different view hierarchy.
Like on Android, we were using a single cell reuse identifier for iOS "items" regardless of if they had different DataTemplates. If you were using a data template selector, and returned different data templates, whenever a recycled cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated. This change ensures we have a unique cell reuse id for every different DataTemplate.Type that might be returned by the template selector so that when a cell is recycled it should always be reused for the same Data Template it was created for, and so the view hierarchy won't be recreated. This does require the DetermineCellReuseId to know the indexpath we're getting it for, so a new method was introduced, internal for NET7, protected for NET8+, and in NET8+ the old method is obsoleted.
Redth
added
the
area-controls-collectionview
mattleibow
reviewed
Dec 10, 2022
hartez
suggested changes
Dec 12, 2022
src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs
Outdated
Show resolved
Hide resolved
hartez
suggested changes
Jan 27, 2023
src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs
Outdated
Show resolved
Hide resolved
rmarinho
reviewed
Feb 10, 2023
src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs
Outdated
Show resolved
Hide resolved
rmarinho
reviewed
Feb 10, 2023
src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs
Outdated
Show resolved
Hide resolved
rmarinho
reviewed
Feb 10, 2023
src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs
Outdated
Show resolved
Hide resolved
…wAdapter.cs Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com>
…wAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net>
…wAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net>
hartez
approved these changes
Feb 15, 2023
rmarinho
approved these changes
Feb 16, 2023
|
/rebase |
1 similar comment
|
/rebase |
The `CarouselViewAdapter` loops by basically returning a very large value for item count of its adapter/source which means the recyclerview is going to ask for view types with a position value that does not actually exist. Previously this was ok because we returned only a single item view type ever, however now that we want to return a different type for template selector scenarios where there are potentially multiple template types, we need to override also the `GetItemViewType` and pass in the calculated position in the list properly (as we do in `OnBindViewHolder` here already) since the base `ItemsViewAdapter` class now calls `ItemsSource.GetItem(position)` in its `GetItemViewType()` call.
PureWeen
approved these changes
May 15, 2023
expensivecow
pushed a commit
to expensivecow/maui
that referenced
this pull request
May 22, 2023
…otnet#12011) * [Android] Use different view types per item template Previously we were returning a single view type id for all 'items' in collectionview, which meant if you were using a template selector, and had a variety of returned different DataTemplates, each would share the same recycled item. When a recycled item is reused for a different template, it causes the view for that template to be recreated to be correct for the template selected. This change returns a unique item id per DataTemplate.Type, so that recycled items should always have the correct DataTemplate.Type and not need to be recreated with a different view hierarchy. * [iOS] Distinct Cell reuse id's for different DataTemplates Like on Android, we were using a single cell reuse identifier for iOS "items" regardless of if they had different DataTemplates. If you were using a data template selector, and returned different data templates, whenever a recycled cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated. This change ensures we have a unique cell reuse id for every different DataTemplate.Type that might be returned by the template selector so that when a cell is recycled it should always be reused for the same Data Template it was created for, and so the view hierarchy won't be recreated. This does require the DetermineCellReuseId to know the indexpath we're getting it for, so a new method was introduced, internal for NET7, protected for NET8+, and in NET8+ the old method is obsoleted. * Switch up some xaml controls in the sample * Expose Id internally to use for android view types * Cache datatemplates for unique item view types * Api changes (overrides) * Improve sample * Remove api txt change that shouldn't have been added * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Use DataTemplate Id for cellReuseId * Calculate correct position for carousel adapter The `CarouselViewAdapter` loops by basically returning a very large value for item count of its adapter/source which means the recyclerview is going to ask for view types with a position value that does not actually exist. Previously this was ok because we returned only a single item view type ever, however now that we want to return a different type for template selector scenarios where there are potentially multiple template types, we need to override also the `GetItemViewType` and pass in the calculated position in the list properly (as we do in `OnBindViewHolder` here already) since the base `ItemsViewAdapter` class now calls `ItemsSource.GetItem(position)` in its `GetItemViewType()` call. --------- Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
expensivecow
pushed a commit
to expensivecow/maui
that referenced
this pull request
May 22, 2023
…lector (dotnet#12011)" This reverts commit 962d1a9.
expensivecow
pushed a commit
to expensivecow/maui
that referenced
this pull request
May 22, 2023
…otnet#12011) * [Android] Use different view types per item template Previously we were returning a single view type id for all 'items' in collectionview, which meant if you were using a template selector, and had a variety of returned different DataTemplates, each would share the same recycled item. When a recycled item is reused for a different template, it causes the view for that template to be recreated to be correct for the template selected. This change returns a unique item id per DataTemplate.Type, so that recycled items should always have the correct DataTemplate.Type and not need to be recreated with a different view hierarchy. * [iOS] Distinct Cell reuse id's for different DataTemplates Like on Android, we were using a single cell reuse identifier for iOS "items" regardless of if they had different DataTemplates. If you were using a data template selector, and returned different data templates, whenever a recycled cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated. This change ensures we have a unique cell reuse id for every different DataTemplate.Type that might be returned by the template selector so that when a cell is recycled it should always be reused for the same Data Template it was created for, and so the view hierarchy won't be recreated. This does require the DetermineCellReuseId to know the indexpath we're getting it for, so a new method was introduced, internal for NET7, protected for NET8+, and in NET8+ the old method is obsoleted. * Switch up some xaml controls in the sample * Expose Id internally to use for android view types * Cache datatemplates for unique item view types * Api changes (overrides) * Improve sample * Remove api txt change that shouldn't have been added * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Use DataTemplate Id for cellReuseId * Calculate correct position for carousel adapter The `CarouselViewAdapter` loops by basically returning a very large value for item count of its adapter/source which means the recyclerview is going to ask for view types with a position value that does not actually exist. Previously this was ok because we returned only a single item view type ever, however now that we want to return a different type for template selector scenarios where there are potentially multiple template types, we need to override also the `GetItemViewType` and pass in the calculated position in the list properly (as we do in `OnBindViewHolder` here already) since the base `ItemsViewAdapter` class now calls `ItemsSource.GetItem(position)` in its `GetItemViewType()` call. --------- Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
rmarinho
added a commit
that referenced
this pull request
May 30, 2023
…12011) * [Android] Use different view types per item template Previously we were returning a single view type id for all 'items' in collectionview, which meant if you were using a template selector, and had a variety of returned different DataTemplates, each would share the same recycled item. When a recycled item is reused for a different template, it causes the view for that template to be recreated to be correct for the template selected. This change returns a unique item id per DataTemplate.Type, so that recycled items should always have the correct DataTemplate.Type and not need to be recreated with a different view hierarchy. * [iOS] Distinct Cell reuse id's for different DataTemplates Like on Android, we were using a single cell reuse identifier for iOS "items" regardless of if they had different DataTemplates. If you were using a data template selector, and returned different data templates, whenever a recycled cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated. This change ensures we have a unique cell reuse id for every different DataTemplate.Type that might be returned by the template selector so that when a cell is recycled it should always be reused for the same Data Template it was created for, and so the view hierarchy won't be recreated. This does require the DetermineCellReuseId to know the indexpath we're getting it for, so a new method was introduced, internal for NET7, protected for NET8+, and in NET8+ the old method is obsoleted. * Switch up some xaml controls in the sample * Expose Id internally to use for android view types * Cache datatemplates for unique item view types * Api changes (overrides) * Improve sample * Remove api txt change that shouldn't have been added * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Use DataTemplate Id for cellReuseId * Calculate correct position for carousel adapter The `CarouselViewAdapter` loops by basically returning a very large value for item count of its adapter/source which means the recyclerview is going to ask for view types with a position value that does not actually exist. Previously this was ok because we returned only a single item view type ever, however now that we want to return a different type for template selector scenarios where there are potentially multiple template types, we need to override also the `GetItemViewType` and pass in the calculated position in the list properly (as we do in `OnBindViewHolder` here already) since the base `ItemsViewAdapter` class now calls `ItemsSource.GetItem(position)` in its `GetItemViewType()` call. --------- Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
samhouts
added
the
fixed-in-8.0.0-preview.5.8529
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
For Android and iOS we were using a single value for the 'view/cell' item type in the platform adapter/source. If you were using a data template selector, and returned different data templates, whenever a recycled view/cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated.
This PR ensures we have a unique view/cell type for every different DataTemplate.Type that might be returned by the template selector so that when a view/cell is recycled it should always be reused for the same DataTemplate it was created for, and so the view hierarchy won't be recreated.
Issues Fixed