benruehl / adonis-ui

Lightweight UI toolkit for WPF applications offering classic but enhanced windows visuals
https://benruehl.github.io/adonis-ui/
MIT License
1.73k stars 145 forks source link

Support drawing the TitleBar over the client area #126

Closed marcinkurek closed 3 years ago

marcinkurek commented 3 years ago

I would like to be able to accomplish this with Adonis UI:

Bez tytułu

Meaning, that the sidebar background would reach all the way to the top of the window.

I'm not sure what's the best way to accomplish this technically, but I'm guessing one way would be to allow Adonis UI to render the title bar with transparent background over the client area.

As a bonus, that could be used to achieve really cool effects, like the blurred content behind the title bar on iOS:

Bez tytułu

I can PoC and contribute this change if this feature request gets accepted.

benruehl commented 3 years ago

Yes, I like the idea!

How would you like to control this behavior? Maybe a bool property on AdonisWindow like IsTitleBarOverlay. Or something similar to what UWP does with ExtendViewIntoTitleBar?

Thanks for the suggestion.

marcinkurek commented 3 years ago

Yeah a bool property on AdonisWindow sounds fine. I'd avoid using ExtendViewIntoTitleBar though unless we're sure that the semantics are 100% the same, so as not to mislead UWP folk... though it would be a pretty good name in its own right.

marcinkurek commented 3 years ago

OK I made a first draft of the acceptance criteria for this feature:

I made a proof of concept using a Grid element as container for both scenarios, but I ran into an issue - I couldn't get the row containing the titlebar to shrink, when the window was maximized by dragging it to the top of the screen (it still worked correctly when maximized with the button).

Because of this I'm not convinced that this is the best approach, and instead I've been trying to create 2 separate templates - one using the existing DockPanel XAML and a new one using a Grid- for each scenario, and somehow bind to one of them according to the value of the new property. I'll see if I can come up with anything by tomorrow.

marcinkurek commented 3 years ago

Because of this I'm not convinced that this is the best approach, and instead I've been trying to create 2 separate templates - one using the existing DockPanel XAML and a new one using a Grid- for each scenario, and somehow bind to one of them according to the value of the new property. I'll see if I can come up with anything by tomorrow.

I couldn't get this to work so I continued with the Grid approach and found a neat (I think!) way to get around the problem we discussed earlier!

I will add a trigger to set the caption buttons container height to the height of the close button. They're all of equal height and this continues to work regardless of the Visibility of the button.

OK so I decided to refactor one important aspect of the title bar height logic.

The way it works now is that the title bar height is dictated by its tallest children - which are the caption buttons. As a side effect, when the AdonisWindow.TitleBarContent is larger than the caption buttons, two (potentially undesirable) things happen:

obraz

What I did is:

Bez tytułu

All existing constraints still apply so that if a tall AdonisWindow.TitleBarContent is not specified, everything matches the default Windows 10 look, and ShrinkTitleBarWhenMaximized works as expected.

What I need from you is a confirmation that you're OK with moving forward with this somewhat extended scope of change :-) I considered if this would be a breaking change for some, and I could only come up with a scenario, where someone has additional caption buttons - in this case they would also get stretched vertically to match the look.

Alternatively, I can keep the refactoring that helped me solve the title bar height issue, but keep the caption buttons aligned to top as they are now. Then we could maybe consider this in a separate issue.

I made a new PR that also addresses the other issues you mentioned. Work in progress, as I still need to add the TitleBarActualHeight property.

marcinkurek commented 3 years ago

As a side note, I think I know why I had these sizing issues to begin with.

The ToggleWindowState() method in AdonisWindow.cs which updates the WindowState property is handled manually in the event of double-clicking the title bar, or clicking the Maximize/Restore button.

You might want to add handling of the other ways of maximizing the window manually, and I can think of at least two:

marcinkurek commented 3 years ago

Done!

Here's a MainWindow.xaml you can use to test:

