Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ public partial class ShellHandler
public static PropertyMapper<Shell, ShellHandler> Mapper =
new PropertyMapper<Shell, ShellHandler>(ElementMapper)
{
#if WINDOWS || TIZEN

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚠️ Cross-Platform Behavioral Consistency — this reorder also changes Tizen's initial mapper order, and Tizen was not tested.
Flagged by: 2/3 reviewers (low risk)

On Windows the move is safe because MapItems/MapFlyoutItems both end with handler.UpdateValue(nameof(Shell.CurrentItem)), so MapCurrentItem re-runs after them and SwitchShellItem is idempotent. On Tizen, however, MapItems/MapFlyoutItems call only UpdateItems() with no CurrentItem re-trigger — so this genuinely flips execution from UpdateCurrentItem()-before-UpdateItems() (current base) to UpdateItems()-first (this PR). That order existed in neither the original nor the current base. The two paths touch independent views so the risk is low, but please either smoke-test Shell startup on Tizen or confirm the Tizen ordering change is intentional (alternatively, scope the reorder to #if WINDOWS since the bug and verification are Windows-only).

💡 Because this ordering is load-bearing (it's now the second TitleView-centering regression caused by mapper order — #34758), consider a short comment right here explaining why the Toolbar/Items block must precede CurrentItem, so the next refactor doesn't silently undo it. (1/3 reviewers — optional.)

[nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
[nameof(IFlyoutView.Flyout)] = MapFlyout,
[nameof(Shell.Items)] = MapItems,
[nameof(Shell.FlyoutItems)] = MapFlyoutItems,
[nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
#endif
Comment on lines 17 to 20

[nameof(Shell.CurrentItem)] = MapCurrentItem,
[nameof(Shell.FlyoutBackground)] = MapFlyoutBackground,
[nameof(Shell.FlyoutBackgroundColor)] = MapFlyoutBackground,
Expand All @@ -33,14 +41,6 @@ public partial class ShellHandler
[nameof(Shell.FlyoutBackgroundImageAspect)] = MapFlyoutBackgroundImage,
[nameof(Shell.FlyoutVerticalScrollMode)] = MapFlyoutVerticalScrollMode,

#if WINDOWS || TIZEN
[nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
[nameof(IFlyoutView.Flyout)] = MapFlyout,
[nameof(Shell.Items)] = MapItems,
[nameof(Shell.FlyoutItems)] = MapFlyoutItems,
[nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
#endif

#if ANDROID
[nameof(Shell.FlyoutHeight)] = MapFlyoutHeight,
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,49 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Controls.Window(shell),
});
}

[Fact(DisplayName = "Shell TitleView Is Centered On Initial Load")]
public async Task ShellTitleViewIsCenteredOnInitialLoad()
{
SetupBuilder();

var page = new ContentPage
{
Title = "Page 1",
Content = new Label { Text = "Body" }
};

var titleView = new Label
{
Text = "TitleView",
HorizontalOptions = LayoutOptions.Center
};

var shell = await CreateShellAsync(s =>
{
s.CurrentItem = page;
Shell.SetTitleView(s, titleView);
});

await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
{
await OnFrameSetToNotEmpty(titleView);
await AssertEventually(() =>
{
var toolbar = GetPlatformToolbar(handler);
var platformTitleView = GetTitleView(handler);

return toolbar?.ActualWidth > 0 && platformTitleView?.ActualWidth > 0;
});

var toolbar = GetPlatformToolbar(handler);
var platformTitleView = GetTitleView(handler);
var titleViewCenterX = platformTitleView.GetLocationRelativeTo(toolbar).Value.X + (platformTitleView.ActualWidth / 2);
var toolbarCenterX = toolbar.ActualWidth / 2;

Assert.InRange(Math.Abs(titleViewCenterX - toolbarCenterX), 0, 2);
});
Comment thread
sheiksyedm marked this conversation as resolved.
Outdated
}

static bool IsViewLaidOut(object platformView)
{
var frameworkElement = platformView as WFrameworkElement;
Expand Down
Loading