[iOS][Shell] Fix navigation lifecycle and back button for More tab (>5 tabs)

[kubaflo]

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

Fixes navigation lifecycle events (OnAppearing, OnNavigatedTo) and back button behavior for Shell pages accessed through the "More" tab (when TabBar has >5 tabs on iOS).

Root cause: When IsInMoreTab is true, PushViewController and PopViewController delegate to UITabBarController.MoreNavigationController instead of the base navigation controller. The MoreNavigationController doesn't use our NavDelegate, so DidShowViewController never fires, leaving navigation completion tasks incomplete. This causes lifecycle events to not fire and back button bindings to fail.

Fix:

1. Completion tasks: Added HandleMoreNavigationCompletionTasks() helper method that manually completes navigation tasks after More push/pop operations
2. Back button: Set BackAction on pushed view controllers to call SendPop() with the correct top view controller
3. API cleanup: Modified SendPop() to accept optional topViewController parameter (defaults to TopViewController for backward compatibility)

Key insight: MoreNavigationController is a separate UINavigationController that doesn't inherit our delegate setup. Tasks were being added to _completionTasks dictionary but never completed because the delegate callback that completes them (DidShowViewController) only fires for the base navigation controller.

What to avoid: Don't try to set a delegate on MoreNavigationController - it's managed by iOS and delegate changes may be overridden. The manual completion approach after each operation is more reliable.

Issues Fixed

Fixes #27799
Fixes #27800
Fixes #30862
Fixes #31166

Before	After
Screen.Recording.2025-02-17.at.20.30.39.mov	Screen.Recording.2025-02-17.at.20.27.08.mov

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[rmarinho]

/rebase

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

The test is failing on Android, Mac and Windows:

   at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2352
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2369
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 665
   at Microsoft.Maui.TestCases.Tests.Issues.Issue27800.ShellBackButtonBehaviorShouldWorkWithMoreTab() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27800.cs:line 22
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

[kubaflo]

Okay, it should be the iOS-only test. I've fixed it. Thanks for letting me know!

[kubaflo]

@jsuarezruiz can you please /azp?

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 27932

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 27932"

[kubaflo]

🤖 AI Summary

📊 Expand Full Review

---

🔍 Pre-Flight — Context & Validation

---

📝 Review Session — Test fix · 6e519b5

---

Issue: #27799, #27800, #30862, #31166 - Navigation lifecycle events (OnAppearing, OnNavigatedTo) and back button behavior fail for Shell pages in "More" tab (>5 tabs) on iOS
PR: #27932 by @kubaflo
Platforms Affected: iOS only (Android works correctly)
Files Changed: 1 fix file, 4 test files

Issue Summary

When a TabBar has more than 5 tabs on iOS, the extra tabs appear under a "More" navigation controller (UITabBarController.MoreNavigationController). Pages accessed through this More tab experience:

1. OnAppearing and OnNavigatedTo does not work when using extended Tabbar (tabbar with more than 5 tabs) on IOS. #27799/Content page onappearing not firing if tabs are on the more tab on IOS #31166 - OnAppearing and OnNavigatedTo lifecycle events not firing correctly
2. Shell.BackButtonBehavior does not work when using extended Tabbar (tabbar with more than 5 tabs)on IOS. #27800 - Shell.BackButtonBehavior commands not working
3. Shell TabBar More button causes ViewModel command binding disconnection on back navigation #30862 - ViewModel command binding disconnection on back navigation

Root Cause (from PR description): When IsInMoreTab is true, PushViewController/PopViewController delegate to UITabBarController.MoreNavigationController. This controller doesn't use our custom NavDelegate, so DidShowViewController never fires, leaving _completionTasks entries never completed. This causes lifecycle events and back button bindings to fail.

Files Changed

Fix files:

• src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cs (+24, -3)

Test files:

• src/Controls/tests/TestCases.HostApp/Issues/Issue27800.cs (+72, new)
• src/Controls/tests/TestCases.HostApp/Issues/Issue27999.cs (+74, new) ⚠️ filename should be Issue27799.cs
• src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27800.cs (+38, new)
• src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27999.cs (+37, new) ⚠️ filename should be Issue27799.cs

