[Windows] Fixed Unstable order of Flyout Items with conditional visibility #29197

SubhikshaSf4851 · 2025-04-25T11:04:19Z

Root Cause:

When the visibility of a Flyout item is changed, the UpdateMenuItemSource method simply adds the item to the end, resulting in the original order of the Flyout items being mismatched.

Description of change:

Instead of adding the item, I have retrieved its original position and inserted it back into the Flyout items accordingly, which results the original order being maintained.

Tested the behavior in the following platforms:

Windows
Android
iOS
Mac

Issue Fixed:

Fixes #23834

Before Issue Fix	After Issue Fix
Issue23834_BeforeFix.mp4	Issue23834_AfterFix.mp4

jsuarezruiz · 2025-04-25T11:41:43Z

src/Controls/src/Core/Handlers/Shell/Windows/ShellView.cs

+					if (flyoutItemIndex == -1)
 					{
-						FlyoutItems.Add(item);
+						if (index < FlyoutItems.Count)


Store FlyoutItems.Count in a variable to minimize repeated property accesses.

I have updated the changes per your suggestion.

jsuarezruiz · 2025-04-25T11:44:30Z

src/Controls/src/Core/Handlers/Shell/Windows/ShellView.cs

+					var item = newItems[index];
+					var flyoutItemIndex = FlyoutItems.IndexOf(item);
+
+					if (flyoutItemIndex == -1)


Not big performance differences, but can simplify this using Contains:

if (!FlyoutItems.Contains(item)) { // Use Insert when within bounds, otherwise Add if (index < count) { FlyoutItems.Insert(index, item); } else { FlyoutItems.Add(item); } }

NOTE: count is a variable with the FlyoutItems.Count value.

I have updated the changes per your suggestion.

jsuarezruiz · 2025-04-25T11:44:59Z

/azp run MAUI-UITests-public

azure-pipelines · 2025-04-25T11:45:11Z

Azure Pipelines successfully started running 1 pipeline(s).

jsuarezruiz · 2025-04-25T11:45:12Z

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23834.cs

+		App.WaitForElement("button");
+		App.Tap("button");
+		App.ShowFlyout();
+		VerifyScreenshot();


Pending snapshots.

Snapshots already available in the latest build:

For example:

I have committed the pending snapshots

jsuarezruiz · 2025-04-30T09:35:39Z

/azp run MAUI-UITests-public

azure-pipelines · 2025-04-30T09:35:51Z

Azure Pipelines successfully started running 1 pipeline(s).

bronteq · 2025-05-22T08:45:27Z

@jsuarezruiz friendly ping, is it possible to merge in the next inflight round this pr? It seems ready.
My company is hitting the bug solved by this pr, then I saw that pr #28977 about Shell Flyout was merged.

bronteq · 2025-06-03T13:36:32Z

@jsuarezruiz friendly ping, is it possible to merge this pr in the next inflight round?

jsuarezruiz · 2025-06-17T08:53:46Z

src/Controls/src/Core/Handlers/Shell/Windows/ShellView.cs

+				var itemsCount = FlyoutItems.Count;

-				foreach (var item in newItems)
+				for (int index = 0; index < newItems.Count; index++)


Usually cannot expect to have a big collection here, but use the Contains with the two nested for loops have a big performance impact. Ideally we could kept it simple, but it should have the best possible behavior in all situations.

Let's use a HashSet here.

@jsuarezruiz I have used HashSet as per your suggestion

jsuarezruiz · 2025-06-17T09:02:57Z

src/Controls/src/Core/Handlers/Shell/Windows/ShellView.cs

 			{
 				_flyoutGrouping = newGrouping;
 				var newItems = IterateItems(newGrouping).ToList();
+				var itemsCount = FlyoutItems.Count;


This count becomes stale after Insert operations.

@jsuarezruiz I have used FlyoutItems Count directly.

jsuarezruiz · 2025-06-17T09:04:10Z

src/Controls/src/Core/Handlers/Shell/Windows/ShellView.cs

 				{
+					var item = newItems[index];
+
 					if (!FlyoutItems.Contains(item))


The performance degrades quadratically with collection size, for example, 100 items = 100,000 operations instead of 100.

@jsuarezruiz I've addressed the performance concern by using HashSet. Kindly let me know if you have any suggestions

bronteq · 2025-07-15T13:45:25Z

@jsuarezruiz friendly ping

bronteq · 2025-08-05T14:00:07Z

@jsuarezruiz @PureWeen friendly ping

bronteq · 2025-08-25T12:33:37Z

@PureWeen @jfversluis friendly ping, this should be ready to merge

jfversluis · 2025-08-25T13:03:01Z

/rebase

jfversluis · 2025-08-25T13:03:17Z

@bronteq did you download the artifacts and verified this fixes your issue?

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

jfversluis · 2025-08-25T13:10:12Z

/azp run MAUI-UITests-public

azure-pipelines · 2025-08-25T13:10:23Z

Azure Pipelines successfully started running 1 pipeline(s).

bronteq · 2025-08-26T08:14:35Z

@bronteq did you download the artifacts and verified this fixes your issue?

@jfversluis i followed this https://github.com/dotnet/maui/wiki/Testing-PR-Builds but i do not find nuget folder in the artifacts, probably because MAUI-public was not executed

I also tried this https://github.com/dotnet/maui/wiki/Nightly-Builds but i think that nightlies do not contain not merged PRs, this has the wrong behavior

jfversluis · 2025-08-27T08:09:11Z

/azp run MAUI-public

azure-pipelines · 2025-08-27T08:09:24Z

Azure Pipelines successfully started running 1 pipeline(s).

jfversluis · 2025-08-27T08:09:24Z

@bronteq you're totally right, my bad! Triggered that one now, please let me know :)

bronteq · 2025-08-27T10:20:28Z

@jfversluis thank you so much, i can confirm that this PR solves the issue

Updated concerns Updated review concern Added snapshots and requested changes Modified the sample used HashSet Used FlyoutItems Count directly

dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Apr 25, 2025

jsuarezruiz added platform/windows area-controls-flyout Flyout labels Apr 25, 2025

jsuarezruiz requested changes Apr 25, 2025

View reviewed changes

jsuarezruiz reviewed Apr 25, 2025

View reviewed changes

SubhikshaSf4851 marked this pull request as ready for review April 28, 2025 11:15

SubhikshaSf4851 requested a review from a team as a code owner April 28, 2025 11:15

SubhikshaSf4851 requested review from StephaneDelcroix and tj-devel709 April 28, 2025 11:15

jsuarezruiz requested changes Jun 17, 2025

View reviewed changes

SubhikshaSf4851 added 6 commits August 25, 2025 13:03

[Windows] Fix for flyout order mismatch

57f506d

Updated concerns

50f7f34

Updated review concern

285b570

Added snapshots and requested changes

606ba21

Modified the sample used HashSet

c53dfc8

Used FlyoutItems Count directly

663ec0e

github-actions bot force-pushed the fix-23834 branch from 66cd5b0 to 663ec0e Compare August 25, 2025 13:03

Copilot AI review requested due to automatic review settings August 25, 2025 13:03

Copilot AI reviewed Aug 25, 2025

View reviewed changes

jfversluis approved these changes Aug 27, 2025

View reviewed changes

jfversluis changed the base branch from main to inflight/current August 27, 2025 13:44

jfversluis added this to the .NET 9 SR11 milestone Aug 27, 2025

jfversluis merged commit a489638 into dotnet:inflight/current Aug 27, 2025
94 checks passed

PureWeen pushed a commit that referenced this pull request Sep 8, 2025

[Windows] Fix for flyout order mismatch (#29197)

0638abd

Updated concerns Updated review concern Added snapshots and requested changes Modified the sample used HashSet Used FlyoutItems Count directly

github-actions bot locked and limited conversation to collaborators Sep 27, 2025

[Windows] Fixed Unstable order of Flyout Items with conditional visibility #29197

[Windows] Fixed Unstable order of Flyout Items with conditional visibility #29197

Uh oh!

Conversation

SubhikshaSf4851 commented Apr 25, 2025

Root Cause:

Description of change:

Tested the behavior in the following platforms:

Issue Fixed:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz commented Apr 25, 2025

Uh oh!

azure-pipelines bot commented Apr 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz commented Apr 30, 2025

Uh oh!

azure-pipelines bot commented Apr 30, 2025

Uh oh!

bronteq commented May 22, 2025

Uh oh!

bronteq commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bronteq commented Jul 15, 2025

Uh oh!

bronteq commented Aug 5, 2025

Uh oh!

bronteq commented Aug 25, 2025

Uh oh!

jfversluis commented Aug 25, 2025

Uh oh!

jfversluis commented Aug 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

jfversluis commented Aug 25, 2025

Uh oh!

azure-pipelines bot commented Aug 25, 2025

Uh oh!

bronteq commented Aug 26, 2025

Uh oh!

jfversluis commented Aug 27, 2025

Uh oh!

azure-pipelines bot commented Aug 27, 2025

Uh oh!

jfversluis commented Aug 27, 2025

Uh oh!

bronteq commented Aug 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees