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

Style Selector not Matching without Pseudoclass #13601

Open
robloo opened this issue Nov 14, 2023 · 6 comments
Open

Style Selector not Matching without Pseudoclass #13601

robloo opened this issue Nov 14, 2023 · 6 comments
Labels
by-design The behavior reported in the issue is actually correct.

Comments

@robloo
Copy link
Contributor

robloo commented Nov 14, 2023

Describe the bug

Attempting to re-style the selection pill height in FluentAvalonia for some specific ListBoxes I ran across an interesting situation. The following code doesn't work. The style is never matched and never appears in DevTools either.

                    <ListBox
                        x:DataType="MyViewModel"
                        ScrollViewer.HorizontalScrollBarVisibility="Disabled"
                        ItemsSource="{Binding Path=Items, Mode=OneWay}"
                        SelectedItem="{Binding Path=SelectedItem, Mode=TwoWay}">
                        <ListBox.Styles>
                            <Style Selector="ListBoxItem /template/ Rectangle#SelectionIndicator">
                                <Setter Property="Height" Value="64" />
                            </Style>
                        </ListBox.Styles>
                        <ListBox.ItemTemplate>
                            <DataTemplate
                                x:DataType="MyViewModel">
                                <Grid>
                                </Grid>
                            </DataTemplate>
                        </ListBox.ItemTemplate>
                    </ListBox>

However, changing the selector to the below works. It must have the pseudoclass.

<Style Selector="ListBoxItem:selected /template/ Rectangle#SelectionIndicator">

For reference:

https://github.com/amwx/FluentAvalonia/blob/master/src/FluentAvalonia/Styling/ControlThemes/BasicControls/ListBoxStyles.axaml#L55

This appears to be a problem across the board. None of the other styles in the app match without a pseudoclass defined. They won't match on just the type in this situation..

To Reproduce

Seems all styles like this won't match.

Expected behavior

Should match by the concrete control type alone.

Screenshots

N/A

Environment

  • OS: Windows 10 Pro
  • Avalonia-Version: 11.0.5

Additional context

This seems to be an Avalonia bug rather than FluentAvalonia. If I'm wrong there I'll close this issue though.

@robloo robloo added the bug label Nov 14, 2023
@timunie
Copy link
Contributor

timunie commented Nov 14, 2023

The height seems to be hard coded in the control template. This is known to be not styleable. We should create a similar test theme which doesn't hard code the size and see if style works then correctly

@timunie
Copy link
Contributor

timunie commented Nov 14, 2023

This is the confliciting line. If I remove the hard coded values, it starts working:

https://github.com/amwx/FluentAvalonia/blob/759d12a67f623a15f44c18c10e0e7480a69aa8e1/src/FluentAvalonia/Styling/ControlThemes/BasicControls/ListBoxStyles.axaml#L83-L87

I honestly think that this is kind of by design. Don't know if it is possible to relax these local set values somehow.

@robloo
Copy link
Contributor Author

robloo commented Nov 14, 2023

I honestly think that this is kind of by design. Don't know if it is possible to relax these local set values somehow.

Thanks, I should have considered style precedence more carefully. HOWEVER, I was under the impression this case is now allowed with v 11.0 after this PR and discussion: #7679. Control Themes no longer set with local priority and should be overridable. Is that my misunderstanding?

I will say it is extremely unintuitive for a Selector to match with a seemingly different priority based on whether or not it applies to a pseudoclass for a type or the type itself. That just can't be right -- the selector SHOULD NOT change the priority of the style itself.

If this really is as intended, and knowing it won't likely be addressed or changed for years to come, I will close this.

Thanks for your help as always.

@timunie
Copy link
Contributor

timunie commented Nov 14, 2023

it was planned yes. But then there were other issues which made us revert this as it was just not really possible as we wanted it to have. @maxkatz6 may have more input on this.

@maxkatz6 maxkatz6 added the by-design The behavior reported in the issue is actually correct. label Nov 16, 2023
@timunie timunie removed the bug label Nov 16, 2023
@timunie timunie closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2023
@maxkatz6
Copy link
Member

It is by design. But honestly, I still think it can be discussed.
I remember there were a possible problem with normal Styles, that the existing style setter would override property value of my new style with a template. Like:

<Style Selector="Button">
   <Setter Property="Background" Value="Blue" />
</Style>

<Style Selector="MyControl">
   <Setter Property="Template">
      <ControlTemplate>
        <!-- This button always will be Blue -->
         <Buton Background="Green" />
      </ControlTemplate>
   </Setter>
<Style>

And yes, it did break Fluent theme completely the last time I tried to "fix" this issue. It was before 11.0.

But with ControlThemes we shouldn't care about it. There are no styles applied before ControlTheme. And even more - we already have some special priority rules for control themes.
Ideally, it should be discussed with @grokys, when he has more time.

That just can't be right -- the selector SHOULD NOT change the priority of the style itself.

CSS selectors do that I am pretty sure.

@maxkatz6 maxkatz6 reopened this Nov 16, 2023
@robloo
Copy link
Contributor Author

robloo commented Nov 19, 2023

That just can't be right -- the selector SHOULD NOT change the priority of the style itself.

CSS selectors do that I am pretty sure.

I probably need to change the way I think about it then. Selectors seem to me as just choosing when a style should apply. They wouldn't change the priority. However, a pseudoclass for pointerover or disabled is designed and intended to apply overtop of existing styles/themes. So I guess it makes some sense for those pseudoclasses to have high priority because of their intended use case. I'm not a web dev but I probably would have designed that differently if it was me...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by-design The behavior reported in the issue is actually correct.
Projects
None yet
Development

No branches or pull requests

3 participants