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

WinUI TitleBar API Spec #10056

Merged
merged 17 commits into from
Dec 13, 2024
Merged

WinUI TitleBar API Spec #10056

merged 17 commits into from
Dec 13, 2024

Conversation

karkarl
Copy link
Contributor

@karkarl karkarl commented Oct 11, 2024

This PR adds the dev and functional spec for WinUI TitleBar.

Community members can provide feedback by commenting directly on the PR, or through a code suggestion with Github tools.
Feedback should be constructive and specific, addressing potential use cases, design concerns, and functionality.

This PR will be open for feedback for a month: October 11th – November 11th
For more info on the public api review process: https://aka.ms/winappsdk/api-specs-review

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Oct 11, 2024
@karkarl karkarl mentioned this pull request Oct 11, 2024
@karkarl karkarl self-assigned this Oct 11, 2024
@karkarl karkarl removed the needs-triage Issue needs to be triaged by the area owners label Oct 11, 2024
@whiskhub whiskhub mentioned this pull request Oct 12, 2024
@ghost1372
Copy link
Contributor

Are the following modes considered?

  • Right To Left
  • Dark/Light Theme support for System Context menu (Right Click on TitleBar)
  • Support Fluent (Menu Flyout) Context Menu (Disable System Context menu and using Custom Context menu)

@Jay-o-Way
Copy link
Contributor

Jay-o-Way commented Oct 15, 2024

TitleBar currently needs to be set explicitly in the grid.row and referenced by Window in codebehind as shown above. Improvements to Window are being considered to avoid this extra grid layout and codebehind.

Yes, this is an inherent and logical result of the whole ExtendsContentIntoTitleBar concept. You're telling the app/window to hide its actual title bar. If you want a control (like this) to be used as a window's title bar, from an app's perspective, this raises the bigger question of "Is this control part of the window's content or not?"

If we could set the window's title bar properties in the following manner, then the ExtendsContentIntoTitleBar part will become unneeded. Just make sure the background of the title bar is transparent so that it show the window background.

Think of it like this:

<Window
    x:Class="App1.MainWindow"
    xmlns:local="using:App1"
    mc:Ignorable="d">

    <TitleBar x:Name="DefaultTitleBar" Title="Default TitleBar" Subtitle="Preview">
        <TitleBar.IconSource>
            <SymbolIconSource Symbol="Home"/>
        </TitleBar.IconSource>
    </TitleBar>

      <!-- App content - in the 100% correct term -->
</Window>

- Custom Height - 32px as default / 48px if content != null
![TitleBar Size Variants](images/titlebar-sizevariant.png)

## Considerations
Copy link
Contributor

Choose a reason for hiding this comment

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

The WinUI title bar should meet Windows app design guidelines (see #9907). For example:

A single-click/tap on the icon should show system window menu.
A double-click/tap should close the window.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great callout. I will make sure it is included in the Considerations section.

@aboodic
Copy link

aboodic commented Oct 22, 2024

This PR adds the dev and functional spec for WinUI TitleBar.

Community members can provide feedback by commenting directly on the PR, or through a code suggestion with Github tools.

Feedback should be constructive and specific, addressing potential use cases, design concerns, and functionality.

This PR will be open for feedback for a month: 10/11-11/11

For more info on the public api review process: https://aka.ms/winappsdk/api-specs-review

@mdtauk
Copy link
Contributor

mdtauk commented Oct 22, 2024

a month: 10/11-11/11

This took my UK brain a moment to decipher lol
11th October – 11th November right?

@duncanmacmichael
Copy link
Member

a month: 10/11-11/11

This took my UK brain a moment to decipher lol 11th October – 11th November right?

Correct :)

