benruehl / adonis-ui

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

Hide buttons in AdonisWindow according to ResizeMode #38

Closed alxnull closed 4 years ago

alxnull commented 4 years ago

The default WPF window hides the minimize and maximize buttons if ResizeMode is set to NoResize. Also, the maximize button is disabled if it's set to CanMinimize [link]. This PR adds this behaviour for AdonisWindow.

PS: Thanks for your great work on this library!

benruehl commented 4 years ago

We should also check if the window buttons are the only things that need to respect the ResizeMode. Maximizing can also be done using shortcuts, double-clicking the title bar and dragging the window to the top of the screen. Maybe these things need to be adressed as well. I will look into that soon.

alxnull commented 4 years ago

@benruehl Thanks for your feedback.

I think the solution could be improved by moving the setter of the disabled foreground to the WindowButton style.

I did try this before, but somehow could not get it to work. It seems the triggers in the WindowButton style are overriden by the values of the control template. The current is the only solution I could think of.

Maximizing can also be done using shortcuts, double-clicking the title bar and dragging the window to the top of the screen.

Made a quick check for those and everything already seems to behave as expected. Looks like someone already thought about that :) https://github.com/benruehl/adonis-ui/blob/bb2bfbe09ed509e0f9f2e39dcf5d4eeef717d7f6/AdonisUI/Controls/AdonisWindow.cs#L225-L229

benruehl commented 4 years ago

You are right, I see what you mean, sorry!

The only alternative I can think of is a trigger in the WindowButton style which sets the foreground of an inner control in the template. That way the foreground is no longer overridden because the inheritance is cut off. Something like this:

<ControlTemplate.Triggers>
    <Trigger Property="IsEnabled" Value="False">
        <Setter Property="TextElement.Foreground" TargetName="ContentPresenter" Value="{DynamicResource {x:Static adonisUi:Brushes.DisabledForegroundBrush}}"/>
    </Trigger>
</ControlTemplate.Triggers>

(Assuming the ContentPresenter in the WindowButton control template is named 'ContentPresenter')

This solution has the disadvantage that it is built directly into the template which means the behavior cannot be customized from the outside anymore.

What do you think? If you don't like the idea, we can use your current solution as well.

alxnull commented 4 years ago

@benruehl You're alternative solution makes more sense to me as it is more generic. However, setting TextElement.Foreground on the ContentPresenter does not seem to have an effect on the window button icons as they are canvas elements which use the foreground color of the surrounding ContentControl. Have I overseen something important here?

benruehl commented 4 years ago

Seems like I forgot to mention that the button's Content needs to go into the ContentTemplate instead.

<Button x:Name="PART_MaximizeRestoreButton"
        Grid.Column="1"
        Style="{DynamicResource {x:Static adonisUi:Styles.WindowButton}}"
        Foreground="{TemplateBinding TitleBarForeground}"
        Background="{TemplateBinding WindowButtonHighlightBrush}">
    <Button.ContentTemplate>
        <DataTemplate>
            <ContentControl ContentTemplate="{DynamicResource {x:Static adonisUi:Icons.WindowMaximize}}"
                            Width="10"
                            Height="10"/>
        </DataTemplate>
    </Button.ContentTemplate>
</Button>

I am not sure why this changes the behavior though. Read about it in punker76's blog.

Does it work for you now?

alxnull commented 4 years ago

Thanks, this does work for me now. I pushed the changes.

benruehl commented 4 years ago

Awesome. Again, thanks a lot for this contribution. Appreciate it.