[Android, Windows] Fix for FlyoutPage toolbar button not updating on orientation change#31962
Conversation
|
Hey there @@praveenkumarkarunanithi! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the FlyoutPage toolbar button doesn't update correctly when device orientation changes on Android and Windows platforms. The bug occurred because the NavigationPageToolbar wasn't detecting orientation changes, so the toolbar button visibility wasn't re-evaluated after rotation.
Key changes:
- Added orientation change detection for Android and Windows platforms in NavigationPageToolbar
- Implemented proper cleanup of event subscriptions to prevent memory leaks
- Created comprehensive test coverage for the orientation change behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/NavigationPage/NavigationPageToolbar.cs | Added orientation change handling with DeviceDisplay.MainDisplayInfoChanged subscription and cleanup logic |
| src/Controls/tests/TestCases.HostApp/Issues/Issue24468.cs | Created UI test page demonstrating the toolbar button update behavior on orientation changes |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs | Added automated test to verify toolbar button updates correctly during orientation changes |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs
Outdated
Show resolved
Hide resolved
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 31962Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 31962" |
|
No action planned here? |
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Merge branch 'dotnet:main' into fix-24468 ·
|
| File:Line | Reviewer | Feedback | Status |
|---|---|---|---|
| NavigationPageToolbar.cs:56 | jsuarezruiz | "Where is re-evaluated on iOS? Why is not required in that case?" | ✅ RESOLVED - iOS uses native ViewWillTransitionToSize() |
| Test Issue24468.cs:22 | jsuarezruiz | "Can use try-finally to always restore the state" | ✅ RESOLVED - Added try-finally |
| Test Issue24468.cs:25 | jsuarezruiz | "Could also check the counter to validate is not called more than necessary?" | ✅ RESOLVED - Added counter validation |
| HostApp Issue24468.cs:4 | Copilot (bot) | "Issue attribute indicates Android only, but fix covers both Android and Windows" |
Edge Cases
From discussion comments:
- ✅ Memory leaks - Handled via proper cleanup in
DisconnectHandler() - ✅ iOS platform - Not affected (already works correctly via native UIKit lifecycle)
- ✅ Excessive calls - Counter validation added to test
⚠️ Platform metadata - Issue attribute mentions Android only, but fix applies to Android + Windows
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31962 | Subscribe to DeviceDisplay.MainDisplayInfoChanged for Android/Windows |
⏳ PENDING (Gate) | NavigationPageToolbar.cs (+27) |
Original PR - includes proper cleanup |
Platform Selection for Testing
User specified platform: Android
Validation:
- ✅ Android IS affected by the bug (issue labels:
platform/android) - ✅ Android IS available on current host (works on any host)
- ✅ PR modifies platform-agnostic code (
NavigationPageToolbar.cs) that applies to both Android and Windows - ✅ Test files exist and include Android-specific test setup
Decision: Test on Android as specified by user.
🚦 Gate — Test Verification
📝 Review Session — Merge branch 'dotnet:main' into fix-24468 · 48d5c39
Result: ✅ PASSED
Platform: Android
Mode: Full Verification (RequireFullVerification: true)
Verification Results
- Tests FAIL without fix ✅ (bug is correctly detected)
- Tests PASS with fix ✅ (fix correctly resolves the issue)
Test Details
Test Filter: Issue24468
Test Files:
src/Controls/tests/TestCases.HostApp/Issues/Issue24468.cs- UI test pagesrc/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs- Automated test
What was tested:
- FlyoutPage with custom
ShouldShowToolbarButton()that tracks call count - Device rotation from portrait to landscape
- Toolbar button visibility after rotation
- Counter validation to ensure method was re-evaluated
Without fix (bug present):
- Test correctly FAILED - toolbar button did not update after rotation
ShouldShowToolbarButton()was not re-called after orientation change
With fix (bug resolved):
- Test correctly PASSED - toolbar button updates after rotation
ShouldShowToolbarButton()is re-evaluated viaDeviceDisplay.MainDisplayInfoChangedevent
Conclusion
✅ Tests properly validate the fix. The test suite correctly:
- Detects the bug when the fix is not present
- Verifies the fix resolves the issue when applied
- Validates that orientation changes trigger toolbar button re-evaluation
🔧 Fix — Analysis & Comparison
📝 Review Session — Merge branch 'dotnet:main' into fix-24468 · 48d5c39
Fix Candidates Summary
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-sonnet-4.5) | FlyoutPage triggers toolbar refresh via internal method | ✅ PASS | FlyoutPage.cs, NavigationPageToolbar.cs | FlyoutPage notifies toolbar directly when orientation changes |
| 2 | try-fix (claude-opus-4.6) | FlyoutPage-side Property Change Notification (OnPropertyChanged) | ✅ PASS | FlyoutPage.cs, NavigationPageToolbar.cs | Uses existing property change pipeline |
| 3 | try-fix (gpt-5.2) | Subscribe to FlyoutPage.SizeChanged | ❌ FAIL | NavigationPageToolbar.cs | SizeChanged didn't trigger reliably during rotation |
| 4 | try-fix (gpt-5.2-codex) | Subscribe to FlyoutPage.SizeChanged (similar) | ❌ FAIL | NavigationPageToolbar.cs | Same issue as #3 |
| 5 | try-fix (gemini-3-pro-preview) | Event Propagation from FlyoutPage (internal event) | ✅ PASS | FlyoutPage.cs, NavigationPageToolbar.cs | Custom internal event from FlyoutPage |
| 6 | try-fix (claude-sonnet-4.5 R2) | Direct Notification in OnMainDisplayInfoChanged | ✅ PASS | FlyoutPage.cs, NavigationPage.cs, NavigationPageToolbar.cs | FlyoutPage directly calls toolbar.ApplyChanges() |
| 7 | try-fix (gpt-5.2 R2) | Window size change with double dispatch | ✅ PASS | FlyoutPage.cs, NavigationPageToolbar.cs | Subscribe to Window.SizeChanged, dispatch twice |
| PR | PR #31962 | Subscribe to DeviceDisplay.MainDisplayInfoChanged | ✅ PASS (Gate) | NavigationPageToolbar.cs | NavigationPageToolbar subscribes to global orientation event |
Cross-Pollination Results
Round 2:
| Model | Response | Action Taken |
|---|---|---|
| claude-sonnet-4.5 | NEW IDEAS: Property listener, Handler Mapper, Weak Event Manager, BindableProperty callback, LayoutChanged | Tested BindableProperty callback variation → Attempt 6 (PASS) |
| claude-opus-4.6 | NO NEW IDEAS | - |
| gpt-5.2 | NEW IDEA: Window.SizeChanged with double dispatch | Tested → Attempt 7 (PASS) |
| gpt-5.2-codex | NEW IDEA: One-time refresh on Window attach | Not relevant to bug (initialization vs rotation) |
| gemini-3-pro-preview | NEW IDEA: iOS renderer hook | Not applicable (iOS already works, fix is for Android/Windows) |
Exhausted Status
Exhausted: Yes
All 5 models consulted in Round 2. Models 2-5 either said "NO NEW IDEAS" or provided ideas not applicable to the bug. The design space has been thoroughly explored:
- ✅ Toolbar subscribes directly to system events (PR's approach)
- ✅ FlyoutPage pushes via method call ([Draft] Readme WIP #1, [Spec] Transitions #6)
- ✅ FlyoutPage pushes via property change (Update README.md #2)
- ✅ FlyoutPage pushes via custom event (Update README.md #5)
- ✅ Window-level event subscription ([Spec] TabView #7)
- ❌ FlyoutPage/Page size-based triggers (Third #3, Aloha System.Maui! #4) - unreliable
Analysis of Passing Candidates
Comparison of the 6 passing alternatives:
| Criterion | PR's Fix | #1 | #2 | #5 | #6 | #7 |
|---|---|---|---|---|---|---|
| Simplicity | Good (27 lines) | Excellent (12 lines) | Excellent (7 lines) | Good (24 lines) | Moderate (11 lines, 3 files) | Complex (65 lines) |
| Robustness | Excellent (direct orientation signal) | Excellent (reuses FlyoutPage handler) | Excellent (reuses property pipeline) | Excellent (custom event) | Excellent (direct call) | Good (double dispatch timing) |
| Architecture | Toolbar manages own events | FlyoutPage notifies children | FlyoutPage notifies via properties | FlyoutPage notifies via event | FlyoutPage notifies directly | Toolbar manages Window events |
| Lifecycle | Manual subscribe/unsubscribe | No lifecycle needed | No lifecycle needed | Manual subscribe/unsubscribe | No lifecycle needed | Complex Window tracking |
| Lines Changed | 27 | 12 | 7 | 24 | 11 (3 files) | 65 |
| Files Modified | 1 | 2 | 2 | 2 | 3 | 2 |
Winner: Candidate #2 (FlyoutPage-side Property Change Notification)
Reasoning:
- Simplest implementation - Only 7 lines changed total
- No new infrastructure - Reuses existing property change pipeline (
ToolbarTracker.PagePropertyChanged) - No lifecycle management - No subscribe/unsubscribe needed
- Architecturally sound - FlyoutPage already owns orientation concerns
- No platform-specific code in toolbar - All #if blocks stay in FlyoutPage
- Proven reliable - Uses the same property change mechanism as other toolbar updates
Root Cause Analysis
Root Cause: NavigationPageToolbar.ShouldShowToolbarButton() determines flyout button visibility based on DeviceDisplay.MainDisplayInfo.Orientation, but is only called during initialization and property changes. On Android/Windows, orientation changes don't trigger any of the existing property change listeners, so the toolbar button visibility becomes stale after rotation.
Why iOS works: iOS platform's PhoneFlyoutPageRenderer handles orientation changes through native ViewWillTransitionToSize() callback, which calls UpdateLeftBarButton() and internally re-evaluates ShouldShowToolbarButton().
Why the fix is needed: Android and Windows lack equivalent native lifecycle callbacks that automatically trigger toolbar updates on orientation change. A manual orientation detection mechanism is required.
Selected Fix
Selected Fix: Candidate #2 - FlyoutPage-side Property Change Notification
Implementation:
FlyoutPage.OnMainDisplayInfoChanged()callsOnPropertyChanged(nameof(FlyoutLayoutBehavior))NavigationPageToolbar.OnPagePropertyChanged()addsFlyoutPage.FlyoutLayoutBehaviorPropertyto its filter- When orientation changes → property change fires → toolbar calls
ApplyChanges()→ShouldShowToolbarButton()re-evaluated
Why this is better than PR's fix:
- Fewer lines (7 vs 27) - Simpler implementation
- No lifecycle management - PR requires subscribe in constructor, unsubscribe in Disconnect
- Reuses existing infrastructure - Property change pipeline already exists for other toolbar updates
- Architecturally cleaner - FlyoutPage notifies through established property change mechanism instead of toolbar subscribing to global events
- Less platform-specific code - All Android/Windows-specific code stays in FlyoutPage
Trade-off: PR's approach is more explicit (direct event subscription makes the dependency obvious), while this approach is implicit (relies on understanding the property change pipeline). However, the simplicity and reuse of existing patterns outweigh this concern.
Recommendations for PR #31962
Recommendation:
The PR's fix works correctly and solves the bug, but there's a significantly simpler alternative (Candidate #2) that:
- Requires only 7 lines of code vs 27
- Eliminates lifecycle management (subscribe/unsubscribe)
- Reuses existing property change infrastructure
- Keeps all platform-specific code in one place (FlyoutPage)
Suggested changes:
Option 1 (Recommended): Adopt Candidate #2 approach
diff --git a/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs b/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs
@@ -351,6 +351,11 @@ void OnMainDisplayInfoChanged(object sender, DisplayInfoChangedEventArgs e)
void OnMainDisplayInfoChanged(object sender, DisplayInfoChangedEventArgs e)
{
Handler?.UpdateValue(nameof(FlyoutBehavior));
+
+ // Notify that FlyoutLayoutBehavior effectively changed due to orientation change.
+ // This propagates through ToolbarTracker.PagePropertyChanged to NavigationPageToolbar
+ // so it re-evaluates ShouldShowToolbarButton() and updates DrawerToggleVisible.
+ OnPropertyChanged(nameof(FlyoutLayoutBehavior));
}
diff --git a/src/Controls/src/Core/NavigationPage/NavigationPageToolbar.cs b/src/Controls/src/Core/NavigationPage/NavigationPageToolbar.cs
@@ -55,7 +55,8 @@ void OnPagePropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
NavigationPage.BarTextColorProperty) ||
e.IsOneOf(
PlatformConfiguration.WindowsSpecific.Page.ToolbarDynamicOverflowEnabledProperty,
- PlatformConfiguration.WindowsSpecific.Page.ToolbarPlacementProperty))
+ PlatformConfiguration.WindowsSpecific.Page.ToolbarPlacementProperty) ||
+ e.Is(FlyoutPage.FlyoutLayoutBehaviorProperty))
{
ApplyChanges(_currentNavigationPage);
}Option 2: Keep current PR approach
If the team prefers the explicit direct subscription approach for clarity, the current PR is acceptable. It works correctly and is a valid solution.
Test Coverage
All candidates were tested with:
- Test:
Issue24468.cs- FlyoutPage with customShouldShowToolbarButton()that tracks call count - Platform: Android
- Validation:
- Toolbar button exists after rotation ✅
ShouldShowToolbarButton()called during rotation ✅- Call count within expected range (not excessive) ✅
📋 Report — Final Recommendation
📝 Review Session — Merge branch 'dotnet:main' into fix-24468 · 48d5c39
✅ Final Recommendation: APPROVE
Summary
PR #31962 successfully fixes the FlyoutPage toolbar button not updating on orientation changes for Android and Windows. The implementation is clean, well-tested, and follows MAUI patterns correctly. While try-fix exploration revealed simpler alternatives, the PR's approach is solid and appropriate for the problem.
Key Findings:
- ✅ Fix works correctly (validated by Gate)
- ✅ Clean implementation with proper lifecycle management
- ✅ Comprehensive test coverage
- ✅ Good PR description (only needs NOTE block)
⚠️ Simpler alternatives exist but not critical
Root Cause
NavigationPageToolbar.ShouldShowToolbarButton() determines flyout button visibility based on device orientation, but is only evaluated during initialization and property changes. On Android/Windows, orientation changes don't trigger any existing property change listeners, so the toolbar button visibility becomes stale after rotation.
Why iOS works: iOS platform's PhoneFlyoutPageRenderer handles orientation changes through native ViewWillTransitionToSize() callback, which automatically triggers toolbar updates.
Why Android/Windows need a fix: These platforms lack equivalent native lifecycle callbacks, requiring manual orientation detection.
Fix Analysis
PR's Approach:
- NavigationPageToolbar subscribes to
DeviceDisplay.MainDisplayInfoChanged - On orientation change: calls
ApplyChanges()→UpdateBackButton()→ShouldShowToolbarButton()re-evaluated - Cleanup in
Disconnect()prevents memory leaks - Platform-specific guards (
#if ANDROID || WINDOWS)
Implementation Quality: ✅ Excellent
- Proper event subscription/unsubscription pairing
- Null safety checks
- Clear comments explaining platform differences
- Follows existing patterns in the codebase
Alternative Fix Exploration
During try-fix exploration, 5 alternative approaches were tested:
| Approach | Result | Lines Changed | Complexity |
|---|---|---|---|
| PR's Fix | ✅ PASS | 27 | Simple |
| #1: FlyoutPage direct method call | ✅ PASS | 12 | Simpler |
| #2: Property change notification | ✅ PASS | 7 | Simplest |
| #3: FlyoutPage.SizeChanged | ❌ FAIL | N/A | Unreliable |
| #4: FlyoutPage.SizeChanged (retry) | ❌ FAIL | N/A | Unreliable |
| #5: Internal event propagation | ✅ PASS | 24 | Simple |
| #6: Direct notification in handler | ✅ PASS | 11 (3 files) | Moderate |
| #7: Window.SizeChanged | ✅ PASS | 65 | Complex |
Simplest Alternative (#2 - Property Change Notification):
- 7 lines vs PR's 27 lines
- No lifecycle management needed
- Reuses existing property change pipeline
FlyoutPage.OnMainDisplayInfoChanged()callsOnPropertyChanged(nameof(FlyoutLayoutBehavior))NavigationPageToolbar.OnPagePropertyChanged()addsFlyoutLayoutBehaviorPropertyto filter
Trade-offs:
- PR's approach: More explicit, direct event subscription makes dependency clear
- Alternative Update README.md #2: More implicit, relies on understanding property change pipeline
- Verdict: Both are valid. PR's approach is slightly more lines but more obvious.
Test Quality
Test Coverage: ✅ Excellent
- UI test page with custom
ShouldShowToolbarButton()that tracks call count - Rotates device to landscape
- Verifies toolbar button exists after rotation
- Verifies
ShouldShowToolbarButton()was called - Validates counter to prevent excessive calls (per reviewer feedback)
- Uses
try-finallyto restore orientation (per reviewer feedback)
Gate Results: ✅ PASSED
- Tests FAIL without fix ✅ (bug detected correctly)
- Tests PASS with fix ✅ (fix resolves issue)
PR Finalization Review
Title: ✅ Good
- Current: "[Android, Windows] Fix for FlyoutPage toolbar button not updating on orientation change"
- Optional enhancement: Add "FlyoutPage:" component prefix for git history
Description: ✅ Good Quality
- Clear root cause explanation
- Detailed implementation approach
- Platforms tested documented
- Before/after videos included
- Only missing: NOTE block at top (required for users to test PR builds)
Code Review: ✅ No critical issues
- Clean event subscription pattern
- Proper memory leak prevention
- Good null safety
- Follows MAUI patterns
Reviewer Feedback
All reviewer feedback has been addressed:
| Reviewer | Feedback | Status |
|---|---|---|
| jsuarezruiz | Why not needed on iOS? | ✅ Answered - iOS uses native lifecycle |
| jsuarezruiz | Use try-finally in test | ✅ Implemented |
| jsuarezruiz | Validate counter | ✅ Implemented |
| Copilot (bot) | Update platform metadata in test |
Recommendations
Primary Recommendation: ✅ APPROVE
The PR correctly fixes the issue with clean, well-tested code. While simpler alternatives exist, they are not critical for approval. The current approach is:
- More explicit than alternatives
- Easy to understand
- Properly documented
- Thoroughly tested
Suggested Minor Improvements (Non-Blocking):
- Add NOTE block to PR description (so users can test PR builds):
<!-- Please let the below note in for people that find this PR -->
> [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!-
Optional: Consider Alternative Update README.md #2 (if team prefers fewer lines):
- 7 lines vs 27 lines
- No lifecycle management
- Same functionality
- See try-fix content.md for implementation
-
Update test platform metadata (cosmetic):
[Issue(..., PlatformAffected.Android | PlatformAffected.WinUI)]
Context for Future Agents
What was tried and failed:
- ❌ FlyoutPage.SizeChanged - Doesn't trigger reliably during rotation
- ❌ Window-level size change - Too complex, unnecessary lifecycle overhead
What works:
- ✅ DeviceDisplay.MainDisplayInfoChanged (PR's approach)
- ✅ Property change notifications through existing pipeline
- ✅ Direct method calls from FlyoutPage to toolbar
Key lesson: On Android/Windows, orientation changes require explicit detection. Using DeviceDisplay.MainDisplayInfoChanged is the most direct signal for orientation changes.
Final Verdict
✅ APPROVE - PR #31962 is ready for merge after adding NOTE block to description.
Reasoning:
- Fix works correctly (Gate validated)
- Implementation is clean and well-structured
- Tests are comprehensive
- Memory leaks are prevented
- All reviewer feedback addressed
- Simpler alternatives exist but current approach is acceptable
📋 Expand PR Finalization Review
Title: ✅ Good
Current: [Android, Windows] Fix for FlyoutPage toolbar button not updating on orientation change
Description: ✅ Excellent
Description needs updates. See details below.
Missing Elements:
**
- ❌ NOTE block missing - Required at the top so users can test PR artifacts
Recommendation: The existing description is excellent and should be preserved. Only add the required NOTE block at the very top.
✨ Suggested PR Description
[!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!
Root Cause
On Android and Windows, the FlyoutPage toolbar button did not update correctly when the device orientation changed. The NavigationPageToolbar lacked orientation change detection, so ShouldShowToolbarButton() was never re-evaluated after rotation.
Description of Change
Added orientation change handling in NavigationPageToolbar for Android and Windows by subscribing to DeviceDisplay.MainDisplayInfoChanged. On rotation, the toolbar button visibility is re-evaluated by calling ApplyChanges, which internally checks ShouldShowToolbarButton(). The subscription is cleaned up on disconnect to prevent leaks. This ensures the toolbar button updates correctly across orientation changes while following MAUI's existing patterns for cross-platform event handling.
Issues Fixed
Fixes #24468
Tested the behaviour in the following platforms
- Android
- Windows
- iOS
- Mac
Note:
The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS.
Output Video
| Before Issue Fix | After Issue Fix |
|---|---|
Beforefix.mov |
Afterfix.mov |
Code Review: ✅ Passed
Code Review Findings for PR #31962
✅ Overall Assessment: Clean Implementation
The code changes are well-implemented with proper platform guards, event cleanup, and defensive programming. No critical issues found.
🟢 Positive Observations
1. Proper Platform Isolation
- ✅ Uses
#if ANDROID || WINDOWSconsistently to isolate platform-specific code - ✅ iOS/Mac don't need this fix (handled automatically by platform), comment explains this
- ✅ Clean separation of concerns
2. Event Subscription Cleanup
- ✅ Properly unsubscribes in
Disconnect()method to prevent memory leaks - ✅ Matches the existing pattern for event cleanup in the class
- ✅ Same platform guard in cleanup as in subscription
3. Defensive Programming
- ✅ Null check for
_currentNavigationPagebefore callingApplyChanges - ✅ Only subscribes when
parent is FlyoutPage(avoids unnecessary subscriptions) - ✅ Reuses existing
ApplyChangesmethod rather than duplicating logic
4. Test Coverage
- ✅ Comprehensive UI test (
Issue24468.cs) verifies the fix - ✅ Test uses proper platform guards (
#if ANDROID || IOS) - ✅ Test validates that
ShouldShowToolbarButton()is called on orientation change - ✅ Test includes assertion range (2-4 calls) to catch excessive re-evaluation
- ✅ Proper cleanup with
try/finallyto reset orientation
🟡 Minor Suggestions (Non-Blocking)
1. Test Platform Comment Clarity
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs
Current:
#if ANDROID || IOS //The test fails on Windows and MacCatalyst because the SetOrientation method...Suggestion:
#if ANDROID || IOS
// SetOrientation only supported on mobile platforms (iOS and Android)
// Windows/MacCatalyst don't support programmatic orientation changes in UI testsReasoning: Slightly clearer that it's a test infrastructure limitation, not a runtime failure.
🔵 Architecture Alignment
1. Event Pattern
- ✅ Uses
DeviceDisplay.MainDisplayInfoChangedwhich is the MAUI cross-platform API for orientation changes - ✅ Follows existing MAUI patterns for display info monitoring
- ✅ Properly typed event args (
DisplayInfoChangedEventArgs)
2. Reuse of Existing Logic
- ✅ Calls existing
ApplyChanges()method rather than duplicating toolbar button logic - ✅
ApplyChangesinternally callsShouldShowToolbarButton(), maintaining the existing evaluation flow - ✅ No duplication of toolbar button visibility logic
3. Minimal Changes
- ✅ Only adds orientation monitoring, doesn't modify existing toolbar logic
- ✅ Surgical fix that doesn't risk breaking existing behavior
- ✅ Platform-specific guards ensure no impact on iOS/Mac
🔍 Test Quality Analysis
Test Design
File: src/Controls/tests/TestCases.HostApp/Issues/Issue24468.cs
✅ Excellent test design:
- Inherits from
TestFlyoutPage(correct base class for FlyoutPage tests) - Uses
ShouldShowToolbarButton()override to track calls - Labels with AutomationIds for UI test verification
- Clear visual feedback via
_eventLabeland_countLabel
Test Assertions
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24468.cs
✅ Smart assertion strategy:
Assert.That(callCount, Is.GreaterThan(1).And.LessThan(5))This catches:
- Too few calls (0-1) → Fix not working
- Too many calls (5+) → Excessive re-evaluation (performance issue)
The range (2-4 calls) allows for platform differences while detecting problems.
Summary
| Category | Status |
|---|---|
| Memory Management | ✅ Clean (proper event cleanup) |
| Platform Guards | ✅ Correct (#if ANDROID || WINDOWS) |
| Null Safety | ✅ Defensive (null check before use) |
| Code Reuse | ✅ Excellent (uses existing ApplyChanges) |
| Test Coverage | ✅ Comprehensive (UI test with orientation change) |
| Test Quality | ✅ Robust (smart assertion range) |
Verdict: No blocking issues. The single minor suggestion (comment clarity) is optional and non-critical. This PR is ready to merge.
kubaflo
left a comment
There was a problem hiding this comment.
Hi @praveenkumarkarunanithi could you please try Winner: Candidate https://github.com/dotnet/maui/pull/2 (FlyoutPage-side Property Change Notification) from the AI Summary?
@kubaflo, per the AI summary (Candidate #2), I simplified the fix by raising |
…orientation change (#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes #24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
…orientation change (#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes #24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
…orientation change (#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes #24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
…orientation change (#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes #24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
…orientation change (dotnet#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes dotnet#24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
…orientation change (#31962) ### Root Cause On Android and Windows, the `FlyoutPage` toolbar button did not update correctly when the device orientation changed. The `NavigationPageToolbar` lacked orientation change detection, so `ShouldShowToolbarButton()` was never re-evaluated after rotation. ### Description of Change On orientation change, `FlyoutPage.OnMainDisplayInfoChanged` now fires `OnPropertyChanged(nameof(FlyoutLayoutBehavior))` on Android and Windows. This propagates through the existing `ToolbarTracker.PagePropertyChanged` pipeline to `NavigationPageToolbar`, which re-evaluates `ShouldShowToolbarButton()` via `ApplyChanges()`. `NavigationPageToolbar` was updated to listen for `FlyoutLayoutBehavior` property changes alongside its existing property filters. iOS is excluded as it handles this natively through `PhoneFlyoutPageRenderer.ViewWillTransitionToSize()`. ### Issues Fixed Fixes #24468 Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [ ] Mac **Note:** The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS. ### Output Video Before Issue Fix | After Issue Fix | |----------|----------| |<video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/1ad9c143-c087-4fd1-aa8c-5bd59e86519e">|<video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/b219267a-e830-490f-994a-570fa6d928b3">|
## What's Coming .NET MAUI inflight/candidate introduces significant improvements across all platforms with focus on quality, performance, and developer experience. This release includes 46 commits with various improvements, bug fixes, and enhancements. ## Button - [Android] Implemented material3 support for Button by @Dhivya-SF4094 in #33173 <details> <summary>🔧 Fixes</summary> - [Implement Material3 support for Button](#33172) </details> ## CollectionView - [Android] Fix RemainingItemsThresholdReachedCommand not firing when CollectionView has Header and Footer both defined by @SuthiYuvaraj in #29618 <details> <summary>🔧 Fixes</summary> - [Android : RemainingItemsThresholdReachedCommand not firing when CollectionVew has Header and Footer both defined](#29588) </details> - [iOS/MacCatalyst] Fix CollectionView ScrollTo for horizontal layouts by @Shalini-Ashokan in #33853 <details> <summary>🔧 Fixes</summary> - [[iOS/MacCatalyst] CollectionView ScrollTo does not work with horizontal Layout](#33852) </details> - [iOS & Mac] Fixed IndicatorView Size doesnt update dynamically by @SubhikshaSf4851 in #31129 <details> <summary>🔧 Fixes</summary> - [[iOS, Catalyst] IndicatorView.IndicatorSize does not update dynamically at runtime](#31064) </details> - [Android] Fix for CollectionView Scrolled event is triggered on the initial app load. by @BagavathiPerumal in #33558 <details> <summary>🔧 Fixes</summary> - [[Android] CollectionView Scrolled event is triggered on the initial app load.](#33333) </details> - [iOS, Android] Fix for CollectionView IsEnabled=false allows touch interactions by @praveenkumarkarunanithi in #31403 <details> <summary>🔧 Fixes</summary> - [More issues with CollectionView IsEnabled, InputTransparent, Opacity via Styles and code behind](#19771) </details> - [iOS] Fix VerticalOffset Update When Modifying CollectionView.ItemsSource While Scrolled by @devanathan-vaithiyanathan in #34153 <details> <summary>🔧 Fixes</summary> - [[iOS]VerticalOffset Not Reset to Zero After Clearing ItemSource in CollectionView](#26798) </details> ## DateTimePicker - [Android] Fix DatePicker MinimumDate/MaximumDate not updating dynamically by @HarishwaranVijayakumar in #33687 <details> <summary>🔧 Fixes</summary> - [[regression/8.0.3] [Android] DatePicker control minimum date issue](#19256) - [[Android] DatePicker does not update MinimumDate / MaximumDate in the Popup when set in the viewmodel after first opening](#33583) </details> ## Drawing - Android drawable perf by @albyrock87 in #31567 ## Editor - [Android] Implemented material3 support for Editor by @SyedAbdulAzeemSF4852 in #33478 <details> <summary>🔧 Fixes</summary> - [Implement Material3 Support for Editor](#33476) </details> ## Entry - [iOS, Mac] Fix for CursorPosition not updating when typing into Entry control by @SyedAbdulAzeemSF4852 in #30505 <details> <summary>🔧 Fixes</summary> - [Entry control CursorPosition does not update on TextChanged event [iOS Maui 8.0.7] ](#20911) - [CursorPosition not calculated correctly on behaviors events for iOS devices](#32483) </details> ## Flyoutpage - [Android, Windows] Fix for FlyoutPage toolbar button not updating on orientation change by @praveenkumarkarunanithi in #31962 <details> <summary>🔧 Fixes</summary> - [Flyout page in Android does not show flyout button (burger) consistently](#24468) </details> - Fix for First Item in CollectionView Overlaps in FlyoutPage.Flyout on iOS by @praveenkumarkarunanithi in #29265 <details> <summary>🔧 Fixes</summary> - [[iOS] CollectionView not rendering first item correctly in FlyoutPage.Flyout](#29170) </details> ## Image - [Android] Fix excessive memory usage for stream and resource-based image loading by @Shalini-Ashokan in #33590 <details> <summary>🔧 Fixes</summary> - [[Android] Unexpected high Bitmap.ByteCount when loading image via ImageSource.FromResource() or ImageSource.FromStream() in .NET MAUI](#33239) </details> - [Android] Fix for Resize method returns an image that has already been disposed by @SyedAbdulAzeemSF4852 in #29964 <details> <summary>🔧 Fixes</summary> - [In GraphicsView, the Resize method returns an image that has already been disposed](#29961) - [IIMage.Resize bugged behaviour](#31103) </details> ## Label - Fixed Label Span font property inheritance when applied via Style by @SubhikshaSf4851 in #34110 <details> <summary>🔧 Fixes</summary> - [`Span` does not inherit text styling from `Label` if that styling is applied using `Style` ](#21326) </details> - [Android] Implemented material3 support for Label by @SyedAbdulAzeemSF4852 in #33599 <details> <summary>🔧 Fixes</summary> - [Implement Material3 Support for Label](#33598) </details> ## Map - [Android] Fix Circle Stroke color is incorrectly updated as Fill color. by @NirmalKumarYuvaraj in #33643 <details> <summary>🔧 Fixes</summary> - [[Android] Circle Stroke color is incorrectly updated as Fill color.](#33642) </details> ## Mediapicker - [iOS] Fix: invoke MediaPicker completion handler after DismissViewController by @yuriikyry4enko in #34250 <details> <summary>🔧 Fixes</summary> - [[iOS] Media Picker UIImagePickerController closing issue](#21996) </details> ## Navigation - Fix ContentPage memory leak on Android when using NavigationPage modally (fixes #33918) by @brunck in #34117 <details> <summary>🔧 Fixes</summary> - [[Android] Modal TabbedPage whose tabs are NavigationPage(ContentPage) is retained after PopModalAsync()](#33918) </details> ## Picker - [Android] Implement material3 support for TimePicker by @HarishwaranVijayakumar in #33646 <details> <summary>🔧 Fixes</summary> - [Implement Material3 support for TimePicker](#33645) </details> - [Android] Implemented Material3 support for Picker by @SyedAbdulAzeemSF4852 in #33668 <details> <summary>🔧 Fixes</summary> - [Implement Material3 support for Picker](#33665) </details> ## RadioButton - [Android] Implemented material3 support for RadioButton by @SyedAbdulAzeemSF4852 in #33468 <details> <summary>🔧 Fixes</summary> - [Implement Material3 Support for RadioButton](#33467) </details> ## Setup - Clarify MA003 error message by @jeremy-visionaid in #34067 <details> <summary>🔧 Fixes</summary> - [MA003 false positive with 9.0.21](#26599) </details> ## Shell - [Android] Fix TabBar FlowDirection not updating dynamically by @SubhikshaSf4851 in #33091 <details> <summary>🔧 Fixes</summary> - [[Android, iOS] FlowDirection RTL is not updated dynamically on Shell TabBar](#32993) </details> - [Android] Fix page not disposed on Shell replace navigation by @Vignesh-SF3580 in #33426 <details> <summary>🔧 Fixes</summary> - [[Android] [Shell] replace navigation leaks current page](#25134) </details> - [Android] Fixed Shell flyout does not disable scrolling when FlyoutVerticalScrollMode is set to Disabled by @NanthiniMahalingam in #32734 <details> <summary>🔧 Fixes</summary> - [[Android] Shell.FlyoutVerticalScrollMode="Disabled" does not disable scrolling](#32477) </details> ## Single Project - Fix: Throw a clear error when an SVG lacks dimensions instead of a NullReferenceException by @Shalini-Ashokan in #33194 <details> <summary>🔧 Fixes</summary> - [MAUI Fails To Convert Valid SVG Files Into PNG Files (Object reference not set to an instance of an object)](#32460) </details> ## SwipeView - [iOS] Fix SwipeView stays open on iOS after updating content by @devanathan-vaithiyanathan in #31248 <details> <summary>🔧 Fixes</summary> - [[iOS] - Swipeview with collectionview issue](#19541) </details> ## TabbedPage - [Windows] Fixed IsEnabled Property not works on Tabs by @NirmalKumarYuvaraj in #26728 <details> <summary>🔧 Fixes</summary> - [ShellContent IsEnabledProperty does not work](#5161) - [[Windows] Shell Tab IsEnabled Not Working](#32996) </details> - [Android] Fix NavigationBar overlapping StatusBar when NavigationBar visibility changes by @Vignesh-SF3580 in #33359 <details> <summary>🔧 Fixes</summary> - [[Android] NavigationBar overlaps with StatusBar when mixing HasNavigationBar=true/false in TabbedPage on Android 15 (API 35)](#33340) </details> ## Templates - Fix for unable to open task using keyboard navigation on windows platform by @SuthiYuvaraj in #33647 <details> <summary>🔧 Fixes</summary> - [Unable to open task using keyboard: A11y_.NET maui_User can get all the insights of Dashboard_Keyboard](#30787) </details> ## TitleView - Fix for NavigationPage.TitleView does not expand with host window in iPadOS 26+ by @SuthiYuvaraj in #33088 ## Toolbar - [iOS] Fix toolbar items ignoring BarTextColor on iOS/MacCatalyst 26+ by @Shalini-Ashokan in #34036 <details> <summary>🔧 Fixes</summary> - [[iOS 26] ToolbarItem color with custom BarTextColor not working](#33970) </details> - [Android] Fix for ToolbarItem retaining the icon from the previous page on Android when using NavigationPage. by @BagavathiPerumal in #32311 <details> <summary>🔧 Fixes</summary> - [Toolbaritem keeps the icon of the previous page on Android, using NavigationPage (not shell)](#31727) </details> ## WebView - [Android] Fix WebView in a grid expands beyond it's cell by @devanathan-vaithiyanathan in #32145 <details> <summary>🔧 Fixes</summary> - [Android - WebView in a grid expands beyond it's cell](#32030) </details> ## Xaml - ContentPresenter: Propagate binding context to children with explicit TemplateBinding by @HarishwaranVijayakumar in #30880 <details> <summary>🔧 Fixes</summary> - [Binding context in ContentPresenter](#23797) </details> <details> <summary>🔧 Infrastructure (1)</summary> - [Revert] ContentPresenter: Propagate binding context to children with explicit TemplateBinding by @Ahamed-Ali in #34332 </details> <details> <summary>🧪 Testing (6)</summary> - [Testing] Feature Matrix UITest Cases for Shell Flyout Page by @NafeelaNazhir in #32525 - [Testing] Feature Matrix UITest Cases for Brushes by @LogishaSelvarajSF4525 in #31833 - [Testing] Feature Matrix UITest Cases for BindableLayout by @LogishaSelvarajSF4525 in #33108 - [Android] Add UI tests for Material 3 CheckBox by @HarishwaranVijayakumar in #34126 <details> <summary>🔧 Fixes</summary> - [[Android] Add UI tests for Material 3 CheckBox](#34125) </details> - [Testing] Feature Matrix UITest Cases for Shell Tabbed Page by @NafeelaNazhir in #33159 - [Testing] Fixed Test case failure in PR 34294 - [03/2/2026] Candidate - 1 by @TamilarasanSF4853 in #34334 </details> <details> <summary>📦 Other (2)</summary> - Bumps Syncfusion.Maui.Toolkit dependency to version 1.0.9 by @PaulAndersonS in #34178 - Fix crash when closing Windows based app when using TitleBar by @MFinkBK in #34032 <details> <summary>🔧 Fixes</summary> - [Unhandled exception "Value does not fall within the expected range" when closing Windows app](#32194) </details> </details> **Full Changelog**: main...inflight/candidate
Root Cause
On Android and Windows, the
FlyoutPagetoolbar button did not update correctly when the device orientation changed. TheNavigationPageToolbarlacked orientation change detection, soShouldShowToolbarButton()was never re-evaluated after rotation.Description of Change
On orientation change,
FlyoutPage.OnMainDisplayInfoChangednow firesOnPropertyChanged(nameof(FlyoutLayoutBehavior))on Android and Windows. This propagates through the existingToolbarTracker.PagePropertyChangedpipeline toNavigationPageToolbar, which re-evaluatesShouldShowToolbarButton()viaApplyChanges().NavigationPageToolbarwas updated to listen forFlyoutLayoutBehaviorproperty changes alongside its existing property filters. iOS is excluded as it handles this natively throughPhoneFlyoutPageRenderer.ViewWillTransitionToSize().Issues Fixed
Fixes #24468
Tested the behaviour in the following platforms
Note:
The fix was implemented and tested on Android, iOS, and Windows platforms, as orientation changes could not be performed on macOS.
Output Video
Beforefix.mov
Afterfix.mov