Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for setting content in place of the title in a TitleBar #1030

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

jonlipsky
Copy link

@jonlipsky jonlipsky commented Mar 31, 2024

Pull request type

Please check the type of change your PR introduces:

  • Update
  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes

What is the current behavior?

Currently there is no way to set content in a TitleBar that is left justified and/or use content other than the default TextBlock for the title content.

Issue Number: [#775] [#1025]

What is the new behavior?

This PR adds support for (optionally) specifying content to be displayed as a replacement for the TextBlock which displays the title. The following changes have been made:

  • Added a LeftHeader property that can be optionally set to specify the view to be presented in place of the default TextBlock that displays the title (when set).
  • A TextBlock is initialized in the TitleBar constructor and assigned to the LeftHeader property so that setting the title is still the default behavior.
  • Renamed Header to RightHeader to make the API more self-documenting as to the purpose of the property.
  • Updated the sandbox window example to demonstrate this new functionality.
image

Other information

  • This change is a breaking change from the previous version since Header has been renamed to RightHeader. One remedy would be to add a new property called Header, which would delegate to RightHeader and be marked as deprecated to give users time to update their code.

  • One additional enhancement that may be nice to have would be the ability to vertically align the Minimize, Maximize and Close buttons of the window. It may be more aesthetically pleasing to have them center aligned depending on the content in the RightHeader. As an example, JetBrains Rider does this in their custom titlebar:

image

@github-actions github-actions bot added controls Changes to the appearance or logic of custom controls. styles Topic is related to styles PR Pull request gallery WPF UI Gallery dotnet labels Mar 31, 2024
@537mfb
Copy link

537mfb commented Apr 1, 2024

To prevent backward compatibility issues you could have kept Header and just have added LeftHeader.

Once there's a major version change that can break backwards compatibility then a name change from Header to RightHeader could be considered without issue

@jonlipsky
Copy link
Author

@537mfb Yes, I'd be happy to submit an update to the pull request to change the naming of RightHeader back to Header. I renamed that property because the original behavior was not obvious (in my personal opinion).

@chucker
Copy link
Contributor

chucker commented Apr 4, 2024

Given that Header's behavior changed between 2.x and 3.x, and this would change it back, I would argue the rename makes sense. Giving it a new name makes it clear that the behavior has changed.

@Jay-o-Way
Copy link

I think you guys are forgetting that "left" and "right" should be swapped when the OS is using a right-to-left language. Better use something like "primary" and "secondary, or something like that.

@Jay-o-Way
Copy link

Jay-o-Way commented Apr 4, 2024

...the ability to vertically align the Minimize, Maximize and Close buttons

That would be setting the PreferredHeightOption to TitleBarHeightOption.Tall
https://learn.microsoft.com/windows/apps/develop/title-bar#tall-title-bar-support-for-custom-title-bars

<ContentPresenter
Grid.Column="0"
HorizontalAlignment="Left"

Choose a reason for hiding this comment

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

No need for this property when width=auto

@jonlipsky
Copy link
Author

@Jay-o-Way I initially called the renamed properties LeadingHeader and TrailingHeader; however it wasn't clear to me that the titlebar class was adaptive for right-to-left languages the way it is built, thus I renamed them. I agree that if it is adaptive, then they should have an alternate name.

@537mfb
Copy link

537mfb commented Apr 8, 2024

Not sure how easy this would be to achieve but how about having Header on the side of the system buttons (as is now) and make Title work like Content in other controls where if you give a string it works like a textblock but you can give it other arbitrary content.

Think of a Label - you can give it a string or a layout with several controls as content

@jonlipsky
Copy link
Author

@537mfb That's essentially what my PR does right now. By default, it's a TextBlock, however if you set LeftHeader, that content becomes whatever you set it to.

@537mfb
Copy link

537mfb commented Apr 8, 2024

@jonlipsky my idea is slightly different.

you could have:

<iu:TitleBar Title="My App Name" ...

and it would work as it does now. But you could also do:

<iu:TitleBar ....>
    <iu:TitleBar.Title>
         <StackPanel Orientation="Horizontal">
             <ui:SymbolIcon Symbol="Fluent24"/>
             ....
         </StackPanel>
    </iu:TitleBar.Title>
</iu:TitleBar>

@pomianowski
Copy link
Member

pomianowski commented Apr 14, 2024

To be honest, I'm not a fan of the division into HeaderLeft and HeaderRight. I would prefer one big Header as a whole Titlebar up to navigation buttons with default value - Icon and Application Name, similar to @537mfb's suggestion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controls Changes to the appearance or logic of custom controls. dotnet gallery WPF UI Gallery PR Pull request styles Topic is related to styles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants