-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix Shell Navigating event not firing on ShellContent change #34351
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.
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
Merged
kubaflo
merged 12 commits into
dotnet:inflight/current
from
jpd21122012:fix/34318-ShellNavigating-NotTriggered-Windows
Jun 27, 2026
+172
−2
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
eff4297
Fix Shell Navigating event not firing on ShellContent change
jpd21122012 2caeb0b
Add UI test for Shell Navigating on ShellContent change
jpd21122012 091b1c5
Fix missing using statements
jpd21122012 fc4e85f
Fix Shell.Navigating not firing on ShellContent change + add test cov…
jpd21122012 b7651de
Fix Shell Navigating event timing and navigation state consistency
jpd21122012 d5dd556
Fix Issue34318 UI test by removing duplicate AutomationId usage
jpd21122012 ce81732
Fix Shell Navigating double-fire on ShellContent change
jpd21122012 b2efae8
Add regression test to ensure Navigating fires exactly once on ShellC…
jpd21122012 344cde4
Avoid duplicate Shell.Navigating notifications on ShellContent changes
jpd21122012 9f536ac
Remove unused Shell parameter from Issue34318 test
jpd21122012 f9586d0
Use OnCurrentItemChanged for ShellContent Navigating notification
jpd21122012 9bcf38f
Avoid duplicate ShellContent Navigating from handler-originated changes
jpd21122012 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
117 changes: 117 additions & 0 deletions
117
src/Controls/tests/TestCases.HostApp/Issues/Issue34318.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| using Microsoft.Maui.ApplicationModel; | ||
| using Microsoft.Maui.Controls; | ||
| using Microsoft.Maui.Controls.CustomAttributes; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 34318, "Shell Navigating event should fire on ShellContent change", PlatformAffected.All)] | ||
| public class Issue34318 : Shell | ||
| { | ||
| Label labelA; | ||
| Label labelB; | ||
| Label navigatingCountLabel; | ||
| int navigatingCount; | ||
|
|
||
| public Issue34318() | ||
| { | ||
| labelA = new Label | ||
| { | ||
| Text = "Waiting", | ||
| AutomationId = "ResultLabelA" | ||
| }; | ||
|
|
||
| labelB = new Label | ||
| { | ||
| Text = "Waiting", | ||
| AutomationId = "ResultLabelB" | ||
| }; | ||
|
|
||
| navigatingCountLabel = new Label | ||
| { | ||
| Text = "0", | ||
| AutomationId = "NavigatingCountLabel" | ||
| }; | ||
|
|
||
| var section = new ShellSection(); | ||
|
|
||
| var pageA = new Issue34318_PageA(labelA); | ||
|
|
||
| var contentA = new ShellContent | ||
| { | ||
| Content = pageA | ||
| }; | ||
|
|
||
| var contentB = new ShellContent | ||
| { | ||
| Content = new ContentPage | ||
| { | ||
| Title = "PageB", | ||
| Content = new VerticalStackLayout | ||
| { | ||
| Children = | ||
| { | ||
| new Label | ||
| { | ||
| Text = "Page B", | ||
| AutomationId = "PageBLabel" | ||
| }, | ||
| labelB, | ||
| navigatingCountLabel | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| section.Items.Add(contentA); | ||
| section.Items.Add(contentB); | ||
|
|
||
| var item = new ShellItem(); | ||
| item.Items.Add(section); | ||
|
|
||
| Items.Add(item); | ||
|
|
||
| Navigating += (_, __) => | ||
| { | ||
| MainThread.BeginInvokeOnMainThread(() => | ||
| { | ||
| navigatingCount++; | ||
|
|
||
| labelA.Text = "Navigating"; | ||
| labelB.Text = "Navigating"; | ||
| navigatingCountLabel.Text = navigatingCount.ToString(); | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
| public class Issue34318_PageA : ContentPage | ||
| { | ||
| public Issue34318_PageA(Label label) | ||
| { | ||
| Title = "PageA"; | ||
|
|
||
| var button = new Button | ||
| { | ||
| Text = "Change Content", | ||
| AutomationId = "ChangeContentButton" | ||
| }; | ||
|
|
||
| button.Clicked += (s, e) => | ||
| { | ||
| Element parent = this; | ||
|
|
||
| while (parent != null && parent is not ShellSection) | ||
| parent = parent.Parent; | ||
|
|
||
| if (parent is ShellSection section && section.Items.Count > 1) | ||
| { | ||
| section.CurrentItem = section.Items[1]; | ||
|
jpd21122012 marked this conversation as resolved.
|
||
| } | ||
| }; | ||
|
|
||
| Content = new VerticalStackLayout | ||
| { | ||
| Children = { button, label } | ||
| }; | ||
| } | ||
| } | ||
| } | ||
37 changes: 37 additions & 0 deletions
37
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue34318 : _IssuesUITest | ||
| { | ||
| public Issue34318(TestDevice device) : base(device) { } | ||
|
|
||
| public override string Issue => "Shell Navigating event should fire on ShellContent change"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.Shell)] | ||
| public void NavigatingFiresWhenShellContentChanges() | ||
| { | ||
| App.WaitForElement("ChangeContentButton"); | ||
|
|
||
| App.WaitForElement("ResultLabelA"); | ||
|
|
||
| var initialText = App.FindElement("ResultLabelA").GetText() ?? string.Empty; | ||
| Assert.That(initialText, Is.EqualTo("Waiting")); | ||
|
|
||
| App.Tap("ChangeContentButton"); | ||
|
|
||
| App.WaitForElement("PageBLabel"); | ||
|
|
||
| var result = App.WaitForTextToBePresentInElement("ResultLabelB", "Navigating"); | ||
|
|
||
| Assert.That(result, Is.True, "Navigating event should have fired and updated the label text"); | ||
|
|
||
| var countText = App.FindElement("NavigatingCountLabel").GetText() ?? string.Empty; | ||
|
|
||
| Assert.That(countText, Is.EqualTo("1"), | ||
| "Navigating event should fire exactly once, not multiple times"); | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Regression Prevention — The regression handler ignores
ShellNavigatingEventArgs, so the test only proves that someNavigatingevent fired once. It would still pass if the fix used the wrongSource, exposed incorrectCurrent/Targetroutes, or fired afterShellSection.CurrentItemwas already changed. Capture the args here and assertSource == ShellNavigationSource.ShellContentChanged,CanCancel == false, the expected current/target locations, and the selected content observed during the handler so the test protects the Shell navigation semantics, not just the event count.