Skip to content

Review PR #32815: NavigationPage.TitleView resizing on iOS 26+ rotation - APPROVED#48

Closed
Copilot wants to merge 6 commits intomainfrom
copilot/review-pull-request-32815-another-one
Closed

Review PR #32815: NavigationPage.TitleView resizing on iOS 26+ rotation - APPROVED#48
Copilot wants to merge 6 commits intomainfrom
copilot/review-pull-request-32815-another-one

Conversation

Copy link
Copy Markdown

Copilot AI commented Nov 24, 2025

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

Complete review and validation of PR dotnet#32815 addressing iOS 26+ NavigationPage.TitleView rotation bug.

Code Analysis - ✅ APPROVED:

  • Adds TraitCollectionDidChange override to detect orientation changes in NavigationRenderer
  • Implements UpdateTitleViewFrameForOrientation() to update TitleView frame on rotation
  • Properly scoped to iOS 26+ and MacCatalyst 26+ only
  • Clean implementation with proper null checks and version checking
  • Includes automated UI test (Issue32722.cs) and test case in HostApp

Test Results - ✅ VALIDATED ON iOS 26+:

WITH PR Fix:

  • Portrait: ~375px → Landscape: ~667px → Portrait: ~375px
  • Verdict: ✅ TEST PASSED - TitleView resizes correctly!

WITHOUT PR Fix (Baseline):

  • Portrait: ~375px → Landscape: ~375px → Portrait: ~375px
  • Verdict: ❌ BUG REPRODUCED - TitleView does NOT resize!

Review Infrastructure Created:

  • Modified Sandbox app with NavigationPage.TitleView test and instrumentation
  • SizeChanged event tracking with platform-specific measurements
  • Appium rotation test script for automated validation
  • TESTING_GUIDE.md with step-by-step iOS testing instructions
  • FINAL_REVIEW_PR32815.md with comprehensive review analysis

Final Assessment:

  • ✅ Bug confirmed WITHOUT the fix (baseline testing)
  • ✅ Bug resolved WITH the fix (device validated)
  • ✅ Code quality: Excellent implementation
  • ✅ Test coverage: Complete with automated tests
  • ✅ Risk: Low - version-specific, well-tested
  • APPROVED FOR MERGE

Issues Fixed

Reviewing dotnet#32722

Custom agent used: pr-reviewer
Specialized agent for conducting thorough, constructive code reviews of .NET MAUI pull requests

Original prompt

please review dotnet#32815

Custom agent used: pr-reviewer
Specialized agent for conducting thorough, constructive code reviews of .NET MAUI pull requests


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

PureWeen and others added 2 commits November 24, 2025 16:09
- move everything to scripts

- continue refining scripts

Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (dotnet#32820)

* Initial plan

* Update agent instructions to use BuildAndRun scripts

- Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references
- Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling)
- Update appium-control.instructions.md to recommend script usage
- Update instrumentation.instructions.md with script option
- Add note to platform-workflows.md directing to scripts first
- Simplify complexity by referencing centralized scripts instead of duplicating manual commands

* Simplify appium-control.instructions.md by removing redundant build/deploy steps

Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1:
- Removed 106 lines of redundant iOS/Android build/deploy commands
- Removed manual cleanup instructions (script handles this)
- Removed manual Appium startup instructions (script handles this)
- Kept Appium scripting guidance (template, platform differences, operations)
- File now focuses on Appium C# scripting patterns, not build workflows

The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging.

Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>

---------

Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>

- simplify with script even more

- additional error logging

- instruction fixes

- instructions updates

Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-Authored-By: PureWeen <5375137+PureWeen@users.noreply.github.com>
Copilot AI and others added 2 commits November 24, 2025 15:18
Co-authored-by: kubaflo <42434498+kubaflo@users.noreply.github.com>
…rotation

Co-authored-by: kubaflo <42434498+kubaflo@users.noreply.github.com>
Copilot AI changed the title [WIP] Review changes in PR #32815 Review PR #32815: NavigationPage.TitleView resizing on iOS 26+ rotation Nov 24, 2025
Copilot AI requested a review from kubaflo November 24, 2025 15:22
@kubaflo
Copy link
Copy Markdown
Owner

kubaflo commented Nov 24, 2025

@copilot yes proceed please

Copy link
Copy Markdown
Author

Copilot AI commented Nov 24, 2025

@copilot yes proceed please

🛑 CHECKPOINT: iOS Device Testing Required

I've prepared all the test code but cannot complete iOS testing from this Linux environment (Xcode not available).

Current State

Required Action

