[iOS] Fix ShellFeatureMatrix test failures on candidate branch

[Vignesh-SF3580]

Root Cause

SetHeaderContentInset() in ShellFlyoutLayoutManager.cs contains an offset compensation block that executes for all FlyoutHeaderBehavior values. This logic was originally intended for Scroll and CollapseOnScroll, where the inset changes dynamically during scrolling.
PR #34936 introduced a frame-based layout for Default and Fixed, but the compensation logic was not excluded for those modes. As a result, it still computes a non-zero offset adjustment based on the inset change and incorrectly shifts the content upward.

Regression PR: #34936

Test name:

VerifyShellFlyout_HeightAndWidthWithBackgroundColor
VerifyShellFlyout_BackgroundColorWithHeaderAndFooter
VerifyShellFlyout_BackgroundImageWithHeaderAndFooter
VerifyShellFlyout_DisplayOptionsWithHeaderAndFooter, VerifyShellFlyout_DisplayOptionsWithHeaderTemplateAndFooterTemplate

How the Regression Occurred

PR #34936 updated SetHeaderContentInset() to set ContentInset.Top = 0 for Default and Fixed behaviors, since the scroll frame now starts physically below the header. However, the existing offset compensation block later in the same method continued to run for all behaviors.
When a header is applied at runtime, this block reads the previous inset value, calculates the difference from the new inset (0), and applies that delta to ContentOffset.Y. This unintentionally scrolls the item list upward, causing the first item to disappear behind the header.

Description of Changes

After setting ContentInset to zero:

• Added UpdateVerticalScrollMode() to apply the current scroll mode before exiting the method.
• Added an early return to prevent execution of the offset compensation block for Default and Fixed behaviors.

No other logic was modified. The Scroll and CollapseOnScroll path, including the compensation block itself, remains unchanged from PR #34936, preserving the intended collapsing-header behavior.

Test result

Before Issue Fix	After Issue Fix

[MauiBot]

🤖 AI Summary

👋 @Vignesh-SF3580 — new AI review results are available. Please review the latest session below.

📊 Review Session — 4c90d08 · Update ShellFlyoutLayoutManager.cs · 2026-05-08 10:21 UTC

---

🚦 Gate — Test Before & After Fix

---

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

🧪 UI Tests — Category Detection

---

Full UI test matrix will run (no specific categories detected from PR changes).

---

🔍 Regression Cross-Reference

---

🔍 Regression Cross-Reference

🟢 No regression risks detected. No labeled bug-fix PRs in the last 6 months touched the modified files.

---

🔍 Pre-Flight — Context & Validation

---

Phase 1 — Pre-Flight: PR #35346

PR Summary

• Title: [iOS] Fix ShellFeatureMatrix test failures on candidate branch
• Author: Vignesh-SF3580 (community ✨ / partner/syncfusion)
• Base branch: inflight/candidate
• Head branch: fix-34936TestFailures
• State: OPEN
• Files changed: 1 file, +6/-2
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellFlyoutLayoutManager.cs
• Labels: platform/ios, area-testing, community, partner/syncfusion, shell-flyout
• Platform: iOS only

Bug / Regression

PR #34936 introduced a frame-based layout for FlyoutHeaderBehavior.Default and Fixed (scroll view positioned physically below the header, content inset top set to 0). However, the existing offset-compensation block in SetHeaderContentInset() continued to run for ALL behaviors. That block reads the previous ContentInset.Top, computes the delta vs. the new inset (now 0), and applies that delta to ContentOffset.Y. When a header is added at runtime, the content gets scrolled upward by the magnitude of the previous inset, hiding the first flyout item behind the header.

Affected tests (UITests):

• VerifyShellFlyout_HeightAndWidthWithBackgroundColor
• VerifyShellFlyout_BackgroundColorWithHeaderAndFooter
• VerifyShellFlyout_BackgroundImageWithHeaderAndFooter
• VerifyShellFlyout_DisplayOptionsWithHeaderAndFooter
• VerifyShellFlyout_DisplayOptionsWithHeaderTemplateAndFooterTemplate

Regression PR: #34936

The Fix (PR #35346)

Inside SetHeaderContentInset(), after setting ContentInset = (0,0,0,0) for Default/Fixed:

1. Call UpdateVerticalScrollMode() to preserve scroll-mode application.
2. return; early to skip the offset-compensation block (which is only correct for Scroll/CollapseOnScroll where the inset varies dynamically).

if (headerBehavior == FlyoutHeaderBehavior.Default || headerBehavior == FlyoutHeaderBehavior.Fixed)
{
    ScrollView.ContentInset = new UIEdgeInsets(0, 0, 0, 0);
    UpdateVerticalScrollMode();
    return;
}

File Classification

File	Area	Risk
ShellFlyoutLayoutManager.cs	Shell iOS handler / Compatibility layer	Low — change is localized to one branch (Default/Fixed) of one method, behind an existing flyout-header behavior conditional.

Code Review Summary (Independence-First)

Findings

• ✅ Root cause is correctly identified. The compensation block at lines 247–251 (offset -= ScrollView.ContentInset.Top; ... ContentOffset = ... + offset;) is only meaningful when the inset varies across calls (Scroll / CollapseOnScroll). For Default/Fixed where the inset is now constantly 0 (post-Fix Shell flyout items scrolling behind FlyoutHeader on iOS #34936), running that block applies a phantom upward scroll equal to the old inset.
• ✅ Early return is the cleanest gate. Restructuring with nested else's would be more invasive; the early return matches the pattern the method already uses (line 213 return; when ScrollView is null).
• ✅ Comment is clear and explains both what and why.
• ✅ Preserves UpdateVerticalScrollMode() invocation, which the original fall-through path called at line 253. Without this explicit call, vertical scroll mode would not be applied for Default/Fixed.

Failure-mode probes (advisory for try-fix)

1. Switching FlyoutHeaderBehavior dynamically (e.g., Scroll → Fixed): the early return skips the compensation. Since the new mode is Fixed, ContentInset.Top is now 0 and the previously-applied offset compensation from the Scroll path no longer needs to be undone — Default/Fixed uses a frame-based layout and does not rely on ContentOffset for header positioning. Acceptable.
2. HeaderView == null path (line 242 else branch): unchanged. Still applies safe-area inset and runs compensation. No regression.
3. HeaderView != null but MeasuredHeaderViewHeightWithNoMargin == NaN: returns at line 221. No change.
4. No header behavior change (Default at startup): inset goes 0→0, delta is 0, compensation is a no-op. Returning early is still correct and slightly cheaper.

Blast radius

• Limited to the iOS Shell flyout's SetHeaderContentInset() method when FlyoutHeaderBehavior is Default or Fixed.
• Does NOT affect Android, Windows, MacCatalyst.
• Does NOT affect Scroll / CollapseOnScroll behaviors.

Verdict

LGTM (confidence: high). The fix is minimal, targeted, and addresses the root cause introduced by #34936. The early return + explicit UpdateVerticalScrollMode() is the correct minimum-risk pattern.

Concerns / Suggestions

• ⚠️ Minor: the original method ended with a single UpdateVerticalScrollMode() call. Now that call is duplicated (once in the early-return branch, once at the original tail). Acceptable — alternative would be a try/finally-style restructure that adds risk.
• 💡 Optional: a UI test specifically guarding against the "first item hidden behind header" regression would future-proof against a similar Fix Shell flyout items scrolling behind FlyoutHeader on iOS #34936-style regression. The PR does not add a new test (gate phase already noted this).

Code Review Hints for Try-Fix (Phase 2)

• Errors: None.
• Failure modes: First flyout item hidden behind header when HeaderBehavior is Default/Fixed and a header is applied at runtime; root cause is the post-inset offset-compensation block running unconditionally.
• Blast radius: iOS Shell flyout, SetHeaderContentInset() for Default/Fixed only.
• Verdict: LGTM (high confidence).

---

🔧 Fix — Analysis & Comparison

---

Phase 2 — Try-Fix Aggregate Summary

Four independent alternative fixes were generated for PR #35346, each from a different MAUI-expert dimension. Each candidate was applied against the broken baseline (PR's diff reverted) and built.

Fix Candidates

#	Source	Model	Approach	Build	Files Changed	Notes
PR	PR #35346	n/a	Early return; + UpdateVerticalScrollMode() inside Default/Fixed branch	n/a (already merged into branch)	1 (+6/-2)	Smallest, most surgical
pr-plus-reviewer	maui-expert-reviewer	n/a	(no actionable findings → identical to PR)	n/a	0 changes vs PR	LGTM
1	try-fix-1	claude-opus-4.6	Restructure: move compensation block INSIDE Scroll/CollapseOnScroll and no-header branches; eliminate the unconditional tail block	✅ PASS	1 (+10/-3 vs broken)	Slightly more LOC; impossible for any future code to accidentally compensate Default/Fixed
2	try-fix-2	claude-sonnet-4.6	Tail guard if (newInsetTop > 0) — flow continues to end, compensation skipped when inset is zero	✅ PASS	1	Report mentions a LayoutContent change that is not actually in PR #35346; treat with caution
3	try-fix-3	gpt-5.3-codex	Behavior-anchored: snapshot ContentOffset.Y at top, for Default/Fixed restore it verbatim at the tail; non-frame modes keep delta-compensation	❌ FAIL (env: wrong project path Controls-net10.csproj vs Core/Controls.Core.csproj)	1	Code-level approach is sound; build failure is environmental
4	try-fix-4	gpt-5.4	Single-pass plan: compute newInsetTop once, set inset once, gate the offset mutation with shouldCompensateOffset flag; also touches LayoutContent to use HeaderView.Frame.Height for Default/Fixed	❌ FAIL (env: NETSDK1005 missing net10.0-ios in project.assets.json)	1	More invasive — touches two methods; build failure environmental

Cross-Pollination

Skipped — the bug has a small, well-contained surface area (one method branch, ~8 lines) and four diverse independent ideas were already explored:

• Early return (PR)
• Scoped per-branch (try-fix-1)
• Post-condition tail guard (try-fix-2)
• Snapshot/restore of ContentOffset (try-fix-3)
• Single-pass plan with explicit flag + LayoutContent change (try-fix-4)

These exhaust the meaningful design axes (control flow exit, branch scoping, post-condition, state snapshot, plan-then-apply). Additional rounds would yield combinations rather than novel approaches.

Exhausted: Yes

Selected Fix

Selected: PR's existing fix (pr).

Reason:

• Smallest diff (+6/-2 in a single method), minimal blast radius.
• Addresses the root cause directly at the decision point.
• Expert reviewer (maui-expert-reviewer) verdict: LGTM (high confidence) with no actionable findings.
• The two build-PASSING alternatives (try-fix-1 and try-fix-2) are both functionally equivalent but each adds meaningful complexity (try-fix-1: more LOC and shared code duplication; try-fix-2: introduces a guard whose semantics are slightly less clear than an explicit early-return).
• The two build-FAILING alternatives (try-fix-3 and try-fix-4) are blocked by environmental issues, not by code correctness, but per ranking rules failed candidates rank lower than passing ones — and even ignoring the build state, both touch more surface area than the PR.

Per ranking policy: Candidates that failed regression tests rank lower than those that passed. Since gate is SKIPPED (no tests added), we use build success as the proxy:

1. pr / pr-plus-reviewer — implicit PASS (PR is the baseline)
2. try-fix-1 — build PASS
3. try-fix-2 — build PASS
4. try-fix-3 — build FAIL (env)
5. try-fix-4 — build FAIL (env)

---

📋 Report — Final Recommendation

---

Phase 3 — Comparative Report: PR #35346

TL;DR

The PR's existing fix is the winner. It is the smallest, most surgical change that addresses the root cause of the regression introduced by PR #34936. The expert reviewer found no actionable issues. Two alternative approaches build cleanly but add complexity for no functional benefit; two more failed to build due to environment issues unrelated to the fix logic.

Recommendation

APPROVE / LGTM — accept the PR as submitted. The author should consider (optional, non-blocking) adding a UI test to lock in the fix and prevent future #34936-style regressions, but this is not a merge-blocker.

Comparative Analysis

Candidate matrix

Candidate	Mechanism	Files	Build	Equivalence to PR	Risk vs PR
pr ⭐	Early return in Default/Fixed branch + explicit UpdateVerticalScrollMode() call	1 method, 1 branch	✅ (baseline)	—	Baseline
pr-plus-reviewer	Identical to pr (no actionable expert findings)	0 deltas vs PR	✅	Identical	None
try-fix-1	Restructure: compensation moved into per-branch scopes	1 method, restructured	✅ PASS	Functionally equivalent	+complexity (more LOC, slight code duplication)
try-fix-2	Post-condition guard if (newInsetTop > 0) at tail	1 method, single guard	✅ PASS	Functionally equivalent	Less self-evident invariant; report contains an inaccurate LayoutContent claim
try-fix-3	Snapshot/restore ContentOffset.Y for Default/Fixed	1 method	❌ FAIL (env)	Plausibly equivalent	Cannot verify
try-fix-4	Single-pass plan + shouldCompensateOffset flag + LayoutContent edit	2 methods	❌ FAIL (env)	More invasive; touches LayoutContent unnecessarily	Higher — broader blast radius

Why PR wins on each dimension

Correctness: Root cause is the unconditional offset-compensation block running for Default/Fixed (where the new inset is constantly 0 post-#34936). The PR gates this at the decision point. All passing alternatives gate at different points but achieve the same outcome.

Simplicity: The PR adds 6 lines (4 of comment + UpdateVerticalScrollMode(); return;). try-fix-1 adds ~10 lines and duplicates the offset-snapshot variable declaration. try-fix-2 introduces a new variable and a guard whose threshold semantics (> 0) require the reader to also know that Math.Max(HeaderMinimumHeight, ...) cannot be negative.

Defensive posture: try-fix-1 is marginally more "future-proof" (any new FlyoutHeaderBehavior enum value would default to the correct behavior of skipping compensation, since the per-branch logic is opt-in). The PR is the inverse: a new enum value would fall through to the compensation block. However, given that FlyoutHeaderBehavior enum changes are rare and reviewed, this benefit does not justify the added complexity.

Reviewer confidence: maui-expert-reviewer returned LGTM with high confidence on the PR. No inline findings (inline-findings.json = []).

Test coverage gap: Gate phase noted no tests were added in this PR. The PR description references existing failing ShellFeatureMatrix UI tests (VerifyShellFlyout_*) that this fix is intended to make pass. A regression test specifically for the "first item hidden behind header" scenario would be valuable but is not blocking — the existing UITest matrix covers the visual outcome.

Risks of NOT picking the alternatives

• try-fix-1: Adds shared code duplication; harder to maintain if the compensation formula needs to change in the future.
• try-fix-2: The > 0 guard's correctness depends on a non-obvious post-condition; less self-documenting than the PR's explicit early-return + comment.
• try-fix-3 / try-fix-4: Cannot empirically verify due to environment build issues; more invasive surface area.

Issues to flag (non-blocking, optional)

1. 💡 Consider adding a UITest specifically asserting that the first flyout item is visible (not occluded by the header) when FlyoutHeaderBehavior is Default or Fixed. This would prevent another Fix Shell flyout items scrolling behind FlyoutHeader on iOS #34936-style regression.
2. ℹ️ The PR currently calls UpdateVerticalScrollMode() twice across paths (once in the early-return branch, once at the original tail). This is acceptable; a try/finally-style restructure would add risk for negligible benefit.

Final Verdict

✅ LGTM — approve the PR as submitted.

• Winner: pr
• Confidence: high
• No code changes required
• Suggested follow-up: regression test (non-blocking)

---