dotnet · PureWeen · Jan 9, 2026 · Jan 8, 2026 · Jan 8, 2026 · Jan 8, 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/agents/pr.md b/.github/agents/pr.md
@@ -216,7 +216,7 @@ This file:
 - Serves as your TODO list for all phases
 - Tracks progress if interrupted
 - Must exist before you start gathering context
-- Gets committed to `.github/agent-pr-session/` directory
+- **Always include when committing changes** (to `.github/agent-pr-session/`)
 - **Phases 4-5 sections are added AFTER Gate passes** (see `pr/post-gate.md`)
 
 **Then gather context and update the file as you go.**

diff --git a/.github/instructions/uitests.instructions.md b/.github/instructions/uitests.instructions.md
@@ -167,47 +167,36 @@ grep -E "public const string [A-Za-z]+ = " src/Controls/tests/TestCases.Shared.T
 
 ### Default Behavior
 
-**DO NOT** use platform-specific conditional compilation directives (`#if ANDROID`, `#if IOS`, `#if WINDOWS`, `#if MACCATALYST`, etc.) unless there is a specific reason.
+Tests should run on all applicable platforms by default. The test infrastructure handles platform detection automatically.
 
-Tests in the `TestCases.Shared.Tests` project should run on all applicable platforms by default. The test infrastructure automatically handles platform detection.
+### No Inline #if Directives in Test Methods
 
-### When Platform Directives Are Acceptable
+**Do NOT use `#if ANDROID`, `#if IOS`, etc. directly in test methods.** Platform-specific behavior must be hidden behind extension methods for readability.
 
-Only use platform-specific directives when:
+**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.
 
-1. **Platform-specific API is being tested** - The test validates behavior that only exists on one platform
-2. **Known platform limitation** - There is a documented bug or limitation that prevents the test from running on a specific platform
-3. **Different expected behavior** - The platforms are expected to behave differently for valid reasons
-
-### Examples
-
-**✅ Correct - Runs on all platforms:**
 ```csharp
+// ❌ BAD - inline #if in test method (hard to read)
 [Test]
-[Category(UITestCategories.SafeAreaEdges)]
-public void SoftInputBehaviorTest()
+public void MyTest()
 {
-    // This test runs on all applicable platforms
-    App.WaitForElement("ContentGrid");
-    // Test code...
+#if ANDROID
+    App.TapCoordinates(100, 200);
+#else
+    App.Tap("MyElement");
+#endif
 }
-```
 
-**❌ Incorrect - Unnecessarily limits to one platform:**
-```csharp
-#if ANDROID
+// ✅ GOOD - platform logic in extension method (clean test)
 [Test]
-[Category(UITestCategories.SafeAreaEdges)]
-public void SoftInputBehaviorTest()
+public void MyTest()
 {
-    // This unnecessarily limits the test to Android only
-    // Unless there's a specific reason, tests should run on all platforms
-    App.WaitForElement("ContentGrid");
-    // Test code...
+    App.TapElementCrossPlatform("MyElement");
 }
-#endif
 ```
 
+Move platform-specific logic to extension methods to keep test code clean and readable.
+
 ## Running UI Tests Locally
 
 **CRITICAL: ALWAYS use the BuildAndRunHostApp.ps1 script to run UI tests. NEVER run `dotnet test` or `dotnet build` commands manually.**
@@ -330,9 +319,8 @@ Verify the following checklist before committing UI tests:
 - [ ] Ensure file names follow the `IssueXXXXX` pattern and match between projects
 - [ ] Ensure test methods have descriptive names
 - [ ] Verify test inherits from `_IssuesUITest`
-- [ ] Confirm only ONE `[Category]` attribute is used per test
-- [ ] Verify tests run on all applicable platforms (iOS, Android, Windows, MacCatalyst) unless platform-specific
-- [ ] Document any platform-specific limitations with clear comments
+- [ ] Confirm only ONE `[Category]` attribute per test
+- [ ] No inline `#if` directives in test code (use extension methods)
 - [ ] Test passes locally on at least one platform
 
 ### Test State Management