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

[Experiment] TitleBar #459

Merged
merged 41 commits into from
May 18, 2024
Merged

[Experiment] TitleBar #459

merged 41 commits into from
May 18, 2024

Conversation

niels9001
Copy link
Collaborator

@niels9001 niels9001 commented Jul 4, 2023

Addressing: #454

Titlebar

@ghost1372
Copy link
Contributor

ghost1372 commented Jul 4, 2023

Tnx @niels9001

ContextMenu theme is wrong and always light (i can fix this issue, can i make a pr for this?)

this code can fix this issues:

public static void SetPreferredAppMode(ElementTheme theme)
    {
        if (theme == ElementTheme.Dark)
        {
            NativeMethods.SetPreferredAppMode(NativeMethods.PreferredAppMode.ForceDark);
        }
        else if (theme == ElementTheme.Light)
        {
            NativeMethods.SetPreferredAppMode(NativeMethods.PreferredAppMode.ForceLight);
        }
        else
        {
            NativeMethods.SetPreferredAppMode(NativeMethods.PreferredAppMode.Default);
        }
        NativeMethods.FlushMenuThemes();
    }

02

@ghost1372
Copy link
Contributor

@niels9001
For some of the problems I reported, I can create a PR. Is that possible?

@niels9001
Copy link
Collaborator Author

Tnx @niels9001

Some Issues:

  1. TitleBar Buttons Should be updated based on windows theme in Active/DeActive State: (i can fix this issue, can i make a pr for this?)

Sure, go ahead - feel free to PR against this branch!

  1. ContextMenu theme is wrong and always light (i can fix this issue, can i make a pr for this?)

I believe WASDK 1.4 should I address this issue.. let me sync with that team to see if that's correct and we don't need to do any registry stuff?

@ghost1372 ghost1372 mentioned this pull request Jul 4, 2023
@ghost1372

This comment was marked as resolved.

@dongle-the-gadget
Copy link

ContextMenu theme is wrong and always light (i can fix this issue, can i make a pr for this?)

IIRC it requires a private API to be used extremely early into the loading process.

@ghost1372 ghost1372 mentioned this pull request Jul 6, 2023
@Jay-o-Way
Copy link
Contributor

in TitleBar Height = Standard , we can not use VerticalAlignment="Center" for AutoSuggestBox or other controls

That's the reason "tall" should be used as soon as there are interactive/input controls. Don't know if it's possible to force this setting, in the logic, based on content?

@ghost1372

This comment was marked as outdated.

@Jay-o-Way
Copy link
Contributor

@ghost1372 you might want to re-count those pixels

@ghost1372
Copy link
Contributor

ghost1372 commented Jul 11, 2023

you might want to re-count those pixels

I don't want the size of the buttons to be big, just like the store, I want it to be small, but the autosuggestbox stays in the center!
Screenshot 2023-07-11 185529

Copy link
Contributor

@Jay-o-Way Jay-o-Way left a comment

Choose a reason for hiding this comment

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

Hope we can use this in PowerToys soon 🙂

components/TitleBar/samples/TitleBar.md Outdated Show resolved Hide resolved
@Jay-o-Way
Copy link
Contributor

@ghost1372 oh sorry, now i see those buttons. @niels9001 those should stay untouched.

@niels9001
Copy link
Collaborator Author

@Jay-o-Way Please leave any generic questions/comments in the discussion and not in this PR.

@niels9001
Copy link
Collaborator Author

in TitleBar Height = Standard , we can not use VerticalAlignment="Center" for AutoSuggestBox or other controls

That's the reason "tall" should be used as soon as there are interactive/input controls. Don't know if it's possible to force this setting, in the logic, based on content?

Microsoft itself has used the standard mode, so we definitely do not need the tall mode to implement this mode, Store is a perfect example Screenshot 2023-07-11 183528

@niels9001 When will you merge this PR? It's been almost 2 weeks...

I have some other stuff that got prioritized. I think in terms of features, we are in a good spot with this PR - or do you see any must-have things that need to be fixed still? I think having some basic tests in place would be good to have - if that's something you want to help out with let me know!

