-
Notifications
You must be signed in to change notification settings - Fork 228
Fixes GroupBox lack of ControlTheme applied #563
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
Conversation
|
Technically this works, but it's not the way that it's done by other --- <ControlTheme TargetType="suki:GroupBox" x:Key="SukiGroupBoxTheme">
+++ <ControlTheme TargetType="suki:GroupBox" x:Key="{x:Type suki:GroupBox}"> |
SukiUI/Controls/GroupBox.axaml
Outdated
| <ContentPresenter.Styles> | ||
| <Style Selector="TextBlock"> | ||
| <Setter Property="Foreground" Value="{DynamicResource SukiLowText}" /> | ||
| <Setter Property="FontWeight" Value="DemiBold" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This disallows users from overriding the FontWeight inline:
<suki:GroupBox Header="Title" FontWeight="Bold" />or with a style setter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, but if we are gona be that picky I think the forcement of Foreground is even worse.
I think avalonia should have separate properties for each control property, should be much better than having to laydown styles, classes and whatever to make it simple.
In this case is often to have header with larger or bolder font, having whole content bold is 1% or less of the usage
Other thing I miss in avalonia is a way to increase size partially and not rely on absolute values
I go revert the DemiBold to keep the original
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree. I tried to address it in my PR as well but failed because Avalonia lacks a style selector for when a property is null/default. There is an open issue about it since 2024: AvaloniaUI/Avalonia#14147.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so far we are doomed to use global styles to override such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the hell of it, I tried writing a MultiBinding converter but it didn't seem to work. Maybe because one of its children was a DynamicResource?
The only other hack I can think of is a converter that uses the fallback value as the key for a resource lookup, but then it becomes a magic string that isn't checked by the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's a rat nest, I think its better to keep it simple.
Avalonia really need a rewrite on styling system, much better than wpf or winui but still looks very restrictive for today pratices.
So I was wrong - some files do it the way you did it, while others do it the way I suggested. Personally, I still prefer the 1-LOC solution though. |
|
Closing in favor of #564 |
Fixes #561 ControlTheme was not applied to control.