Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
32 changes: 26 additions & 6 deletions src/Controls/src/Core/ShellToolbar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,27 +169,47 @@ internal void UpdateTitle()
Shell.TitleViewProperty,
Shell.GetTitleView(_shell));

var title = GetCurrentTitle();
if (!IsShellTitleSetByUser())
_shell.SetValueFromRenderer(Shell.TitleProperty, title);

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.

[major] Logic and Correctness — This still treats an explicit TwoWay binding on Shell.Title as handler-owned state and overwrites it with the current page title. BindableObject.SetBinding stores TwoWay bindings with SetterSpecificity.FromHandler, so IsShellTitleSetByUser() returns false for <Shell Title="{Binding AppTitle, Mode=TwoWay}">; when a TitleView is applied or the page title changes, this line pushes the page title into Shell.Title and back into the view model instead of preserving the app's binding. Please distinguish an actual binding in the property context from renderer-generated title values before calling SetValueFromRenderer.


if (TitleView != null)
{
Title = String.Empty;
return;
}

Title = title;
}

string GetCurrentTitle()
{
Page? currentPage = _shell.GetCurrentShellPage();
if (currentPage?.IsSet(Page.TitleProperty) == true)
{
Title = currentPage.Title ?? String.Empty;
return currentPage.Title ?? String.Empty;
}
// We only want to use the ShellContent as a title if no pages have been
// Pushed onto the stack
else if (_shell.Navigation?.NavigationStack?.Count <= 1)
{
Title = _shell.CurrentContent?.Title ?? String.Empty;
}
else
{
Title = String.Empty;
return _shell.CurrentContent?.Title ?? String.Empty;
}

return String.Empty;
}

bool IsShellTitleSetByUser()
{
var titleContext = _shell.GetContext(Shell.TitleProperty);
if (titleContext == null)
return false;

if (titleContext.Bindings.Count > 0)
return true;

var specificity = titleContext.Values.GetSpecificity();
return specificity != SetterSpecificity.DefaultValue && specificity != SetterSpecificity.FromHandler;

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.

[major] Navigation & Shell — This treats SetterSpecificity.FromHandler as toolbar-owned, but explicit TwoWay bindings on Shell.Title are also registered with FromHandler (BindableObject.SetBinding). When a user binds Shell.Title TwoWay and a TitleView is active, UpdateTitle() can overwrite the bound view-model value with the current page title. Please treat an existing binding on Shell.TitleProperty as user intent before falling back to the specificity check.

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 fix it?

}
}
}
58 changes: 58 additions & 0 deletions src/Controls/tests/Core.UnitTests/ShellToolbarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,64 @@ public async Task TitleAndTitleViewAreMutuallyExclusive()
Assert.Equal("Test Title", toolbar.Title);
}

[Fact]
public void ShellTitleReflectsCurrentPageTitleForTitleViewBindings()
{
var contentPage = new ContentPage() { Title = "Test Title" };
var label = new Label();
var titleView = new VerticalStackLayout()
{
Children =
{
label
}
};

TestShell testShell = new TestShell(contentPage);
_ = new Window()
{
Page = testShell
};

label.SetBinding(Label.TextProperty, new Binding(nameof(Shell.Title), source: testShell));
Shell.SetTitleView(contentPage, titleView);

Assert.Empty(testShell.Toolbar.Title);
Assert.Equal("Test Title", testShell.Title);
Assert.Equal("Test Title", label.Text);

contentPage.Title = "Updated Test Title";

Assert.Equal("Updated Test Title", testShell.Title);
Assert.Equal("Updated Test Title", label.Text);
}

[Fact]
public void ShellTitleBindingIsNotOverwrittenByCurrentPageTitle()
{
var contentPage = new ContentPage() { Title = "Page Title" };
var titleView = new VerticalStackLayout();
var viewModel = new TestShellViewModel() { Text = "App Title" };

TestShell testShell = new TestShell(contentPage);
_ = new Window()
{
Page = testShell
};

testShell.SetBinding(Shell.TitleProperty, new Binding(nameof(TestShellViewModel.Text), BindingMode.TwoWay, source: viewModel));
Shell.SetTitleView(contentPage, titleView);

Assert.Empty(testShell.Toolbar.Title);
Assert.Equal("App Title", testShell.Title);
Assert.Equal("App Title", viewModel.Text);

contentPage.Title = "Updated Page Title";

Assert.Equal("App Title", testShell.Title);
Assert.Equal("App Title", viewModel.Text);
}

[Fact]
public void ContentPageColorsPropagateToShellToolbar()
{
Expand Down
Loading