Skip to content

[Mac] Fixed the Issue with the TitleBar TrailingContent not being properly aligned - #36541

Merged
kubaflo merged 2 commits into
dotnet:inflight/currentfrom
Ahamed-Ali:fix-29516Titlebar
Sep 13, 2026
Merged

kubaflo merged 2 commits into
dotnet:inflight/currentfrom
Ahamed-Ali:fix-29516Titlebar

Conversation

@Ahamed-Ali

Copy link
Copy Markdown
Contributor

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 of the issue

  • TitleBar.TrailingContent contains a HorizontalStackLayout, which includes a Border. The Border itself contains a StackLayout with content. For layouts used within the Window.TitleBar, setting IgnoreSafeArea = true is essential to ensure proper rendering within the window bounds. However, when a Border (or any IContentView) containing a layout is added, this setting is not applied recursively. As a result, the inner layout does not ignore the safe area, leading to rendering issues.

Description of Change

  • To resolve this, the IgnoreLayoutSafeArea extension method was updated to recursively apply IgnoreSafeArea = true not only to direct child layouts, but also to layouts contained within IContentView instances.

Issues Fixed

Fixes #29516

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Screenshot

Before Issue Fix After Issue Fix
TitleBarIssue.mov
TitleBarfix.mov

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36541

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36541"

@Ahamed-Ali Ahamed-Ali added platform/macos macOS / Mac Catalyst area-controls-titlebar community ✨ Community Contribution labels Jul 13, 2026
@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jul 13, 2026
@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 13, 2026
@sheiksyedm
sheiksyedm marked this pull request as ready for review July 13, 2026 09:18
Copilot AI review requested due to automatic review settings July 13, 2026 09:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a macOS/MacCatalyst TitleBar rendering issue where TrailingContent becomes misaligned when it contains an IContentView (e.g., Border) whose Content is a layout, by expanding the safe-area ignore logic and adding a UI test coverage scenario.

Changes:

  • Update LayoutExtensions.IgnoreLayoutSafeArea to recurse into layouts hosted inside IContentView.Content.
  • Add a HostApp repro page (Issue29516) that sets Window.TitleBar and places a layout inside a Border in TrailingContent.
  • Add an Appium screenshot test (Issue29516) validating correct TitleBar rendering (including the TitleBar region).

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.

File Description
src/Controls/src/Core/Layout/LayoutExtensions.cs Expands safe-area ignore recursion to reach layout content inside IContentView.
src/Controls/tests/TestCases.HostApp/Issues/Issue29516.cs Adds a TitleBar repro with TrailingContent containing a Border wrapping a layout.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29516.cs Adds a screenshot-based UI test to validate TitleBar trailing content alignment.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 1 findings

See inline comments for details.

@MauiBot MauiBot added s/agent-gate-failed AI could not verify tests catch the bug s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jul 13, 2026
MauiBot

This comment was marked as outdated.

@kubaflo

kubaflo commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added s/agent-review-in-progress AI review is currently running for this PR and removed s/agent-review-in-progress AI review is currently running for this PR labels Jul 14, 2026

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 3 findings

See inline comments for details.

MauiBot

This comment was marked as outdated.

@MauiBot MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 14, 2026

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the ai's suggestions?

@kubaflo

kubaflo commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

/review -b improved-reviewer -p catalyst

1 similar comment
@kubaflo

This comment has been minimized.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 8 findings

See inline comments for details.

{
IgnoreLayoutSafeArea(childLayout);
}
else if (child is IContentView contentView)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

❌ Error — [critical] Logic and Correctness / Layout Safe Area: the walk still only re-enters through Layout, so the traversal terminates at the first non-Layout container in a chain.

Concrete failing scenarios:

  1. TrailingContent = new Border { Content = new Border { Content = new StackLayout {...} } } — the outer Border's Content is a Border, not a Layout, so neither branch matches and the inner StackLayout never gets IgnoreSafeArea. Same for ContentView → Border → Grid, Frame → ScrollView → VerticalStackLayout, etc.
  2. More importantly, the three entry points are (newValue as Layout)?.IgnoreLayoutSafeArea() (src/Controls/src/Core/TitleBar/TitleBar.cs:145, :210, :235). If LeadingContent/Content/TrailingContent is set to a Border, ContentView, or ScrollView directly — a very common TitleBar shape, and arguably the shape the reported issue describes — the extension is never invoked at all and this PR changes nothing for it.

The fix only covers the exact one-level Layout → IContentView → Layout shape used by the new HostApp repro. Suggest restructuring as an IView-based recursive walk (set IgnoreSafeArea when the node is a Layout, then recurse into Layout.Children, IContentView.PresentedContent/Content, and IBorderView content regardless of whether the intermediate node is a Layout), and changing the TitleBar call sites so a non-Layout root is also traversed.

{
IgnoreLayoutSafeArea(presentedLayout);
}
else if (contentView.Content is Layout contentLayout)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

⚠️ Warning — [major] Logic and Correctness: PresentedContent and Content are treated as mutually exclusive alternatives, but they are not.

When a templated ContentView/Border has a ControlTemplate, PresentedContent is the template root, and Content is the user content hosted by the ContentPresenter. If the template root is not a Layout (e.g. the template root is a Border or a Label), PresentedContent is Layout is false, but because it is an else if the Content branch is evaluated — which is accidental and inconsistent; conversely when the template root is a Layout the user Content layout subtree is skipped entirely even though it is a distinct subtree that also needs IgnoreSafeArea.

Both references should be visited (deduplicated by reference equality), not chained with else.

{
if (contentView.PresentedContent is Layout presentedLayout)
{
IgnoreLayoutSafeArea(presentedLayout);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

💡 Suggestion — [moderate] Handler Mapper and Property Patterns / lifecycle: this is a one-shot snapshot taken at TitleBar.LeadingContent/Content/TrailingContent assignment time (TitleBar.cs:145/210/235). Descendants materialized after that assignment never receive IgnoreSafeArea.

Failing scenario: titleBar.TrailingContent = stack; then later border.Content = new StackLayout {...} or stack.Children.Add(new Grid()) — the new subtree keeps IgnoreSafeArea = false and the alignment defect reappears. This pre-exists for Layout.Children, but the new IContentView branch widens the set of shapes where the propagation silently does not apply. Consider driving this from a descendant-added hook (or applying it at layout/attach time) rather than only at property-set time.

}
}
}
} No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

💡 Suggestion — [minor] Build: the added block mixes leading spaces and tabs (\t\t else if, and \t\t\t\t{ on the inner braces), and the trailing newline at end-of-file was removed here and in both new Issue29516.cs files. dotnet format --verify-no-changes runs in CI and will flag these, turning the lane red for a reason unrelated to the fix. Please re-run dotnet format / restore the final newlines.

public void TitleBarTrailingContentShouldRenderProperly()
{
App.WaitForElement("ContentLabel");
VerifyScreenshot(includeTitleBar: true);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

❌ Error — [critical] Regression Prevention and Test Coverage: this test does not discriminate the fix, per the gate run on Mac Catalyst:

  • Without the LayoutExtensions.cs change: TitleBarTrailingContentShouldRenderProperly FAILED (expected).
  • With the change applied: FAILED on all three executions — Snapshot different than baseline: TitleBarTrailingContentShouldRenderProperly.png (size differs - baseline is 789x592 pixels, actual is 2560x1600 pixels).

Two separate problems:

  1. The baseline does not match the capture method. 789x592 is the logical-point Catalyst window size (existing snapshots/mac/*.png are 789x563 = the same window with the default cropFromTop: 29, which includeTitleBar: true sets to 0 — see UITest.cs:432). The actual capture is 2560x1600, i.e. the full physical/Retina screen. A size mismatch means the baseline was produced by a different capture path than the one CI executes, so the comparison never even reaches pixel diffing. The .png baselines must be regenerated from an actual run of this test on the CI capture path.
  2. Even once the sizes align, this assertion cannot prove the fix. A whole-window pixel diff of a page whose only body content is a wrapping paragraph label is dominated by unrelated pixels; it will fail for any incidental rendering change and passes/fails for reasons unrelated to trailing-content safe-area padding. Consider cropping to the title-bar strip (cropBottom) or asserting the trailing element's rect/position via Appium instead of a full-window snapshot.

As submitted, CI cannot be green and the change is unverified.

[Category(UITestCategories.Window)]
public void TitleBarTrailingContentShouldRenderProperly()
{
App.WaitForElement("ContentLabel");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

⚠️ Warning — [major] Regression Prevention / Gestures precondition rule: the only synchronization before the screenshot is App.WaitForElement("ContentLabel"), which waits on the page body, not on the TitleBar. The TitleBar is attached in OnAppearing (Issue29516.cs:64 in the HostApp), so the body label can be present and queryable before the title bar is attached, measured, and drawn — the screenshot can be taken mid-attach.

There is also no retryTimeout. Per the repo guidance, prefer VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2)) over relying on a single capture, and wait on an AutomationId placed on the trailing content itself so the test's precondition (the thing under test is actually on screen) is verified rather than assumed.

Title = ".NET MAUI",
BackgroundColor = Colors.Blue,
HeightRequest = 50,
Content = new SearchBar

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

⚠️ Warning — [major] Regression Prevention: the repro puts a SearchBar in TitleBar.Content, which is unrelated to the trailing-content alignment being fixed and injects platform-variable rendering (placeholder metrics, focus ring, native search-field chrome, and a blinking caret on Catalyst) into a pixel-exact snapshot baseline. This is a flakiness source that will cause unrelated future failures on this baseline. Recommend replacing it with a static Label, or omitting Content entirely so the snapshot isolates the trailing content.

Margin = new Thickness(4),
Children =
{
new Border

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

💡 Suggestion — [moderate] Regression Prevention and Test Coverage: the repro exercises exactly one shape — HorizontalStackLayout → Border → StackLayout — which is precisely the single case the new else if (child is IContentView) branch handles. There is no coverage for the adjacent shapes that the fix still leaves broken (see the LayoutExtensions.cs comment): a Border assigned directly as TrailingContent (the extension is never called, because the call site is (newValue as Layout)?), and a two-level Border → Border → Layout chain (traversal terminates at the intermediate non-Layout). Adding those two variants to this page would show whether the fix is complete or only covers the demo shape.

MauiBot

This comment was marked as outdated.

@kubaflo

This comment has been minimized.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the ai's suggestions?

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 5 findings

See inline comments for details.

{
IgnoreLayoutSafeArea(childLayout);
}
else if (child is IContentView contentView)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — The traversal still terminates on nested content-view chains. The new branch only recurses when the content view's content is a Layout; when it is another IContentView, no recursion happens and the subtree below it never gets IgnoreSafeArea = true.

Concrete failing scenario (same shape as #29516, one level deeper):

TrailingContent = new HorizontalStackLayout {
    Children = { new Border { Content = new ContentView { Content = new StackLayout {
        Children = { new Label { Text = "TrailingContent" } } } } } }
};

Border matches IContentView, but its content is a ContentView (not a Layout), so both new if/else if arms miss and the inner StackLayout keeps IgnoreSafeArea = false — the original misalignment reproduces. Border → Border → layout fails the same way.

Suggested shape: make the walk element-typed rather than layout-typed, so Layout and IContentView are handled by the same recursive step:

static void IgnoreSafeAreaForView(IView? view)
{
    if (view is Layout layout)
    {
        layout.IgnoreSafeArea = true;
        foreach (var child in layout.Children)
            IgnoreSafeAreaForView(child as IView);
    }
    else if (view is IContentView contentView)
    {
        IgnoreSafeAreaForView((contentView.PresentedContent ?? contentView.Content) as IView);
    }
}

}
else if (child is IContentView contentView)
{
if (contentView.PresentedContent is Layout presentedLayout)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Handler Mapper and Property Patterns — This walk is one-shot: IgnoreLayoutSafeArea runs only from TitleBar.OnLeadingChanged/OnContentChanged/OnTrailingContentChanged (TitleBar.cs:145,210,235) at the moment the property is assigned, and it is not re-run afterwards. Two consequences for the newly added IContentView path specifically:

  1. PresentedContent is generally not materialized yet at assignment time (the element is not parented and no ControlTemplate has been applied when a XAML/code-constructed TrailingContent is set), so the first arm silently falls through to Content — the branch the PR relies on is mostly untested in practice.
  2. Any subtree mutated after assignment — border.Content = new StackLayout(...) set later, children appended to the HorizontalStackLayout, or a ControlTemplate applied later — never receives IgnoreSafeArea = true, so the bug returns for dynamically built title-bar content.

If this walk is meant to be authoritative, it needs to re-run on descendant changes (e.g. on ChildAdded/content change of the assigned subtree), or IgnoreSafeArea needs to propagate at layout time rather than being pushed once at property-set time.

public void TitleBarTrailingContentShouldRenderProperly()
{
App.WaitForElement("ContentLabel");
VerifyScreenshot(includeTitleBar: true);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — The committed Mac baseline cannot match what this call captures, so the test fails regardless of the fix. TestCases.Mac.Tests/snapshots/mac/TitleBarTrailingContentShouldRenderProperly.png is 789x592, while the run captures 2560x1600 (the gate run failed with exactly this size mismatch, with the fix applied, in all three attempts). A size mismatch means the capture method differs from how the baseline was produced — includeTitleBar: true captures the full window/screen at device resolution, not the scaled app-view image the baseline was recorded from. The Windows baseline is a third size (1010x761), which reinforces that these were not recorded through the CI capture path.

Re-record both baselines from an actual VerifyScreenshot(includeTitleBar: true) run on the CI image before this can be considered validated; as submitted, the PR's only evidence of the fix is a test that never passes.


public override string Issue => "Issue with the TitleBar TrailingContent not being properly aligned on macOS";

[Test]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — The only coverage for this fix is a screenshot comparison, which is both currently unrunnable (see baseline size mismatch below) and non-discriminating: a screenshot diff cannot tell a reviewer why it changed, and if it ever regresses the usual remedy is re-recording the baseline, which silently erases the regression signal.

The changed code (LayoutExtensions.IgnoreLayoutSafeArea) is internal and Controls.Core.UnitTests already has IVT, so a deterministic regression test is cheap and platform-independent:

[Fact]
public void IgnoreLayoutSafeAreaRecursesThroughContentViews()
{
    var inner = new StackLayout { Children = { new Label() } };
    var root = new HorizontalStackLayout { Children = { new Border { Content = inner } } };
    root.IgnoreLayoutSafeArea();
    Assert.True(inner.IgnoreSafeArea);
}

Please add the unit test covering the exact nesting from #29516 (and, per the finding on LayoutExtensions.cs:17, a Border → ContentView → Layout case, which currently fails).

else if (child is IContentView contentView)
{
if (contentView.PresentedContent is Layout presentedLayout)
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[minor] Complexity Reduction — Mechanical cleanups on the new block: lines 20-26 are indented with a leading space followed by tabs (" \t\t\t\t{") while the rest of the file uses tabs only, and line 17 mixes spaces with tabs ("\t\t else if"). The diff also deletes the trailing newline at end of file (\ No newline at end of file), which is an unrelated change to a line the PR does not otherwise touch. Both trip dotnet format --verify-no-changes. The two new test files have the same issues (4-space indentation in TestCases.Shared.Tests/Tests/Issues/Issue29516.cs, missing EOF newline in both).

MauiBot

This comment was marked as outdated.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the ai's suggestions?

@kubaflo

This comment has been minimized.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 12 findings

See inline comments for details.

{
IgnoreLayoutSafeArea(childLayout);
}
else if (child is IContentView contentView)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness Verification — The traversal is extended one hop deep, but the three entry points that call it are still gated on Layout: TitleBar.cs:145, TitleBar.cs:210, and TitleBar.cs:235 all do (newValue as Layout)?.IgnoreLayoutSafeArea();. Concrete failing scenario: <TitleBar.TrailingContent><Border><StackLayout>… (a Border assigned directly as TrailingContent, without an outer HorizontalStackLayout) — newValue as Layout is null, the extension method is never invoked, and the nested StackLayout keeps IgnoreSafeArea = false, reproducing exactly the misalignment #29516 reports. This is the same class of bug the PR is fixing, left unfixed one level up. The repro page added in this PR happens to wrap the Border in a HorizontalStackLayout (Issue29516.cs:34), so the test cannot detect this gap. Fixing this requires either changing the callers to accept IView/IContentView, or moving the entry-point dispatch into this helper.

}
else if (child is IContentView contentView)
{
if (contentView.PresentedContent is Layout presentedLayout)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness Verification — Recursion terminates when an IContentView's content is itself an IContentView rather than a Layout. Concrete failing scenario: TrailingContent = new HorizontalStackLayout { new Border { Content = new Border { Content = new StackLayout { … } } } }. First Border: PresentedContent is a Border (not a Layout) → falls to the else if, where Content is also a Border (not a Layout) → both branches miss and the walk stops. The inner StackLayout never gets IgnoreSafeArea = true. Same failure for Border > ContentView > Layout, ContentView > Border > Layout, and Border > ScrollView > Layout (ScrollView.PresentedContent => Content, ScrollView is not a Controls.Layout). The recursion needs to descend through IContentView chains, not only IContentView → Layout.

{
IgnoreLayoutSafeArea(presentedLayout);
}
else if (contentView.Content is Layout contentLayout)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness Verification — PresentedContent is not resolved yet at the moment this code runs, so the fallback silently no-ops for templated content. ContentView.PresentedContent is ((this as IControlTemplated).TemplateRoot as IView) ?? Content (ContentView.cs:78) and TemplatedView.PresentedContent (TemplatedView.cs:159) is template-root-only; TemplateRoot is null until the ControlTemplate is applied during parenting/measure. This method is invoked synchronously from the TitleBar property-changed callback, i.e. before the assigned subtree is parented. Concrete failing scenario: a ContentView subclass with a ControlTemplate (and no plain Content) inside TrailingContent — at call time PresentedContent is null and Content is null, both branches miss, and when the template root is later materialized nothing re-runs the walk. Same applies to a nested TitleBar (TitleBar.cs:345, PresentedContent => TemplateRoot as IView with no Content fallback).

{
if (contentView.PresentedContent is Layout presentedLayout)
{
IgnoreLayoutSafeArea(presentedLayout);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — This propagation runs exactly once, at TitleBar.LeadingContent/Content/TrailingContent assignment time. Any subtree mutated afterwards is not covered: e.g. border.Content = new StackLayout{…} assigned after the TitleBar was built, or stack.Children.Add(new Border{ Content = new VerticalStackLayout{…} }). The pre-existing Layout-only walk had the same one-shot characteristic, but this change materially widens the surface (content views are commonly populated lazily / via BindingContext), so the intermittent "works on first render, broken after a content swap" failure mode becomes reachable. There is no test covering re-assignment after the title bar is set. Consider hooking Children/Content change notifications, or documenting the limitation.

else if (child is IContentView contentView)
{
if (contentView.PresentedContent is Layout presentedLayout)
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[minor] Complexity Reduction — Indentation is inconsistent with the rest of the file and will not survive dotnet format: line 17 begins with two tabs followed by four spaces (\t\t else if), and lines 20–26 each begin with a space before their tabs (\u0020\t\t\t\t{). The file otherwise uses tabs exclusively. This is a mechanical diff artifact, not a preference — it will show up as a formatting-check failure and as spurious whitespace noise in git blame for this block.


[Test]
[Category(UITestCategories.Window)]
public void TitleBarTrailingContentShouldRenderProperly()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Gestures / Regression Prevention — The test's precondition does not cover what it claims to verify. WaitForElement("ContentLabel") waits on the page body label (Issue29516.cs:16 in the HostApp), not on anything inside the TitleBar. The trailing content (Border > StackLayout > Label "TrailingContent") carries no AutomationId, so the screenshot can be taken before the title bar has laid out, and — more importantly — the test would still pass its wait even if the trailing content failed to render at all. Add an AutomationId to the trailing Label/Border and WaitForElement on it before VerifyScreenshot, so the assertion is anchored to the element the issue is about.

@@ -0,0 +1,23 @@
#if MACCATALYST || WINDOWS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency — The issue is scoped to macOS (the HostApp page declares PlatformAffected.macOS), but this #if MACCATALYST || WINDOWS also introduces a new Windows screenshot test plus a 1010x761 Windows baseline. The gate ran catalyst only, so the Windows path and its baseline are entirely unvalidated by this PR. Since the production change (LayoutExtensions.cs) is shared code that now also alters Windows TitleBar content safe-area state, either (a) validate the Windows run and keep it, or (b) scope the test to MACCATALYST and drop the unverified Windows baseline. Committing an unverified snapshot baseline adds a flake source to a required check.


if (Window is not null)
{
Window.TitleBar = _titleBar;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — Window.TitleBar is assigned in OnAppearing but never cleared, and TestCases.HostApp runs all issue pages in one long-lived app session. After this test navigates away, the blue 50px title bar with the SearchBar and trailing content stays attached to the Window and will appear in unrelated pages' screenshots on Mac Catalyst and Windows — a cross-test contamination source for every subsequent VerifyScreenshot(includeTitleBar: true) test in the same run. Add OnDisappearing that restores Window.TitleBar = null.

BackgroundColor = Colors.Red,
Children =
{
new Label { Text = "TrailingContent" }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — The reproduction subtree that the fix is supposed to correct carries no AutomationId, so nothing in the UI test can query or assert on it. Add AutomationId to this Label (and/or the parent Border) so the test can wait on it and, ideally, assert its rect is inside the title bar's 50px height — which would give a deterministic pre-fix failure instead of relying solely on a pixel snapshot.

HorizontalOptions = LayoutOptions.Fill,
VerticalOptions = LayoutOptions.Center
},
TrailingContent = new HorizontalStackLayout

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — The repro wraps the Border in a HorizontalStackLayout, which is the one shape the new code handles. It therefore cannot detect the two gaps flagged in LayoutExtensions.cs: (a) TrailingContent assigned directly to a Border/ContentView (entry points still do newValue as Layout), and (b) Border > Border > StackLayout nesting where the IContentView → Layout hop breaks. Please extend this page (or add sibling cases) to cover a direct-Border trailing content and a two-level content-view nest, so the regression test actually pins the fixed behavior rather than only the single reported arrangement.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review Summary

@Ahamed-Ali — new AI review results are available based on commit fba7afb.

Gate Failed Confidence Low Platform MacCatalyst


🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix

Gate Result: ❌ FAILED

Platform: CATALYST · Base: main · Merge base: 19a18889

🩺 Fix does not pass the tests — every test still fails after applying the fix. The PR's change does not resolve the failure(s).

Test Without Fix (expect FAIL) With Fix (expect PASS)
🖥️ Issue29516 Issue29516 ✅ FAIL — 139s ❌ FAIL — 265s
🔴 Without fix — 🖥️ Issue29516: FAIL ✅ · 139s

Error-relevant lines (filtered from the build log):

     at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
   at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
   at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 480
   at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance, Boolean includeTitleBar) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 312
   at Microsoft.Maui.TestCases.Tests.Issues.Issue29516.TitleBarTrailingContentShouldRenderProperly() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29516.cs:line 20
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
🟢 With fix — 🖥️ Issue29516: FAIL ❌ · 265s

Error-relevant lines (filtered from the build log):

     at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
   at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
   at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 480
   at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance, Boolean includeTitleBar) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 312
   at Microsoft.Maui.TestCases.Tests.Issues.Issue29516.TitleBarTrailingContentShouldRenderProperly() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29516.cs:line 20
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

⚠️ Failure Details

  • ❌ Issue29516 FAILED with fix (should pass)
    • TitleBarTrailingContentShouldRenderProperly [4 s]
    • VisualTestUtils.VisualTestFailedException : Snapshot different than baseline: TitleBarTrailingContentShouldRenderProperly.png (size differs - baseline is 789x592 pixels, actual is 2560x1600 pixels) I...
📁 Fix files reverted (1 files)
  • src/Controls/src/Core/Layout/LayoutExtensions.cs

📋 Pre-Flight — Context & Validation

PR #36541 Pre-Flight

Context

  • PR: [Mac] Fixed the Issue with the TitleBar TrailingContent not being properly aligned
  • Issue: #29516, where a Label inside Border -> StackLayout in TitleBar.TrailingContent renders outside the title bar's vertical bounds on macOS.
  • Base / merge base: main / 19a188896c0e518c0d201db181f67166c740724d
  • Review branch: pr-review-36541 at 24a1580acd78cf7b9d08009b903419f0102d1f95
  • Platform: catalyst

Existing Fix

The PR changes src/Controls/src/Core/Layout/LayoutExtensions.cs. Existing title-bar property callbacks invoke IgnoreLayoutSafeArea() only when the assigned leading, center, or trailing content is a Layout. The helper already sets IgnoreSafeArea = true recursively on nested Layout children.

The PR extends that traversal for direct IContentView children. It checks PresentedContent first and falls back to Content, recursing only when the selected value is a Layout. This is intended to reach the StackLayout nested under the reproduction's Border.

The PR also adds:

  • src/Controls/tests/TestCases.HostApp/Issues/Issue29516.cs
  • src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29516.cs
  • Mac Catalyst and Windows screenshot baselines

Validation Scope

  • Detected primary test: Issue29516
  • Command: pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform catalyst -TestFilter "Issue29516"
  • Mandatory regression tests supplied for STEP 5a: none
  • Detected categories: Layout, Window

The already-completed gate must not be rerun. It established that Issue29516 fails without the production fix and still fails with the PR fix. The observed with-fix failure is a screenshot-cropping mismatch: the expected image is 789x592, while the actual retained full-screen image is 2560x1600; the log reports that the app window starts at a negative X coordinate and cannot be cropped. This evidence prevents treating the gate result as proof that the production behavior is corrected.

Try-Fix Constraints

  • The production fix file detected by the gate is only src/Controls/src/Core/Layout/LayoutExtensions.cs.
  • Each attempt must use EstablishBrokenBaseline.ps1, honor its RevertedFiles allow-list, run only the detected primary test, perform the inline expert self-review, capture artifacts, and restore through the script.
  • Candidate 2 must avoid both the PR's direct IContentView traversal mechanism and candidate 1's recorded mechanism.
  • The worktree contains substantial pre-existing changes outside the PR fix. They are not part of this review and must remain untouched.

🔬 Code Review — Deep Analysis

Expert Code Review - PR #36541

Verdict

NEEDS_CHANGES

Confidence: low for merge safety because the trusted gate failed and the new regression test never produced a passing with-fix result. Confidence is high in the concrete traversal gaps identified from the source.

Independent Assessment

The submitted change extends IgnoreLayoutSafeArea so a layout walk can pass through one IContentView and reach a layout stored in PresentedContent or Content. This addresses the submitted HorizontalStackLayout > Border > StackLayout reproduction in principle, because the walk can now reach the layout inside the Border.

The implementation does not cover the general content-view shape it claims to support:

  • The three TitleBar property callbacks still invoke the helper only when the assigned root is a Layout. A directly assigned Border or ContentView bypasses the fix.
  • Traversal stops when an IContentView contains another IContentView; only an IContentView > Layout hop is supported.
  • Templated content can be unresolved when the one-shot property callback runs, and later content or child changes do not trigger propagation again.

Findings

  1. Error - incomplete entry-point and recursive coverage. LayoutExtensions.cs handles one IContentView hop, but direct content-view roots and nested content-view chains remain unfixed. Concrete failing shapes include TitleBar.TrailingContent = Border > StackLayout and HorizontalStackLayout > Border > Border > StackLayout.
  2. Error - regression test has no passing fixed state. The trusted gate reports that Issue29516 failed both without and with the production fix. The with-fix result was blocked by Mac Catalyst screenshot cropping (789x592 expected versus retained 2560x1600 full-screen output), so the test supplies no empirical proof of the fix.
  3. Warning - test readiness and isolation. The test waits for the page-body ContentLabel, not the title-bar content; the reproduction element has no automation identifier; Window.TitleBar is not cleared when leaving the page; and the Windows snapshot path was not covered by the catalyst gate.
  4. Warning - lifecycle coverage. Safe-area propagation is performed only when a title-bar property is assigned. Templated or subsequently replaced content can remain unprocessed.
  5. Minor - formatting. The submitted source files contain mixed indentation and missing final newlines.

The complete raw file-and-line findings are persisted in ../inline-findings.json.

Prior Review Reconciliation

The reviewer could not query the three GitHub review surfaces because gh was unauthenticated. The pre-flight artifact contained no prior error finding to reconcile, so duplication with an existing online review remains possible.

Blast Radius and Failure-Mode Probing

  • The helper has three callers, all title-bar content property callbacks. There is no public API change or new shared/static state.
  • The change widens safe-area suppression for descendants on platforms that support title bars, including an unvalidated Windows path.
  • A direct Border root bypasses the helper.
  • A second nested IContentView terminates traversal.
  • A plain Border > StackLayout beneath an outer layout is reached by the submitted change.
  • Null content is safe, but a template root or content assigned after the callback is not revisited.

CI and Gate

The trusted gate failed and was not rerun. Its screenshot-cropping failure is not evidence that the production logic is wrong, but it leaves the regression test unvalidated and prevents approval. No full-suite result is inferred.

External Output Contract

Not applicable.


🛠️ Try-Fix — Analysis & Comparison

STEP 5a Alternative Fix Candidates — PR #36541

Candidate 1 — Explicit safe-area policy across the visual subtree

Model: claude-opus-5
Detailed record: ../try-fix-1/content.md
Attempt artifacts: attempt-1/
Result: Blocked

Approach

Replace the PR's type-specific Layout/IContentView recursion with a generic
IVisualTreeElement.GetVisualChildren() traversal. For every visited element that
implements ISafeAreaElement, explicitly set
SafeAreaElement.SafeAreaEdgesProperty to SafeAreaRegions.None.

The PR assumes the nested StackLayout is the missed safe-area participant and
only sets the obsolete Layout.IgnoreSafeArea property on layouts reached
through one IContentView hop. Candidate 1 instead hypothesizes that the
intermediate Border is itself the relevant safe-area participant. A generic
visual-tree traversal would reach Border, StackLayout, and later descendants
without type-specific chain breaks, while the explicit modern property would be
consumed by Border rather than only by Layout.

Planned diff

No diff was applied because baseline establishment failed. The designed
single-file change was:

internal static void IgnoreLayoutSafeArea(this Layout layout)
	=> DisableSafeAreaForSubtree(layout);

static void DisableSafeAreaForSubtree(Element element)
{
	if (element is ISafeAreaElement)
	{
		element.SetValue(
			SafeAreaElement.SafeAreaEdgesProperty,
			new SafeAreaEdges(SafeAreaRegions.None));
	}

	foreach (var child in ((IVisualTreeElement)element).GetVisualChildren())
	{
		if (child is Element childElement)
		{
			DisableSafeAreaForSubtree(childElement);
		}
	}
}

Test result and failure analysis

The mandatory baseline command failed before creating
.github/.baseline-state.json:

ERROR: DIRTY WORKING DIRECTORY - Cannot establish baseline
EstablishBrokenBaseline.ps1 failed: Working directory is not clean.

There were 41 pre-existing modified tracked files outside this attempt. The
attempt was forbidden to alter or stash them, and the try-fix restoration
contract prohibits editing without a baseline state file. Therefore no code was
changed and the primary command
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform catalyst -TestFilter "Issue29516"
was not run. No other tests were run.

The captured diff is empty. Inline expert self-review recorded zero findings.
The required restore command returned the accepted no-state result
No baseline state found / Restored False; the target production file remained
untouched. Candidate 2 must avoid both the PR's IContentView recursion and this
candidate's visual-tree traversal plus explicit SafeAreaEdges = None mechanism.

Candidate 2 — Clip rendering at the title-bar root

Model: gpt-5.6-sol
Detailed record: ../try-fix-2/content.md
Attempt artifacts: attempt-2/
Result: Blocked

Approach

Keep the root layout's existing legacy IgnoreSafeArea = true assignment,
remove descendant recursion, and set layout.IsClippedToBounds = true on the
root layout supplied to the title bar:

internal static void IgnoreLayoutSafeArea(this Layout layout)
{
#pragma warning disable CS0618 // Type or member is obsolete
	layout.IgnoreSafeArea = true;
#pragma warning restore CS0618 // Type or member is obsolete
	layout.IsClippedToBounds = true;
}

This candidate avoids both earlier descendant-propagation hypotheses. It does
not inspect IContentView, traverse IVisualTreeElement, or modify descendant
safe-area state. Instead, it hypothesizes that Mac Catalyst allows a nested
native view to draw outside the finite title-bar content slot. Native clipping
would make the root title-bar frame authoritative so descendant pixels cannot
render beyond it, regardless of wrapper type or safe-area resolution.

Diff

No diff was applied. fix.diff and reviewer-findings.diff are empty.

Test result and failure analysis

pwsh .github/scripts/EstablishBrokenBaseline.ps1 again failed its clean-tree
guard against the same 41 protected pre-existing tracked modifications. It did
not create .github/.baseline-state.json, so there was no RevertedFiles
allow-list and editing the target file was prohibited.

The sole permitted primary command,
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform catalyst -TestFilter "Issue29516",
was not run because no candidate code could be applied. No gate verification,
regression test, or full suite was run. This result supplies no empirical

This try-fix section was shortened to keep every required review section visible. Full details remain available in the pipeline build artifacts.


📝 PR Finalize — Recommended Title & Description

Assessment: ✏️ Recommend updating — the current title is not in the repository's component-oriented format, and the description overstates the submitted implementation as general recursive IContentView handling.

Recommended title

[Mac] TitleBar: Fix nested layout alignment in TrailingContent

Recommended description

### Root Cause of the issue

`TitleBar.TrailingContent` can contain a layout nested inside an `IContentView`, such as `HorizontalStackLayout > Border > StackLayout`. Title-bar layouts must set `IgnoreSafeArea = true` to render within the window bounds, but `IgnoreLayoutSafeArea` previously traversed only child `Layout` instances. Traversal therefore stopped at the `Border`, leaving its nested `StackLayout` subject to safe-area insets and causing the trailing content to render outside the title bar's vertical bounds on macOS.

### Description of Change

Update `IgnoreLayoutSafeArea` so that, while traversing a `Layout`, it also inspects direct `IContentView` children. The helper prefers `PresentedContent`, falls back to `Content`, and continues recursively when the selected content is a `Layout`.

Add an `Issue29516` HostApp reproduction and screenshot coverage with Mac Catalyst and Windows baselines.

### Issues Fixed

Fixes #29516

### Tested the behaviour in the following platforms

- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac

### Screenshot

| Before Issue Fix | After Issue Fix |
|----------|----------|
| <video src="https://github.com/user-attachments/assets/b74c8f23-e549-419b-8ca0-d6ee563b8f09"> | <video src="https://github.com/user-attachments/assets/cfaa839b-e3fb-47a9-89f6-55935eae589e"> |

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winner: pr-plus-reviewer

The reviewer-enhanced candidate is the strongest implementation because it fixes the submitted patch's concrete root-dispatch and nested-IContentView traversal gaps while preserving the intended safe-area behavior for layouts. It is not ready to merge: its sole required regression run failed before screenshot comparison when Appium could not find the title-bar label, and the trusted gate for the raw PR also failed.

Evidence Priority

No candidate passed the required regression test, so the rule preferring passing candidates does not distinguish this set. Both executable PR-based candidates failed; both STEP 5a alternatives were blocked before producing a diff or running a test.

Rank Candidate Implementation Required validation Assessment
1 pr-plus-reviewer Extends the helper to any title-bar IView, recurses through nested IContentView chains, and hardens test isolation/targeting Failed: build passed; Appium timed out waiting for TrailingContentLabel, before screenshot comparison Best code coverage, but the regression test must be made queryable and pass before merge
2 pr Adds one IContentView > Layout traversal hop beneath an outer Layout Failed gate: the test failed without and with the fix; with-fix evidence was blocked by screenshot cropping (789x592 expected, 2560x1600 retained) Plausibly fixes the exact submitted tree, but leaves direct content-view roots and nested content-view chains unfixed
3 try-fix-1 Proposed generic visual-tree traversal plus explicit modern SafeAreaEdges = None Blocked: no diff and no test run Broader and more speculative behavior change with no empirical evidence
4 try-fix-2 Proposed clipping the root layout instead of propagating safe-area behavior Blocked: no diff and no test run Treats visible overflow rather than the safe-area propagation cause and has no empirical evidence

Expert Review Comparison

The raw PR's traversal begins only when TitleBar.LeadingContent, Content, or TrailingContent is a Layout. A directly assigned Border therefore bypasses the helper. Once invoked, the submitted recursion also stops at IContentView > IContentView. The pr-plus-reviewer candidate resolves both concrete gaps by moving dispatch to an IView helper and recursively following content-view chains.

The reviewer-enhanced candidate also fixes formatting, targets the affected title-bar label rather than the unrelated page label, and clears the test title bar on disappearance. The attempted stronger wait exposed a separate testability limitation: the title-bar label was not discoverable through Appium. Because refinement and validation are limited to one pass, that uncertainty is recorded rather than repaired.

Remaining Risks

  • Neither PR-based candidate has a passing regression result.
  • Templated content materialized after the one-shot title-bar callback and content added later remain outside the candidate's propagation model.
  • The shared production path and committed Windows baseline were not validated on Windows in this run.
  • Prior online review reconciliation was unavailable because GitHub CLI authentication was unavailable; the required raw inline findings are nevertheless persisted locally.

Required Change

Adopt the pr-plus-reviewer production changes, revise the regression test so it can deterministically observe readiness without querying an inaccessible title-bar element, and obtain a passing Mac Catalyst result. The submitted raw PR should not be approved in its current form.


📱 UI Tests — Layout,Window

Detected UI test categories: Layout,Window

❌ Deep UI tests — 11 passed, 191 failed, 4 skipped across 2 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category Tests Snapshot diffs
Layout 11/190 (175 ❌, 4 skipped) 6 diff PNGs
Window 0/16 (16 ❌) 10 diff PNGs
🔍 AI analysis of failures — PR-related vs unrelated

🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.

Likely PR-related: one or more failures appear connected to this PR's changes.

  • ✗ PR-related — Added Mac Catalyst TitleBar trailing-content visual test (1 test): TitleBarTrailingContentShouldRenderProperly and its macOS snapshot are introduced by this PR, so the snapshot-size failure is directly tied to the new test/baseline.
  • ● Unrelated — Layout-suite Appium timeouts (175 tests): the changed helper is only invoked for TitleBar content layouts, while these FlexLayout, TwoPaneView, EmptyView, BindableLayout, AbsoluteLayout, and Grid failures show generic wait/command timeout patterns with no changed-code path.
  • ● Unrelated — Existing TitleBar snapshot mismatches (~4+ tests): repeated baselines near 789x563/592 versus actual images near 2560x1571/1600 indicate a Catalyst capture-environment mismatch, and the proportional TitleBarWithSmallHeightShell case does not exercise the newly added IContentView traversal.
  • ● Unrelated — Remaining Window Appium timeouts (~3+ tests): fullscreen, modal/alert, and navigation cases fail in driver click, tap, or element-wait operations without referencing the changed traversal, matching run instability rather than this PR.

Strongest signal: only the newly added trailing-content test is directly in changed test/baseline scope; the repeated full-screen resolution mismatch and 175 broad layout timeouts point to a bad Catalyst UI-test run.

❌ Layout — 175 failed tests
FlexLayout_JustifyContentCenter
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.FlexLayoutFeatureTests.FlexLayout_JustifyContentCenter() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/FlexLayoutFeatureTests.cs:line 368
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** argume
...
TwoPaneView_TallMode
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.TwoPaneViewFeatureTests.TwoPaneView_TallMode() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/TwoPaneViewFeatureTests.cs:line 131
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, Obje
...
CEmptyViewTemplateHidesWhenItemsSourceIsFilled
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.Issues.Issue6932_emptyviewtemplate.CEmptyViewTemplateHidesWhenItemsSourceIsFilled() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/XFIssue/Issue6932_emptyviewtemplate.cs:line 48
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHan
...
VerifyBindableLayoutWithItemTemplateSelector
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.BindableLayoutFeatureTests.VerifyBindableLayoutWithItemTemplateSelector() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/BindableLayoutFeatureTests.cs:line 138
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack 
...
TwoPaneView_ZIsShadowEnabled
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.TwoPaneViewFeatureTests.TwoPaneView_ZIsShadowEnabled() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/TwoPaneViewFeatureTests.cs:line 243
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** argumen
...
TwoPaneView_Wide_UsingRect
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.TwoPaneViewFeatureTests.TwoPaneView_Wide_UsingRect() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/TwoPaneViewFeatureTests.cs:line 89
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments,
...
GridIsEnabled
System.TimeoutException : An Appium command did not complete within 45s. The application may be unresponsive (e.g., due to an infinite layout loop).
at UITest.Appium.HelperExtensions.RunWithTimeout[T](Func`1 action, Nullable`1 timeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 3056
   at UITest.Appium.HelperExtensions.RunWithTimeout(Action action, Nullable`1 timeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 3075
   at UITest.Appium.HelperExtensions.Tap(IApp app, String element) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 36
   at Microsoft.Maui.TestCases.Tests.Issues.Bugzilla46458.GridIsEnabled() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Bugzilla/Bugzilla46458.cs:line 20
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, 
...
VerifyAbsoluteLayout_WidthAndHeight
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.AbsoluteLayoutFeatureTests.VerifyAbsoluteLayout_WidthAndHeight() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/AbsoluteLayoutFeatureTests.cs:line 60
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object ob
...
VerifyGrid_SetRowSpacingAndPadding
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757

This uitests section was shortened to keep every required review section visible. Full details remain available in the pipeline build artifacts.


🧭 Next Steps — reviewer changes required

The reviewer-enhanced candidate identified changes that are not yet in the submitted PR.

Why: The reviewer-enhanced candidate fixes the raw PR's direct-root and nested-IContentView traversal gaps while preserving its intended layout safe-area behavior. It wins on implementation completeness, but its targeted test still failed before screenshot comparison, so the PR requires changes and a passing regression run.

Address the actionable findings in this review before merging.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the ai's suggestions and test failures?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-titlebar community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/macos macOS / Mac Catalyst s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-win AI found a better alternative fix than the PR s/agent-gate-failed AI could not verify tests catch the bug s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with the TitleBar TrailingContent not being properly aligned on macOS.

5 participants