dotnet · PureWeen · Jan 16, 2026 · Jan 6, 2026 · Jan 6, 2026 · Jan 6, 2026
diff --git a/.github/agent-pr-session/pr-33406.md b/.github/agent-pr-session/pr-33406.md
@@ -0,0 +1,247 @@
+# PR Review: #33406 - [iOS] Fixed Shell navigation on search handler suggestion selection
+
+**Date:** 2026-01-08 | **Issue:** [#33356](https://github.com/dotnet/maui/issues/33356) | **PR:** [#33406](https://github.com/dotnet/maui/pull/33406)
+
+**Related Prior Attempt:** [PR #33396](https://github.com/dotnet/maui/pull/33396) (closed - Copilot CLI attempt)
+
+## ⏳ Status: IN PROGRESS
+
+| Phase | Status |
+|-------|--------|
+| Pre-Flight | ✅ COMPLETE |
+| 🧪 Tests | ⏳ PENDING |
+| 🚦 Gate | ⏳ PENDING |
+| 🔧 Fix | ⏳ PENDING |
+| 📋 Report | ⏳ PENDING |
+
+---
+
+<details>
+<summary><strong>📋 Issue Summary</strong></summary>
+
+**Issue #33356**: [iOS] Clicking on search suggestions fails to navigate to detail page correctly
+
+**Bug Description**: Clicking on a search suggestion using NavigationBar/SearchBar/custom SearchHandler does not navigate to the detail page correctly on iOS 26.1 & 26.2 with MAUI 10.
+
+**Root Cause (from PR #33406)**: Navigation fails because `UISearchController` was dismissed (`Active = false`) BEFORE `ItemSelected` was called. This triggers a UIKit transition that deactivates the Shell navigation context and prevents the navigation from completing.
+
+**Reproduction App**: https://github.com/dotnet/maui-samples/tree/main/10.0/Fundamentals/Shell/Xaminals
+
+**Steps to Reproduce:**
+1. Open the Xaminals sample app
+2. Deploy to iPhone 17 Pro 26.2 simulator (Xcode 26.2)
+3. Put focus on the search box
+4. Type 'b' (note: search dropdown position is wrong - see Issue #32930)
+5. Click on 'Bengal' in search suggestions
+6. **Issue 1:** No navigation happens (expected: navigate to Bengal cat detail page)
+7. Click on 'Bengal' from the main list - this works correctly
+8. Click back button
+9. **Issue 2:** Navigates to an empty page (expected: navigate back to list)
+10. Click back button again - actually navigates back
+
+**Platforms Affected:**
+- [ ] Android
+- [x] iOS (26.1 & 26.2)
+- [ ] Windows
+- [ ] MacCatalyst
+
+**Regression Info:**
+- **Confirmed regression** starting in version 9.0.90
+- Labels: `t/bug`, `platform/ios`, `s/verified`, `s/triaged`, `i/regression`, `shell-search-handler`, `regressed-in-9.0.90`
+- Issue 2 (empty page on back navigation) specifically reproducible from 9.0.90
+
+**Validated by:** TamilarasanSF4853 (Syncfusion partner) - Confirmed reproducible in VS Code 1.107.1 with MAUI versions 9.0.0, 9.0.82, 9.0.90, 9.0.120, 10.0.0, and 10.0.20 on iOS.
+
+</details>
+
+<details>
+<summary><strong>📁 Files Changed - PR #33406 (Community PR)</strong></summary>
+
+| File | Type | Changes |
+|------|------|---------|
+| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | 2 lines (swap order) |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` | Test (HostApp) | +261 lines |
+| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +46 lines |
+
+**PR #33406 Fix** (simpler approach - just swap order):
+```diff
+ void OnSearchItemSelected(object? sender, object e)
+ {
+     if (_searchController is null)
+         return;
+
+-    _searchController.Active = false;
+     (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
++    _searchController.Active = false;
+ }
+```
+
+</details>
+
+<details>
+<summary><strong>📁 Files Changed - PR #33396 (Prior Copilot Attempt - CLOSED)</strong></summary>
+
+| File | Type | Changes |
+|------|------|---------|
+| `.github/agent-pr-session/pr-33396.md` | Session | +210 lines |
+| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | +17 lines |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml` | Test (XAML) | +41 lines |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs` | Test (C#) | +138 lines |
+| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +70 lines |
+
+**PR #33396 Fix** (more defensive approach with BeginInvokeOnMainThread):
+```diff
+ void OnSearchItemSelected(object? sender, object e)
+ {
+     if (_searchController is null)
+         return;
+
++    // Store the search controller reference before any state changes
++    var searchController = _searchController;
++    
++    // Call ItemSelected first to trigger navigation before dismissing the search UI.
++    // On iOS 26+, setting Active = false before navigation can cause the navigation
++    // to be lost due to the search controller dismissal animation.
+     (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
+-    _searchController.Active = false;
++    
++    // Deactivate the search controller after navigation has been initiated.
++    // Using BeginInvokeOnMainThread ensures this happens after the current run loop,
++    // allowing the navigation to proceed without interference from the dismissal animation.
++    ViewController?.BeginInvokeOnMainThread(() =>
++    {
++        if (searchController is not null)
++        {
++            searchController.Active = false;
++        }
++    });
+ }
+```
+
+</details>
+
+<details>
+<summary><strong>💬 Discussion Summary</strong></summary>
+
+**Key Comments from Issue #33356:**
+- TamilarasanSF4853 (Syncfusion): Validated issue across multiple MAUI versions (9.0.0 through 10.0.20)
+- Issue 2 (empty page on back) specifically regressed in 9.0.90
+- Issue 1 (no navigation on search suggestion tap) affects all tested versions on iOS
+
+**PR #33406 Review Comments:**
+- Copilot PR reviewer caught typo: "searchHander" should be "searchHandler" (5 duplicate comments, all resolved/outdated now)
+- Prior agent review by kubaflo marked it as ✅ APPROVE with comprehensive analysis
+- PureWeen requested `/rebase` (latest comment)
+
+**PR #33396 Review Comments:**
+- PureWeen asked to update state file to match PR number
+- Copilot had firewall issues accessing GitHub API
+
+**Disagreements to Investigate:**
+| File:Line | Reviewer Says | Author Says | Status |
+|-----------|---------------|-------------|--------|
+| N/A | N/A | N/A | No active disagreements |
+
+**Author Uncertainty:**
+- None noted in either PR
+
+</details>
+
+<details>
+<summary><strong>⚖️ Comparison: PR #33406 vs PR #33396</strong></summary>
+
+### Fix Approach Comparison
+
+| Aspect | PR #33406 (Community) | PR #33396 (Copilot) |
+|--------|----------------------|---------------------|
+| **Author** | SubhikshaSf4851 (Syncfusion) | Copilot |
+| **Status** | Open | Closed (draft) |
+| **Lines Changed** | 2 (swap order) | 17 (more defensive) |
+| **Fix Strategy** | Simply swap order of operations | Swap order + dispatch to next run loop |
+| **Test Style** | Code-only (no XAML) | XAML + code-behind |
+| **Test Count** | 1 test method | 2 test methods |
+
+### Which Fix is Better?
+
+**PR #33406 (simpler approach):**
+- ✅ Minimal change - just swaps two lines
+- ✅ Addresses root cause: ItemSelected called while navigation context is valid
+- ⚠️ Dismissal happens synchronously after ItemSelected
+- ⚠️ Could theoretically still interfere if dismissal animation is fast
+
+**PR #33396 (defensive approach):**
+- ✅ Uses BeginInvokeOnMainThread for explicit async deactivation
+- ✅ Stores reference to search controller before state changes
+- ✅ More detailed comments explaining the fix
+- ⚠️ More code complexity
+- ⚠️ Was closed/abandoned
+
+### Recommendation
+
+Both approaches should work. PR #33406 is simpler and has been reviewed/approved. The extra defensive measures in PR #33396 (BeginInvokeOnMainThread) may provide additional safety margin but add complexity.
+
+**Prior agent review on PR #33406** already verified:
+- Tests FAIL without fix (bug reproduced - timeout)
+- Tests PASS with fix (navigation successful)
+
+</details>
+
+<details>
+<summary><strong>🧪 Tests</strong></summary>
+
+**Status**: ⏳ PENDING (need to verify tests compile and reproduce issue)
+
+**PR #33406 Tests:**
+- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` (code-only, no XAML)
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
+- 1 test: `Issue33356NavigateShouldOccur` - Tests search handler navigation AND back navigation + collection view navigation
+
+**PR #33396 Tests (for reference):**
+- HostApp XAML: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml`
+- HostApp Code: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs`
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
+- 2 tests: `SearchSuggestionTapNavigatesToDetailPage`, `BackNavigationFromDetailPageWorks`
+
+**Test Checklist:**
+- [ ] PR includes UI tests
+- [ ] Tests reproduce the issue
+- [ ] Tests follow naming convention (`Issue33356`)
+
+</details>
+
+<details>
+<summary><strong>🚦 Gate - Test Verification</strong></summary>
+
+**Status**: ⏳ PENDING
+
+- [ ] Tests FAIL without fix (bug reproduced)
+- [ ] Tests PASS with fix (fix validated)
+
+**Prior Agent Review Result (kubaflo on PR #33406):**
+```
+WITHOUT FIX: FAILED - System.TimeoutException: Timed out waiting for element "Issue33356CatNameLabel"
+WITH FIX: PASSED - All 1 tests passed in 21.73 seconds
+```
+
+**Result:** [PENDING - needs re-verification]
+
+</details>
+
+<details>
+<summary><strong>🔧 Fix Candidates</strong></summary>
+
+**Status**: ⏳ PENDING
+
+| # | Source | Approach | Test Result | Files Changed | Notes |
+|---|--------|----------|-------------|---------------|-------|
+| PR | PR #33406 | Swap order: ItemSelected before Active=false | ⏳ PENDING (Gate) | `ShellPageRendererTracker.cs` (2 lines) | Current PR - simpler fix |
+| Alt | PR #33396 | Swap order + BeginInvokeOnMainThread | ✅ VERIFIED (prior test) | `ShellPageRendererTracker.cs` (17 lines) | Prior attempt - more defensive |
+
+**Exhausted:** No
+**Selected Fix:** [PENDING]
+
+</details>
+
+---
+
+**Next Step:** Verify PR #33406 tests compile and Gate passes. Read `.github/agents/pr/post-gate.md` after Gate passes.
diff --git a/.github/instructions/uitests.instructions.md b/.github/instructions/uitests.instructions.md
@@ -171,9 +171,9 @@ Tests should run on all applicable platforms by default. The test infrastructure
 
 ### No Inline #if Directives in Test Methods
 
-**Do NOT use `#if ANDROID`, `#if IOS`, etc. inside test method bodies.** Platform-specific behavior must be hidden behind extension methods for readability.
+**Do NOT use `#if ANDROID`, `#if IOS`, etc. directly in test methods.** Platform-specific behavior must be hidden behind extension methods for readability.
 
-**Note:** File-level `#if` (to exclude entire files) or wrapping entire test methods is acceptable. This rule targets inline conditionals within test logic that make code hard to read and maintain.
+**Note:** This rule is about **code cleanliness**, not platform scope. Using `#if ANDROID ... #else ...` still compiles for all platforms - the issue is that inline directives make test logic hard to read and maintain.
 
 ```csharp
 // ❌ BAD - inline #if in test method (hard to read)

diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs
@@ -1045,8 +1045,8 @@ void OnSearchItemSelected(object? sender, object e)
 				return;
 			}
 
-			_searchController.Active = false;
 			(SearchHandler as ISearchHandlerController)?.ItemSelected(e);
+			_searchController.Active = false;
 		}
 
 		void SearchButtonClicked(object? sender, EventArgs e)