<adonisControls:AdonisWindow x:Class="AdonisUI.Demo.MainWindow"
                             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
                             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
                             xmlns:adonisUi="clr-namespace:AdonisUI;assembly=AdonisUI"
                             xmlns:adonisControls="clr-namespace:AdonisUI.Controls;assembly=AdonisUI"
                             PlaceTitleBarOverContent="True"
                             Title="Adonis UI">
    <Window.Style>
        <Style TargetType="Window" BasedOn="{StaticResource {x:Type Window}}"/>
    </Window.Style>
    <Grid>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="128" />
            <ColumnDefinition />
        </Grid.ColumnDefinitions>
        <GroupBox Grid.Column="0"
                  Padding="0">
            <StackPanel>
                <Border Height="{Binding Path=TitleBarActualHeight, RelativeSource={RelativeSource AncestorType={x:Type adonisControls:AdonisWindow}}}" />
                <TextBlock Margin="{adonisUi:Space}">Sidebar content</TextBlock>
            </StackPanel>
        </GroupBox>
        <StackPanel Grid.Column="1">
            <Border Height="{Binding Path=TitleBarActualHeight, RelativeSource={RelativeSource AncestorType={x:Type adonisControls:AdonisWindow}}}" />
            <TextBlock Margin="{adonisUi:Space}">Main content</TextBlock>
        </StackPanel>
    </Grid>
</adonisControls:AdonisWindow>

adonis-titlebar-place-over-false adonis-titlebar-place-over-true

benruehl commented 3 years ago

Looks good, thanks for putting so much effort into that feature :)

Regarding the stretched window buttons, I am not sure myself which approach makes more sense. When I built them I could not think of any application that has larger buttons than the native ones. That doesn't mean that none exist of course. I thought users would not expect the buttons to grow when they place something like a search bar in the title bar. My reference application was Google Chrome at the time which aligns the buttons to the top and leaves a little gap below them (which can be used to drag the window as well).

Google Chrome: image

So personally I'd prefer to have them aligned to the top but if that's too complicated or breaks other parts of the feature I'm fine with the stretching approach as well.

marcinkurek commented 3 years ago

So personally I'd prefer to have them aligned to the top but if that's too complicated or breaks other parts of the feature I'm fine with the stretching approach as well.

Yeah I actually tried to revert this change so it could be made separately if desired but I found that it reintroduced the bug.

I will timebox this and let you know if I come up with anything else that can be done.

marcinkurek commented 3 years ago

Sorry about abandoning this, life happened.

I promise to take a look at this soon.

benruehl commented 3 years ago

Hey, no problem. Take your time! I am really slow with my own PRs as well at the moment.

benruehl commented 3 years ago

I took a look at the current state of the feature and tried to figure out what was missing for it to be completed. I couldn't really remember but from reading through the issue again I think it was about having the window buttons aligned to the top. This seemed to cause an issue when maximizing the window.

So I played around a little and I think I might have found a solution. At least for me it seems to work in all scenarios I tested. I pushed this small change to your branch. I hope this is ok for you. I am not entirely sure if I reproduced the original issue correctly, so I would be glad if you could try it out and see for yourself if it works as expected.

If you don't have time for that I think I'm going to just merge it. I love the feature and definitely want to release it. I think it is better to have a minor bug in it (if it is not already fixed now) instead of not having the feature at all.

marcinkurek commented 3 years ago

Hey @benruehl, happy new year!

I really appreciate your help with this.

I'm going to take a look at this and see if it covers all the scenarios I remember.

marcinkurek commented 3 years ago

@benruehl Fantastic! I tested for both values of PlaceTitleBarOverContent and ShrinkTitleBarWhenMaximized and for each of these I tried maximizing and restoring using dragging, [Win]+[Up] and the maximize button. Everything behaves as expected. I can see that the area under the buttons is now draggable as well :)

Thank you so much for helping me wrap this up. I spent some time trying to figure this out but I really couldn't get to the bottom of it at the time.

Is there anything else I can do for you regarding this enhancement?

benruehl commented 3 years ago

@marcinkurek Happy new year to you as well! And thank you for testing! I'm glad it works for you, too.

After checking the demo application I noticed that for message boxes the window close button is really off. I don't know if this has anything to to with this feature yet, but it might have. I'll have a look at it soon.

Other than that, I think this can be considered done. Great work on your part, thanks a lot for contributing! :)