public class TitleBar : Control
```

## TitleBar.Header property
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised nobody has brought this up, after the heated discussions in #9700.
Header and Footer, while being concise terms, can be confusing in this context of a horizontal layout. Other terminology could be:

In the end, it will be hard to objectively decide on what's best. Personally, I prefer StartElement and EndElement here. Before and After imply the center of the title bar would have the actual content, while in most apps the center of the title bar is actually empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the thought, but don't agree. A head is not a synonym for the top but for the leading part. Animals that are "built horizontally" have their head at the front, not on top.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm with Whiskhub here. Header/Footer is misleading. The only argument I see for keeping the naming is that TabView also got this wrong, but I'd rather you don't continue down the confusing naming and address it here with better names, like Leading and Trailing.

Choose a reason for hiding this comment

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

I'm for header/footer if they're not going to change it everywhere else at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

As per the original feedback thread, agree and prefer Before/After #9700 (comment)

@mdtauk
Copy link
Contributor

mdtauk commented Oct 28, 2024

I hope there will be some sample, guidance, or official support for apps which use tabbed navigation within the titlebar area.

Comment on lines +384 to +385
## TitleBarTemplateSettings Class
<!-- TODO -->
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see information on this class. I had pointed out in #9700 that this was an odd way to set the icon here, rather than just setting the icon directly on the window or titlebar instance. It feels a bit like this is just internal plumbing that is bubbling up into the public API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The TitleBarTemplateSettings class would not be used to set the Icon Property. It was added originally to implement interactive / non-interactive regions in the TitleBar. I will most likely be removing it, as it is no longer needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I misspoke in my previous comment. The Icon in TitleBar will continue to be applied through TitleBarTemplateSettings.IconElement. By best practice, we should not be modifying xaml elements in codebehind, should the element be removed in re-templating.

Copy link
Contributor

Choose a reason for hiding this comment

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

This confuses me. Normally templatesettings classes are used to simplify binding things into the template, and not as the main way to set a property (which would also make it harder to set from xaml). Reference: https://learn.microsoft.com/en-us/windows/uwp/xaml-platform/template-settings-classes#the-scenario-for-templatesettings-classes

@ghost1372
Copy link
Contributor

Today is November 12! 😐

@karkarl
Copy link
Contributor Author

karkarl commented Nov 12, 2024

The feedback period is up! Thank you all very much for your contributions thus far. I will be closing this PR while we parse through the feedback.

Updates to the spec will still be done on this branch, and I will re-open the PR when it is ready for final review and merge. Closing this PR is simply to indicate that we are no longer actively gathering feedback.

@karkarl karkarl closed this Nov 12, 2024
public class TitleBar : Control
```

## TitleBar.Header property
Copy link
Contributor

Choose a reason for hiding this comment

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

As per the original feedback thread, agree and prefer Before/After #9700 (comment)

Gets or sets the TitleBar's icon.

