[iOS] Fixed OnNavigatedTo not triggered for tabs in More section of TabbedPage#29495
[iOS] Fixed OnNavigatedTo not triggered for tabs in More section of TabbedPage#29495Vignesh-SF3580 wants to merge 6 commits intodotnet:inflight/currentfrom
Conversation
|
Hey there @@Vignesh-SF3580! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
community ✨
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue on iOS where the OnNavigatedTo lifecycle event was not triggered when navigating to tabs in the More section by updating the CurrentPage through a delegate mechanism.
- Added a UITest to verify the OnNavigatedTo event firing in the More section.
- Updated the HostApp to include a TabbedPage with multiple tabs covering the More scenario.
- Modified the iOS TabbedRenderer to assign a MoreTab delegate and update the CurrentPage when a More tab is shown.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue16175.cs | Added a test validating the navigation event for the More tab. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue16175.cs | Introduced a TabbedPage with multiple tabs to trigger the navigation event. |
| src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs | Implemented a delegate (MoreTabDelegate) to update CurrentPage for More tabs. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue16175.cs
Outdated
Show resolved
Hide resolved
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jsuarezruiz
left a comment
jsuarezruiz
left a comment
There was a problem hiding this comment.
Seems to keep failing:
TabbedPageTabEventsTriggeredInMoreSection
The app was expected to be running still, investigate as possible crash
TearDown : The app was expected to be running still, investigate as possible crash
Could you review it?
@jsuarezruiz I have verified the test |
Yes, running it again. |
|
|
||
| [Test] | ||
| [Category(UITestCategories.TabbedPage)] | ||
| public void TabbedPageTabEventsTriggeredInMoreSection() |
There was a problem hiding this comment.
The test is failing on iOS:
The app was expected to be running still, investigate as possible crash
Seems like the App is crashing.
There was a problem hiding this comment.
@jsuarezruiz I have verified it again, and the test still passes locally without any issues. However, I added an additional WaitForElement when the page appears. Could you please re-run the CI to check the results?
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| void UpdateCurrentPage() | ||
| { | ||
| if (SelectedViewController?.Title?.Equals("More", StringComparison.Ordinal) == true) |
There was a problem hiding this comment.
Does checking for "More" also works if the device is set to a language other than English?
There was a problem hiding this comment.
@Jabbah76 Yes, I tested by changing the device language to a non-English language, and it works as expected.
Vignesh-SF3580
force-pushed
the
fix-16175
branch
from
39ad841 to
2f2972c
Compare
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29495 | Add a navigation delegate for the More controller and update TabbedPage.CurrentPage when a More-tab child is shown; set CurrentPage = null while the More root is PENDING (Gate) |
src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs, src/Controls/tests/TestCases.HostApp/Issues/Issue16175.cs, src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue16175.cs |
Original PR; open concern about using the localized title "More" as control flow |
selected |
🚦 Gate — Test Verification
Gate Result PASSED:
Platform: ios
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence: CustomAgentLogsTmp/PRState/29495/PRAgent/gate/verify-tests-fail/verification-report.md shows Issue16175 failed on the broken baseline and passed with the PR fix on iOS.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Handle More-navigation callback directly on via exported ; resolve pages from instead of scanning controllers or checking Missing filename ("less --help" for help) | PASS | 1 file | Simpler and locale-safe; avoids the PR's string comparison concern |
| 2 | try-fix | Create a small action-based in and map the shown controller back to the page with | PASS | 1 file | Also avoids "More"; no renderer back-reference, but still requires a separate delegate object |
| 3 | try-fix | Synchronize More-tab selection during ViewDidLayoutSubviews by mapping MoreNavigationController.TopViewController to a child FAIL |
1 file | Layout timing was too late / unreliable; Issue16175 still failed |
page |
| 4 | try-fix | Use KVO on to react to stack changes and sync | PASS | 1 file | Distinct from delegate-based fixes, but more complex / more UIKit-observer-specific |
| 5 | try-fix | Implement the More-navigation callback directly on as and sync from | PASS | 1 file | Best balance of simplicity and robustness: no extra delegate type, no string check, controller-identity based |
| 6 | try-fix | Attach lightweight child observer view controllers to overflow tabs and let UIKit appearance forwarding update when an overflow page appears | PASS | 1 file | Appearance-based and delegate-free, but more invasive than the navigation-callback options |
| 7 | try-fix | Subscribe to child Page.Appearing events and sync CurrentPage when an overflow child FAIL |
1 file | Too late / wrong lifecycle hook; test still observed OnNavigatedTo not firing |
appears |
| 8 | try-fix | Wrap overflow child controllers in lightweight container VCs whose callback updates | PASS | 1 file | Works, but is the most invasive option by far |
| 9 | try-fix | Hook MoreNavigationController.NavigationBar via UINavigationBarDelegate.DidPushItem to sync ` FAIL |
1 file | Blocked by iOS API: navigation bars managed by UINavigationController reject manual delegate assignment |
CurrentPage` |
| 10 | try-fix | Attach a gesture recognizer to the More navigation view and asynchronously reconcile after user interaction | PASS | 1 file | Passes, but is interaction-driven and less deterministic than direct navigation callbacks |
| PR | PR #29495 | Add a and update for More-tab children, while using to null the current page on the More root | PASSED (Gate) | 3 files | Works, but the literal "More" control-flow check is brittle and unnecessary |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Inline exported delegate callback on TabbedRenderer, using TabBarItem.Tag / controller identity instead of the "More" title string |
| claude-sonnet-4.6 | 1 | Yes | Action-based UINavigationControllerDelegate wired in SetElement(), resolving the shown page via ViewControllers index |
| gpt-5.3-codex | 1 | Yes | Layout-time sync from MoreNavigationController.TopViewController to CurrentPage |
| gemini-3-pro-preview | 1 | Yes | KVO on the More-navigation stack |
| claude-opus-4.6 | 2 | Yes | Have TabbedRenderer itself implement IUINavigationControllerDelegate and sync directly from the More-navigation callback |
| claude-sonnet-4.6 | 2 | Yes | Use child-controller appearance forwarding for overflow tabs instead of navigation-controller callbacks |
| gpt-5.3-codex | 2 | Yes | Use child Page.Appearing as the sync trigger for overflow tabs |
| gemini-3-pro-preview | 2 | Yes | Wrap overflow children in appearance-reporting container controllers |
| claude-opus-4.6 | 3 | No | NO NEW IDEAS |
| claude-sonnet-4.6 | 3 | Yes | Use UINavigationBarDelegate.DidPushItem on the More navigation bar |
| gpt-5.3-codex | 3 | Yes | Add a gesture recognizer to the More navigation view and schedule an async top-controller sync after interaction |
| gemini-3-pro-preview | 3 | No | NO NEW IDEAS |
Exhausted: Yes
Selected Fix: Candidate # it passes the same iOS issue test as the PR, avoids the brittle localized "More" string, removes the extra delegate type, and stays far less invasive than observer/container/gesture alternatives.5
📋 Report — Final Recommendation
Final Recommendation: REQUEST CHANGES##
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight COMPLETE | Confirmed issue #16175, iOS scope, and an open review concern about the literal "More" check |
|
| Gate PASSED | Issue16175 failed on the broken baseline and passed with the PR fix on iOS |
|
| Try-Fix COMPLETE | 10 attempts, 7 passing, 3 failing/blocked; best alternative found | |
| Report COMPLETE |
Summary
The PR fixes the bug and its test coverage is valid, but Try-Fix found a cleaner implementation. The current PR solution adds a separate MoreTabDelegate and uses SelectedViewController?.Title == "More" as control flow. Multiple passing alternatives proved that the same behavior can be implemented without relying on the localized "More" title, and the best one keeps the logic directly on TabbedRenderer using controller identity.
Root Cause
Overflow tabs in iOS TabbedPage are navigated through MoreNavigationController, which does not update TabbedPage.CurrentPage through the normal selected-tab path. As a result, lifecycle hooks like OnNavigatedTo never fire for pages reached from the More stack unless the renderer explicitly synchronizes CurrentPage during that navigation flow.
Fix Quality
The PR is functionally correct, but not the best fix. Gate proved it works, yet the implementation is more brittle than necessary because it branches on the literal "More" title and adds an extra delegate type. Candidate #5 passed the same test with a simpler, locale-safe controller-identity-based callback on TabbedRenderer itself, making it the better long-term option.
MauiBot
added
s/agent-changes-requested
|
Fixed here: #27932 |
Issue Detail
OnNavigatedTo not called when navigate to the tabs in more tab in iOS.
Root cause
On iOS, when a tab is selected from the "More" section of a TabbedPage, the navigation event OnNavigatedTo is not triggered. This is because the CurrentPage property is not updated when selecting tabs from the "More" section, resulting in missing navigation lifecycle events for those tabs.
Description of change
Updated the CurrentPage using delegate when accessing tabs in More section resolves the issue.
Issues Fixed
Fixes #16175
Screenshots
16175BeforeFix.mov
16175AfterFix.mov