Skip to content

Refactor ItemsLayout handling: dynamic default + virtual view-managed subscriptions v2#29683

Closed
bhavanesh2001 wants to merge 35 commits intodotnet:mainfrom
bhavanesh2001:fix_collectionview_ItemsLayout_pro
Closed

Refactor ItemsLayout handling: dynamic default + virtual view-managed subscriptions v2#29683
bhavanesh2001 wants to merge 35 commits intodotnet:mainfrom
bhavanesh2001:fix_collectionview_ItemsLayout_pro

Conversation

@bhavanesh2001
Copy link
Copy Markdown
Contributor

@bhavanesh2001 bhavanesh2001 commented May 27, 2025 •
edited
Loading

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

For more information on the problem and the fix, see: #29638 (comment)

Why this ?

In #29638, we currently call MapItemsLayout when properties such as ItemSpacing, Span, etc., change.
This means that any property change within ItemsLayout triggers a full layout remap.

  • On iOS, this is required: the layout must be fully remapped at the platform level.
  • On Windows and Android, however, full remapping is often unnecessary.
    For example:
    When changing the Span of a GridItemsLayout, we can simply update the platform view’s Span without re-creating the layout.

The problem

It's not always correct to use the same mapper (MapItemsLayout) for:

  • Reassigning the entire ItemsLayout, and
  • Responding to property changes inside an existing layout instance.

The solution

This PR introduces a dedicated command mapper to handle property changes within ItemsLayout.
It separates:

  • MapItemsLayout — for when the layout object itself is replaced.
  • A new command mapper — for property changes like Span, ItemSpacing, etc.

Supersedes #29638 , #29635 , #28675 , #29190 , #28311

Issues Fixed

Fixes #29619
Fixes #27666
Fixes #27667
Fixes #28656
Fixes #29696
Fixes #28023
Fixes #23377
Fixes #31259
Fixes #31071

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label May 27, 2025
@jsuarezruiz jsuarezruiz added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label May 27, 2025
@bhavanesh2001 bhavanesh2001 marked this pull request as ready for review May 27, 2025 10:38
@bhavanesh2001 bhavanesh2001 requested a review from a team as a code owner May 27, 2025 10:38
@bhavanesh2001 bhavanesh2001 force-pushed the fix_collectionview_ItemsLayout_pro branch from e026d3b to 8012865 Compare May 29, 2025 10:25
@jsuarezruiz
Copy link
Copy Markdown
Contributor

jsuarezruiz commented May 30, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented May 30, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added the p/0 Current heighest priority issues that we are targeting for a release. label Jun 2, 2025
@PureWeen PureWeen added this to the .NET 9 SR8 milestone Jun 2, 2025
@PureWeen PureWeen moved this from Todo to Ready To Review in MAUI SDK Ongoing Jun 2, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR8, .NET 9 SR9 Jun 9, 2025
@PureWeen
Copy link
Copy Markdown
Member

PureWeen commented Jun 17, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jun 17, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@bhavanesh2001 bhavanesh2001 force-pushed the fix_collectionview_ItemsLayout_pro branch from 31ae867 to 6573300 Compare June 30, 2025 13:40
@PureWeen PureWeen mentioned this pull request Jul 1, 2025
Closed
Copilot AI added a commit that referenced this pull request Jul 1, 2025
@PureWeen
Replace targeted fix with comprehensive architectural changes from PR #…
9936c18 
…29683

Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
@PureWeen PureWeen modified the milestones: .NET 9 SR9, .NET 9 SR10 Jul 3, 2025
@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Jul 10, 2025

/rebase

@github-actions github-actions bot force-pushed the fix_collectionview_ItemsLayout_pro branch from 1cf7474 to 380090e Compare July 10, 2025 10:26
@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Jul 10, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jul 10, 2025

Azure Pipelines successfully started running 3 pipeline(s).

rmarinho
rmarinho requested changes Jul 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add more UITests for the spacing changes , and after update, maybe add the sample from #27667 and also test for #28656

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Jul 10, 2025
@bhavanesh2001 bhavanesh2001 requested a review from rmarinho July 10, 2025 19:39
@PureWeen PureWeen force-pushed the fix_collectionview_ItemsLayout_pro branch from 4cdb6d8 to dbb3e5d Compare October 19, 2025 12:10
@PureWeen
Copy link
Copy Markdown
Member