Comment on lines 179 to 180
[DllImport("Shcore.dll", SetLastError = true)]
internal static extern int GetDpiForMonitor(IntPtr hmonitor, Monitor_DPI_Type dpiType, out uint dpiX, out uint dpiY);
Copy link
Member

Choose a reason for hiding this comment

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

Seems silly for a single method, but can use CsWin32 to generate these instead...

@Jay-o-Way
Copy link
Contributor

Although, I don't have any image in mind of integrating the titlebar with the tabview.
Is there an example?

Pretty much any tabbed window. Notepad, Explorer, browsers...

@ghost1372
Copy link
Contributor

Thank you @Jay-o-Way @AnalogFeelings
I would like to work on it😁

@ghost1372
Copy link
Contributor

Hi @pratikone
Can you help us?
which one we should use?

Window.ExtendsContentIntoTitleBar + Window.SetTitleBar()
or
Window.AppWindow.TitleBar.ExtendsContentIntoTitleBar

@pratikone
Copy link

pratikone commented Jan 3, 2024

Hi @pratikone
Can you help us?
which one we should use?

Window.ExtendsContentIntoTitleBar + Window.SetTitleBar()
or
Window.AppWindow.TitleBar.ExtendsContentIntoTitleBar

Both are correct answers because both use same code underneath. The former is for ease of use, latter is more low level and provides finer control . So if you are creating custom drag regions, latter is good too. You can also mix and match APIs. Like setup titlebar using Window APIs and modify drag regions using appwindow APIs.

First see if default titlebar is good enough. I've changed default titlebar with winappsdk 1.4 release to represent the most common case : full width custom titlebar

@insomniachi
Copy link

Is there anything i need to do while hiding the titlebar ?
i referred this from msdocs to hide the title bar

public MainWindow()
{
    this.InitializeComponent();

    m_AppWindow = this.AppWindow;
    m_AppWindow.Changed += AppWindow_Changed;
}

private void AppWindow_Changed(AppWindow sender, AppWindowChangedEventArgs args)
{
    if (args.DidPresenterChange)
    {
        switch (sender.Presenter.Kind)
        {
            case AppWindowPresenterKind.CompactOverlay:
                // Compact overlay - hide custom title bar
                // and use the default system title bar instead.
                AppTitleBar.Visibility = Visibility.Collapsed;
                sender.TitleBar.ResetToDefault();
                break;

            case AppWindowPresenterKind.FullScreen:
                // Full screen - hide the custom title bar
                // and the default system title bar.
                AppTitleBar.Visibility = Visibility.Collapsed;
                sender.TitleBar.ExtendsContentIntoTitleBar = true;
                break;

            case AppWindowPresenterKind.Overlapped:
                // Normal - hide the system title bar
                // and use the custom title bar instead.
                AppTitleBar.Visibility = Visibility.Visible;
                sender.TitleBar.ExtendsContentIntoTitleBar = true;
                break;

            default:
                // Use the default system title bar.
                sender.TitleBar.ResetToDefault();
                break;
        }
    }
}

because my application is crashing while returning from fullscreen mode to Overlappedmode. (this is not always, but it randomly happens. i'm not able to identify why or when it happens.)
but i'm sure it's something wrong with the titlebar because when i don't use it, App never crashes.

@ghost1372
Copy link
Contributor

Hi @insomniachi
I did not test hiding labs titlebar, but if you call ResetToDefault, titlebar will be reseted to default so custom titlebar will be gone. If you want to hide titlebar, i think using visibility property is standard way.
Also if you want to use ResetToDefault, you can set AutoConfigureCustomTitleBar = "false"

@insomniachi
Copy link

insomniachi commented Jan 9, 2024

in my case, it'll never call reset to default because I'm only switching between Overlapped/Fullscreen, in that I'm only changing visibility

@ghost1372
Copy link
Contributor

in my case, it'll never call reset to default because I'm only switching between Overlapped/Fullscreen, in that I'm only changing visibility

So you should set AutoConfigureCustomTitleBar = false when you are switching between Overlapped/Fullscreen

If you can please upload a sample so i can test it better.

@insomniachi
Copy link

insomniachi commented Jan 9, 2024

reproduceable sample
https://github.com/insomniachi/CTK_TitleBar_Crash (default template from Template Studio)
it will not crash when running from visual studio.

  • publish unpackaged win-x64 in release mode
  • run exe
  • there should be a media player element in the main page
  • play video
  • double click the media player element to go to Fullscreen.
  • while playing double click again to come back to overlapped. it should crash, or repeat the Fullscreen and back steps a few times, it should eventually crash.

image

@ghost1372
Copy link
Contributor

Tnx @insomniachi
i tested 100 times but i did not see any crash, mybe something related to MediaPlayerElement or WinUI Windowing iteself, so i suggest you report issue in Microsoft.UI.Xaml or WindowsAppSDK repo

@insomniachi
Copy link

did you try from visual studio or published exe ?
it does not happen when running from visual studio
Recording 2024-01-10 094232

@ghost1372
Copy link
Contributor

did you try from visual studio or published exe ?
it does not happen when running from visual studio
Recording 2024-01-10 094232

I tried both visual studio and publish

@ghost1372
Copy link
Contributor

ghost1372 commented Mar 15, 2024

i found another bug
if we collapse NavigationView, then we choose an item, we can not select first item and act as a titlebar drag region

222

@TyJOrtiz
Copy link

Is it possible to just adjust the pop up margin?

@niels9001 niels9001 marked this pull request as ready for review April 22, 2024 16:41
@ghost1372
Copy link
Contributor

i found another bug if we collapse NavigationView, then we choose an item, we can not select first item and act as a titlebar drag region

This is an expected behavior!
microsoft/microsoft-ui-xaml#9448 (comment)

@Arlodotexe
Copy link
Member

Arlodotexe commented Apr 24, 2024

Looks like this is blocking the CI:

"D:\a\Labs-Windows\Labs-Windows\components\TitleBar\src\CommunityToolkit.WinUI.Controls.TitleBar.csproj" (default target) (21:67) ->
(CoreCompile target) -> 
  D:\a\Labs-Windows\Labs-Windows\components\TitleBar\src\TitleBar.Properties.cs(45,73): error CS1574: XML comment has cref attribute 'Display' that could not be resolved [D:\a\Labs-Windows\Labs-Windows\components\TitleBar\src\CommunityToolkit.WinUI.Controls.TitleBar.csproj::TargetFramework=net7.0-android33.0]

Should be clear to merge once this is resolved.

Copy link
Member

@Arlodotexe Arlodotexe left a comment

Choose a reason for hiding this comment

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

We'd like to close this PR so that iterations and fixes can be done separately, and so that people can start using the experiment from a package in the Labs feed instead of a PR feed.

@Arlodotexe
Copy link
Member

The current warning in the CI:

"D:\a\Labs-Windows\Labs-Windows\CommunityToolkit.AllComponents.sln" (default target) (1:2) ->
"D:\a\Labs-Windows\Labs-Windows\tooling\ProjectHeads\AllComponents\Tests.Uwp\CommunityToolkit.Tests.Uwp.csproj" (default target) (31:6) ->
(CoreCompile target) -> 
  D:\a\Labs-Windows\Labs-Windows\components\TitleBar\tests\ExampleTitleBarTestClass.cs(80,23): error CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread. [D:\a\Labs-Windows\Labs-Windows\tooling\ProjectHeads\AllComponents\Tests.Uwp\CommunityToolkit.Tests.Uwp.csproj]
  D:\a\Labs-Windows\Labs-Windows\components\TitleBar\tests\ExampleTitleBarTestClass.cs(88,23): error CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread. [D:\a\Labs-Windows\Labs-Windows\tooling\ProjectHeads\AllComponents\Tests.Uwp\CommunityToolkit.Tests.Uwp.csproj]
  D:\a\Labs-Windows\Labs-Windows\components\TitleBar\tests\ExampleTitleBarTestClass.cs(95,23): error CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread. [D:\a\Labs-Windows\Labs-Windows\tooling\ProjectHeads\AllComponents\Tests.Uwp\CommunityToolkit.Tests.Uwp.csproj]

@Arlodotexe Arlodotexe merged commit 8d49097 into main May 18, 2024
6 of 7 checks passed
@delete-merged-branch delete-merged-branch bot deleted the niels9001/titlebar-experiment branch May 18, 2024 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.