Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 37 commits into from

Conversation

bhavanesh2001
Copy link
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
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added the p/0 Work that we can't release without 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
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

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
@Copilot @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
Member

/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
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

rmarinho
rmarinho requested changes Jul 10, 2025
View reviewed changes
Copy link
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
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Sep 16, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen
PureWeen requested changes Sep 18, 2025
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

looks like this one is failing consistently
CollectionViewWithFallbackVauleShouldUpdateAtRunTime

@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Sep 18, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz requested changes Sep 23, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Failing tests on Mac with small screenshots differences:
image

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Sep 23, 2025
@jsuarezruiz jsuarezruiz mentioned this pull request Sep 23, 2025
Closed
@jsuarezruiz
Copy link
Contributor

Failing tests on Mac with small screenshots differences: image

Fixed.

@jsuarezruiz
Copy link
Contributor

Added samples and tests related with #31071

Before After
fix-31071-before fix-31071-after

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@sheiksyedm sheiksyedm mentioned this pull request Sep 25, 2025
Open
@PureWeen PureWeen modified the milestones: .NET 9 SR12, .NET 10 SR1 Oct 8, 2025
@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsuarezruiz jsuarezruiz jsuarezruiz requested changes

@rmarinho rmarinho Awaiting requested review from rmarinho

@mattleibow mattleibow Awaiting requested review from mattleibow

@PureWeen PureWeen Awaiting requested review from PureWeen

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution p/0 Work that we can't release without

Projects

MAUI SDK Ongoing
Status: Ready To Review

Milestone

.NET 10 SR1

Development

Successfully merging this pull request may close these issues.

CollectionView CollectionViewHandler2 doesnt change ItemsLayout on DataTrigger 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 [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

5 participants

@bhavanesh2001 @jsuarezruiz @PureWeen @rmarinho @mattleibow