PureWeen commented Oct 19, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Oct 19, 2025

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz reviewed Oct 23, 2025
View reviewed changes
src/Controls/src/Core/Handlers/Items/CarouselViewHandler.cs
[Controls.CarouselView.CurrentItemProperty.PropertyName] = MapCurrentItem
};

//TODO Make this public in .NET10
Copy link
Copy Markdown
Contributor

@jsuarezruiz jsuarezruiz Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we could retarget to the net10.0 branch, and remove this TODOs, but I think not is too late.

@PureWeen PureWeen modified the milestones: .NET 10.0 SR1, .NET 10.0 SR2 Nov 12, 2025
@jaimeatsherpa
Copy link
Copy Markdown

jaimeatsherpa commented Nov 25, 2025

any updates?

@PureWeen PureWeen modified the milestones: .NET 10.0 SR2, .NET 10.0 SR3 Dec 3, 2025
@PureWeen PureWeen modified the milestones: .NET 10.0 SR3, .NET 10.0 SR4 Jan 21, 2026
@sheiksyedm sheiksyedm modified the milestones: .NET 10.0 SR4, .NET 10 SR6 Feb 13, 2026
@kubaflo kubaflo mentioned this pull request Feb 20, 2026
Closed
@PureWeen PureWeen modified the milestones: .NET 10 SR6, .NET 10 SR7 Mar 3, 2026
@jfversluis
Copy link
Copy Markdown
Member

jfversluis commented Mar 5, 2026

I think we merged a couple of other PRs now to fix some of the things that are happening here, so this one is a bit stale now. Also this one is a bit too big and complicated to just take at once. Probably for a next time its better to have multiple smaller PRs that will make it easier for us to merge.

If you still want to take this one over the finish line, please have a look at what is still left to be done and split that into several PRs. Thanks!

@jfversluis jfversluis added the s/pr-needs-author-input PR needs an update from the author label Mar 5, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

dotnet-policy-service bot commented Mar 5, 2026

Hi @@bhavanesh2001. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time.

1 similar comment
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

dotnet-policy-service bot commented Mar 5, 2026

Hi @@bhavanesh2001. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time.

@kubaflo kubaflo added the stale Indicates a stale issue/pr and will be closed soon label Mar 7, 2026
@dotnet-policy-service dotnet-policy-service bot removed this from the .NET 10 SR7 milestone Mar 11, 2026
@github-project-automation github-project-automation bot moved this from Ready To Review to Done in MAUI SDK Ongoing Mar 11, 2026
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rmarinho rmarinho Awaiting requested review from rmarinho

@mattleibow mattleibow Awaiting requested review from mattleibow

@PureWeen PureWeen Awaiting requested review from PureWeen

1 more reviewer

@jsuarezruiz jsuarezruiz jsuarezruiz requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution p/0 Current heighest priority issues that we are targeting for a release. s/pr-needs-author-input PR needs an update from the author stale Indicates a stale issue/pr and will be closed soon

Projects

MAUI SDK Ongoing
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

I2 - Entering the vertical and horizontal lists again, Monkeys still retain the spacing value changed last time [.NET10]I1_Vertical Grid for Multi Span Layout - After clicking "UpdateColumns" multiple times, the app will become very slow until it hangs [.NET10]I2_Spacing - Vertical list and Vertical grid pages have different abnormal behaviors when clicking Update after changing the spacing value CollectionView CollectionViewHandler2 doesnt change ItemsLayout on DataTrigger [MAUI] I2_Spacing - The horizontal spacing should be changed but it is actually changed vertically
[iOS] Review possible memory leak with CollectionViewHandler2.SubscribeToItemsLayoutPropertyChanged Changing one CollectionView's ItemSpacing changes another CollectionView's ItemSpacing as well. CollectionView2 (CollectionViewHandler2) much more slow than CV1 on updating columns count at runtime iOS CollectionView doesn't respect a change to ItemsLayout when using Items2.CollectionViewHandler2

9 participants

@bhavanesh2001 @jsuarezruiz @PureWeen @rmarinho @jaimeatsherpa @jfversluis @mattleibow @sheiksyedm @kubaflo