Avalonia icon indicating copy to clipboard operation
Avalonia copied to clipboard

When styling multiple sliders with class selectors, only the last selector works

Open T-0pel opened this issue 1 year ago • 9 comments

Describe the bug When there are multiple styles for sliders defined using class selectors, only the last selector is valid and works.

To Reproduce

  1. Add 2 sliders with different classes to a control.
  2. Create two styles for each slider.class (they can have the same Minimum and Maximum values).
  3. Only the slider with class that is defined last in the selectors will work.

OR use the following code:

<Window.Styles>
    <Style Selector="Slider.percentSlider">
        <Setter Property="Minimum" Value="0"/>
        <Setter Property="Maximum" Value="100"/>
    </Style>
    <Style Selector="Slider.colorSlider">
        <Setter Property="Minimum" Value="0"/>
        <Setter Property="Maximum" Value="100"/>
    </Style>

</Window.Styles>

<Design.DataContext>
    <vm:MainWindowViewModel/>
</Design.DataContext>

<StackPanel>
    <Slider Classes="colorSlider"/>
    <Slider Classes="percentSlider"/>
</StackPanel>

Expected behavior Both sliders should work correctly.

Screenshots image The second slider cannot be moved

Desktop (please complete the following information):

  • OS: Windows 10
  • Version 10.14

T-0pel avatar Oct 13 '22 08:10 T-0pel

Actually only max/min not working, if you set other properties like foreground, it works well. definitely a bug.

rabbitism avatar Oct 13 '22 10:10 rabbitism

I don't think it's a bug, because they're direct properties.

jp2masa avatar Oct 13 '22 10:10 jp2masa

I don't think it's a bug, because they're direct properties.

Yeah that's true, but I think this scenario is valid and should be supported. Actually the behavior is very unreasonable. If I have multiple sliders not assigned to any class, then the local max/min values are reset to 0. you can simply add those two styles in ControlCatalog slider page, and all sliders are locked.

rabbitism avatar Oct 13 '22 10:10 rabbitism

~~I also think min and max should be styleable. I'll ask the core team if we should fix it and if that fix can be backported.~~

I was told that changing this will have other drawbacks, so data setters will not be stylable. It's by design.

timunie avatar Oct 13 '22 18:10 timunie

In general we don't make "data" properties styleable, we only make "style" properties styleable (though there are some exceptions).

Slider.Value should probably be made styleable to allow animations, but the problem is that currently styled properties don't support data validation. Not really sure why min/max should be styleable, and this issue doesn't give a use-case.

If it's simply in order to apply properties to a large number of controls without copy/pasting the property assignments, then maybe data templates would be a decent fit?

grokys avatar Oct 13 '22 20:10 grokys

To give some background: the reason we choose direct properties for "data" properties is mainly due to performance:

  • Styled properties are a lot slower than direct properties (direct properties are just CLR properties with metadata)
  • Bindings on styled properties are slower than direct properties
  • Styled properties take up a lot more space than direct properties
  • Styled properties don't support data validation currently

So for properties that will be set almost always on a control (which is usually the case for "data" properties), we tend to use direct properties to avoid these overheads.

grokys avatar Oct 13 '22 20:10 grokys

I do understand the decision of making max/min to be direct property, but the actual behavior is unexpected.

Try below in any boilerplate code. There is only one style with class selector, and all sliders are not assigned to any class. but all sliders are affected, their local max/min are ignored.

If direct property should not be assigned in style/theme, then why do we allow such syntax?

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:vm="using:SliderSample.ViewModels"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
        x:Class="SliderSample.Views.MainWindow"
        Icon="/Assets/avalonia-logo.ico"
        Title="SliderSample">

	<Window.Styles>
		<Style Selector="Slider.colorSlider">
			<Setter Property="Minimum" Value="0"/>
			<Setter Property="Maximum" Value="100"/>
		</Style>
	</Window.Styles>
    <Design.DataContext>
        <vm:MainWindowViewModel/>
    </Design.DataContext>

	<StackPanel>
		<Slider Minimum="0" Maximum="100"/>
		<Slider Minimum="0" Maximum="100"/>
	</StackPanel>

</Window>

rabbitism avatar Oct 14 '22 01:10 rabbitism

@rabbitism correct, expected behavior would be to throw an exception. And it is done with that PR https://github.com/AvaloniaUI/Avalonia/pull/8600

maxkatz6 avatar Oct 14 '22 01:10 maxkatz6

Thanks for the detailed explanation and discussion. I agree that throwing an exception would be best since this is not supported. The use case was indeed to apply the same properties to a large number of controls, so I will try using data templates to achieve that.

One more weird thing that I do not understand - the styling of data properties seems to work correctly when I set the Maximum to a Binding. That is how it originally was in the code I have inherited. Do you know why that works if it is not supported?

image

<Window.Styles>
    <Style Selector="Slider.percentSlider">
        <Setter Property="Minimum" Value="0"/>
        <Setter Property="Maximum" Value="{Binding Percent}"/>
    </Style>
    <Style Selector="Slider.colorSlider">
        <Setter Property="Minimum" Value="0"/>
        <Setter Property="Maximum" Value="{Binding Percent2}"/>
    </Style>
</Window.Styles>

<Design.DataContext>
    <vm:MainWindowViewModel/>
</Design.DataContext>

<StackPanel>
    <TextBox Text="{Binding Slider1}"></TextBox>
    <Slider Classes="colorSlider" Value="{Binding Slider1}"/>
    <TextBox Text="{Binding Slider2}"></TextBox>
    <Slider Classes="percentSlider" Value="{Binding Slider2}"/>
</StackPanel>
public class MainWindowViewModel : ViewModelBase
{
    private int _slider1;
    private int _slider2;

    public int Percent => 100;
    public int Percent2 => 1000;

    public int Slider1
    {
        get => _slider1;
        set => this.RaiseAndSetIfChanged(ref _slider1, value);
    }

    public int Slider2
    {
        get => _slider2;
        set => this.RaiseAndSetIfChanged(ref _slider2, value);
    }
}

T-0pel avatar Oct 14 '22 06:10 T-0pel