PR's Fix Approach

1. Added HandleMoreNavigationCompletionTasks() helper to manually complete navigation tasks after More push/pop operations
2. Modified SendPop() to accept optional topViewController parameter (defaults to TopViewController)
3. Set BackAction on pushed view controllers to call SendPop() with correct VC reference
4. Does NOT set delegate on MoreNavigationController (iOS manages this and overrides changes)

Code Review Findings

File	Issue	Status
Issue27999.cs (NUnit)	Duplicate [Category] attribute	⚠️ Code quality
Issue27999.cs (NUnit)	Typo in method name: "ShouldBeCalles" → "ShouldBeCalled"	⚠️ Code quality
Issue27800.cs (NUnit)	Was failing on Android/Mac/Windows (not iOS-scoped)	✅ Fixed by author
Issue27999.cs / Issue27800.cs	Filename mismatch: 27999 should be 27799	⚠️ Naming convention

PR Discussion Summary

• @jsuarezruiz: Triggered Azure Pipelines runs
• @rmarinho: Triggered rebase
• @jsuarezruiz: Flagged that Issue27800.cs test was failing on Android/Mac/Windows
• @kubaflo: Fixed by scoping to iOS only

Fix Candidates Table

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #27932	Manual completion tasks + BackAction override after More tab push/pop	✅ PASS (Gate)	ShellSectionRenderer.cs (+24 -3)	Original PR

---

🚦 Gate — Test Verification

---

📝 Review Session — Test fix · 6e519b5

---

Result: ✅ PASSED
Platform: iOS (simulator)
Mode: Full Verification (imported from prior agent run)

• Tests FAIL without fix ✅ (bug reproduced - Issue27799: OnNavigatedTo count was 0 instead of 3; Issue27800: count was 1 instead of 2)
• Tests PASS with fix ✅ (fix validated - both tests passed)

Test Filter: Issue27800|Issue27799
Tests: 2/2 FAILED without fix, 2/2 PASSED with fix

Conclusion: Tests correctly detect the issue and validate the fix works.

---

🔧 Fix — Analysis & Comparison

---

📝 Review Session — Test fix · 6e519b5

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (sonnet)	MoreTabNavDelegate on MoreNavigationController.Delegate	✅ PASS	ShellSectionRenderer.cs (+47)	Intercepts DidShowViewController after animation; PR warns iOS may override delegate
2	try-fix (opus)	CATransaction.CompletionBlock wrapping push/pop	✅ PASS	ShellSectionRenderer.cs (+28)	Post-animation timing via Core Animation; clean 1-file approach
3	try-fix (gpt-5.2)	TransitionCoordinator + NavigationBarDelegateProxy	❌ FAIL	ShellSectionRenderer.cs	App got stuck, back button "Go Back" element never appeared
4	try-fix (gpt-5.3-codex)	DispatchQueue.MainQueue.DispatchAsync deferred completion	❌ FAIL	ShellSectionRenderer.cs	OnNavigatedTo stayed at 1 (expected 2); timing insufficient
5	try-fix (opus round 2)	Forward from existing ShellItemRenderer.DidShowViewController WeakDelegate to ShellSectionRenderer.HandleMoreNavigationCompletionTasks	✅ PASS	ShellItemRenderer.cs (+10), ShellSectionRenderer.cs (+34)	Uses EXISTING WeakDelegate hookup (line 82); post-animation timing matches normal navigation
6	try-fix (gpt-5.2 round 2)	UIView containment hook (MoreNavigationCompletionHookView with MovedToSuperview/MovedToWindow)	✅ PASS	ShellSectionRenderer.cs (+118/-8)	Novel approach bypasses delegate entirely; complex implementation
PR	PR #27932	Manual HandleMoreNavigationCompletionTasks() + BackAction on pushed VCs + optional topViewController on SendPop()	✅ PASS (Gate)	ShellSectionRenderer.cs (+24/-3)	Simplest, synchronous completion, well-documented reasoning

Cross-Pollination Summary

Round	Model	Response
1	claude-sonnet-4.6	NEW IDEA: viewDidAppear hook on pushed VCs
1	claude-opus-4.6	NEW IDEA: ShellItemRenderer.DidShowViewController forwarding → ran as Attempt 5
1	gpt-5.2	NEW IDEA: NSNotificationCenter
1	gpt-5.3-codex	NEW IDEA: KVO on MoreNavigationController.ViewControllers
1	gemini-3-pro-preview	BLOCKED (no response after 2 retries)
2	claude-sonnet-4.6	NEW IDEA: UINavigationBar didPush/didPop callbacks
2	claude-opus-4.6	NO NEW IDEAS
2	gpt-5.2	NEW IDEA: WillMoveToParentViewController/DidMoveToParentViewController → ran as Attempt 6
2	gpt-5.3-codex	NEW IDEA: Delegate multiplexer/swizzle
3	claude-sonnet-4.6	NEW IDEA: beginAppearanceTransition/endAppearanceTransition
3	claude-opus-4.6	NO NEW IDEAS
3	gpt-5.2	NEW IDEA: ViewDidAppear/ViewDidDisappear hooks
3	gpt-5.3-codex	NO NEW IDEAS

Exhausted: No (reached max 3 rounds; gemini blocked; 2/4 responding models still had ideas)

Selected Fix: PR's fix

Reasoning: The PR's fix (+24/-3 lines in 1 file) is the simplest passing candidate and its reasoning is well-documented in the PR description. Three independent approaches also pass (Attempts 1, 2, 5, 6), which strongly validates the fix is correct.

Strongest alternative: Attempt 5 (ShellItemRenderer.DidShowViewController forwarding) is architecturally cleaner — it uses the existing WeakDelegate hookup at line 82 of ShellItemRenderer.cs and achieves proper post-animation timing matching normal navigation. However, it requires changes in 2 files.

Why not Attempt 1: The PR explicitly warns "Don't try to set a delegate on MoreNavigationController - it's managed by iOS and delegate changes may be overridden." Although it passed tests, the reliability concern is valid.

Why not Attempt 2 or 6: Both pass but are more complex than the PR's direct approach.

Conclusion: PR's fix is simple, direct, well-documented, and validated. APPROVE.

---

📋 Report — Final Recommendation

---

📝 Review Session — Test fix · 6e519b5

---

PR #27932 - Agent Review Report

Summary

PR: [iOS][Shell] Fix navigation lifecycle and back button for More tab (>5 tabs)
Author: @kubaflo
Fixes: #27799, #27800, #27801, #27802

Recommendation: ✅ APPROVE WITH MINOR FIXES

The core fix is correct and well-implemented. Tests pass on iOS. Several test file quality issues should be addressed before merge.

---

Phase 1: Pre-Flight ✅

Issue Summary:

• #27799 / #27800: OnAppearing, OnNavigatedTo not firing; BackButtonBehavior broken for Shell pages in "More" tab (>5 tabs, iOS only)
• Root cause: When IsInMoreTab, iOS uses UITabBarController.MoreNavigationController instead of our ShellSectionRenderer navigation controller. Our NavDelegate.DidShowViewController never fires, leaving _completionTasks entries pending forever, blocking lifecycle events and navigation.

PR Discussion:

• @jsuarezruiz flagged Android test failures → author fixed to #if IOS scope ✅
• Inline Copilot comments flagged: duplicate [Category], method name typo, filename mismatch → not yet addressed

---

Phase 2: Gate ✅ PASSED (iOS)

Tests verified without fix (FAILING):

• Issue27799: OnNavigatedTo: 0 (expected 3) — events blocked by pending completion tasks
• Issue27800: OnNavigatedTo: 1 (expected 2) — back button navigation hung

Tests verified with PR fix (PASSING):

• Issue27799: OnNavigatedTo: 3, OnAppearing: 3 ✅
• Issue27800: OnNavigatedTo: 2, OnAppearing: 2 ✅

---

Phase 3: Fix Exploration

6 alternative approaches explored across 4 models:

Attempt	Model	Approach	Result
1	claude-sonnet-4.6	MoreTabNavDelegate on MoreNavigationController.Delegate	✅ PASS
2	claude-opus-4.6	CATransaction.CompletionBlock after push/pop	✅ PASS
3	gpt-5.2	TransitionCoordinator + NavigationBarDelegateProxy	❌ FAIL (app stuck)
4	gpt-5.3-codex	DispatchQueue.MainQueue.DispatchAsync	❌ FAIL (OnNavigatedTo=1)
5	claude-opus-4.6 (R2)	Forward from ShellItemRenderer.DidShowViewController	✅ PASS
6	gpt-5.2 (R2)	UIView containment hook (MovedToSuperview)	✅ PASS

Selected: PR's own fix — synchronous HandleMoreNavigationCompletionTasks() called after push/pop. Simplest (+24 lines in 1 file), no delegate manipulation, well-documented.

Notable architectural insight: ShellItemRenderer already sets MoreNavigationController.WeakDelegate = this, and DidShowViewController IS called for MoreNavController — but it doesn't forward completion to ShellSectionRenderer._completionTasks. Alternative Attempt 5 leverages this existing hookup.

Warning documented in PR: Don't set a delegate on MoreNavigationController — managed by iOS, may be overridden (disqualifies Attempt 1 for robustness).

---

Phase 4: Code Review (pr-finalize)

Title ✅

"[iOS][Shell] Fix navigation lifecycle and back button for More tab (>5 tabs)" — excellent, no change needed.

Description ✅

Good quality. Has NOTE block, root cause, 3-part fix description, key insight, "what to avoid" section, all 4 issue links, video demo. No rewrite needed.

Code Review Issues

🟡 1. Static counters cause test flakiness (Issue27999.cs HostApp)

// Current (buggy - persists across test runs):
private static int _onNavigatedToCount = 0;
private static int _onAppearingCount = 0;

// Fix:
private int _onNavigatedToCount = 0;
private int _onAppearingCount = 0;

The test verifies exact counts (3, 3). Static fields accumulate across runs → test fails on second execution.

🟡 2. Filename/class mismatch (both Issue27999.cs files)

• TestCases.HostApp/Issues/Issue27999.cs → class is Issue27799 → rename to Issue27799.cs
• TestCases.Shared.Tests/Tests/Issues/Issue27999.cs → class is Issue27799 → rename to Issue27799.cs

🟡 3. Overly broad #if guard in Issue27800.cs HostApp

// Current:
#if ANDROID || IOS

// Fix:
#if IOS

PlatformAffected.iOS and the NUnit test both correctly scope to iOS. HostApp should match.

🟡 4. Duplicate [Category] attribute in Issue27999.cs NUnit test
Two [Category(UITestCategories.Shell)] attributes on the same test method — remove one per guidelines.

🟡 5. Method name typo in Issue27999.cs NUnit test
OnAppearingAndOnNavigatedToShouldBeCalles → OnAppearingAndOnNavigatedToShouldBeCalled

🟡 6. Missing newline at end of all 4 test files

What Looks Good ✅

• HandleMoreNavigationCompletionTasks() is well-named, properly private, correctly handles both _completionTasks and _popCompletionTask fallback
• SendPop(UIViewController topViewController = null) maintains backward compatibility
• BackAction closure correctly evaluates TopViewController at invocation time (not capture time)
• The "don't set delegate on MoreNavigationController" warning is documented in PR description

---

Final Recommendation

✅ APPROVE WITH MINOR FIXES

The fix is architecturally sound, gate-verified on iOS, and has multiple validated alternatives as evidence the approach is correct. Issues found are all in test files, not the production fix. Key items to address:

1. Rename Issue27999.cs → Issue27799.cs in both projects (naming convention)
2. Make static counters instance fields (test reliability)
3. Fix #if guard, duplicate category, method name typo

---

📋 Expand PR Finalization Review

---

Title: ✅ Good

Current: [iOS][Shell] Fix navigation lifecycle and back button for More tab (>5 tabs)

Description: ✅ Good

Description needs updates. See details below.

Code Review: ⚠️ Issues Found

Code Review — PR #27932

PR: [iOS][Shell] Fix navigation lifecycle and back button for More tab (>5 tabs)

---

🔴 Critical Issues

File Naming Mismatch in Both Test Projects

• Files: src/Controls/tests/TestCases.HostApp/Issues/Issue27999.cs and src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27999.cs
• Problem: Both files are named Issue27999.cs but contain the class Issue27799 (and [Issue(... 27799, ...)] attribute). They fix issue OnAppearing and OnNavigatedTo does not work when using extended Tabbar (tabbar with more than 5 tabs) on IOS. #27799, not a nonexistent [iOS] Wrong TimePicker black bar #27999. Test infrastructure, CI categorization, and developer lookup all rely on file names matching the issue number they test.
• Recommendation: Rename both files to Issue27799.cs.

---

🟡 Medium Suggestions

#if ANDROID || IOS Guard Should Be #if IOS

• File: src/Controls/tests/TestCases.HostApp/Issues/Issue27800.cs
• Problem: The file wraps the entire class in #if ANDROID || IOS, but the More tab (>5 tabs) feature is iOS-only. The [Issue(... PlatformAffected.iOS)] attribute already documents this. Android doesn't have the MoreNavigationController concept at all — including Android in the compilation guard is misleading and could confuse future readers into thinking there's Android relevance.
• Note: The corresponding SharedTests file correctly uses #if IOS only.
• Recommendation: Change #if ANDROID || IOS to #if IOS.

---

Static Counter Fields Cause Test Isolation Risk

• File: src/Controls/tests/TestCases.HostApp/Issues/Issue27999.cs
• Problem: _onNavigatedToCount and _onAppearingCount are private static int fields on the outer Issue27799 class, shared by the inner Tab2, Tab6, and Tab6Subpage classes. This is intentional (to count total lifecycle events across all inner pages), but static means the counts persist across test runs within the same process. The NUnit test asserts exact counts (OnNavigatedTo: 3, OnAppearing: 3). On a second run (e.g., test retry, parallel execution), the counts would be 6, 6 and the test would fail.
• Recommendation: Reset the static counters in Issue27799.Init() (e.g., _onNavigatedToCount = 0; _onAppearingCount = 0;), or make them instance fields and adjust the design so Tab2 can observe the cumulative count.

---

Test Method Indentation in SharedTests Files

• Files: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27800.cs and Issue27999.cs

• Problem: The [Test], [Category], and method declarations are unindented (at column 0) instead of being indented at the class level. While this compiles, it's inconsistent with all other test files in the project.

  // Current (wrong)
  [Test]
  [Category(UITestCategories.Shell)]
  public void ShellBackButtonBehaviorShouldWorkWithMoreTab()
  		{
  
  // Expected (correct)
  		[Test]
  		[Category(UITestCategories.Shell)]
  		public void ShellBackButtonBehaviorShouldWorkWithMoreTab()
  		{

• Recommendation: Fix indentation to match class-level indentation (2 tabs).

---

Missing EOF Newline

• Files: All 4 new test files end with \ No newline at end of file.
• Impact: Minor — causes noisy diffs in some tools and violates the repository's standard formatting.
• Recommendation: Add a trailing newline to all 4 files.

---

✅ Looks Good

Core Fix Logic in ShellSectionRenderer.cs

• HandleMoreNavigationCompletionTasks() is correctly scoped (private), well-named, and follows the existing completion task pattern. The fallback to _popCompletionTask correctly handles the pop case (the popped view controller won't be in _completionTasks, so the else-branch handles it).
• SendPop(UIViewController topViewController = null) optional parameter correctly defaults to TopViewController for all existing call sites, maintaining full backward compatibility.
• BackAction closure correctly evaluates tabBarController.MoreNavigationController.TopViewController at call time (not capture time), since it's inside a lambda that runs when the back button is tapped.
• Not setting a delegate on MoreNavigationController — the PR description correctly calls out why this is the right approach (iOS manages that controller's delegate and will override it).

Test Coverage

• Two tests covering two distinct failure modes: lifecycle events (OnAppearing/OnNavigatedTo) and BackButtonBehavior. Good coverage of the reported issues.
• Tests correctly gate with #if IOS in SharedTests since More tab is iOS-specific.

---

[marcojak]

Does it also fix the same issue while not using Shell? Because I'm not using shell and I have the same issue so I'm wondering if this would fix it.