```cs
public IconSource IconSource { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Make it clear who owns the underlying icon handle here, to avoid common handle leaks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see any talk about wiring this up to Window.Titlebar.

Comment on lines +270 to +272
## TitleBar.IconSource property

Gets or sets the TitleBar's icon.
Copy link
Contributor

@riverar riverar Nov 13, 2024

Choose a reason for hiding this comment

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

Is this going to be wired up to WM_SETICON for the notification area? (If so, I have a laundry list of things you need to keep in mind...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great callout. Please do share the list of considerations here.

Copy link
Contributor

@riverar riverar Dec 12, 2024

Choose a reason for hiding this comment

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

Some considerations include:

  • Ownership and lifetime of icon handles: You'll need to decide whether the developer is responsible for managing the HICON handle, or if ownership is transferred to the framework. If ownership is transferred, will the framework duplicate the handle for use in multiple windows, or allow developers to set per-window icons? And don't forget about globally shared handles, which should not be freed automatically.

  • Icon size variants: The API should support setting both small and big icon sizes, as both are used across different UI surfaces for visual consistency.

  • System theme and accessibility: The API should allow for setting multiple icon variants to accommodate different system themes (e.g., light/dark mode), high contrast mode, and high DPI scenarios. Again, it's all about visual consistency.

@dotMorten
Copy link
Contributor

Is the v1.7experimental release the final TitleBar API? I'm a little confused that it is identical to the 1.6 one, and from my understanding it was pulled from the 1.6 release to address the design feedback?

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Nov 21, 2024
@michael-hawker
Copy link
Collaborator

Is the v1.7experimental release the final TitleBar API? I'm a little confused that it is identical to the 1.6 one, and from my understanding it was pulled from the 1.6 release to address the design feedback?

No, the version in 1.7-experimental is still the one from 1.6-experimental. The feedback period for collection on the spec here just closed last week; so now that'll be go into improving the control. I agree the timing here is bit off, but part of it was us shoring up the definition of the process here.

I'll let @karkarl add any more specifics from a timeline/planning perspective, as I don't have up-to-date info on that, just the info above. I had called this out during the .NET Conf talk last week when we mentioned this as well.

@karkarl karkarl reopened this Dec 12, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Dec 12, 2024
Comment on lines +177 to +181
Alternate naming considerations for `Header`, `Content`, `Footer`:
- Header and Footer aligns with TabView.TabStripHeader and TabView.TabStripFooter
- Start and End (e.g. [Windows.UI.Xaml.TextAlignment](https://learn.microsoft.com/en-us/uwp/api/windows.ui.xaml?view=winrt-26100))
- Before and After (e.g. CSS, [Fluent Web UI](https://react.fluentui.dev/?path=/docs/components-input--docs#content-before-after))
- Leading and Trailing (e.g. [Apple SwiftUI Toolbar](https://developer.apple.com/documentation/swiftui/toolbaritemplacement/topbarleading))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean this is up for discussion still in this design as a todo? Or is this more of a comparison to explain what is meant by these concepts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are notes for posterity. I am submitting this spec for review and wanted to call out the comparisons.

@karkarl karkarl merged commit 650b2c1 into main Dec 13, 2024
1 of 2 checks passed
@karkarl karkarl deleted the user/karkarl/TitleBarSpecReview branch December 13, 2024 01:04
@manoj-sunagar
Copy link

hey sorry to post it here, can anyone look at this?#10319 (comment)

@ghost1372
Copy link
Contributor

ghost1372 commented Feb 12, 2025

Hi @karkarl
we are in v1.7-Preview1 and still Many TitleBar feedbacks are unresolved, Most importantly, AppWindow.TitleBar APIs do not work with TitleBar control.

#9784
#9864
#9907
#9866
#9865
#9863

@ghost1372
Copy link
Contributor

For those interested (@dotMorten, @whiskhub, @Jay-o-Way , @riverar ), the new titlebar now uses RightContent, LeftContent, CenterContent

@riverar
Copy link
Contributor

riverar commented Feb 12, 2025

That’s unfortunate and may confuse RTL customers, but the decision has been made. The good news, I suppose, is that the team is still making progress.

@whiskhub
Copy link
Contributor

I agree, this decision is quite weird. Why do all this effort for a spec discussion, if you internally decide for something entirely different? The final decision should’ve been at least documented here.

Probably it was just forgotten since all the work is done in the internal Microsoft repo and there is actually rarely anyone looking at this GitHub repo :/

@karkarl
Copy link
Contributor Author

karkarl commented Feb 15, 2025

Hey everyone,

We want to take a moment to sincerely thank you for your feedback and contributions to improving WinUI controls, in this case, TitleBar. Your insights help us refine the control and ensure we're delivering the best possible experience for developers and users alike.

Based on your feedback, we've made several improvements since 1.6Experimental, including:

Upcoming post 1.7 stable fixes that we are committed to:

In addition, we’ve documented and marked the following suggestions for future versions:

Your engagement makes a real impact, and we truly appreciate the time and effort you put into testing, reporting issues, and suggesting enhancements. This is the first iteration for open spec reviews for WinUI 3, and while there are growing pains and gaps in communication, we are working on streamlining this process.

Please keep the feedback coming—we're excited to keep improving together!

@karkarl
Copy link
Contributor Author

karkarl commented Feb 15, 2025

For those interested (@dotMorten, @whiskhub, @Jay-o-Way , @riverar ), the new titlebar now uses RightContent, LeftContent, CenterContent

During internal review, after presenting the options that the community has offered, we found precedence with both Pivot and ScrollViewer having LeftHeader/RightHeader as a concept. The current naming ensued from that.

Would folks have preferred LeftHeader/Content/RightHeader instead to align with more established patterns?

@dotMorten
Copy link
Contributor

dotMorten commented Feb 15, 2025

Would folks have preferred LeftHeader/Content/RightHeader instead to align with more established patterns?

My main concern here is I don't understand how this should work in an RTL scenario.

  1. Would Left go on the right side and vice versa? (confusing/misleading naming) or:
  2. Would left stay on the left and forcing developers to write code to detect RTL and swap the contents of the two? (too complicated developer story that will set developers up to fail).

Right and Left content was explicitly mentioned in a few of the design comments as not tenable because of this, so I was rather confused it was ignored and instead became part of the shipping design, even though the spec never mentioned this naming as an option (having said that the spec was merged with only options for naming, not final naming which was a little confusing to me in a design spec that is merged with no final conclusion).

I think comparing to Pivot and ScrollViewer doesn't make that much sense, because the expectation for these controls isn't to swap content in RTL (you still scroll/pivot left/right when clicking that content)

@karkarl
Copy link
Contributor Author

karkarl commented Feb 15, 2025

@dotMorten

Would Left go on the right side and vice versa? (confusing/misleading naming) or:

Yes, consistent with the rest of Xaml where the Left* elements are swapped to the right side in RTL and vice versa.

Would left stay on the left and forcing developers to write code to detect RTL and swap the contents of the two? (too complicated developer story that will set developers up to fail).

A developer would have to handle the RTL language change and update the FlowDirection property on the app first to enable RTL scenario. This is an obvious gap in WinUI developer experience but this code should probably live in the XamlWindow layer. It is outside of the scope of WinUI TitleBar.

@whiskhub
Copy link
Contributor

I appreciate your response to the community, this is what we need more of!
In the end I don’t really mind the exact naming. It was just confusing for us that this came out of nowhere. If it’s a spec, IMO all considerations must be documented in advance. And the taken decision should also be documented here together with a short reason. If some information stays internal, it’s just challenging for the community to understand what is happening.

@mominshaikhdevs
Copy link

mominshaikhdevs commented Feb 15, 2025

@karkarl can you please file a dedicated issue in the WindowsAppSDK repo that includes whole of your this comment, so that we people outside of MS can be aware of its progress? because right now, it's buried deep down in the github-PR-comments soup.

@Jay-o-Way
Copy link
Contributor

Remember that iconic situation of two kids, facing each other, one tells the other to raise their left hand and, because they are not on the same side, the first one tells the other that they're doing it wrong...?

The words RIGHT and LEFT, like east and west, are supposed to be objective and not to be mixed up! It's the whole reason that we were suggesting other words! Oh, Microsoft, you made another dumb choice once again. Mark my words, I predict a huge pile of complaints coming in the near future...

1739615413272.jpg

@Dubzer
Copy link

Dubzer commented Feb 16, 2025

My main concern here is I don't understand how this should work in an RTL scenario.

By the way, there is such a case in CSS, and in Flexbox API it's solved by using terms like "start" and "end". When the writing direction is changed, the positions are swapped. So I think these are good words to describe it

@michael-hawker
Copy link
Collaborator

We've talked a lot about various naming conventions here. They all have their drawbacks. The main goal for us is to try and remain as consistent as possible with what's been done in the past to make things more discoverable or consumable by others.

Header/Content/Footer has been the established pattern for a long time, even for horizontal layouts (though not as common). This was the direction from the initial reviews during the preliminary development as well. However, this got a lot of pushback last year, so after evaluation of other possibilities, it was discovered there was another fallback with the Left/Right Header convention done by Pivot and ScrollViewer.

Unfortunately for its roots based over a decade ago, Left/Right and swapping them in RTL mode with FlowDirection is the current pattern established by the framework for handling these scenarios. Even if it's not well understood or confusing, though hopefully we can do a better job documenting this convention in the interim.

It's better for us to be more consistent now than try to one-off a new paradigm that'll cause further confusion, and look towards an opportunity in the future to revisit things as a whole to rework across the platform with some future breaking change (whenever that opportunity arises).

@dotMorten
Copy link
Contributor

@michael-hawker ugh that's tough but I can understand the issues you're dealing with here, and the reasoning for making this change. Thank you for the above description. I do wish that would have been part of the design discussion rather than the blind-sided change in the release. It really devalued the purpose of the public design review.

I do wonder though if people would have hated header/footer as much had they known this was the only other alternative to keep consistency.

@karkarl
Copy link
Contributor Author

karkarl commented Feb 20, 2025

Hey everyone! I just want to give an update that the API names have been offically updated to LeftHeader, Content, and RightHeader.

@eduardobragaxz
Copy link

Does it make sense to have different names for these properties when you can put exactly the same things in them? What makes the headers different from Content apart from position? Left/RightContent makes more sense to me, even though I dislike it for not being consistent with other controls.

@michael-hawker
Copy link
Collaborator

@eduardobragaxz the LeftHeader is before the Icon/Title/Subtitle, so it's not just left of the content area.

@eduardobragaxz
Copy link

I just mean they have different names yet achieve the same: to set some content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Issue needs to be triaged by the area owners SpecInReview
Projects
Status: API Review Completed
Development

Successfully merging this pull request may close these issues.