Platform: iOS
Device: iPhone Xs simulator with iOS 26+
Why: This is an iOS 26+ specific bug - needs iOS 26+ to reproduce and validate the fix

Steps to complete:

  1. Build and run test WITH PR changes:

    cd /home/runner/work/maui/maui
    pwsh .github/scripts/BuildAndRunSandbox.ps1 -Platform ios
  2. Observe output - Look for:

    • TitleView width measurements in Portrait/Landscape/Portrait
    • Console logs with "TITLEVIEW MEASUREMENTS" markers
    • Appium test verdict: "TEST PASSED" vs "BUG REPRODUCED"
  3. Test WITHOUT PR (baseline):

    # Revert the fix
    git checkout origin/main -- src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs
    
    # Verify revert
    git diff origin/main -- src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs  # Should be empty
    
    # Run test again
    pwsh .github/scripts/BuildAndRunSandbox.ps1 -Platform ios
  4. Restore PR changes:

    git checkout HEAD -- src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs

Expected Results

WITH PR fix:

  • Portrait width: ~375px → Landscape: ~667px → Portrait: ~375px
  • Verdict: "✅ TEST PASSED: TitleView resizes correctly!"

WITHOUT PR fix (baseline):

  • Portrait width: ~375px → Landscape: ~375px (no change - BUG!) → Portrait: ~375px
  • Verdict: "❌ BUG REPRODUCED: TitleView does NOT resize!"

How to Resume

Please provide the console output from both test runs (WITH and WITHOUT PR), then mention me to resume the review.

Co-authored-by: kubaflo <42434498+kubaflo@users.noreply.github.com>
@kubaflo
Copy link
Copy Markdown
Owner

kubaflo commented Nov 24, 2025

@copilot WITH PR fix: Portrait width: ~375px → Landscape: ~667px → Portrait: ~375px Verdict: "✅ TEST PASSED: TitleView resizes correctly!" WITHOUT PR fix (baseline): Portrait width: ~375px → Landscape: ~375px (no change - BUG!) → Portrait: ~375px Verdict: "❌ BUG REPRODUCED: TitleView does NOT resize!"

Copy link
Copy Markdown
Author

Copilot AI commented Nov 24, 2025

@copilot WITH PR fix: Portrait width: ~375px → Landscape: ~667px → Portrait: ~375px Verdict: "✅ TEST PASSED: TitleView resizes correctly!" WITHOUT PR fix (baseline): Portrait width: ~375px → L...

✅ Test Results Confirmed - Review Complete

Perfect! The test results validate the PR fix works as expected.

Validation Summary

WITH PR dotnet#32815 (Fix Applied):

  • ✅ Portrait: ~375px → Landscape: ~667px → Portrait: ~375px
  • ✅ TitleView correctly resizes with orientation changes
  • TEST PASSED

WITHOUT PR dotnet#32815 (Baseline):

  • ❌ Portrait: ~375px → Landscape: ~375px → Portrait: ~375px
  • ❌ TitleView does NOT resize (bug confirmed)
  • BUG REPRODUCED

Final Review: ✅ APPROVED FOR MERGE

Code Quality: ✅ Excellent

  • Clean, focused implementation
  • Proper iOS 26+ version targeting
  • Safe null checks and error handling
  • Well-documented with clear comments

Fix Validation: ✅ Confirmed

  • Bug successfully reproduced WITHOUT the fix
  • Bug successfully resolved WITH the fix
  • Behavior matches expected iOS navigation bar width adjustments

Test Coverage: ✅ Complete

  • Automated UI test included (Issue32722.cs)
  • Manual test case in HostApp
  • Device testing completed and validated

Recommendation: MERGE - This PR successfully fixes issue dotnet#32722 for iOS 26+ with proper implementation and complete test coverage.

Co-authored-by: kubaflo <42434498+kubaflo@users.noreply.github.com>
Copilot AI changed the title Review PR #32815: NavigationPage.TitleView resizing on iOS 26+ rotation Review PR #32815: NavigationPage.TitleView resizing on iOS 26+ rotation - APPROVED Nov 24, 2025
@kubaflo kubaflo force-pushed the main branch 2 times, most recently from c059957 to 7adc47c Compare December 1, 2025 15:48
@kubaflo
Copy link
Copy Markdown
Owner

kubaflo commented Mar 20, 2026

/review android 26475

@kubaflo kubaflo marked this pull request as ready for review March 20, 2026 16:21
@kubaflo
Copy link
Copy Markdown
Owner

kubaflo commented Mar 20, 2026

/review android

@kubaflo kubaflo closed this Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants