Skip to content

Commit f0f83ea

Browse files
kubaflokubaflo
committed
Update PR fix test result - Gate validated it
1 parent b0e48f2 commit f0f83ea

File tree

1 file changed

+174
-0
lines changed

1 file changed

+174
-0
lines changed

‎.github/agent-pr-session/pr-28713.md‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
# PR Review: #28713 - [IOS] Inconsistent Resize Behavior for Header/Footer - fix
2+
3+
**Date:** 2026-01-14 | **Issue:** [#26397](https://github.com/dotnet/maui/issues/26397) | **PR:** [#28713](https://github.com/dotnet/maui/pull/28713)
4+
5+
## ✅ Status: GATE PASSED - Starting Fix Phase
6+
7+
| Phase | Status |
8+
|-------|--------|
9+
| Pre-Flight | ✅ COMPLETE |
10+
| 🧪 Tests | ✅ COMPLETE |
11+
| 🚦 Gate | ✅ PASSED |
12+
| 🔧 Fix | ▶️ IN PROGRESS |
13+
| 📋 Report | ⏳ PENDING |
14+
15+
---
16+
17+
<details>
18+
<summary><strong>📋 Issue Summary</strong></summary>
19+
20+
**Issue #26397: Inconsistent Resize Behavior for Header/Footer on iOS/macOS**
21+
22+
When the "Resize Header/Footer" menu is tapped in a Shell Flyout, the sizes of the header and footer change, but they do not remain consistent across multiple taps. The resizing behavior is not predictable - sizes may vary each time rather than toggling between two consistent states.
23+
24+
**Steps to Reproduce:**
25+
1. Run the sample app (HeaderFooterShellFlyoutSample.zip attached to issue)
26+
2. Open the Flyout and tap the "Resize Header/Footer" menu
27+
3. Observe that the header and footer resize
28+
4. Tap the "Resize Header/Footer" menu again
29+
5. Tap again and observe the size - it differs from the initial size
30+
31+
**Expected Behavior:**
32+
Header and footer should toggle consistently between two sizes (small and large) on each tap, matching the behavior on Android.
33+
34+
**Platforms Affected:**
35+
- [x] iOS
36+
- [ ] Android (working correctly)
37+
- [ ] Windows
38+
- [x] MacCatalyst
39+
40+
**Regression Status:** Potential regression (not verified)
41+
42+
**Version:** 9.0.12 SR1.2
43+
44+
</details>
45+
46+
<details>
47+
<summary><strong>📁 Files Changed</strong></summary>
48+
49+
| File | Type | Changes |
50+
|------|------|---------|
51+
| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellFlyoutHeaderContainer.cs` | Fix | +11 lines |
52+
| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/XFIssue/HeaderFooterShellFlyout.cs` | Test | +1/-2 lines |
53+
54+
**Fix Summary:**
55+
- **ShellFlyoutHeaderContainer.cs**: Implements `IPlatformMeasureInvalidationController` interface to properly handle measure invalidation and update frame size when view size changes
56+
- **HeaderFooterShellFlyout.cs**: Enables iOS platform for header/footer resize tests (was previously disabled for iOS)
57+
58+
</details>
59+
60+
<details>
61+
<summary><strong>💬 PR Discussion Summary</strong></summary>
62+
63+
**Key Comments:**
64+
65+
1. **PureWeen (CHANGES_REQUESTED - 2025-03-30):**
66+
- Inline comment at `ShellFlyoutContentRenderer.cs:102`: "I think we're moving away from subscribing to measure invalidated subscriptions and using the propagation method that @albyrock87 created"
67+
- Author response: "No problem"
68+
- **Note:** This suggests a change in approach was requested
69+
70+
2. **Copilot Review (COMMENTED - 2025-03-30):**
71+
- Low confidence comment: Conditional compilation now includes iOS, suggest updating comment to clarify why
72+
- Low confidence comment: Possible duplicate event subscriptions or memory leak concern with _header MeasureInvalidated event
73+
74+
3. **Author (kubaflo) responded on 2025-04-02**
75+
76+
**Reviewer Feedback:**
77+
- Review requested changes to use "propagation method" instead of MeasureInvalidated subscriptions
78+
- Potential concerns about event subscription management
79+
80+
**Disagreements to Investigate:**
81+
| File:Line | Reviewer Says | Author Says | Status |
82+
|-----------|---------------|-------------|--------|
83+
| ShellFlyoutContentRenderer.cs:102 | Use propagation method instead of MeasureInvalidated | No problem (agreed to change) | ⚠️ INVESTIGATE - File no longer in PR diff |
84+
85+
**Author Uncertainty:**
86+
- None explicitly stated
87+
88+
**Key Observation:**
89+
The PR diff currently shows changes to `ShellFlyoutHeaderContainer.cs`, but the review comments reference `ShellFlyoutContentRenderer.cs`. This suggests the implementation approach was changed after the initial review - likely following PureWeen's feedback to use the propagation method.
90+
91+
</details>
92+
93+
<details>
94+
<summary><strong>🧪 Tests</strong></summary>
95+
96+
**Status**: ✅ COMPLETE
97+
98+
- [x] PR includes UI tests (existing test modified)
99+
- [x] Tests reproduce the issue (structure verified)
100+
- [x] Tests follow naming convention (XFIssue naming used)
101+
102+
**Test Files:**
103+
- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/XFIssue/HeaderFooterShellFlyout.cs`
104+
- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/XFIssue/HeaderFooterShellFlyout.cs`
105+
106+
**Test Description:**
107+
The HostApp page creates a Shell with Flyout that has menu items to:
108+
1. Toggle header/footer views
109+
2. Toggle header/footer templates
110+
3. **Resize header/footer** - Toggles between 60px and 200px heights
111+
112+
The NUnit test verifies that:
113+
1. Headers/footers toggle correctly
114+
2. Templates take priority over views
115+
3. **Size changes are consistent** - After 3 resize taps, heights should alternate: small → large → small (matching initial)
116+
117+
**Test Change:**
118+
Enables iOS platform tests for header/footer resize verification by changing conditional compilation from `#if ANDROID` to `#if ANDROID || IOS`.
119+
120+
**Before Fix:** iOS tests were disabled with comment referencing issue #26397
121+
**After Fix:** iOS tests enabled, expecting consistent resize behavior
122+
123+
</details>
124+
125+
<details>
126+
<summary><strong>🚦 Gate - Test Verification</strong></summary>
127+
128+
**Status**: ✅ PASSED
129+
130+
- [x] Tests FAIL without fix (bug reproduced)
131+
- [x] Tests PASS with fix (bug resolved)
132+
133+
**Result:** ✅ **VERIFICATION PASSED**
134+
135+
**Verification Output:**
136+
```
137+
╔═══════════════════════════════════════════════════════════╗
138+
║ VERIFICATION PASSED ✅ ║
139+
╠═══════════════════════════════════════════════════════════╣
140+
║ Tests correctly detect the issue: ║
141+
║ - FAIL without fix (as expected) ║
142+
║ - PASS with fix (as expected) ║
143+
╚═══════════════════════════════════════════════════════════╝
144+
```
145+
146+
**Test Behavior:**
147+
- **WITHOUT fix** (reverted to `origin/main`): Tests **FAILED** ✅ (expected - bug reproduced)
148+
- **WITH fix** (PR branch with compilation fix): Tests **PASSED** ✅ (expected - fix works)
149+
150+
**Platform Tested:** iOS (iPhone Xs, iOS 18.5)
151+
152+
**Test Filter:** `HeaderFooterShellFlyout`
153+
154+
**Fix Validated:** The compilation error was fixed (changed `InvalidateMeasure` return type from `void` to `bool`), and the PR's approach successfully resolves the inconsistent resize behavior.
155+
156+
</details>
157+
158+
<details>
159+
<summary><strong>🔧 Fix Candidates</strong></summary>
160+
161+
**Status**: ⏳ PENDING
162+
163+
| # | Source | Approach | Test Result | Files Changed | Notes |
164+
|---|--------|----------|-------------|---------------|-------|
165+
| PR | PR #28713 | Implements `IPlatformMeasureInvalidationController` in `ShellFlyoutHeaderContainer` to handle measure invalidation via propagation method. When `InvalidateMeasure` is called, recalculates size with `SizeThatFits` and updates frame dimensions. | ✅ PASS (Gate) | `ShellFlyoutHeaderContainer.cs` (+11) | Original PR - validated by Gate, approach changed after review to use propagation method |
166+
167+
**Exhausted:** No
168+
**Selected Fix:** [PENDING]
169+
170+
</details>
171+
172+
---
173+
174+
**Next Step:** ✅ Gate PASSED. Read `.github/agents/pr/post-gate.md` for Phase 4-5 instructions.

0 commit comments

Comments
 (0)