From f0e93683f00df286384f77405f481a992fb0ab66 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 6 May 2023 20:51:59 -0400 Subject: [PATCH 01/29] Add special coercion logic to initial ColorSpectrum selection This specially handles #00000000 to be much more intuitive for end-users. --- .../ColorSpectrum/ColorSpectrum.cs | 86 ++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs b/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs index 9198a2f237..98a721ec91 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs @@ -45,6 +45,7 @@ namespace Avalonia.Controls.Primitives private bool _updatingColor = false; private bool _updatingHsvColor = false; + private bool _coercedInitialColor = false; private bool _isPointerPressed = false; private bool _shouldShowLargeSelection = false; private List _hsvValues = new List(); @@ -601,14 +602,97 @@ namespace Avalonia.Controls.Primitives } } + /// + /// Changes the currently selected color (always in HSV) and applies all necessary updates. + /// + /// + /// Some additional logic is applied in certain situations to coerce and sync color values. + /// Use this method instead of update the or directly. + /// + /// The new HSV color to change to. private void UpdateColor(Hsv newHsv) { _updatingColor = true; _updatingHsvColor = true; - Rgb newRgb = newHsv.ToRgb(); double alpha = HsvColor.A; + // It is common for the ColorPicker (and therefore the Spectrum) to be initialized + // with a #00000000 color value in some use cases. This is usually used to indicate + // that no color has been selected by the user. Note that #00000000 is different than + // #00FFFFFF (Transparent). + // + // In this situation, the first time the user clicks on the spectrum the third + // component and alpha component will remain zero. This is because the spectrum only + // controls two components at any given time. + // + // This is very unintuitive from a user-standpoint as after the user clicks on the + // spectrum they must then increase the alpha and then the third component sliders + // to the desired value. In fact, until they increase these slider values no color + // will show at all since it is fully transparent and black. In almost all cases + // though the desired value is simply full color. + // + // To work around this usability issue with an initial #00000000 color, the selected + // color is coerced (only the first time) into a color with maximum third component + // value and maximum alpha. This can only happen once and only if those two components + // are already zero. + // + // Also note this is NOT currently done for #00FFFFFF (Transparent) but based on + // further usability study that case may need to be handled here as well. Right now + // Transparent is treated as a normal color value with the alpha intentionally set + // to zero so the alpha slider must still be adjusted after the spectrum. + if (!_coercedInitialColor && + IsLoaded) + { + bool isAlphaComponentZero = (alpha == 0.0); + bool isThirdComponentZero = false; + + switch (Components) + { + case ColorSpectrumComponents.HueValue: + case ColorSpectrumComponents.ValueHue: + isThirdComponentZero = (newHsv.S == 0.0); + break; + + case ColorSpectrumComponents.HueSaturation: + case ColorSpectrumComponents.SaturationHue: + isThirdComponentZero = (newHsv.V == 0.0); + break; + + case ColorSpectrumComponents.ValueSaturation: + case ColorSpectrumComponents.SaturationValue: + isThirdComponentZero = (newHsv.H == 0.0); + break; + } + + if (isAlphaComponentZero && isThirdComponentZero) + { + alpha = 1.0; + + switch (Components) + { + case ColorSpectrumComponents.HueValue: + case ColorSpectrumComponents.ValueHue: + newHsv.S = 1.0; + break; + + case ColorSpectrumComponents.HueSaturation: + case ColorSpectrumComponents.SaturationHue: + newHsv.V = 1.0; + break; + + case ColorSpectrumComponents.ValueSaturation: + case ColorSpectrumComponents.SaturationValue: + newHsv.H = 1.0; + break; + } + + _coercedInitialColor = true; + } + } + + Rgb newRgb = newHsv.ToRgb(); + SetCurrentValue(ColorProperty, newRgb.ToColor(alpha)); SetCurrentValue(HsvColorProperty, newHsv.ToHsvColor(alpha)); From 4fd4970ce7c0cab42830f2d061798bb7a2622421 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 6 May 2023 21:38:15 -0400 Subject: [PATCH 02/29] Fix Hue in new ColorSpectrum coercion logic --- .../ColorSpectrum/ColorSpectrum.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs b/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs index 98a721ec91..6683346eeb 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSpectrum/ColorSpectrum.cs @@ -683,7 +683,12 @@ namespace Avalonia.Controls.Primitives case ColorSpectrumComponents.ValueSaturation: case ColorSpectrumComponents.SaturationValue: - newHsv.H = 1.0; + // Hue is mathematically NOT a special case; however, is one conceptually. + // It doesn't make sense to change the selected Hue value, so why is it set here? + // Setting to 360.0 is equivalent to the max set for other components and is + // internally wrapped back to 0.0 (since 360 degrees = 0 degrees). + // This means effectively there is no change to the hue component value. + newHsv.H = 360.0; break; } From 2f2f290e7f427c1d8847e6f088b6c8825f4b7afc Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 6 May 2023 21:40:37 -0400 Subject: [PATCH 03/29] Always force max saturation and value in the third component slider --- .../Themes/Fluent/ColorPicker.xaml | 2 +- src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml | 2 +- .../Themes/Simple/ColorPicker.xaml | 2 +- src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml index b3c7cd9f9c..73dbb3b527 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml @@ -114,7 +114,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaMaxForced="True" - IsSaturationValueMaxForced="False" + IsSaturationValueMaxForced="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml index acd2c7ff15..14a10f4943 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml @@ -361,7 +361,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaMaxForced="True" - IsSaturationValueMaxForced="False" + IsSaturationValueMaxForced="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml index ff4e1d93a8..b7f07b1ef0 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml @@ -113,7 +113,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaMaxForced="True" - IsSaturationValueMaxForced="False" + IsSaturationValueMaxForced="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml index a26d3179b5..7279ef6369 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml @@ -323,7 +323,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaMaxForced="True" - IsSaturationValueMaxForced="False" + IsSaturationValueMaxForced="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" From 0c223cbcee5d56017a9304351c145c87a0921831 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 6 May 2023 22:31:48 -0400 Subject: [PATCH 04/29] Rename ColorSlider.IsAlphaMaxForced to IsAlphaVisible --- .../ColorSlider/ColorSlider.Properties.cs | 25 +++++++++++-------- .../ColorSlider/ColorSlider.cs | 15 ++++++----- .../Helpers/ColorPickerHelpers.cs | 8 +++--- .../Themes/Fluent/ColorPicker.xaml | 2 +- .../Themes/Fluent/ColorView.xaml | 2 +- .../Themes/Simple/ColorPicker.xaml | 2 +- .../Themes/Simple/ColorView.xaml | 2 +- 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs index e2a34a7f90..ef734cf1db 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs @@ -41,12 +41,12 @@ namespace Avalonia.Controls.Primitives defaultBindingMode: BindingMode.TwoWay); /// - /// Defines the property. + /// Defines the property. /// - public static readonly StyledProperty IsAlphaMaxForcedProperty = + public static readonly StyledProperty IsAlphaVisibleProperty = AvaloniaProperty.Register( - nameof(IsAlphaMaxForced), - true); + nameof(IsAlphaVisible), + false); /// /// Defines the property. @@ -109,14 +109,19 @@ namespace Avalonia.Controls.Primitives } /// - /// Gets or sets a value indicating whether the alpha component is always forced to maximum for components - /// other than . - /// This ensures that the background is always visible and never transparent regardless of the actual color. + /// Gets or sets a value indicating whether the alpha component is visible and rendered in the spectrum. + /// When false, this ensures that the spectrum is always visible and never transparent regardless of + /// the actual color. /// - public bool IsAlphaMaxForced + /// + /// Setting to false means the alpha component is always forced to maximum for components other than + /// during rendering. This doesn't change the value of the alpha component + /// in the color – it is only for display. + /// + public bool IsAlphaVisible { - get => GetValue(IsAlphaMaxForcedProperty); - set => SetValue(IsAlphaMaxForcedProperty, value); + get => GetValue(IsAlphaVisibleProperty); + set => SetValue(IsAlphaVisibleProperty, value); } /// diff --git a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs index ce47a797ec..6aaf6aab35 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs @@ -52,8 +52,7 @@ namespace Avalonia.Controls.Primitives // This means under a certain alpha threshold, neither a white or black selector thumb // should be shown and instead the default slider thumb color should be used instead. if (Color.A < 128 && - (IsAlphaMaxForced == false || - ColorComponent == ColorComponent.Alpha)) + (IsAlphaVisible || ColorComponent == ColorComponent.Alpha)) { PseudoClasses.Set(pcDarkSelector, false); PseudoClasses.Set(pcLightSelector, false); @@ -122,7 +121,7 @@ namespace Avalonia.Controls.Primitives ColorModel, ColorComponent, HsvColor, - IsAlphaMaxForced, + IsAlphaVisible, IsSaturationValueMaxForced); if (_backgroundBitmap != null) @@ -319,10 +318,10 @@ namespace Avalonia.Controls.Primitives private HsvColor GetEquivalentBackgroundColor(HsvColor hsvColor) { var component = ColorComponent; - var isAlphaMaxForced = IsAlphaMaxForced; + var isAlphaVisible = IsAlphaVisible; var isSaturationValueMaxForced = IsSaturationValueMaxForced; - if (isAlphaMaxForced && + if (isAlphaVisible == false && component != ColorComponent.Alpha) { hsvColor = new HsvColor(1.0, hsvColor.H, hsvColor.S, hsvColor.V); @@ -362,9 +361,9 @@ namespace Avalonia.Controls.Primitives private Color GetEquivalentBackgroundColor(Color rgbColor) { var component = ColorComponent; - var isAlphaMaxForced = IsAlphaMaxForced; + var isAlphaVisible = IsAlphaVisible; - if (isAlphaMaxForced && + if (isAlphaVisible == false && component != ColorComponent.Alpha) { rgbColor = new Color(255, rgbColor.R, rgbColor.G, rgbColor.B); @@ -401,7 +400,7 @@ namespace Avalonia.Controls.Primitives } else if (change.Property == ColorComponentProperty || change.Property == ColorModelProperty || - change.Property == IsAlphaMaxForcedProperty || + change.Property == IsAlphaVisibleProperty || change.Property == IsSaturationValueMaxForcedProperty) { ignorePropertyChanged = true; diff --git a/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs b/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs index dbd92d4ac5..cb64b3f6d5 100644 --- a/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs +++ b/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs @@ -29,8 +29,8 @@ namespace Avalonia.Controls.Primitives /// The color model being used: RGBA or HSVA. /// The specific color component to sweep. /// The base HSV color used for components not being changed. - /// Fix the alpha component value to maximum during calculation. - /// This will remove any alpha/transparency from the other component backgrounds. + /// Whether the alpha component is visible and rendered in the bitmap. + /// This property is ignored when the alpha component itself is being rendered. /// Fix the saturation and value components to maximum /// during calculation with the HSVA color model. /// This will ensure colors are always discernible regardless of saturation/value. @@ -42,7 +42,7 @@ namespace Avalonia.Controls.Primitives ColorModel colorModel, ColorComponent component, HsvColor baseHsvColor, - bool isAlphaMaxForced, + bool isAlphaVisible, bool isSaturationValueMaxForced) { if (width == 0 || height == 0) @@ -67,7 +67,7 @@ namespace Avalonia.Controls.Primitives bgraPixelDataWidth = width * 4; // Maximize alpha component value - if (isAlphaMaxForced && + if (isAlphaVisible == false && component != ColorComponent.Alpha) { baseHsvColor = new HsvColor(1.0, baseHsvColor.H, baseHsvColor.S, baseHsvColor.V); diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml index 73dbb3b527..592494a1e3 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml @@ -113,7 +113,7 @@ Date: Sat, 6 May 2023 23:45:50 -0400 Subject: [PATCH 05/29] Replace ColorSlider.IsSaturationValueMaxForced with more powerful/general-purpose IsPerceptive --- .../ColorSlider/ColorSlider.Properties.cs | 53 ++++++++------ .../ColorSlider/ColorSlider.cs | 73 +++++++++++-------- .../Helpers/ColorPickerHelpers.cs | 52 ++++++++----- .../Themes/Fluent/ColorPicker.xaml | 17 ++++- .../Themes/Fluent/ColorView.xaml | 17 ++++- .../Themes/Simple/ColorPicker.xaml | 17 ++++- .../Themes/Simple/ColorView.xaml | 17 ++++- 7 files changed, 173 insertions(+), 73 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs index ef734cf1db..b2deb913f1 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs @@ -49,20 +49,20 @@ namespace Avalonia.Controls.Primitives false); /// - /// Defines the property. + /// Defines the property. /// - public static readonly StyledProperty IsRoundingEnabledProperty = + public static readonly StyledProperty IsPerceptiveProperty = AvaloniaProperty.Register( - nameof(IsRoundingEnabled), - false); + nameof(IsPerceptive), + true); /// - /// Defines the property. + /// Defines the property. /// - public static readonly StyledProperty IsSaturationValueMaxForcedProperty = + public static readonly StyledProperty IsRoundingEnabledProperty = AvaloniaProperty.Register( - nameof(IsSaturationValueMaxForced), - true); + nameof(IsRoundingEnabled), + false); /// /// Gets or sets the currently selected color in the RGB color model. @@ -109,9 +109,9 @@ namespace Avalonia.Controls.Primitives } /// - /// Gets or sets a value indicating whether the alpha component is visible and rendered in the spectrum. - /// When false, this ensures that the spectrum is always visible and never transparent regardless of - /// the actual color. + /// Gets or sets a value indicating whether the alpha component is visible and rendered. + /// When false, this ensures that the gradient is always visible and never transparent regardless of + /// the actual color. This property is ignored when the alpha component itself is being displayed. /// /// /// Setting to false means the alpha component is always forced to maximum for components other than @@ -124,6 +124,26 @@ namespace Avalonia.Controls.Primitives set => SetValue(IsAlphaVisibleProperty, value); } + /// + /// Gets or sets a value indicating whether the slider adapts rendering to improve user-perception + /// over exactness. When true in the HSVA color model, this ensures that the gradient is always visible and + /// never washed out regardless of the actual color. When true in the RGBA color model, this ensures + /// the components always appear as red, green or blue. + /// + /// + /// For example, with Hue in the HSVA color model, the Saturation and Value components are always forced + /// to maximum values during rendering. In the RGBA color model, all components other than + /// are forced to minimum values during rendering. + ///

+ /// Also note this property will only adjust components other than during rendering. + /// This doesn't change the values of any components in the color – it is only for display. + /// + public bool IsPerceptive + { + get => GetValue(IsPerceptiveProperty); + set => SetValue(IsPerceptiveProperty, value); + } + /// /// Gets or sets a value indicating whether rounding of color component values is enabled. /// @@ -136,16 +156,5 @@ namespace Avalonia.Controls.Primitives get => GetValue(IsRoundingEnabledProperty); set => SetValue(IsRoundingEnabledProperty, value); } - - /// - /// Gets or sets a value indicating whether the saturation and value components are always forced to maximum values - /// when using the HSVA color model. Only component values other than will be changed. - /// This ensures, for example, that the Hue background is always visible and never washed out regardless of the actual color. - /// - public bool IsSaturationValueMaxForced - { - get => GetValue(IsSaturationValueMaxForcedProperty); - set => SetValue(IsSaturationValueMaxForcedProperty, value); - } } } diff --git a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs index 6aaf6aab35..a6d9ca459f 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.cs @@ -63,11 +63,11 @@ namespace Avalonia.Controls.Primitives if (ColorModel == ColorModel.Hsva) { - perceivedColor = GetEquivalentBackgroundColor(HsvColor).ToRgb(); + perceivedColor = GetPerceptiveBackgroundColor(HsvColor).ToRgb(); } else { - perceivedColor = GetEquivalentBackgroundColor(Color); + perceivedColor = GetPerceptiveBackgroundColor(Color); } if (ColorHelper.GetRelativeLuminance(perceivedColor) <= 0.5) @@ -107,7 +107,7 @@ namespace Avalonia.Controls.Primitives { // As a fallback, attempt to calculate using the overall control size // This shouldn't happen as a track is a required template part of a slider - // However, if it does, the spectrum will still be shown + // However, if it does, the spectrum gradient will still be shown pixelWidth = Convert.ToInt32(Bounds.Width * scale); pixelHeight = Convert.ToInt32(Bounds.Height * scale); } @@ -122,7 +122,7 @@ namespace Avalonia.Controls.Primitives ColorComponent, HsvColor, IsAlphaVisible, - IsSaturationValueMaxForced); + IsPerceptive); if (_backgroundBitmap != null) { @@ -315,11 +315,11 @@ namespace Avalonia.Controls.Primitives /// /// The actual color to get the equivalent background color for. /// The equivalent, perceived background color. - private HsvColor GetEquivalentBackgroundColor(HsvColor hsvColor) + private HsvColor GetPerceptiveBackgroundColor(HsvColor hsvColor) { var component = ColorComponent; var isAlphaVisible = IsAlphaVisible; - var isSaturationValueMaxForced = IsSaturationValueMaxForced; + var isPerceptive = IsPerceptive; if (isAlphaVisible == false && component != ColorComponent.Alpha) @@ -327,28 +327,23 @@ namespace Avalonia.Controls.Primitives hsvColor = new HsvColor(1.0, hsvColor.H, hsvColor.S, hsvColor.V); } - switch (component) + if (isPerceptive) { - case ColorComponent.Component1: - return new HsvColor( - hsvColor.A, - hsvColor.H, - isSaturationValueMaxForced ? 1.0 : hsvColor.S, - isSaturationValueMaxForced ? 1.0 : hsvColor.V); - case ColorComponent.Component2: - return new HsvColor( - hsvColor.A, - hsvColor.H, - hsvColor.S, - isSaturationValueMaxForced ? 1.0 : hsvColor.V); - case ColorComponent.Component3: - return new HsvColor( - hsvColor.A, - hsvColor.H, - isSaturationValueMaxForced ? 1.0 : hsvColor.S, - hsvColor.V); - default: - return hsvColor; + switch (component) + { + case ColorComponent.Component1: + return new HsvColor(hsvColor.A, hsvColor.H, 1.0, 1.0); + case ColorComponent.Component2: + return new HsvColor(hsvColor.A, hsvColor.H, hsvColor.S, 1.0); + case ColorComponent.Component3: + return new HsvColor(hsvColor.A, hsvColor.H, 1.0, hsvColor.V); + default: + return hsvColor; + } + } + else + { + return hsvColor; } } @@ -358,10 +353,11 @@ namespace Avalonia.Controls.Primitives /// /// The actual color to get the equivalent background color for. /// The equivalent, perceived background color. - private Color GetEquivalentBackgroundColor(Color rgbColor) + private Color GetPerceptiveBackgroundColor(Color rgbColor) { var component = ColorComponent; var isAlphaVisible = IsAlphaVisible; + var isPerceptive = IsPerceptive; if (isAlphaVisible == false && component != ColorComponent.Alpha) @@ -369,7 +365,24 @@ namespace Avalonia.Controls.Primitives rgbColor = new Color(255, rgbColor.R, rgbColor.G, rgbColor.B); } - return rgbColor; + if (isPerceptive) + { + switch (component) + { + case ColorComponent.Component1: + return new Color(rgbColor.A, rgbColor.R, 0, 0); + case ColorComponent.Component2: + return new Color(rgbColor.A, 0, rgbColor.G, 0); + case ColorComponent.Component3: + return new Color(rgbColor.A, 0, 0, rgbColor.B); + default: + return rgbColor; + } + } + else + { + return rgbColor; + } } /// @@ -401,7 +414,7 @@ namespace Avalonia.Controls.Primitives else if (change.Property == ColorComponentProperty || change.Property == ColorModelProperty || change.Property == IsAlphaVisibleProperty || - change.Property == IsSaturationValueMaxForcedProperty) + change.Property == IsPerceptiveProperty) { ignorePropertyChanged = true; diff --git a/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs b/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs index cb64b3f6d5..c2332751af 100644 --- a/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs +++ b/src/Avalonia.Controls.ColorPicker/Helpers/ColorPickerHelpers.cs @@ -31,9 +31,8 @@ namespace Avalonia.Controls.Primitives /// The base HSV color used for components not being changed. /// Whether the alpha component is visible and rendered in the bitmap. /// This property is ignored when the alpha component itself is being rendered. - /// Fix the saturation and value components to maximum - /// during calculation with the HSVA color model. - /// This will ensure colors are always discernible regardless of saturation/value. + /// Whether the slider adapts rendering to improve user-perception over exactness. + /// This will ensure colors are always discernible. /// A new bitmap representing a gradient of color component values. public static async Task> CreateComponentBitmapAsync( int width, @@ -43,7 +42,7 @@ namespace Avalonia.Controls.Primitives ColorComponent component, HsvColor baseHsvColor, bool isAlphaVisible, - bool isSaturationValueMaxForced) + bool isPerceptive) { if (width == 0 || height == 0) { @@ -79,22 +78,41 @@ namespace Avalonia.Controls.Primitives baseRgbColor = baseHsvColor.ToRgb(); } - // Maximize Saturation and Value components when in HSVA mode - if (isSaturationValueMaxForced && - colorModel == ColorModel.Hsva && + // Apply any perceptive adjustments to the color + if (isPerceptive && component != ColorComponent.Alpha) { - switch (component) + if (colorModel == ColorModel.Hsva) { - case ColorComponent.Component1: - baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, 1.0, 1.0); - break; - case ColorComponent.Component2: - baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, baseHsvColor.S, 1.0); - break; - case ColorComponent.Component3: - baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, 1.0, baseHsvColor.V); - break; + // Maximize Saturation and Value components + switch (component) + { + case ColorComponent.Component1: // Hue + baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, 1.0, 1.0); + break; + case ColorComponent.Component2: // Saturation + baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, baseHsvColor.S, 1.0); + break; + case ColorComponent.Component3: // Value + baseHsvColor = new HsvColor(baseHsvColor.A, baseHsvColor.H, 1.0, baseHsvColor.V); + break; + } + } + else + { + // Minimize component values other than the current one + switch (component) + { + case ColorComponent.Component1: // Red + baseRgbColor = new Color(baseRgbColor.A, baseRgbColor.R, 0, 0); + break; + case ColorComponent.Component2: // Green + baseRgbColor = new Color(baseRgbColor.A, 0, baseRgbColor.G, 0); + break; + case ColorComponent.Component3: // Blue + baseRgbColor = new Color(baseRgbColor.A, 0, 0, baseRgbColor.B); + break; + } } } diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml index 592494a1e3..a0daff2e3f 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml @@ -114,7 +114,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsSaturationValueMaxForced="True" + IsPerceptive="False" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -502,6 +502,21 @@ + + + + + + + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml index 0f5a93e75b..e7db283c9e 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml @@ -361,7 +361,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsSaturationValueMaxForced="True" + IsPerceptive="False" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -746,6 +746,21 @@ + + + + + + + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml index 131ea41eba..bad4ae9e44 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml @@ -113,7 +113,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsSaturationValueMaxForced="True" + IsPerceptive="False" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -501,6 +501,21 @@ + + + + + + + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml index c389ac310a..a3cb7bca1d 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml @@ -323,7 +323,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsSaturationValueMaxForced="True" + IsPerceptive="False" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -708,6 +708,21 @@ + + + + + + + From 31a1e514fa9d4075386b032b117fb46eab1b56b7 Mon Sep 17 00:00:00 2001 From: robloo Date: Sun, 7 May 2023 10:11:34 -0400 Subject: [PATCH 06/29] Improve ColorPreviewer margins with drop shadow This sets ClipToBounds to false for the control which allows the drop shadow to fully draw above/below the control itself. Doing this removes the need to adjust margins in almost all places. This is a big simplification to margin calculation and removes strange negative values. --- samples/ControlCatalog/Pages/ColorPickerPage.xaml | 3 ++- .../Themes/Fluent/ColorPicker.xaml | 4 ++-- .../Themes/Fluent/ColorPreviewer.xaml | 13 +++++-------- .../Themes/Fluent/ColorView.xaml | 4 ++-- .../Themes/Simple/ColorPicker.xaml | 4 ++-- .../Themes/Simple/ColorPreviewer.xaml | 13 +++++-------- .../Themes/Simple/ColorView.xaml | 4 ++-- 7 files changed, 20 insertions(+), 25 deletions(-) diff --git a/samples/ControlCatalog/Pages/ColorPickerPage.xaml b/samples/ControlCatalog/Pages/ColorPickerPage.xaml index b759720cf2..25fffabfd2 100644 --- a/samples/ControlCatalog/Pages/ColorPickerPage.xaml +++ b/samples/ControlCatalog/Pages/ColorPickerPage.xaml @@ -103,7 +103,8 @@ HsvColor="{Binding HsvColor, ElementName=ColorSpectrum1}" />--> + HsvColor="{Binding HsvColor, ElementName=ColorSpectrum1}" + Margin="0,2,0,0" /> diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml index a0daff2e3f..bbb3c0c01a 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml @@ -490,11 +490,11 @@ - + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPreviewer.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPreviewer.xaml index e05fa5a907..fabc5d0349 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPreviewer.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPreviewer.xaml @@ -8,7 +8,9 @@ - + + + @@ -21,7 +23,6 @@ Height="{StaticResource ColorPreviewerAccentSectionHeight}" Width="{StaticResource ColorPreviewerAccentSectionWidth}" ColumnDefinitions="*,*" - Margin="0,0,-10,0" VerticalAlignment="Center"> + CornerRadius="{TemplateBinding CornerRadius}"> @@ -82,8 +80,7 @@ + VerticalAlignment="Stretch"> diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml index e7db283c9e..6159a96cc5 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml @@ -737,11 +737,11 @@ - + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml index bad4ae9e44..82c98e22f7 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml @@ -489,11 +489,11 @@ - + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPreviewer.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPreviewer.xaml index a39dd91f52..9e123b2a1f 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPreviewer.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPreviewer.xaml @@ -8,7 +8,9 @@ - + + + @@ -21,7 +23,6 @@ Height="{StaticResource ColorPreviewerAccentSectionHeight}" Width="{StaticResource ColorPreviewerAccentSectionWidth}" ColumnDefinitions="*,*" - Margin="0,0,-10,0" VerticalAlignment="Center"> + CornerRadius="{TemplateBinding CornerRadius}"> @@ -82,8 +80,7 @@ + VerticalAlignment="Stretch"> diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml index a3cb7bca1d..97a8d4b885 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml @@ -699,11 +699,11 @@ - + From 6ddef20ef7847c05855b1ee3af8958a757ffc884 Mon Sep 17 00:00:00 2001 From: robloo Date: Sun, 7 May 2023 10:17:19 -0400 Subject: [PATCH 07/29] Improve comments --- .../ColorView/ColorView.Properties.cs | 6 ++++++ src/Avalonia.Controls.ColorPicker/Helpers/Hsv.cs | 2 +- src/Avalonia.Controls.ColorPicker/Helpers/Rgb.cs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorView/ColorView.Properties.cs b/src/Avalonia.Controls.ColorPicker/ColorView/ColorView.Properties.cs index 532e87a9fc..701dab97f7 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorView/ColorView.Properties.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorView/ColorView.Properties.cs @@ -504,6 +504,12 @@ namespace Avalonia.Controls /// /// Gets or sets the index of the selected tab/panel/page (subview). /// + /// + /// When using the default control theme, this property is designed to be used with the + /// enum. The enum defines the + /// index values of each of the three standard tabs. + /// Use like `SelectedIndex = (int)ColorViewTab.Palette`. + /// public int SelectedIndex { get => GetValue(SelectedIndexProperty); diff --git a/src/Avalonia.Controls.ColorPicker/Helpers/Hsv.cs b/src/Avalonia.Controls.ColorPicker/Helpers/Hsv.cs index 8a425b9581..76f33b3d83 100644 --- a/src/Avalonia.Controls.ColorPicker/Helpers/Hsv.cs +++ b/src/Avalonia.Controls.ColorPicker/Helpers/Hsv.cs @@ -11,7 +11,7 @@ namespace Avalonia.Controls.Primitives /// Contains and allows modification of Hue, Saturation and Value components. /// /// - /// The is a specialized struct optimized for permanence and memory: + /// The is a specialized struct optimized for performance and memory: /// /// This is not a read-only struct like and allows editing the fields /// Removes the alpha component unnecessary in core calculations diff --git a/src/Avalonia.Controls.ColorPicker/Helpers/Rgb.cs b/src/Avalonia.Controls.ColorPicker/Helpers/Rgb.cs index 72e3821c2b..ee81a22ecf 100644 --- a/src/Avalonia.Controls.ColorPicker/Helpers/Rgb.cs +++ b/src/Avalonia.Controls.ColorPicker/Helpers/Rgb.cs @@ -12,7 +12,7 @@ namespace Avalonia.Controls.Primitives /// Contains and allows modification of Red, Green and Blue components. /// /// - /// The is a specialized struct optimized for permanence and memory: + /// The is a specialized struct optimized for performance and memory: /// /// This is not a read-only struct like and allows editing the fields /// Removes the alpha component unnecessary in core calculations From 8fed1f30f0ef19703a1e4fc7dddd12aab363012a Mon Sep 17 00:00:00 2001 From: robloo Date: Thu, 11 May 2023 21:37:05 -0400 Subject: [PATCH 08/29] Always enable ColorSlider.IsPerceptive This works-around #11271 and stabilizes the API. It can be revisited later. --- .../Themes/Fluent/ColorPicker.xaml | 4 +++- .../Themes/Fluent/ColorView.xaml | 4 +++- .../Themes/Simple/ColorPicker.xaml | 4 +++- .../Themes/Simple/ColorView.xaml | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml index bbb3c0c01a..f3100a648a 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorPicker.xaml @@ -114,7 +114,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsPerceptive="False" + IsPerceptive="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -503,6 +503,7 @@ + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml index 6159a96cc5..8793467b36 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Fluent/ColorView.xaml @@ -361,7 +361,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsPerceptive="False" + IsPerceptive="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -747,6 +747,7 @@ + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml index 82c98e22f7..d9ba8bb9d2 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorPicker.xaml @@ -113,7 +113,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsPerceptive="False" + IsPerceptive="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -502,6 +502,7 @@ + diff --git a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml index 97a8d4b885..d4f02933f2 100644 --- a/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml +++ b/src/Avalonia.Controls.ColorPicker/Themes/Simple/ColorView.xaml @@ -323,7 +323,7 @@ AutomationProperties.Name="Third Component" Grid.Column="0" IsAlphaVisible="False" - IsPerceptive="False" + IsPerceptive="True" Orientation="Vertical" ColorModel="Hsva" ColorComponent="{Binding ThirdComponent, ElementName=ColorSpectrum}" @@ -709,6 +709,7 @@ + From 1d5703e5657ac6e9f57c7a0bb40e558c179b89fa Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 13 May 2023 11:44:48 -0400 Subject: [PATCH 09/29] Remove static color conversion methods This functionality is already provided by, for example, HsvColor.ToRgb(). Having these separate methods just clutters the API. --- src/Avalonia.Base/Media/Color.cs | 30 ----------------------------- src/Avalonia.Base/Media/HslColor.cs | 12 +----------- src/Avalonia.Base/Media/HsvColor.cs | 12 +----------- 3 files changed, 2 insertions(+), 52 deletions(-) diff --git a/src/Avalonia.Base/Media/Color.cs b/src/Avalonia.Base/Media/Color.cs index 50c2faacc0..7d74b4d602 100644 --- a/src/Avalonia.Base/Media/Color.cs +++ b/src/Avalonia.Base/Media/Color.cs @@ -517,21 +517,6 @@ namespace Avalonia.Media } } - /// - /// Converts the given RGB color to its HSL color equivalent. - /// - /// The color in the RGB color model. - /// A new equivalent to the given RGBA values. - public static HslColor ToHsl(Color color) - { - // Normalize RGBA components into the 0..1 range - return Color.ToHsl( - (byteToDouble * color.R), - (byteToDouble * color.G), - (byteToDouble * color.B), - (byteToDouble * color.A)); - } - /// /// Converts the given RGBA color component values to their HSL color equivalent. /// @@ -606,21 +591,6 @@ namespace Avalonia.Media return new HslColor(a, 60 * h1, saturation, lightness, clampValues: false); } - /// - /// Converts the given RGB color to its HSV color equivalent. - /// - /// The color in the RGB color model. - /// A new equivalent to the given RGBA values. - public static HsvColor ToHsv(Color color) - { - // Normalize RGBA components into the 0..1 range - return Color.ToHsv( - (byteToDouble * color.R), - (byteToDouble * color.G), - (byteToDouble * color.B), - (byteToDouble * color.A)); - } - /// /// Converts the given RGBA color component values to their HSV color equivalent. /// diff --git a/src/Avalonia.Base/Media/HslColor.cs b/src/Avalonia.Base/Media/HslColor.cs index 897c883875..624cc88ad4 100644 --- a/src/Avalonia.Base/Media/HslColor.cs +++ b/src/Avalonia.Base/Media/HslColor.cs @@ -90,7 +90,7 @@ namespace Avalonia.Media /// The RGB color to convert to HSL. public HslColor(Color color) { - var hsl = Color.ToHsl(color); + var hsl = color.ToHsl(); A = hsl.A; H = hsl.H; @@ -349,16 +349,6 @@ namespace Avalonia.Media return new HslColor(1.0, h, s, l); } - /// - /// Converts the given HSL color to its RGB color equivalent. - /// - /// The color in the HSL color model. - /// A new RGB equivalent to the given HSLA values. - public static Color ToRgb(HslColor hslColor) - { - return HslColor.ToRgb(hslColor.H, hslColor.S, hslColor.L, hslColor.A); - } - /// /// Converts the given HSLA color component values to their RGB color equivalent. /// diff --git a/src/Avalonia.Base/Media/HsvColor.cs b/src/Avalonia.Base/Media/HsvColor.cs index df68252065..3c6336c445 100644 --- a/src/Avalonia.Base/Media/HsvColor.cs +++ b/src/Avalonia.Base/Media/HsvColor.cs @@ -90,7 +90,7 @@ namespace Avalonia.Media /// The RGB color to convert to HSV. public HsvColor(Color color) { - var hsv = Color.ToHsv(color); + var hsv = color.ToHsv(); A = hsv.A; H = hsv.H; @@ -379,16 +379,6 @@ namespace Avalonia.Media return new HsvColor(1.0, h, s, v); } - /// - /// Converts the given HSV color to its RGB color equivalent. - /// - /// The color in the HSV color model. - /// A new RGB equivalent to the given HSVA values. - public static Color ToRgb(HsvColor hsvColor) - { - return HsvColor.ToRgb(hsvColor.H, hsvColor.S, hsvColor.V, hsvColor.A); - } - /// /// Converts the given HSVA color component values to their RGB color equivalent. /// From fac0a045d47e8e5a10ad52393409e62619b2fb55 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 13 May 2023 11:46:16 -0400 Subject: [PATCH 10/29] Add direct conversions for HsvColor.ToHsl() and HslColor.ToHsv() --- src/Avalonia.Base/Media/Color.cs | 2 - src/Avalonia.Base/Media/HslColor.cs | 66 ++++++++++++++++++- src/Avalonia.Base/Media/HsvColor.cs | 66 ++++++++++++++++++- .../Media/ColorTests.cs | 29 ++++++++ 4 files changed, 157 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Media/Color.cs b/src/Avalonia.Base/Media/Color.cs index 7d74b4d602..3ee151389a 100644 --- a/src/Avalonia.Base/Media/Color.cs +++ b/src/Avalonia.Base/Media/Color.cs @@ -478,7 +478,6 @@ namespace Avalonia.Media /// The HSL equivalent color. public HslColor ToHsl() { - // Don't use the HslColor(Color) constructor to avoid an extra HslColor return Color.ToHsl(R, G, B, A); } @@ -488,7 +487,6 @@ namespace Avalonia.Media /// The HSV equivalent color. public HsvColor ToHsv() { - // Don't use the HsvColor(Color) constructor to avoid an extra HsvColor return Color.ToHsv(R, G, B, A); } diff --git a/src/Avalonia.Base/Media/HslColor.cs b/src/Avalonia.Base/Media/HslColor.cs index 624cc88ad4..84f2149367 100644 --- a/src/Avalonia.Base/Media/HslColor.cs +++ b/src/Avalonia.Base/Media/HslColor.cs @@ -165,10 +165,18 @@ namespace Avalonia.Media /// The RGB equivalent color. public Color ToRgb() { - // Use the by-component conversion method directly for performance return HslColor.ToRgb(H, S, L, A); } + /// + /// Returns the HSV color model equivalent of this HSL color. + /// + /// The HSV equivalent color. + public HsvColor ToHsv() + { + return HslColor.ToHsv(H, S, L, A); + } + /// public override string ToString() { @@ -432,13 +440,67 @@ namespace Avalonia.Media b1 = x; } - return Color.FromArgb( + return new Color( (byte)Math.Round(255 * alpha), (byte)Math.Round(255 * (r1 + m)), (byte)Math.Round(255 * (g1 + m)), (byte)Math.Round(255 * (b1 + m))); } + /// + /// Converts the given HSLA color component values to their HSV color equivalent. + /// + /// The Hue component in the HSL color model in the range from 0..360. + /// The Saturation component in the HSL color model in the range from 0..1. + /// The Lightness component in the HSL color model in the range from 0..1. + /// The Alpha component in the range from 0..1. + /// A new equivalent to the given HSLA values. + public static HsvColor ToHsv( + double hue, + double saturation, + double lightness, + double alpha = 1.0) + { + // We want the hue to be between 0 and 359, + // so we first ensure that that's the case. + while (hue >= 360.0) + { + hue -= 360.0; + } + + while (hue < 0.0) + { + hue += 360.0; + } + + // We similarly clamp saturation, lightness and alpha between 0 and 1. + saturation = saturation < 0.0 ? 0.0 : saturation; + saturation = saturation > 1.0 ? 1.0 : saturation; + + lightness = lightness < 0.0 ? 0.0 : lightness; + lightness = lightness > 1.0 ? 1.0 : lightness; + + alpha = alpha < 0.0 ? 0.0 : alpha; + alpha = alpha > 1.0 ? 1.0 : alpha; + + // The conversion algorithm is from the below link + // https://en.wikipedia.org/wiki/HSL_and_HSV#Interconversion + + double s; + double v = lightness + (saturation * Math.Min(lightness, 1.0 - lightness)); + + if (v <= 0) + { + s = 0; + } + else + { + s = 2.0 * (1.0 - (lightness / v)); + } + + return new HsvColor(alpha, hue, s, v); + } + /// /// Indicates whether the values of two specified objects are equal. /// diff --git a/src/Avalonia.Base/Media/HsvColor.cs b/src/Avalonia.Base/Media/HsvColor.cs index 3c6336c445..03949d32aa 100644 --- a/src/Avalonia.Base/Media/HsvColor.cs +++ b/src/Avalonia.Base/Media/HsvColor.cs @@ -195,10 +195,18 @@ namespace Avalonia.Media /// The RGB equivalent color. public Color ToRgb() { - // Use the by-component conversion method directly for performance return HsvColor.ToRgb(H, S, V, A); } + /// + /// Returns the HSL color model equivalent of this HSV color. + /// + /// The HSL equivalent color. + public HslColor ToHsl() + { + return HsvColor.ToHsl(H, S, V, A); + } + /// public override string ToString() { @@ -510,13 +518,67 @@ namespace Avalonia.Media break; } - return Color.FromArgb( + return new Color( (byte)Math.Round(alpha * 255), (byte)Math.Round(r * 255), (byte)Math.Round(g * 255), (byte)Math.Round(b * 255)); } + /// + /// Converts the given HSVA color component values to their HSL color equivalent. + /// + /// The Hue component in the HSV color model in the range from 0..360. + /// The Saturation component in the HSV color model in the range from 0..1. + /// The Value component in the HSV color model in the range from 0..1. + /// The Alpha component in the range from 0..1. + /// A new equivalent to the given HSVA values. + public static HslColor ToHsl( + double hue, + double saturation, + double value, + double alpha = 1.0) + { + // We want the hue to be between 0 and 359, + // so we first ensure that that's the case. + while (hue >= 360.0) + { + hue -= 360.0; + } + + while (hue < 0.0) + { + hue += 360.0; + } + + // We similarly clamp saturation, value and alpha between 0 and 1. + saturation = saturation < 0.0 ? 0.0 : saturation; + saturation = saturation > 1.0 ? 1.0 : saturation; + + value = value < 0.0 ? 0.0 : value; + value = value > 1.0 ? 1.0 : value; + + alpha = alpha < 0.0 ? 0.0 : alpha; + alpha = alpha > 1.0 ? 1.0 : alpha; + + // The conversion algorithm is from the below link + // https://en.wikipedia.org/wiki/HSL_and_HSV#Interconversion + + double s; + double l = value * (1.0 - (saturation / 2.0)); + + if (l <= 0 || l >= 1) + { + s = 0.0; + } + else + { + s = (value - l) / Math.Min(l, 1.0 - l); + } + + return new HslColor(alpha, hue, s, l); + } + /// /// Indicates whether the values of two specified objects are equal. /// diff --git a/tests/Avalonia.Base.UnitTests/Media/ColorTests.cs b/tests/Avalonia.Base.UnitTests/Media/ColorTests.cs index 36929d5e95..1ed3ea50b9 100644 --- a/tests/Avalonia.Base.UnitTests/Media/ColorTests.cs +++ b/tests/Avalonia.Base.UnitTests/Media/ColorTests.cs @@ -335,5 +335,34 @@ namespace Avalonia.Base.UnitTests.Media Assert.True(dataPoint.Item2 == parsedColor); } } + + [Fact] + public void Hsv_To_From_Hsl_Conversion() + { + // Note that conversion of values more representative of actual colors is not done due to rounding error + // It would be necessary to introduce a different equality comparison that accounts for rounding differences in values + // This is a result of the math in the conversion itself + // RGB doesn't have this problem because it uses whole numbers + var data = new Tuple[] + { + Tuple.Create(new HsvColor(1.0, 0.0, 0.0, 0.0), new HslColor(1.0, 0.0, 0.0, 0.0)), + Tuple.Create(new HsvColor(1.0, 359.0, 1.0, 1.0), new HslColor(1.0, 359.0, 1.0, 0.5)), + + Tuple.Create(new HsvColor(1.0, 128.0, 0.0, 0.0), new HslColor(1.0, 128.0, 0.0, 0.0)), + Tuple.Create(new HsvColor(1.0, 128.0, 0.0, 1.0), new HslColor(1.0, 128.0, 0.0, 1.0)), + Tuple.Create(new HsvColor(1.0, 128.0, 1.0, 1.0), new HslColor(1.0, 128.0, 1.0, 0.5)), + + Tuple.Create(new HsvColor(0.23, 0.5, 1.0, 1.0), new HslColor(0.23, 0.5, 1.0, 0.5)), + }; + + foreach (var dataPoint in data) + { + var convertedHsl = dataPoint.Item1.ToHsl(); + var convertedHsv = dataPoint.Item2.ToHsv(); + + Assert.Equal(convertedHsv, dataPoint.Item1); + Assert.Equal(convertedHsl, dataPoint.Item2); + } + } } } From a91571aa49c025b96a4a280c64d1f6e9ba8fdea1 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 13 May 2023 11:47:34 -0400 Subject: [PATCH 11/29] Switch ColorPreviewer's AccentColorConverter to use the FluentTheme's HSL algorithm --- .../Converters/AccentColorConverter.cs | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs b/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs index 2c8e09adc9..7a8644f9de 100644 --- a/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs +++ b/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs @@ -13,11 +13,6 @@ namespace Avalonia.Controls.Primitives.Converters /// public class AccentColorConverter : IValueConverter { - /// - /// The amount to change the Value component for each accent color step. - /// - public const double ValueDelta = 0.1; - /// public object? Convert( object? value, @@ -101,13 +96,54 @@ namespace Avalonia.Controls.Primitives.Converters /// The new accent color. public static HsvColor GetAccent(HsvColor hsvColor, int accentStep) { + // Accent steps are taken from the FluentTheme: + // SystemAccentColors.cs -> CalculateAccentShades() + // + // This is currently believed to be the most accurate representation of the algorithm in Windows. + // It replaces the previous implementation which adjusted Value +/- 10% per step. + // Any changes to FluentTheme's accent color calculation should be copied here. + + const double dark1step = 28.5 / 255d; + const double dark2step = 49 / 255d; + const double dark3step = 74.5 / 255d; + const double light1step = 39 / 255d; + const double light2step = 70 / 255d; + const double light3step = 103 / 255d; + if (accentStep != 0) { - double colorValue = hsvColor.V; - colorValue += (accentStep * AccentColorConverter.ValueDelta); - colorValue = Math.Round(colorValue, 2); + // Temporary colors are used to preserve Hue through the RGB conversion + // Otherwise, at Black/White hue information would be lost + // This should be improved in the future with direct conversions to/from HSV/HSL. + var hslAccent = hsvColor.ToHsl(); + + double lightnessAdjustment = 0.0; + switch (accentStep) + { + case -3: + lightnessAdjustment = -dark3step; + break; + case -2: + lightnessAdjustment = -dark2step; + break; + case -1: + lightnessAdjustment = -dark1step; + break; + case 1: + lightnessAdjustment = light1step; + break; + case 2: + lightnessAdjustment = light2step; + break; + case 3: + lightnessAdjustment = light3step; + break; + } + + var adjustedHsl = new HslColor(hslAccent.A, hslAccent.H, hslAccent.S, hslAccent.L + lightnessAdjustment); + // Rounding may be required here - return new HsvColor(hsvColor.A, hsvColor.H, hsvColor.S, colorValue); + return adjustedHsl.ToHsv(); } else { From 27dff43e53fcc2c78243999f24ee0ab74f6c0e50 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 13 May 2023 11:48:50 -0400 Subject: [PATCH 12/29] Revert "Switch ColorPreviewer's AccentColorConverter to use the FluentTheme's HSL algorithm" This reverts commit a91571aa49c025b96a4a280c64d1f6e9ba8fdea1. --- .../Converters/AccentColorConverter.cs | 54 ++++--------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs b/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs index 7a8644f9de..2c8e09adc9 100644 --- a/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs +++ b/src/Avalonia.Controls.ColorPicker/Converters/AccentColorConverter.cs @@ -13,6 +13,11 @@ namespace Avalonia.Controls.Primitives.Converters /// public class AccentColorConverter : IValueConverter { + /// + /// The amount to change the Value component for each accent color step. + /// + public const double ValueDelta = 0.1; + /// public object? Convert( object? value, @@ -96,54 +101,13 @@ namespace Avalonia.Controls.Primitives.Converters /// The new accent color. public static HsvColor GetAccent(HsvColor hsvColor, int accentStep) { - // Accent steps are taken from the FluentTheme: - // SystemAccentColors.cs -> CalculateAccentShades() - // - // This is currently believed to be the most accurate representation of the algorithm in Windows. - // It replaces the previous implementation which adjusted Value +/- 10% per step. - // Any changes to FluentTheme's accent color calculation should be copied here. - - const double dark1step = 28.5 / 255d; - const double dark2step = 49 / 255d; - const double dark3step = 74.5 / 255d; - const double light1step = 39 / 255d; - const double light2step = 70 / 255d; - const double light3step = 103 / 255d; - if (accentStep != 0) { - // Temporary colors are used to preserve Hue through the RGB conversion - // Otherwise, at Black/White hue information would be lost - // This should be improved in the future with direct conversions to/from HSV/HSL. - var hslAccent = hsvColor.ToHsl(); - - double lightnessAdjustment = 0.0; - switch (accentStep) - { - case -3: - lightnessAdjustment = -dark3step; - break; - case -2: - lightnessAdjustment = -dark2step; - break; - case -1: - lightnessAdjustment = -dark1step; - break; - case 1: - lightnessAdjustment = light1step; - break; - case 2: - lightnessAdjustment = light2step; - break; - case 3: - lightnessAdjustment = light3step; - break; - } - - var adjustedHsl = new HslColor(hslAccent.A, hslAccent.H, hslAccent.S, hslAccent.L + lightnessAdjustment); - // Rounding may be required here + double colorValue = hsvColor.V; + colorValue += (accentStep * AccentColorConverter.ValueDelta); + colorValue = Math.Round(colorValue, 2); - return adjustedHsl.ToHsv(); + return new HsvColor(hsvColor.A, hsvColor.H, hsvColor.S, colorValue); } else { From a4661bbee11c9a52adbb392721403cbccfde35f4 Mon Sep 17 00:00:00 2001 From: robloo Date: Sat, 13 May 2023 11:58:08 -0400 Subject: [PATCH 13/29] Improve IsPerceptive comments --- .../ColorSlider/ColorSlider.Properties.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs index b2deb913f1..a065a7f826 100644 --- a/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs +++ b/src/Avalonia.Controls.ColorPicker/ColorSlider/ColorSlider.Properties.cs @@ -126,17 +126,19 @@ namespace Avalonia.Controls.Primitives /// /// Gets or sets a value indicating whether the slider adapts rendering to improve user-perception - /// over exactness. When true in the HSVA color model, this ensures that the gradient is always visible and - /// never washed out regardless of the actual color. When true in the RGBA color model, this ensures - /// the components always appear as red, green or blue. + /// over exactness. /// /// + /// When true in the HSVA color model, this ensures that the gradient is always visible and + /// never washed out regardless of the actual color. When true in the RGBA color model, this ensures + /// the gradient always appears as red, green or blue. + ///

/// For example, with Hue in the HSVA color model, the Saturation and Value components are always forced /// to maximum values during rendering. In the RGBA color model, all components other than /// are forced to minimum values during rendering. ///

- /// Also note this property will only adjust components other than during rendering. - /// This doesn't change the values of any components in the color – it is only for display. + /// Note this property will only adjust components other than during rendering. + /// This also doesn't change the values of any components in the actual color – it is only for display. /// public bool IsPerceptive { From 7295446fe1b8120190ba19a25b41d77f8bdb4be3 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Fri, 12 May 2023 23:14:39 +0300 Subject: [PATCH 14/29] Ignore xmlns without CLR namespace on type resolution --- .../XamlIl/Runtime/XamlIlRuntimeHelpers.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs index f8eab5b654..a78af8e35c 100644 --- a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs +++ b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs @@ -156,10 +156,13 @@ namespace Avalonia.Markup.Xaml.XamlIl.Runtime throw new ArgumentException("Unable to resolve namespace for type " + qualifiedTypeName); foreach (var entry in lst) { - var asm = Assembly.Load(new AssemblyName(entry.ClrAssemblyName)); - var resolved = asm.GetType(entry.ClrNamespace + "." + name); - if (resolved != null) - return resolved; + if (entry.ClrAssemblyName is { Length: > 0 }) + { + var asm = Assembly.Load(new AssemblyName(entry.ClrAssemblyName)); + var resolved = asm.GetType(entry.ClrNamespace + "." + name); + if (resolved != null) + return resolved; + } } throw new ArgumentException( From 083fcb54e0f4781dfd5a94a93480bfa84e7ddc0a Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Fri, 12 May 2023 23:49:39 +0300 Subject: [PATCH 15/29] Improved XAML type resolution exception --- .../XamlIl/Runtime/XamlIlRuntimeHelpers.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs index a78af8e35c..5b8388594f 100644 --- a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs +++ b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs @@ -154,20 +154,19 @@ namespace Avalonia.Markup.Xaml.XamlIl.Runtime var namespaces = _nsInfo.XmlNamespaces; if (!namespaces.TryGetValue(ns, out var lst)) throw new ArgumentException("Unable to resolve namespace for type " + qualifiedTypeName); - foreach (var entry in lst) + var resolvable = lst.Where(static e => e.ClrAsseblyName is { Length: > 0 }); + foreach (var entry in resolvable) { - if (entry.ClrAssemblyName is { Length: > 0 }) - { - var asm = Assembly.Load(new AssemblyName(entry.ClrAssemblyName)); - var resolved = asm.GetType(entry.ClrNamespace + "." + name); - if (resolved != null) - return resolved; - } + var asm = Assembly.Load(new AssemblyName(entry.ClrAssemblyName)); + var resolved = asm.GetType(entry.ClrNamespace + "." + name); + if (resolved != null) + return resolved; } throw new ArgumentException( $"Unable to resolve type {qualifiedTypeName} from any of the following locations: " + - string.Join(",", lst.Select(e => $"`{e.ClrAssemblyName}:{e.ClrNamespace}.{name}`"))); + string.Join(",", resolvable.Select(e => $"`clr-namespace:{e.ClrNamespace};assembly={e.ClrAssemblyName}`"))) + { HelpLink = "https://docs.avaloniaui.net/guides/basics/introduction-to-xaml#valid-xaml-namespaces" }; } } From 75eb8a78d4e88c709ca21fae54ee6aa34d2ee361 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 15 May 2023 13:21:05 +0300 Subject: [PATCH 16/29] Fixed typo --- .../Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs index 5b8388594f..be6b37bb02 100644 --- a/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs +++ b/src/Markup/Avalonia.Markup.Xaml/XamlIl/Runtime/XamlIlRuntimeHelpers.cs @@ -154,7 +154,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.Runtime var namespaces = _nsInfo.XmlNamespaces; if (!namespaces.TryGetValue(ns, out var lst)) throw new ArgumentException("Unable to resolve namespace for type " + qualifiedTypeName); - var resolvable = lst.Where(static e => e.ClrAsseblyName is { Length: > 0 }); + var resolvable = lst.Where(static e => e.ClrAssemblyName is { Length: > 0 }); foreach (var entry in resolvable) { var asm = Assembly.Load(new AssemblyName(entry.ClrAssemblyName)); From 20064647dd52c6c58eb8e1502313e44037e739d6 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Tue, 16 May 2023 17:59:32 -0400 Subject: [PATCH 17/29] Limit IFocusManager API, extend IInputElement API, remove visible static properties --- src/Avalonia.Base/Input/AccessKeyHandler.cs | 4 +- src/Avalonia.Base/Input/FocusManager.cs | 60 ++++++++++++------- src/Avalonia.Base/Input/IFocusManager.cs | 36 ++--------- src/Avalonia.Base/Input/IFocusScope.cs | 3 + src/Avalonia.Base/Input/IInputElement.cs | 4 +- src/Avalonia.Base/Input/IInputRoot.cs | 8 +++ src/Avalonia.Base/Input/IKeyboardDevice.cs | 8 +-- src/Avalonia.Base/Input/InputElement.cs | 17 +++--- src/Avalonia.Base/Input/KeyboardDevice.cs | 4 +- .../Input/KeyboardNavigationHandler.cs | 4 +- src/Avalonia.Base/Input/MouseDevice.cs | 2 + .../Input/Navigation/TabNavigation.cs | 6 +- src/Avalonia.Controls.DataGrid/DataGrid.cs | 5 +- .../Controls/ViewManager.cs | 2 +- .../AutoCompleteBox/AutoCompleteBox.cs | 2 +- src/Avalonia.Controls/Calendar/Calendar.cs | 2 +- src/Avalonia.Controls/ComboBox.cs | 3 +- src/Avalonia.Controls/ContextMenu.cs | 4 +- .../DateTimePickers/DatePickerPresenter.cs | 11 ++-- .../DateTimePickers/TimePickerPresenter.cs | 6 +- .../Flyouts/PopupFlyoutBase.cs | 4 +- src/Avalonia.Controls/ItemsControl.cs | 11 ++-- src/Avalonia.Controls/Primitives/Popup.cs | 6 +- src/Avalonia.Controls/TopLevel.cs | 5 +- src/Avalonia.Controls/TreeView.cs | 3 +- src/Avalonia.Controls/TreeViewItem.cs | 2 +- src/Avalonia.Controls/WindowBase.cs | 4 +- .../Diagnostics/DevTools.cs | 8 +-- .../LinuxFramebufferPlatform.cs | 2 +- .../WinForms/WinFormsAvaloniaControlHost.cs | 2 +- .../Input/WindowsKeyboardDevice.cs | 2 +- .../Input/InputElement_Focus.cs | 58 +++++++++--------- .../Input/PointerOverTests.cs | 4 +- .../FlyoutTests.cs | 10 ++-- .../ItemsControlTests.cs | 6 +- .../ListBoxTests.cs | 8 +-- .../DefaultMenuInteractionHandlerTests.cs | 4 +- .../Primitives/PopupTests.cs | 11 ++-- .../TabControlTests.cs | 8 +-- .../TreeViewTests.cs | 16 ++--- tests/Avalonia.LeakTests/ControlTests.cs | 4 +- tests/Avalonia.UnitTests/TestRoot.cs | 1 + 42 files changed, 192 insertions(+), 178 deletions(-) diff --git a/src/Avalonia.Base/Input/AccessKeyHandler.cs b/src/Avalonia.Base/Input/AccessKeyHandler.cs index 2bd9fce947..23d1f51730 100644 --- a/src/Avalonia.Base/Input/AccessKeyHandler.cs +++ b/src/Avalonia.Base/Input/AccessKeyHandler.cs @@ -141,9 +141,11 @@ namespace Avalonia.Input if (MainMenu == null || !MainMenu.IsOpen) { + var focusManager = FocusManager.GetFocusManager(e.Source as IInputElement); + // TODO: Use FocusScopes to store the current element and restore it when context menu is closed. // Save currently focused input element. - _restoreFocusElement = FocusManager.Instance?.Current; + _restoreFocusElement = focusManager?.GetFocusedElement(); // When Alt is pressed without a main menu, or with a closed main menu, show // access key markers in the window (i.e. "_File"). diff --git a/src/Avalonia.Base/Input/FocusManager.cs b/src/Avalonia.Base/Input/FocusManager.cs index c8de7267ca..4c464a44ae 100644 --- a/src/Avalonia.Base/Input/FocusManager.cs +++ b/src/Avalonia.Base/Input/FocusManager.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; using Avalonia.Interactivity; +using Avalonia.Metadata; using Avalonia.VisualTree; namespace Avalonia.Input @@ -10,6 +11,7 @@ namespace Avalonia.Input /// /// Manages focus for the application. /// + [Unstable] public class FocusManager : IFocusManager { /// @@ -21,7 +23,7 @@ namespace Avalonia.Input /// /// Initializes a new instance of the class. /// - static FocusManager() + internal FocusManager() { InputElement.PointerPressedEvent.AddClassHandler( typeof(IInputElement), @@ -29,15 +31,12 @@ namespace Avalonia.Input RoutingStrategies.Tunnel); } - /// - /// Gets the instance of the . - /// - public static IFocusManager? Instance => AvaloniaLocator.Current.GetService(); + private IInputElement? Current => KeyboardDevice.Instance?.FocusedElement; /// /// Gets the currently focused . /// - public IInputElement? Current => KeyboardDevice.Instance?.FocusedElement; + public IInputElement? GetFocusedElement() => Current; /// /// Gets the current focus scope. @@ -54,7 +53,7 @@ namespace Avalonia.Input /// The control to focus. /// The method by which focus was changed. /// Any key modifiers active at the time of focus. - public void Focus( + public bool Focus( IInputElement? control, NavigationMethod method = NavigationMethod.Unspecified, KeyModifiers keyModifiers = KeyModifiers.None) @@ -67,7 +66,7 @@ namespace Avalonia.Input if (scope != null) { Scope = scope; - SetFocusedElement(scope, control, method, keyModifiers); + return SetFocusedElement(scope, control, method, keyModifiers); } } else if (Current != null) @@ -79,28 +78,29 @@ namespace Avalonia.Input _focusScopes.TryGetValue(scope, out var element) && element != null) { - Focus(element, method); - return; + return Focus(element, method); } } if (Scope is object) { // Couldn't find a focus scope, clear focus. - SetFocusedElement(Scope, null); + return SetFocusedElement(Scope, null); } } + + return false; } - public IInputElement? GetFocusedElement(IInputElement e) + public void ClearFocus() { - if (e is IFocusScope scope) - { - _focusScopes.TryGetValue(scope, out var result); - return result; - } + Focus(null); + } - return null; + public IInputElement? GetFocusedElement(IFocusScope scope) + { + _focusScopes.TryGetValue(scope, out var result); + return result; } /// @@ -114,7 +114,7 @@ namespace Avalonia.Input /// If the specified scope is the current then the keyboard focus /// will change. /// - public void SetFocusedElement( + public bool SetFocusedElement( IFocusScope scope, IInputElement? element, NavigationMethod method = NavigationMethod.Unspecified, @@ -124,7 +124,7 @@ namespace Avalonia.Input if (element is not null && !CanFocus(element)) { - return; + return false; } if (_focusScopes.TryGetValue(scope, out var existingElement)) @@ -144,6 +144,8 @@ namespace Avalonia.Input { KeyboardDevice.Instance?.SetFocusedElement(element, method, keyModifiers); } + + return true; } /// @@ -185,6 +187,20 @@ namespace Avalonia.Input public static bool GetIsFocusScope(IInputElement e) => e is IFocusScope; + /// + /// Public API customers should use TopLevel.GetTopLevel(control).FocusManager. + /// But since we have split projects, we can't access TopLevel from Avalonia.Base. + /// That's why we need this helper method instead. + /// + internal static FocusManager? GetFocusManager(IInputElement? element) + { + // Element might not be a visual, and not attached to the root. + // But IFocusManager is always expected to be a FocusManager. + return (FocusManager?)((IInputRoot?)(element as Visual)?.VisualRoot)?.FocusManager + // In our unit tests some elements might not have a root. Remove when + ?? (FocusManager?)AvaloniaLocator.Current.GetService(); + } + /// /// Checks if the specified element can be focused. /// @@ -221,7 +237,7 @@ namespace Avalonia.Input /// /// The event sender. /// The event args. - private static void OnPreviewPointerPressed(object? sender, RoutedEventArgs e) + private void OnPreviewPointerPressed(object? sender, RoutedEventArgs e) { if (sender is null) return; @@ -237,7 +253,7 @@ namespace Avalonia.Input { if (element is IInputElement inputElement && CanFocus(inputElement)) { - Instance?.Focus(inputElement, NavigationMethod.Pointer, ev.KeyModifiers); + Focus(inputElement, NavigationMethod.Pointer, ev.KeyModifiers); break; } diff --git a/src/Avalonia.Base/Input/IFocusManager.cs b/src/Avalonia.Base/Input/IFocusManager.cs index 0c85cad2f7..5691172f3f 100644 --- a/src/Avalonia.Base/Input/IFocusManager.cs +++ b/src/Avalonia.Base/Input/IFocusManager.cs @@ -11,40 +11,12 @@ namespace Avalonia.Input /// /// Gets the currently focused . /// - IInputElement? Current { get; } + IInputElement? GetFocusedElement(); /// - /// Gets the current focus scope. + /// Clears currently focused element. /// - IFocusScope? Scope { get; } - - /// - /// Focuses a control. - /// - /// The control to focus. - /// The method by which focus was changed. - /// Any key modifiers active at the time of focus. - void Focus( - IInputElement? control, - NavigationMethod method = NavigationMethod.Unspecified, - KeyModifiers keyModifiers = KeyModifiers.None); - - /// - /// Notifies the focus manager of a change in focus scope. - /// - /// The new focus scope. - /// - /// This should not be called by client code. It is called by an - /// when it activates, e.g. when a Window is activated. - /// - void SetFocusScope(IFocusScope scope); - - /// - /// Notifies the focus manager that a focus scope has been removed. - /// - /// The focus scope to be removed. - /// This should not be called by client code. It is called by an - /// when it deactivates or closes, e.g. when a Window is closed. - void RemoveFocusScope(IFocusScope scope); + [Unstable("This API might be removed in 11.x minor updates. Please consider focusing another element instead of removing focus at all for better UX.")] + void ClearFocus(); } } diff --git a/src/Avalonia.Base/Input/IFocusScope.cs b/src/Avalonia.Base/Input/IFocusScope.cs index 56f558040e..4f7c454263 100644 --- a/src/Avalonia.Base/Input/IFocusScope.cs +++ b/src/Avalonia.Base/Input/IFocusScope.cs @@ -1,5 +1,8 @@ +using Avalonia.Metadata; + namespace Avalonia.Input { + [NotClientImplementable] public interface IFocusScope { } diff --git a/src/Avalonia.Base/Input/IInputElement.cs b/src/Avalonia.Base/Input/IInputElement.cs index 6c20d20b4d..39dc30befd 100644 --- a/src/Avalonia.Base/Input/IInputElement.cs +++ b/src/Avalonia.Base/Input/IInputElement.cs @@ -119,7 +119,9 @@ namespace Avalonia.Input /// /// Focuses the control. /// - void Focus(); + /// The method by which focus was changed. + /// Any key modifiers active at the time of focus. + bool Focus(NavigationMethod method = NavigationMethod.Unspecified, KeyModifiers keyModifiers = KeyModifiers.None); /// /// Gets the key bindings for the element. diff --git a/src/Avalonia.Base/Input/IInputRoot.cs b/src/Avalonia.Base/Input/IInputRoot.cs index 344a4eefd7..0100b22ba7 100644 --- a/src/Avalonia.Base/Input/IInputRoot.cs +++ b/src/Avalonia.Base/Input/IInputRoot.cs @@ -18,6 +18,14 @@ namespace Avalonia.Input /// IKeyboardNavigationHandler KeyboardNavigationHandler { get; } + /// + /// Gets focus manager of the root. + /// + /// + /// Focus manager can be null only if application wasn't initialized yet. + /// + IFocusManager? FocusManager { get; } + /// /// Gets or sets the input element that the pointer is currently over. /// diff --git a/src/Avalonia.Base/Input/IKeyboardDevice.cs b/src/Avalonia.Base/Input/IKeyboardDevice.cs index 0b7b5aaecc..172b58068c 100644 --- a/src/Avalonia.Base/Input/IKeyboardDevice.cs +++ b/src/Avalonia.Base/Input/IKeyboardDevice.cs @@ -44,13 +44,7 @@ namespace Avalonia.Input } [NotClientImplementable] - public interface IKeyboardDevice : IInputDevice, INotifyPropertyChanged + public interface IKeyboardDevice : IInputDevice { - IInputElement? FocusedElement { get; } - - void SetFocusedElement( - IInputElement? element, - NavigationMethod method, - KeyModifiers modifiers); } } diff --git a/src/Avalonia.Base/Input/InputElement.cs b/src/Avalonia.Base/Input/InputElement.cs index 33ddbaedf9..68131e5bf7 100644 --- a/src/Avalonia.Base/Input/InputElement.cs +++ b/src/Avalonia.Base/Input/InputElement.cs @@ -458,9 +458,10 @@ namespace Avalonia.Input SetAndRaise(IsEffectivelyEnabledProperty, ref _isEffectivelyEnabled, value); PseudoClasses.Set(":disabled", !value); - if (!IsEffectivelyEnabled && FocusManager.Instance?.Current == this) + if (!IsEffectivelyEnabled && FocusManager.GetFocusManager(this) is {} focusManager + && Equals(focusManager.GetFocusedElement(), this)) { - FocusManager.Instance?.Focus(null); + focusManager.ClearFocus(); } } } @@ -491,12 +492,10 @@ namespace Avalonia.Input public GestureRecognizerCollection GestureRecognizers => _gestureRecognizers ?? (_gestureRecognizers = new GestureRecognizerCollection(this)); - /// - /// Focuses the control. - /// - public void Focus() + /// + public bool Focus(NavigationMethod method = NavigationMethod.Unspecified, KeyModifiers keyModifiers = KeyModifiers.None) { - FocusManager.Instance?.Focus(this); + return FocusManager.GetFocusManager(this)?.Focus(this, method, keyModifiers) ?? false; } /// @@ -506,7 +505,7 @@ namespace Avalonia.Input if (IsFocused) { - FocusManager.Instance?.Focus(null); + FocusManager.GetFocusManager(this)?.ClearFocus(); } } @@ -649,7 +648,7 @@ namespace Avalonia.Input } else if (change.Property == IsVisibleProperty && !change.GetNewValue() && IsFocused) { - FocusManager.Instance?.Focus(null); + FocusManager.GetFocusManager(this)?.ClearFocus(); } } diff --git a/src/Avalonia.Base/Input/KeyboardDevice.cs b/src/Avalonia.Base/Input/KeyboardDevice.cs index c46834fff4..f6d2a2195a 100644 --- a/src/Avalonia.Base/Input/KeyboardDevice.cs +++ b/src/Avalonia.Base/Input/KeyboardDevice.cs @@ -3,9 +3,11 @@ using System.Runtime.CompilerServices; using Avalonia.Input.Raw; using Avalonia.Input.TextInput; using Avalonia.Interactivity; +using Avalonia.Metadata; namespace Avalonia.Input { + [Unstable] public class KeyboardDevice : IKeyboardDevice, INotifyPropertyChanged { private IInputElement? _focusedElement; @@ -13,7 +15,7 @@ namespace Avalonia.Input public event PropertyChangedEventHandler? PropertyChanged; - public static IKeyboardDevice? Instance => AvaloniaLocator.Current.GetService(); + internal static KeyboardDevice? Instance => AvaloniaLocator.Current.GetService() as KeyboardDevice; public IInputManager? InputManager => AvaloniaLocator.Current.GetService(); diff --git a/src/Avalonia.Base/Input/KeyboardNavigationHandler.cs b/src/Avalonia.Base/Input/KeyboardNavigationHandler.cs index ba909de60f..e96c80da14 100644 --- a/src/Avalonia.Base/Input/KeyboardNavigationHandler.cs +++ b/src/Avalonia.Base/Input/KeyboardNavigationHandler.cs @@ -88,7 +88,7 @@ namespace Avalonia.Input var method = direction == NavigationDirection.Next || direction == NavigationDirection.Previous ? NavigationMethod.Tab : NavigationMethod.Directional; - FocusManager.Instance?.Focus(next, method, keyModifiers); + next.Focus(method, keyModifiers); } } @@ -99,7 +99,7 @@ namespace Avalonia.Input /// The event args. protected virtual void OnKeyDown(object? sender, KeyEventArgs e) { - var current = FocusManager.Instance?.Current; + var current = FocusManager.GetFocusManager(e.Source as IInputElement)?.GetFocusedElement(); if (current != null && e.Key == Key.Tab) { diff --git a/src/Avalonia.Base/Input/MouseDevice.cs b/src/Avalonia.Base/Input/MouseDevice.cs index 44412cd152..4aeffcdd72 100644 --- a/src/Avalonia.Base/Input/MouseDevice.cs +++ b/src/Avalonia.Base/Input/MouseDevice.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using Avalonia.Reactive; using Avalonia.Input.Raw; +using Avalonia.Metadata; using Avalonia.Platform; using Avalonia.Utilities; #pragma warning disable CS0618 @@ -11,6 +12,7 @@ namespace Avalonia.Input /// /// Represents a mouse device. /// + [Unstable] public class MouseDevice : IMouseDevice, IDisposable { private int _clickCount; diff --git a/src/Avalonia.Base/Input/Navigation/TabNavigation.cs b/src/Avalonia.Base/Input/Navigation/TabNavigation.cs index c460ecf3b3..9697e32926 100644 --- a/src/Avalonia.Base/Input/Navigation/TabNavigation.cs +++ b/src/Avalonia.Base/Input/Navigation/TabNavigation.cs @@ -190,9 +190,11 @@ namespace Avalonia.Input.Navigation private static IInputElement? FocusedElement(IInputElement? e) { // Focus delegation is enabled only if keyboard focus is outside the container - if (e != null && !e.IsKeyboardFocusWithin) + if (e != null && !e.IsKeyboardFocusWithin && e is IFocusScope scope) { - var focusedElement = (FocusManager.Instance as FocusManager)?.GetFocusedElement(e); + var focusManager = FocusManager.GetFocusManager(e); + + var focusedElement = focusManager?.GetFocusedElement(scope); if (focusedElement != null) { if (!IsFocusScope(e)) diff --git a/src/Avalonia.Controls.DataGrid/DataGrid.cs b/src/Avalonia.Controls.DataGrid/DataGrid.cs index a55a47fa53..bfcd4750e3 100644 --- a/src/Avalonia.Controls.DataGrid/DataGrid.cs +++ b/src/Avalonia.Controls.DataGrid/DataGrid.cs @@ -3958,7 +3958,7 @@ namespace Avalonia.Controls { bool focusLeftDataGrid = true; bool dataGridWillReceiveRoutedEvent = true; - Visual focusedObject = FocusManager.Instance.Current as Visual; + Visual focusedObject = FocusManager.GetFocusManager(this)?.GetFocusedElement() as Visual; DataGridColumn editingColumn = null; while (focusedObject != null) @@ -4865,7 +4865,8 @@ namespace Avalonia.Controls if (!ctrl) { // If Enter was used by a TextBox, we shouldn't handle the key - if (FocusManager.Instance.Current is TextBox focusedTextBox && focusedTextBox.AcceptsReturn) + if (FocusManager.GetFocusManager(this)?.GetFocusedElement() is TextBox focusedTextBox + && focusedTextBox.AcceptsReturn) { return false; } diff --git a/src/Avalonia.Controls.ItemsRepeater/Controls/ViewManager.cs b/src/Avalonia.Controls.ItemsRepeater/Controls/ViewManager.cs index 6b9d7934bf..674389d77c 100644 --- a/src/Avalonia.Controls.ItemsRepeater/Controls/ViewManager.cs +++ b/src/Avalonia.Controls.ItemsRepeater/Controls/ViewManager.cs @@ -695,7 +695,7 @@ namespace Avalonia.Controls { Control? focusedElement = null; - if (FocusManager.Instance?.Current is Visual child) + if (TopLevel.GetTopLevel(_owner)?.FocusManager?.GetFocusedElement() is Visual child) { var parent = child.GetVisualParent(); var owner = _owner; diff --git a/src/Avalonia.Controls/AutoCompleteBox/AutoCompleteBox.cs b/src/Avalonia.Controls/AutoCompleteBox/AutoCompleteBox.cs index 20711eecbc..d0b894101f 100644 --- a/src/Avalonia.Controls/AutoCompleteBox/AutoCompleteBox.cs +++ b/src/Avalonia.Controls/AutoCompleteBox/AutoCompleteBox.cs @@ -762,7 +762,7 @@ namespace Avalonia.Controls /// otherwise, false. protected bool HasFocus() { - Visual? focused = FocusManager.Instance?.Current as Visual; + Visual? focused = FocusManager.GetFocusManager(this)?.GetFocusedElement() as Visual; while (focused != null) { diff --git a/src/Avalonia.Controls/Calendar/Calendar.cs b/src/Avalonia.Controls/Calendar/Calendar.cs index 10aadfa759..6468d0b4e8 100644 --- a/src/Avalonia.Controls/Calendar/Calendar.cs +++ b/src/Avalonia.Controls/Calendar/Calendar.cs @@ -1567,7 +1567,7 @@ namespace Avalonia.Controls base.OnPointerReleased(e); if (!HasFocusInternal && e.InitialPressMouseButton == MouseButton.Left) { - FocusManager.Instance?.Focus(this); + Focus(); } } diff --git a/src/Avalonia.Controls/ComboBox.cs b/src/Avalonia.Controls/ComboBox.cs index f41c00662d..efa319ad54 100644 --- a/src/Avalonia.Controls/ComboBox.cs +++ b/src/Avalonia.Controls/ComboBox.cs @@ -230,8 +230,7 @@ namespace Avalonia.Controls var firstChild = Presenter?.Panel?.Children.FirstOrDefault(c => CanFocus(c)); if (firstChild != null) { - FocusManager.Instance?.Focus(firstChild, NavigationMethod.Directional); - e.Handled = true; + e.Handled = firstChild.Focus(NavigationMethod.Directional); } } } diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 97a8c6fe97..98b85dc31e 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -360,7 +360,7 @@ namespace Avalonia.Controls private void PopupOpened(object? sender, EventArgs e) { - _previousFocus = FocusManager.Instance?.Current; + _previousFocus = FocusManager.GetFocusManager(this)?.GetFocusedElement(); Focus(); _popupHostChangedHandler?.Invoke(_popup!.Host); @@ -390,7 +390,7 @@ namespace Avalonia.Controls } // HACK: Reset the focus when the popup is closed. We need to fix this so it's automatic. - FocusManager.Instance?.Focus(_previousFocus); + _previousFocus?.Focus(); RaiseEvent(new RoutedEventArgs { diff --git a/src/Avalonia.Controls/DateTimePickers/DatePickerPresenter.cs b/src/Avalonia.Controls/DateTimePickers/DatePickerPresenter.cs index 0ae743f30a..fb61ea679c 100644 --- a/src/Avalonia.Controls/DateTimePickers/DatePickerPresenter.cs +++ b/src/Avalonia.Controls/DateTimePickers/DatePickerPresenter.cs @@ -323,10 +323,11 @@ namespace Avalonia.Controls e.Handled = true; break; case Key.Tab: - if (FocusManager.Instance?.Current is IInputElement focus) + var focusManager = FocusManager.GetFocusManager(this); + if (focusManager?.GetFocusedElement() is { } focus) { var nextFocus = KeyboardNavigationHandler.GetNext(focus, NavigationDirection.Next); - KeyboardDevice.Instance?.SetFocusedElement(nextFocus, NavigationMethod.Tab, KeyModifiers.None); + nextFocus?.Focus(NavigationMethod.Tab); e.Handled = true; } break; @@ -449,15 +450,15 @@ namespace Avalonia.Controls if (monthCol < dayCol && monthCol < yearCol) { - KeyboardDevice.Instance?.SetFocusedElement(_monthSelector, NavigationMethod.Pointer, KeyModifiers.None); + _monthSelector?.Focus(NavigationMethod.Pointer); } else if (dayCol < monthCol && dayCol < yearCol) { - KeyboardDevice.Instance?.SetFocusedElement(_daySelector, NavigationMethod.Pointer, KeyModifiers.None); + _monthSelector?.Focus(NavigationMethod.Pointer); } else if (yearCol < monthCol && yearCol < dayCol) { - KeyboardDevice.Instance?.SetFocusedElement(_yearSelector, NavigationMethod.Pointer, KeyModifiers.None); + _yearSelector?.Focus(NavigationMethod.Pointer); } } diff --git a/src/Avalonia.Controls/DateTimePickers/TimePickerPresenter.cs b/src/Avalonia.Controls/DateTimePickers/TimePickerPresenter.cs index ba06e1b5e6..929ad68c24 100644 --- a/src/Avalonia.Controls/DateTimePickers/TimePickerPresenter.cs +++ b/src/Avalonia.Controls/DateTimePickers/TimePickerPresenter.cs @@ -161,10 +161,10 @@ namespace Avalonia.Controls e.Handled = true; break; case Key.Tab: - if (FocusManager.Instance?.Current is IInputElement focus) + if (FocusManager.GetFocusManager(this)?.GetFocusedElement() is { } focus) { var nextFocus = KeyboardNavigationHandler.GetNext(focus, NavigationDirection.Next); - KeyboardDevice.Instance?.SetFocusedElement(nextFocus, NavigationMethod.Tab, KeyModifiers.None); + nextFocus?.Focus(NavigationMethod.Tab); e.Handled = true; } break; @@ -216,7 +216,7 @@ namespace Avalonia.Controls _periodSelector.SelectedValue = hr >= 12 ? 1 : 0; SetGrid(); - KeyboardDevice.Instance?.SetFocusedElement(_hourSelector, NavigationMethod.Pointer, KeyModifiers.None); + _hourSelector?.Focus(NavigationMethod.Pointer); } private void SetGrid() diff --git a/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs b/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs index 5b23b5030f..7fd9fad605 100644 --- a/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs +++ b/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs @@ -250,14 +250,14 @@ namespace Avalonia.Controls.Primitives // Try and focus content inside Flyout if (Popup.Child.Focusable) { - FocusManager.Instance?.Focus(Popup.Child); + Popup.Child.Focus(); } else { var nextFocus = KeyboardNavigationHandler.GetNext(Popup.Child, NavigationDirection.Next); if (nextFocus != null) { - FocusManager.Instance?.Focus(nextFocus); + nextFocus.Focus(); } } } diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 1613bda45b..0b6fc2b64b 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -566,19 +566,20 @@ namespace Avalonia.Controls { if (!e.Handled) { - var focus = FocusManager.Instance; + var focus = FocusManager.GetFocusManager(this); var direction = e.Key.ToNavigationDirection(); var container = Presenter?.Panel as INavigableContainer; - if (container == null || - focus?.Current == null || + if (focus == null || + container == null || + focus.GetFocusedElement() == null || direction == null || direction.Value.IsTab()) { return; } - Visual? current = focus.Current as Visual; + Visual? current = focus.GetFocusedElement() as Visual; while (current != null) { @@ -588,7 +589,7 @@ namespace Avalonia.Controls if (next != null) { - focus.Focus(next, NavigationMethod.Directional, e.KeyModifiers); + next.Focus(NavigationMethod.Directional, e.KeyModifiers); e.Handled = true; } diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index 80b7841fc7..7ac219aa83 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -727,7 +727,7 @@ namespace Avalonia.Controls.Primitives Closed?.Invoke(this, EventArgs.Empty); - var focusCheck = FocusManager.Instance?.Current; + var focusCheck = FocusManager.GetFocusManager(this)?.GetFocusedElement(); // Focus is set to null as part of popup closing, so we only want to // set focus to PlacementTarget if this is the case @@ -744,7 +744,7 @@ namespace Avalonia.Controls.Primitives if (e is object) { - FocusManager.Instance?.Focus(e); + e.Focus(); } } else @@ -752,7 +752,7 @@ namespace Avalonia.Controls.Primitives var anc = this.FindLogicalAncestorOfType(); if (anc != null) { - FocusManager.Instance?.Focus(anc); + anc.Focus(); } } } diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index 85a35a3489..67847f621b 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -452,6 +452,9 @@ namespace Avalonia.Controls /// public IClipboard? Clipboard => PlatformImpl?.TryGetFeature(); + /// + public IFocusManager? FocusManager => AvaloniaLocator.Current.GetService(); + /// Point IRenderRoot.PointToClient(PixelPoint p) { @@ -725,7 +728,7 @@ namespace Avalonia.Controls void PlatformImpl_LostFocus() { - var focused = (Visual?)FocusManager.Instance?.Current; + var focused = (Visual?)FocusManager?.GetFocusedElement(); if (focused == null) return; while (focused.VisualParent != null) diff --git a/src/Avalonia.Controls/TreeView.cs b/src/Avalonia.Controls/TreeView.cs index ef10dcdb22..a499829d4a 100644 --- a/src/Avalonia.Controls/TreeView.cs +++ b/src/Avalonia.Controls/TreeView.cs @@ -560,8 +560,7 @@ namespace Avalonia.Controls if (next != null) { - FocusManager.Instance?.Focus(next, NavigationMethod.Directional); - e.Handled = true; + e.Handled = next.Focus(NavigationMethod.Directional); } } else diff --git a/src/Avalonia.Controls/TreeViewItem.cs b/src/Avalonia.Controls/TreeViewItem.cs index cf12c12447..a48706cb19 100644 --- a/src/Avalonia.Controls/TreeViewItem.cs +++ b/src/Avalonia.Controls/TreeViewItem.cs @@ -238,7 +238,7 @@ namespace Avalonia.Controls } else { - FocusManager.Instance?.Focus(treeViewItem, NavigationMethod.Directional); + treeViewItem.Focus(NavigationMethod.Directional); } return true; diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index ac47e744e0..b19ad49820 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -234,7 +234,7 @@ namespace Avalonia.Controls if (this is IFocusScope scope) { - FocusManager.Instance?.RemoveFocusScope(scope); + ((FocusManager?)FocusManager)?.RemoveFocusScope(scope); } base.HandleClosed(); @@ -326,7 +326,7 @@ namespace Avalonia.Controls if (scope != null) { - FocusManager.Instance?.SetFocusScope(scope); + ((FocusManager?)FocusManager)?.SetFocusScope(scope); } IsActive = true; diff --git a/src/Avalonia.Diagnostics/Diagnostics/DevTools.cs b/src/Avalonia.Diagnostics/Diagnostics/DevTools.cs index cb896fd633..ff49d0dd87 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/DevTools.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/DevTools.cs @@ -86,7 +86,7 @@ namespace Avalonia.Diagnostics private static IDisposable Open(IDevToolsTopLevelGroup topLevelGroup, DevToolsOptions options, Window? owner, Application? app) { - var focussedControl = KeyboardDevice.Instance?.FocusedElement as Control; + var focusedControl = owner?.FocusManager?.GetFocusedElement() as Control; AvaloniaObject root = topLevelGroup switch { ClassicDesktopStyleApplicationLifetimeTopLevelGroup gr => new Controls.Application(gr, app ?? Application.Current!), @@ -98,7 +98,7 @@ namespace Avalonia.Diagnostics if (s_open.TryGetValue(topLevelGroup, out var mainWindow)) { mainWindow.Activate(); - mainWindow.SelectedControl(focussedControl); + mainWindow.SelectedControl(focusedControl); return Disposable.Empty; } if (topLevelGroup.Items.Count == 1 && topLevelGroup.Items is not INotifyCollectionChanged) @@ -110,7 +110,7 @@ namespace Avalonia.Diagnostics if (group.Key.Items.Contains(singleTopLevel)) { group.Value.Activate(); - group.Value.SelectedControl(focussedControl); + group.Value.SelectedControl(focusedControl); return Disposable.Empty; } } @@ -124,7 +124,7 @@ namespace Avalonia.Diagnostics Tag = topLevelGroup }; window.SetOptions(options); - window.SelectedControl(focussedControl); + window.SelectedControl(focusedControl); window.Closed += DevToolsClosed; s_open.Add(topLevelGroup, window); if (options.ShowAsChildWindow && owner is not null) diff --git a/src/Linux/Avalonia.LinuxFramebuffer/LinuxFramebufferPlatform.cs b/src/Linux/Avalonia.LinuxFramebuffer/LinuxFramebufferPlatform.cs index bc178c8ecb..c352b5f9bf 100644 --- a/src/Linux/Avalonia.LinuxFramebuffer/LinuxFramebufferPlatform.cs +++ b/src/Linux/Avalonia.LinuxFramebuffer/LinuxFramebufferPlatform.cs @@ -107,7 +107,7 @@ namespace Avalonia.LinuxFramebuffer if (_topLevel is IFocusScope scope) { - FocusManager.Instance?.SetFocusScope(scope); + ((FocusManager)_topLevel.FocusManager).SetFocusScope(scope); } } diff --git a/src/Windows/Avalonia.Win32.Interop/WinForms/WinFormsAvaloniaControlHost.cs b/src/Windows/Avalonia.Win32.Interop/WinForms/WinFormsAvaloniaControlHost.cs index a8060d3fbf..8d73bde919 100644 --- a/src/Windows/Avalonia.Win32.Interop/WinForms/WinFormsAvaloniaControlHost.cs +++ b/src/Windows/Avalonia.Win32.Interop/WinForms/WinFormsAvaloniaControlHost.cs @@ -22,7 +22,7 @@ namespace Avalonia.Win32.Embedding UnmanagedMethods.SetParent(WindowHandle, Handle); _root.Prepare(); if (_root.IsFocused) - FocusManager.Instance.Focus(null); + _root.FocusManager.ClearFocus(); _root.GotFocus += RootGotFocus; FixPosition(); diff --git a/src/Windows/Avalonia.Win32/Input/WindowsKeyboardDevice.cs b/src/Windows/Avalonia.Win32/Input/WindowsKeyboardDevice.cs index 7e1e22579b..4f9b6c54d3 100644 --- a/src/Windows/Avalonia.Win32/Input/WindowsKeyboardDevice.cs +++ b/src/Windows/Avalonia.Win32/Input/WindowsKeyboardDevice.cs @@ -5,7 +5,7 @@ using Avalonia.Win32.Interop; namespace Avalonia.Win32.Input { - class WindowsKeyboardDevice : KeyboardDevice + internal class WindowsKeyboardDevice : KeyboardDevice { private readonly byte[] _keyStates = new byte[256]; diff --git a/tests/Avalonia.Base.UnitTests/Input/InputElement_Focus.cs b/tests/Avalonia.Base.UnitTests/Input/InputElement_Focus.cs index ac1547d09f..aa2e47de2d 100644 --- a/tests/Avalonia.Base.UnitTests/Input/InputElement_Focus.cs +++ b/tests/Avalonia.Base.UnitTests/Input/InputElement_Focus.cs @@ -23,7 +23,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); - Assert.Same(target, FocusManager.Instance.Current); + Assert.Same(target, root.FocusManager.GetFocusedElement()); } } @@ -39,14 +39,14 @@ namespace Avalonia.Base.UnitTests.Input Child = target = new Button() { IsVisible = false} }; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); target.Focus(); Assert.False(target.IsFocused); Assert.False(target.IsKeyboardFocusWithin); - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -67,14 +67,14 @@ namespace Avalonia.Base.UnitTests.Input } }; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); target.Focus(); Assert.False(target.IsFocused); Assert.False(target.IsKeyboardFocusWithin); - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -100,11 +100,11 @@ namespace Avalonia.Base.UnitTests.Input first.Focus(); - Assert.Same(first, FocusManager.Instance.Current); + Assert.Same(first, root.FocusManager.GetFocusedElement()); second.Focus(); - Assert.Same(first, FocusManager.Instance.Current); + Assert.Same(first, root.FocusManager.GetFocusedElement()); } } @@ -120,14 +120,14 @@ namespace Avalonia.Base.UnitTests.Input Child = target = new Button() { IsEnabled = false } }; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); target.Focus(); Assert.False(target.IsFocused); Assert.False(target.IsKeyboardFocusWithin); - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -148,14 +148,14 @@ namespace Avalonia.Base.UnitTests.Input } }; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); target.Focus(); Assert.False(target.IsFocused); Assert.False(target.IsKeyboardFocusWithin); - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -201,7 +201,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); target.IsVisible = false; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -224,7 +224,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); container.IsVisible = false; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -243,7 +243,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); target.IsEnabled = false; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -266,7 +266,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); container.IsEnabled = false; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -285,7 +285,7 @@ namespace Avalonia.Base.UnitTests.Input target.Focus(); root.Child = null; - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } @@ -312,13 +312,13 @@ namespace Avalonia.Base.UnitTests.Input target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus")); Assert.False(target2.IsFocused); Assert.False(target2.Classes.Contains(":focus")); - FocusManager.Instance?.Focus(target2, NavigationMethod.Tab); + target2.Focus(NavigationMethod.Tab); Assert.False(target1.IsFocused); Assert.False(target1.Classes.Contains(":focus")); Assert.True(target2.IsFocused); @@ -348,19 +348,19 @@ namespace Avalonia.Base.UnitTests.Input target1.ApplyTemplate(); target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.False(target1.Classes.Contains(":focus-visible")); Assert.False(target2.IsFocused); Assert.False(target2.Classes.Contains(":focus-visible")); - FocusManager.Instance?.Focus(target2, NavigationMethod.Tab); + target2.Focus(NavigationMethod.Tab); Assert.False(target1.IsFocused); Assert.False(target1.Classes.Contains(":focus-visible")); Assert.True(target2.IsFocused); Assert.True(target2.Classes.Contains(":focus-visible")); - FocusManager.Instance?.Focus(target1, NavigationMethod.Directional); + target1.Focus(NavigationMethod.Directional); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus-visible")); Assert.False(target2.IsFocused); @@ -390,7 +390,7 @@ namespace Avalonia.Base.UnitTests.Input target1.ApplyTemplate(); target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus-within")); Assert.True(target1.IsKeyboardFocusWithin); @@ -425,7 +425,7 @@ namespace Avalonia.Base.UnitTests.Input target1.ApplyTemplate(); target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus-within")); Assert.True(target1.IsKeyboardFocusWithin); @@ -436,7 +436,7 @@ namespace Avalonia.Base.UnitTests.Input Assert.True(root.Classes.Contains(":focus-within")); Assert.True(root.IsKeyboardFocusWithin); - FocusManager.Instance?.Focus(target2); + target2.Focus(); Assert.False(target1.IsFocused); Assert.False(target1.Classes.Contains(":focus-within")); @@ -478,7 +478,7 @@ namespace Avalonia.Base.UnitTests.Input target1.ApplyTemplate(); target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus-within")); Assert.True(target1.IsKeyboardFocusWithin); @@ -534,7 +534,7 @@ namespace Avalonia.Base.UnitTests.Input target1.ApplyTemplate(); target2.ApplyTemplate(); - FocusManager.Instance?.Focus(target1); + target1.Focus(); Assert.True(target1.IsFocused); Assert.True(target1.Classes.Contains(":focus-within")); Assert.True(target1.IsKeyboardFocusWithin); @@ -545,7 +545,7 @@ namespace Avalonia.Base.UnitTests.Input Assert.Equal(KeyboardDevice.Instance.FocusedElement, target1); - FocusManager.Instance?.Focus(target2); + target2.Focus(); Assert.False(target1.IsFocused); Assert.False(target1.Classes.Contains(":focus-within")); @@ -578,9 +578,9 @@ namespace Avalonia.Base.UnitTests.Input }; target.Focus(); - FocusManager.Instance.Focus(null); + root.FocusManager.ClearFocus(); - Assert.Null(FocusManager.Instance.Current); + Assert.Null(root.FocusManager.GetFocusedElement()); } } } diff --git a/tests/Avalonia.Base.UnitTests/Input/PointerOverTests.cs b/tests/Avalonia.Base.UnitTests/Input/PointerOverTests.cs index 629188800a..ae6b601896 100644 --- a/tests/Avalonia.Base.UnitTests/Input/PointerOverTests.cs +++ b/tests/Avalonia.Base.UnitTests/Input/PointerOverTests.cs @@ -20,7 +20,9 @@ namespace Avalonia.Base.UnitTests.Input [Fact] public void Close_Should_Remove_PointerOver() { - using var app = UnitTestApplication.Start(new TestServices(inputManager: new InputManager())); + using var app = UnitTestApplication.Start(new TestServices( + inputManager: new InputManager(), + focusManager: new FocusManager())); var renderer = RendererMocks.CreateRenderer(); var device = CreatePointerDeviceMock().Object; diff --git a/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs b/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs index 7767de11c7..db12de9db9 100644 --- a/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs +++ b/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs @@ -288,10 +288,10 @@ namespace Avalonia.Controls.UnitTests window.Show(); button.Focus(); - Assert.True(FocusManager.Instance?.Current == button); + Assert.True(window.FocusManager.GetFocusedElement() == button); button.Flyout.ShowAt(button); Assert.False(button.IsFocused); - Assert.True(FocusManager.Instance?.Current == flyoutTextBox); + Assert.True(window.FocusManager.GetFocusedElement() == flyoutTextBox); } } @@ -322,10 +322,10 @@ namespace Avalonia.Controls.UnitTests window.Content = button; window.Show(); - FocusManager.Instance?.Focus(button); - Assert.True(FocusManager.Instance?.Current == button); + button.Focus(); + Assert.True(window.FocusManager.GetFocusedElement() == button); button.Flyout.ShowAt(button); - Assert.True(FocusManager.Instance?.Current == button); + Assert.True(window.FocusManager.GetFocusedElement() == button); } } diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index 86249c66ff..58e4ec1b75 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -576,8 +576,9 @@ namespace Avalonia.Controls.UnitTests }); var panel = Assert.IsAssignableFrom(target.ItemsPanelRoot); + var focusManager = ((IInputRoot)target.VisualRoot!).FocusManager; - Assert.Equal(panel.Children[1], FocusManager.Instance!.Current); + Assert.Equal(panel.Children[1], focusManager?.GetFocusedElement()); } [Fact] @@ -601,8 +602,9 @@ namespace Avalonia.Controls.UnitTests }); var panel = Assert.IsAssignableFrom(target.ItemsPanelRoot); + var focusManager = ((IInputRoot)target.VisualRoot!).FocusManager; - Assert.Equal(panel.Children[2], FocusManager.Instance!.Current); + Assert.Equal(panel.Children[2], focusManager?.GetFocusedElement()); } [Fact] diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 72f476a3b0..18c1136ccb 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -979,7 +979,7 @@ namespace Avalonia.Controls.UnitTests RaiseKeyEvent(button, Key.Tab); var item = target.ContainerFromIndex(0); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); } [Fact] @@ -1026,17 +1026,17 @@ namespace Avalonia.Controls.UnitTests RaiseKeyEvent(button, Key.Tab); var item = target.ContainerFromIndex(1); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); RaiseKeyEvent(item, Key.Tab); - Assert.Same(button, FocusManager.Instance.Current); + Assert.Same(button, root.FocusManager.GetFocusedElement()); target.Selection.AnchorIndex = 2; RaiseKeyEvent(button, Key.Tab); item = target.ContainerFromIndex(2); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); } private static void RaiseKeyEvent(Control target, Key key, KeyModifiers inputModifiers = 0) diff --git a/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs b/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs index e5c96dcab6..eaf95b0c8c 100644 --- a/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Platform/DefaultMenuInteractionHandlerTests.cs @@ -267,7 +267,7 @@ namespace Avalonia.Controls.UnitTests.Platform target.KeyDown(item.Object, e); parentItem.Verify(x => x.Close()); - parentItem.Verify(x => x.Focus()); + parentItem.Verify(x => x.Focus(It.IsAny(), It.IsAny())); Assert.True(e.Handled); } @@ -351,7 +351,7 @@ namespace Avalonia.Controls.UnitTests.Platform target.KeyDown(item.Object, e); parentItem.Verify(x => x.Close()); - parentItem.Verify(x => x.Focus()); + parentItem.Verify(x => x.Focus(It.IsAny(), It.IsAny())); Assert.True(e.Handled); } diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index 765f2d1c19..51399d1202 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -642,10 +642,11 @@ namespace Avalonia.Controls.UnitTests.Primitives tb.Focus(); - Assert.True(FocusManager.Instance?.Current == tb); + var focusManager = TopLevel.GetTopLevel(tb)!.FocusManager; + tb = Assert.IsType(focusManager.GetFocusedElement()); //Ensure focus remains in the popup - var nextFocus = KeyboardNavigationHandler.GetNext(FocusManager.Instance.Current, NavigationDirection.Next); + var nextFocus = KeyboardNavigationHandler.GetNext(tb, NavigationDirection.Next); Assert.True(nextFocus == b); @@ -684,7 +685,8 @@ namespace Avalonia.Controls.UnitTests.Primitives p.Close(); - var focus = FocusManager.Instance?.Current; + var focusManager = window.FocusManager; + var focus = focusManager.GetFocusedElement(); Assert.True(focus == window); } } @@ -723,7 +725,8 @@ namespace Avalonia.Controls.UnitTests.Primitives windowTB.Focus(); - var focus = FocusManager.Instance?.Current; + var focusManager = window.FocusManager; + var focus = focusManager.GetFocusedElement(); Assert.True(focus == windowTB); diff --git a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs index 0d3eb80ae7..398ac3ff43 100644 --- a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs @@ -461,7 +461,7 @@ namespace Avalonia.Controls.UnitTests RaiseKeyEvent(button, Key.Tab); var item = target.ContainerFromIndex(0); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); } [Fact] @@ -513,17 +513,17 @@ namespace Avalonia.Controls.UnitTests RaiseKeyEvent(button, Key.Tab); var item = target.ContainerFromIndex(1); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); RaiseKeyEvent(item, Key.Tab); - Assert.Same(button, FocusManager.Instance.Current); + Assert.Same(button, root.FocusManager.GetFocusedElement()); target.Selection.AnchorIndex = 2; RaiseKeyEvent(button, Key.Tab); item = target.ContainerFromIndex(2); - Assert.Same(item, FocusManager.Instance.Current); + Assert.Same(item, root.FocusManager.GetFocusedElement()); } private static IControlTemplate TabControlTemplate() diff --git a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs index 51300f343a..604ff1c715 100644 --- a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs @@ -969,7 +969,6 @@ namespace Avalonia.Controls.UnitTests public void Keyboard_Navigation_Should_Move_To_Last_Selected_Node() { using var app = Start(); - var focus = FocusManager.Instance!; var navigation = AvaloniaLocator.Current.GetRequiredService(); var data = CreateTestTreeData(); @@ -984,6 +983,7 @@ namespace Avalonia.Controls.UnitTests { Children = { target, button }, }); + var focus = root.FocusManager; root.LayoutManager.ExecuteInitialLayoutPass(); ExpandAll(target); @@ -994,20 +994,19 @@ namespace Avalonia.Controls.UnitTests target.SelectedItem = item; node.Focus(); - Assert.Same(node, focus.Current); + Assert.Same(node, focus.GetFocusedElement()); - navigation.Move(focus.Current!, NavigationDirection.Next); - Assert.Same(button, focus.Current); + navigation.Move(focus.GetFocusedElement()!, NavigationDirection.Next); + Assert.Same(button, focus.GetFocusedElement()); - navigation.Move(focus.Current!, NavigationDirection.Next); - Assert.Same(node, focus.Current); + navigation.Move(focus.GetFocusedElement()!, NavigationDirection.Next); + Assert.Same(node, focus.GetFocusedElement()); } [Fact] public void Keyboard_Navigation_Should_Not_Crash_If_Selected_Item_Is_not_In_Tree() { using var app = Start(); - var focus = FocusManager.Instance!; var data = CreateTestTreeData(); var selectedNode = new Node { Value = "Out of Tree Selected Item" }; @@ -1025,6 +1024,7 @@ namespace Avalonia.Controls.UnitTests { Children = { target, button }, }); + var focus = root.FocusManager; root.LayoutManager.ExecuteInitialLayoutPass(); ExpandAll(target); @@ -1035,7 +1035,7 @@ namespace Avalonia.Controls.UnitTests target.SelectedItem = selectedNode; node.Focus(); - Assert.Same(node, focus.Current); + Assert.Same(node, focus.GetFocusedElement()); } [Fact] diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index d6318bba0b..b3a6b52d68 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -561,7 +561,7 @@ namespace Avalonia.LeakTests var window = new Window { Focusable = true }; window.Show(); - Assert.Same(window, FocusManager.Instance.Current); + Assert.Same(window, window.FocusManager.GetFocusedElement()); // Context menu in resources means the baseline may not be 0. var initialMenuCount = 0; @@ -608,7 +608,7 @@ namespace Avalonia.LeakTests var window = new Window { Focusable = true }; window.Show(); - Assert.Same(window, FocusManager.Instance.Current); + Assert.Same(window, window.FocusManager.GetFocusedElement()); // Context menu in resources means the baseline may not be 0. var initialMenuCount = 0; diff --git a/tests/Avalonia.UnitTests/TestRoot.cs b/tests/Avalonia.UnitTests/TestRoot.cs index c17eeda3e1..8dabfe2197 100644 --- a/tests/Avalonia.UnitTests/TestRoot.cs +++ b/tests/Avalonia.UnitTests/TestRoot.cs @@ -54,6 +54,7 @@ namespace Avalonia.UnitTests public IAccessKeyHandler AccessKeyHandler => null; public IKeyboardNavigationHandler KeyboardNavigationHandler => null; + public IFocusManager FocusManager => AvaloniaLocator.Current.GetService(); public IInputElement PointerOverElement { get; set; } From 6294093a18bde8ba5e5e0f6718b7c988868be7b9 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Tue, 16 May 2023 21:45:06 -0400 Subject: [PATCH 18/29] Use [PrivateApi] on members --- src/Avalonia.Base/Input/FocusManager.cs | 2 +- src/Avalonia.Base/Input/KeyboardDevice.cs | 2 +- src/Avalonia.Base/Input/MouseDevice.cs | 2 +- src/Avalonia.Base/Input/PenDevice.cs | 2 ++ src/Avalonia.Base/Input/TouchDevice.cs | 2 ++ 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Base/Input/FocusManager.cs b/src/Avalonia.Base/Input/FocusManager.cs index 4c464a44ae..da84a3272f 100644 --- a/src/Avalonia.Base/Input/FocusManager.cs +++ b/src/Avalonia.Base/Input/FocusManager.cs @@ -11,7 +11,7 @@ namespace Avalonia.Input /// /// Manages focus for the application. /// - [Unstable] + [PrivateApi] public class FocusManager : IFocusManager { /// diff --git a/src/Avalonia.Base/Input/KeyboardDevice.cs b/src/Avalonia.Base/Input/KeyboardDevice.cs index f6d2a2195a..a81bc6b2e0 100644 --- a/src/Avalonia.Base/Input/KeyboardDevice.cs +++ b/src/Avalonia.Base/Input/KeyboardDevice.cs @@ -7,7 +7,7 @@ using Avalonia.Metadata; namespace Avalonia.Input { - [Unstable] + [PrivateApi] public class KeyboardDevice : IKeyboardDevice, INotifyPropertyChanged { private IInputElement? _focusedElement; diff --git a/src/Avalonia.Base/Input/MouseDevice.cs b/src/Avalonia.Base/Input/MouseDevice.cs index 4aeffcdd72..f3e77433a9 100644 --- a/src/Avalonia.Base/Input/MouseDevice.cs +++ b/src/Avalonia.Base/Input/MouseDevice.cs @@ -12,7 +12,7 @@ namespace Avalonia.Input /// /// Represents a mouse device. /// - [Unstable] + [PrivateApi] public class MouseDevice : IMouseDevice, IDisposable { private int _clickCount; diff --git a/src/Avalonia.Base/Input/PenDevice.cs b/src/Avalonia.Base/Input/PenDevice.cs index b3cd39212b..832f32fb03 100644 --- a/src/Avalonia.Base/Input/PenDevice.cs +++ b/src/Avalonia.Base/Input/PenDevice.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using Avalonia.Input.Raw; +using Avalonia.Metadata; using Avalonia.Platform; #pragma warning disable CS0618 @@ -11,6 +12,7 @@ namespace Avalonia.Input /// /// Represents a pen/stylus device. /// + [PrivateApi] public class PenDevice : IPenDevice, IDisposable { private readonly Dictionary _pointers = new(); diff --git a/src/Avalonia.Base/Input/TouchDevice.cs b/src/Avalonia.Base/Input/TouchDevice.cs index bab1b9f784..78b570da14 100644 --- a/src/Avalonia.Base/Input/TouchDevice.cs +++ b/src/Avalonia.Base/Input/TouchDevice.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using Avalonia.Input.Raw; +using Avalonia.Metadata; using Avalonia.Platform; #pragma warning disable CS0618 @@ -14,6 +15,7 @@ namespace Avalonia.Input /// /// This class is supposed to be used on per-toplevel basis, don't use a shared one /// + [PrivateApi] public class TouchDevice : IPointerDevice, IDisposable { private readonly Dictionary _pointers = new Dictionary(); From cd9a19387aef7395794d71170071926347dc0be4 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Tue, 16 May 2023 21:52:08 -0400 Subject: [PATCH 19/29] Keep FocusManager.OnPreviewPointerPressed static as it was before --- src/Avalonia.Base/Input/FocusManager.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Base/Input/FocusManager.cs b/src/Avalonia.Base/Input/FocusManager.cs index da84a3272f..2127e19627 100644 --- a/src/Avalonia.Base/Input/FocusManager.cs +++ b/src/Avalonia.Base/Input/FocusManager.cs @@ -23,7 +23,7 @@ namespace Avalonia.Input /// /// Initializes a new instance of the class. /// - internal FocusManager() + static FocusManager() { InputElement.PointerPressedEvent.AddClassHandler( typeof(IInputElement), @@ -237,7 +237,7 @@ namespace Avalonia.Input /// /// The event sender. /// The event args. - private void OnPreviewPointerPressed(object? sender, RoutedEventArgs e) + private static void OnPreviewPointerPressed(object? sender, RoutedEventArgs e) { if (sender is null) return; @@ -253,7 +253,7 @@ namespace Avalonia.Input { if (element is IInputElement inputElement && CanFocus(inputElement)) { - Focus(inputElement, NavigationMethod.Pointer, ev.KeyModifiers); + inputElement.Focus(NavigationMethod.Pointer, ev.KeyModifiers); break; } From 44e3a532b600318db6b7b69e0c3a377cb7e45b88 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Tue, 16 May 2023 21:56:11 -0400 Subject: [PATCH 20/29] Fix comment --- src/Avalonia.Base/Input/FocusManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Input/FocusManager.cs b/src/Avalonia.Base/Input/FocusManager.cs index 2127e19627..214ba21c5c 100644 --- a/src/Avalonia.Base/Input/FocusManager.cs +++ b/src/Avalonia.Base/Input/FocusManager.cs @@ -197,7 +197,7 @@ namespace Avalonia.Input // Element might not be a visual, and not attached to the root. // But IFocusManager is always expected to be a FocusManager. return (FocusManager?)((IInputRoot?)(element as Visual)?.VisualRoot)?.FocusManager - // In our unit tests some elements might not have a root. Remove when + // In our unit tests some elements might not have a root. Remove when we migrate to headless tests. ?? (FocusManager?)AvaloniaLocator.Current.GetService(); } From d3fbc105af38328ff25a30a442b57038e13acf11 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Tue, 16 May 2023 23:26:11 -0400 Subject: [PATCH 21/29] Fix "GetFocusManager" with some unit tests --- src/Avalonia.Base/Input/FocusManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Input/FocusManager.cs b/src/Avalonia.Base/Input/FocusManager.cs index 214ba21c5c..c3316eb278 100644 --- a/src/Avalonia.Base/Input/FocusManager.cs +++ b/src/Avalonia.Base/Input/FocusManager.cs @@ -196,7 +196,7 @@ namespace Avalonia.Input { // Element might not be a visual, and not attached to the root. // But IFocusManager is always expected to be a FocusManager. - return (FocusManager?)((IInputRoot?)(element as Visual)?.VisualRoot)?.FocusManager + return (FocusManager?)((element as Visual)?.VisualRoot as IInputRoot)?.FocusManager // In our unit tests some elements might not have a root. Remove when we migrate to headless tests. ?? (FocusManager?)AvaloniaLocator.Current.GetService(); } From af346d31785633d67ee2ba0db1c26a0a17f59dff Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 17 May 2023 13:30:51 +0100 Subject: [PATCH 22/29] bump build services with latest fixes. --- packages/Avalonia/Avalonia.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/Avalonia/Avalonia.csproj b/packages/Avalonia/Avalonia.csproj index 412251bc9c..9f2cf870f9 100644 --- a/packages/Avalonia/Avalonia.csproj +++ b/packages/Avalonia/Avalonia.csproj @@ -5,7 +5,7 @@ - + all From 4b09e99382cd5f8cc8c4e8c090c061e5dfce1cf6 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 17 May 2023 15:49:58 +0100 Subject: [PATCH 23/29] bump avalonia build services. --- packages/Avalonia/Avalonia.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/Avalonia/Avalonia.csproj b/packages/Avalonia/Avalonia.csproj index 9f2cf870f9..8ac2fb28ed 100644 --- a/packages/Avalonia/Avalonia.csproj +++ b/packages/Avalonia/Avalonia.csproj @@ -5,7 +5,7 @@ - + all From c6e047e6d4dff79dcd08cfd56a5193991eb56cf7 Mon Sep 17 00:00:00 2001 From: wangchuang <515497782@qq.com> Date: Thu, 18 May 2023 09:37:56 +0800 Subject: [PATCH 24/29] fix: CalendarItem PopulateGrids --- src/Avalonia.Controls/Calendar/CalendarItem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Calendar/CalendarItem.cs b/src/Avalonia.Controls/Calendar/CalendarItem.cs index 2e3f1f96ce..e81ed8072e 100644 --- a/src/Avalonia.Controls/Calendar/CalendarItem.cs +++ b/src/Avalonia.Controls/Calendar/CalendarItem.cs @@ -171,7 +171,7 @@ namespace Avalonia.Controls.Primitives var childCount = Calendar.RowsPerMonth + Calendar.RowsPerMonth * Calendar.ColumnsPerMonth; using var children = new PooledList(childCount); - for (int i = 0; i < Calendar.RowsPerMonth; i++) + for (int i = 0; i < Calendar.ColumnsPerMonth; i++) { if (DayTitleTemplate?.Build() is Control cell) { From ca02947c3ee9d24782146e2e784c50b84a16ca5c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 18 May 2023 10:35:08 +0200 Subject: [PATCH 25/29] Added failing test for VirtualizingStackPanel. --- .../VirtualizingStackPanelTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs index aa03a77d70..1fddaab910 100644 --- a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs @@ -657,6 +657,40 @@ namespace Avalonia.Controls.UnitTests root.LayoutManager.ExecuteLayoutPass(); } + [Fact] + public void ScrollIntoView_On_Effectively_Invisible_Panel_Does_Not_Create_Ghost_Elements() + { + var items = new[] { "foo", "bar", "baz" }; + var (target, _, itemsControl) = CreateUnrootedTarget(items: items); + var container = new Decorator { Margin = new Thickness(100), Child = itemsControl }; + var root = new TestRoot(true, container); + + root.LayoutManager.ExecuteInitialLayoutPass(); + + // Clear the items and do a layout to recycle all elements. + itemsControl.ItemsSource = null; + root.LayoutManager.ExecuteLayoutPass(); + + // Should have no realized elements and 3 unrealized elements. + Assert.Equal(0, target.GetRealizedElements().Count); + Assert.Equal(3, target.Children.Count); + + // Make the panel effectively invisible and set items. + container.IsVisible = false; + itemsControl.ItemsSource = items; + + // Try to scroll into view while effectively invisible. + target.ScrollIntoView(0); + + // Make the panel visible and layout. + container.IsVisible = true; + root.LayoutManager.ExecuteLayoutPass(); + + // Should have 3 realized elements and no unrealized elements. + Assert.Equal(3, target.GetRealizedElements().Count); + Assert.Equal(3, target.Children.Count); + } + private static IReadOnlyList GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl) { return target.GetRealizedElements() From 5a718074049b3e0e8dc7ba235ba00ade3dec0249 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 18 May 2023 10:51:54 +0200 Subject: [PATCH 26/29] Don't try to scroll into view on invisible panel. It won't work because layout won't be run, and will result in a ghost element. --- src/Avalonia.Controls/VirtualizingStackPanel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index 7ec1808e63..0580761631 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/src/Avalonia.Controls/VirtualizingStackPanel.cs @@ -343,7 +343,7 @@ namespace Avalonia.Controls { var items = Items; - if (_isInLayout || index < 0 || index >= items.Count || _realizedElements is null) + if (_isInLayout || index < 0 || index >= items.Count || _realizedElements is null || !IsEffectivelyVisible) return null; if (GetRealizedElement(index) is Control element) From 04c50f8bf26c3c867fb6f9a6b2349ef0752e2dad Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 18 May 2023 13:12:15 +0200 Subject: [PATCH 27/29] Make style selector internals internal. --- .../Styling/Activators/IStyleActivator.cs | 3 +- .../Styling/Activators/IStyleActivatorSink.cs | 3 +- src/Avalonia.Base/Styling/ChildSelector.cs | 12 +++---- .../Styling/DescendentSelector.cs | 12 +++---- src/Avalonia.Base/Styling/NestingSelector.cs | 12 +++---- src/Avalonia.Base/Styling/NotSelector.cs | 12 +++---- src/Avalonia.Base/Styling/NthChildSelector.cs | 14 ++++---- .../Styling/NthLastChildSelector.cs | 2 +- src/Avalonia.Base/Styling/OrSelector.cs | 12 +++---- .../Styling/PropertyEqualsSelector.cs | 12 +++---- src/Avalonia.Base/Styling/Selector.cs | 14 ++++---- src/Avalonia.Base/Styling/SelectorMatch.cs | 4 +-- src/Avalonia.Base/Styling/TemplateSelector.cs | 12 +++---- .../Styling/TypeNameAndClassSelector.cs | 12 +++---- .../Styling/SelectorBenchmark.cs | 36 +++++++++---------- tests/Avalonia.UnitTests/StyleHelpers.cs | 4 +-- 16 files changed, 87 insertions(+), 89 deletions(-) diff --git a/src/Avalonia.Base/Styling/Activators/IStyleActivator.cs b/src/Avalonia.Base/Styling/Activators/IStyleActivator.cs index 487198a861..7bc0777c61 100644 --- a/src/Avalonia.Base/Styling/Activators/IStyleActivator.cs +++ b/src/Avalonia.Base/Styling/Activators/IStyleActivator.cs @@ -15,8 +15,7 @@ namespace Avalonia.Styling.Activators /// - The activation state can be re-evaluated at any time by calling /// - No error or completion messages /// - [Unstable] - public interface IStyleActivator : IDisposable + internal interface IStyleActivator : IDisposable { /// /// Gets a value indicating whether the style is subscribed. diff --git a/src/Avalonia.Base/Styling/Activators/IStyleActivatorSink.cs b/src/Avalonia.Base/Styling/Activators/IStyleActivatorSink.cs index 142a3c3517..6d49485c20 100644 --- a/src/Avalonia.Base/Styling/Activators/IStyleActivatorSink.cs +++ b/src/Avalonia.Base/Styling/Activators/IStyleActivatorSink.cs @@ -5,8 +5,7 @@ namespace Avalonia.Styling.Activators /// /// Receives notifications from an . /// - [Unstable] - public interface IStyleActivatorSink + internal interface IStyleActivatorSink { /// /// Called when the subscribed activator value changes. diff --git a/src/Avalonia.Base/Styling/ChildSelector.cs b/src/Avalonia.Base/Styling/ChildSelector.cs index 400ba18530..ac28d2bc46 100644 --- a/src/Avalonia.Base/Styling/ChildSelector.cs +++ b/src/Avalonia.Base/Styling/ChildSelector.cs @@ -19,13 +19,13 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => _parent.InTemplate; + internal override bool InTemplate => _parent.InTemplate; /// - public override bool IsCombinator => true; + internal override bool IsCombinator => true; /// - public override Type? TargetType => null; + internal override Type? TargetType => null; public override string ToString(Style? owner) { @@ -37,7 +37,7 @@ namespace Avalonia.Styling return _selectorString; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { var controlParent = ((ILogical)control).LogicalParent; @@ -64,7 +64,7 @@ namespace Avalonia.Styling } } - protected override Selector? MovePrevious() => null; - protected override Selector? MovePreviousOrParent() => _parent; + private protected override Selector? MovePrevious() => null; + private protected override Selector? MovePreviousOrParent() => _parent; } } diff --git a/src/Avalonia.Base/Styling/DescendentSelector.cs b/src/Avalonia.Base/Styling/DescendentSelector.cs index 20874a6877..6706eb4441 100644 --- a/src/Avalonia.Base/Styling/DescendentSelector.cs +++ b/src/Avalonia.Base/Styling/DescendentSelector.cs @@ -17,13 +17,13 @@ namespace Avalonia.Styling } /// - public override bool IsCombinator => true; + internal override bool IsCombinator => true; /// - public override bool InTemplate => _parent.InTemplate; + internal override bool InTemplate => _parent.InTemplate; /// - public override Type? TargetType => null; + internal override Type? TargetType => null; public override string ToString(Style? owner) { @@ -35,7 +35,7 @@ namespace Avalonia.Styling return _selectorString; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { var c = (ILogical)control; var descendantMatches = new OrActivatorBuilder(); @@ -69,7 +69,7 @@ namespace Avalonia.Styling } } - protected override Selector? MovePrevious() => null; - protected override Selector? MovePreviousOrParent() => _parent; + private protected override Selector? MovePrevious() => null; + private protected override Selector? MovePreviousOrParent() => _parent; } } diff --git a/src/Avalonia.Base/Styling/NestingSelector.cs b/src/Avalonia.Base/Styling/NestingSelector.cs index deb688ca4d..4a6b0cfda9 100644 --- a/src/Avalonia.Base/Styling/NestingSelector.cs +++ b/src/Avalonia.Base/Styling/NestingSelector.cs @@ -7,13 +7,13 @@ namespace Avalonia.Styling /// internal class NestingSelector : Selector { - public override bool InTemplate => false; - public override bool IsCombinator => false; - public override Type? TargetType => null; + internal override bool InTemplate => false; + internal override bool IsCombinator => false; + internal override Type? TargetType => null; public override string ToString(Style? owner) => owner?.Parent?.ToString() ?? "^"; - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { if (parent is Style s && s.Selector is not null) { @@ -32,7 +32,7 @@ namespace Avalonia.Styling "Nesting selector was specified but cannot determine parent selector."); } - protected override Selector? MovePrevious() => null; - protected override Selector? MovePreviousOrParent() => null; + private protected override Selector? MovePrevious() => null; + private protected override Selector? MovePreviousOrParent() => null; } } diff --git a/src/Avalonia.Base/Styling/NotSelector.cs b/src/Avalonia.Base/Styling/NotSelector.cs index f6b1288ac5..9a541cbba7 100644 --- a/src/Avalonia.Base/Styling/NotSelector.cs +++ b/src/Avalonia.Base/Styling/NotSelector.cs @@ -26,13 +26,13 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => _argument.InTemplate; + internal override bool InTemplate => _argument.InTemplate; /// - public override bool IsCombinator => false; + internal override bool IsCombinator => false; /// - public override Type? TargetType => _previous?.TargetType; + internal override Type? TargetType => _previous?.TargetType; /// public override string ToString(Style? owner) @@ -45,7 +45,7 @@ namespace Avalonia.Styling return _selectorString; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { var innerResult = _argument.Match(control, parent, subscribe); @@ -66,7 +66,7 @@ namespace Avalonia.Styling } } - protected override Selector? MovePrevious() => _previous; - protected override Selector? MovePreviousOrParent() => _previous; + private protected override Selector? MovePrevious() => _previous; + private protected override Selector? MovePreviousOrParent() => _previous; } } diff --git a/src/Avalonia.Base/Styling/NthChildSelector.cs b/src/Avalonia.Base/Styling/NthChildSelector.cs index 532179bb2c..bf6247aba1 100644 --- a/src/Avalonia.Base/Styling/NthChildSelector.cs +++ b/src/Avalonia.Base/Styling/NthChildSelector.cs @@ -12,7 +12,7 @@ namespace Avalonia.Styling /// /// Element indices are 1-based. /// - public class NthChildSelector : Selector + internal class NthChildSelector : Selector { private const string NthChildSelectorName = "nth-child"; private const string NthLastChildSelectorName = "nth-last-child"; @@ -39,16 +39,16 @@ namespace Avalonia.Styling } - public override bool InTemplate => _previous?.InTemplate ?? false; + internal override bool InTemplate => _previous?.InTemplate ?? false; - public override bool IsCombinator => false; + internal override bool IsCombinator => false; - public override Type? TargetType => _previous?.TargetType; + internal override Type? TargetType => _previous?.TargetType; public int Step { get; } public int Offset { get; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { if (!(control is ILogical logical)) { @@ -103,8 +103,8 @@ namespace Avalonia.Styling return match ? SelectorMatch.AlwaysThisInstance : SelectorMatch.NeverThisInstance; } - protected override Selector? MovePrevious() => _previous; - protected override Selector? MovePreviousOrParent() => _previous; + private protected override Selector? MovePrevious() => _previous; + private protected override Selector? MovePreviousOrParent() => _previous; public override string ToString(Style? owner) { diff --git a/src/Avalonia.Base/Styling/NthLastChildSelector.cs b/src/Avalonia.Base/Styling/NthLastChildSelector.cs index 6f6abbae6a..aa62ad2b2c 100644 --- a/src/Avalonia.Base/Styling/NthLastChildSelector.cs +++ b/src/Avalonia.Base/Styling/NthLastChildSelector.cs @@ -8,7 +8,7 @@ namespace Avalonia.Styling /// /// Element indices are 1-based. /// - public class NthLastChildSelector : NthChildSelector + internal class NthLastChildSelector : NthChildSelector { /// /// Creates an instance of diff --git a/src/Avalonia.Base/Styling/OrSelector.cs b/src/Avalonia.Base/Styling/OrSelector.cs index 53e4baa2c4..cc77aa9fcf 100644 --- a/src/Avalonia.Base/Styling/OrSelector.cs +++ b/src/Avalonia.Base/Styling/OrSelector.cs @@ -36,13 +36,13 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => false; + internal override bool InTemplate => false; /// - public override bool IsCombinator => false; + internal override bool IsCombinator => false; /// - public override Type? TargetType => _targetType ??= EvaluateTargetType(); + internal override Type? TargetType => _targetType ??= EvaluateTargetType(); /// public override string ToString(Style? owner) @@ -55,7 +55,7 @@ namespace Avalonia.Styling return _selectorString; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { var activators = new OrActivatorBuilder(); var neverThisInstance = false; @@ -94,8 +94,8 @@ namespace Avalonia.Styling } } - protected override Selector? MovePrevious() => null; - protected override Selector? MovePreviousOrParent() => null; + private protected override Selector? MovePrevious() => null; + private protected override Selector? MovePreviousOrParent() => null; internal override void ValidateNestingSelector(bool inControlTheme) { diff --git a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs index 0865a7a8b0..3a50923094 100644 --- a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs +++ b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs @@ -28,13 +28,13 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => _previous?.InTemplate ?? false; + internal override bool InTemplate => _previous?.InTemplate ?? false; /// - public override bool IsCombinator => false; + internal override bool IsCombinator => false; /// - public override Type? TargetType => _previous?.TargetType; + internal override Type? TargetType => _previous?.TargetType; /// public override string ToString(Style? owner) @@ -73,7 +73,7 @@ namespace Avalonia.Styling } /// - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { if (subscribe) { @@ -88,8 +88,8 @@ namespace Avalonia.Styling } - protected override Selector? MovePrevious() => _previous; - protected override Selector? MovePreviousOrParent() => _previous; + private protected override Selector? MovePrevious() => _previous; + private protected override Selector? MovePreviousOrParent() => _previous; [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.TypeConvertionSupressWarningMessage)] [UnconditionalSuppressMessage("Trimming", "IL2067", Justification = TrimmingMessages.TypeConvertionSupressWarningMessage)] diff --git a/src/Avalonia.Base/Styling/Selector.cs b/src/Avalonia.Base/Styling/Selector.cs index c83950f72d..4e1c338553 100644 --- a/src/Avalonia.Base/Styling/Selector.cs +++ b/src/Avalonia.Base/Styling/Selector.cs @@ -14,7 +14,7 @@ namespace Avalonia.Styling /// Gets a value indicating whether either this selector or a previous selector has moved /// into a template. /// - public abstract bool InTemplate { get; } + internal abstract bool InTemplate { get; } /// /// Gets a value indicating whether this selector is a combinator. @@ -22,12 +22,12 @@ namespace Avalonia.Styling /// /// A combinator is a selector such as Child or Descendent which links simple selectors. /// - public abstract bool IsCombinator { get; } + internal abstract bool IsCombinator { get; } /// /// Gets the target type of the selector, if available. /// - public abstract Type? TargetType { get; } + internal abstract Type? TargetType { get; } /// /// Tries to match the selector with a control. @@ -41,7 +41,7 @@ namespace Avalonia.Styling /// or simply return an immediate result. /// /// A . - public SelectorMatch Match(StyledElement control, IStyle? parent = null, bool subscribe = true) + internal SelectorMatch Match(StyledElement control, IStyle? parent = null, bool subscribe = true) { // First match the selector until a combinator is found. Selectors are stored from // right-to-left, so MatchUntilCombinator reverses this order because the type selector @@ -88,17 +88,17 @@ namespace Avalonia.Styling /// or simply return an immediate result. /// /// A . - protected abstract SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe); + private protected abstract SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe); /// /// Moves to the previous selector. /// - protected abstract Selector? MovePrevious(); + private protected abstract Selector? MovePrevious(); /// /// Moves to the previous selector or the parent selector. /// - protected abstract Selector? MovePreviousOrParent(); + private protected abstract Selector? MovePreviousOrParent(); internal virtual void ValidateNestingSelector(bool inControlTheme) { diff --git a/src/Avalonia.Base/Styling/SelectorMatch.cs b/src/Avalonia.Base/Styling/SelectorMatch.cs index 2eac04301a..cbcab612d6 100644 --- a/src/Avalonia.Base/Styling/SelectorMatch.cs +++ b/src/Avalonia.Base/Styling/SelectorMatch.cs @@ -8,7 +8,7 @@ namespace Avalonia.Styling /// /// Describes how a matches a control and its type. /// - public enum SelectorMatchResult + internal enum SelectorMatchResult { /// /// The selector never matches this type. @@ -43,7 +43,7 @@ namespace Avalonia.Styling /// A selector match describes whether and how a matches a control, and /// in addition whether the selector can ever match a control of the same type. /// - public readonly record struct SelectorMatch + internal readonly record struct SelectorMatch { /// /// A selector match with the result of . diff --git a/src/Avalonia.Base/Styling/TemplateSelector.cs b/src/Avalonia.Base/Styling/TemplateSelector.cs index b253efc6d2..1fa2ca2d0f 100644 --- a/src/Avalonia.Base/Styling/TemplateSelector.cs +++ b/src/Avalonia.Base/Styling/TemplateSelector.cs @@ -18,13 +18,13 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => true; + internal override bool InTemplate => true; /// - public override bool IsCombinator => true; + internal override bool IsCombinator => true; /// - public override Type? TargetType => null; + internal override Type? TargetType => null; public override string ToString(Style? owner) { @@ -36,7 +36,7 @@ namespace Avalonia.Styling return _selectorString; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { var templatedParent = control.TemplatedParent as StyledElement; @@ -48,7 +48,7 @@ namespace Avalonia.Styling return _parent.Match(templatedParent, parent, subscribe); } - protected override Selector? MovePrevious() => null; - protected override Selector? MovePreviousOrParent() => _parent; + private protected override Selector? MovePrevious() => null; + private protected override Selector? MovePreviousOrParent() => _parent; } } diff --git a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs index 2bd05242f5..81b204761b 100644 --- a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs @@ -58,7 +58,7 @@ namespace Avalonia.Styling } /// - public override bool InTemplate => _previous?.InTemplate ?? false; + internal override bool InTemplate => _previous?.InTemplate ?? false; /// /// Gets the name of the control to match. @@ -66,10 +66,10 @@ namespace Avalonia.Styling public string? Name { get; set; } /// - public override Type? TargetType => _targetType ?? _previous?.TargetType; + internal override Type? TargetType => _targetType ?? _previous?.TargetType; /// - public override bool IsCombinator => false; + internal override bool IsCombinator => false; /// /// Whether the selector matches the concrete or any object which @@ -89,7 +89,7 @@ namespace Avalonia.Styling } /// - protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent, bool subscribe) { if (TargetType != null) { @@ -134,8 +134,8 @@ namespace Avalonia.Styling return Name == null ? SelectorMatch.AlwaysThisType : SelectorMatch.AlwaysThisInstance; } - protected override Selector? MovePrevious() => _previous; - protected override Selector? MovePreviousOrParent() => _previous; + private protected override Selector? MovePrevious() => _previous; + private protected override Selector? MovePreviousOrParent() => _previous; private string BuildSelectorString(Style? owner) { diff --git a/tests/Avalonia.Benchmarks/Styling/SelectorBenchmark.cs b/tests/Avalonia.Benchmarks/Styling/SelectorBenchmark.cs index 11bc5ce35f..e4799d46b8 100644 --- a/tests/Avalonia.Benchmarks/Styling/SelectorBenchmark.cs +++ b/tests/Avalonia.Benchmarks/Styling/SelectorBenchmark.cs @@ -37,62 +37,62 @@ namespace Avalonia.Benchmarks.Styling } [Benchmark] - public SelectorMatch IsSelector_NoMatch() + public void IsSelector_NoMatch() { - return _isCalendarSelector.Match(_notMatchingControl); + _isCalendarSelector.Match(_notMatchingControl); } [Benchmark] - public SelectorMatch IsSelector_Match() + public void IsSelector_Match() { - return _isCalendarSelector.Match(_matchingControl); + _isCalendarSelector.Match(_matchingControl); } [Benchmark] - public SelectorMatch ClassSelector_NoMatch() + public void ClassSelector_NoMatch() { - return _classSelector.Match(_notMatchingControl); + _classSelector.Match(_notMatchingControl); } [Benchmark] - public SelectorMatch ClassSelector_Match() + public void ClassSelector_Match() { - return _classSelector.Match(_matchingControl); + _classSelector.Match(_matchingControl); } [Benchmark] - public SelectorMatch OrSelector_One_Match() + public void OrSelector_One_Match() { - return _orSelectorTwo.Match(_matchingControl); + _orSelectorTwo.Match(_matchingControl); } [Benchmark] - public SelectorMatch OrSelector_Five_Match() + public void OrSelector_Five_Match() { - return _orSelectorFive.Match(_matchingControl); + _orSelectorFive.Match(_matchingControl); } } internal class AlwaysMatchSelector : Selector { - public override bool InTemplate => false; + internal override bool InTemplate => false; - public override bool IsCombinator => false; + internal override bool IsCombinator => false; - public override Type TargetType => null; + internal override Type TargetType => null; public override string ToString(Style owner) { return "Always"; } - protected override SelectorMatch Evaluate(StyledElement control, IStyle parent, bool subscribe) + private protected override SelectorMatch Evaluate(StyledElement control, IStyle parent, bool subscribe) { return SelectorMatch.AlwaysThisType; } - protected override Selector MovePrevious() => null; + private protected override Selector MovePrevious() => null; - protected override Selector MovePreviousOrParent() => null; + private protected override Selector MovePreviousOrParent() => null; } } diff --git a/tests/Avalonia.UnitTests/StyleHelpers.cs b/tests/Avalonia.UnitTests/StyleHelpers.cs index 00f74769d3..ed95519830 100644 --- a/tests/Avalonia.UnitTests/StyleHelpers.cs +++ b/tests/Avalonia.UnitTests/StyleHelpers.cs @@ -6,9 +6,9 @@ namespace Avalonia.UnitTests { public static class StyleHelpers { - public static SelectorMatchResult TryAttach(Style style, StyledElement element, object? host = null) + public static void TryAttach(Style style, StyledElement element, object? host = null) { - return style.TryAttach(element, host ?? element, PropertyStore.FrameType.Style); + style.TryAttach(element, host ?? element, PropertyStore.FrameType.Style); } } } From 34a83e26d14abf793dedc2b7f1d773847de73e90 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 18 May 2023 14:39:58 +0200 Subject: [PATCH 28/29] Renamed ISetter as SetterBase. Allows us to hide the mechanics of initializing a setter. --- src/Avalonia.Base/Styling/ISetter.cs | 24 ------------------- src/Avalonia.Base/Styling/ISetterInstance.cs | 2 +- src/Avalonia.Base/Styling/ISetterValue.cs | 4 ++-- src/Avalonia.Base/Styling/Setter.cs | 4 ++-- src/Avalonia.Base/Styling/SetterBase.cs | 12 ++++++++++ src/Avalonia.Base/Styling/StyleBase.cs | 6 ++--- src/Avalonia.Controls/ContextMenu.cs | 2 +- src/Avalonia.Controls/Control.cs | 2 +- .../AvaloniaXamlIlBindingPathTransformer.cs | 2 +- .../AvaloniaXamlIlWellKnownTypes.cs | 4 ++-- .../Avalonia.Markup/Data/TemplateBinding.cs | 2 +- 11 files changed, 26 insertions(+), 38 deletions(-) delete mode 100644 src/Avalonia.Base/Styling/ISetter.cs create mode 100644 src/Avalonia.Base/Styling/SetterBase.cs diff --git a/src/Avalonia.Base/Styling/ISetter.cs b/src/Avalonia.Base/Styling/ISetter.cs deleted file mode 100644 index 22af90b446..0000000000 --- a/src/Avalonia.Base/Styling/ISetter.cs +++ /dev/null @@ -1,24 +0,0 @@ -using System; -using Avalonia.Metadata; - -namespace Avalonia.Styling -{ - /// - /// Represents a setter for a . - /// - [NotClientImplementable] - public interface ISetter - { - /// - /// Instances a setter on a control. - /// - /// The style which contains the setter. - /// The control. - /// An . - /// - /// This method should return an which can be used to apply - /// the setter to the specified control. - /// - ISetterInstance Instance(IStyleInstance styleInstance, StyledElement target); - } -} diff --git a/src/Avalonia.Base/Styling/ISetterInstance.cs b/src/Avalonia.Base/Styling/ISetterInstance.cs index 4a65d6deeb..f8a7e7d346 100644 --- a/src/Avalonia.Base/Styling/ISetterInstance.cs +++ b/src/Avalonia.Base/Styling/ISetterInstance.cs @@ -3,7 +3,7 @@ namespace Avalonia.Styling { /// - /// Represents an that has been instanced on a control. + /// Represents a that has been instanced on a control. /// [Unstable] public interface ISetterInstance diff --git a/src/Avalonia.Base/Styling/ISetterValue.cs b/src/Avalonia.Base/Styling/ISetterValue.cs index 0fd245a429..800d8275b5 100644 --- a/src/Avalonia.Base/Styling/ISetterValue.cs +++ b/src/Avalonia.Base/Styling/ISetterValue.cs @@ -3,13 +3,13 @@ namespace Avalonia.Styling { /// - /// Customizes the behavior of a class when added as a value to an . + /// Customizes the behavior of a class when added as a value to a . /// public interface ISetterValue { /// /// Notifies that the object has been added as a setter value. /// - void Initialize(ISetter setter); + void Initialize(SetterBase setter); } } diff --git a/src/Avalonia.Base/Styling/Setter.cs b/src/Avalonia.Base/Styling/Setter.cs index 9b009be6d2..e5b2bed738 100644 --- a/src/Avalonia.Base/Styling/Setter.cs +++ b/src/Avalonia.Base/Styling/Setter.cs @@ -14,7 +14,7 @@ namespace Avalonia.Styling /// A is used to set a value on a /// depending on a condition. /// - public class Setter : ISetter, IValueEntry, ISetterInstance, IAnimationSetter + public class Setter : SetterBase, IValueEntry, ISetterInstance, IAnimationSetter { private object? _value; private DirectPropertySetterInstance? _direct; @@ -66,7 +66,7 @@ namespace Avalonia.Styling void IValueEntry.Unsubscribe() { } [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.ImplicitTypeConvertionSupressWarningMessage)] - ISetterInstance ISetter.Instance(IStyleInstance instance, StyledElement target) + internal override ISetterInstance Instance(IStyleInstance instance, StyledElement target) { if (target is not AvaloniaObject ao) throw new InvalidOperationException("Don't know how to instance a style on this type."); diff --git a/src/Avalonia.Base/Styling/SetterBase.cs b/src/Avalonia.Base/Styling/SetterBase.cs new file mode 100644 index 0000000000..24cd525130 --- /dev/null +++ b/src/Avalonia.Base/Styling/SetterBase.cs @@ -0,0 +1,12 @@ +namespace Avalonia.Styling +{ + /// + /// Represents the base class for value setters. + /// + public abstract class SetterBase + { + internal abstract ISetterInstance Instance( + IStyleInstance styleInstance, + StyledElement target); + } +} diff --git a/src/Avalonia.Base/Styling/StyleBase.cs b/src/Avalonia.Base/Styling/StyleBase.cs index 7dfa516bce..318e8d6890 100644 --- a/src/Avalonia.Base/Styling/StyleBase.cs +++ b/src/Avalonia.Base/Styling/StyleBase.cs @@ -16,7 +16,7 @@ namespace Avalonia.Styling private IResourceHost? _owner; private StyleChildren? _children; private IResourceDictionary? _resources; - private List? _setters; + private List? _setters; private List? _animations; private StyleInstance? _sharedInstance; @@ -60,7 +60,7 @@ namespace Avalonia.Styling } } - public IList Setters => _setters ??= new List(); + public IList Setters => _setters ??= new(); public IList Animations => _animations ??= new List(); bool IResourceNode.HasResources => _resources?.Count > 0; @@ -69,7 +69,7 @@ namespace Avalonia.Styling internal bool HasChildren => _children?.Count > 0; internal bool HasSettersOrAnimations => _setters?.Count > 0 || _animations?.Count > 0; - public void Add(ISetter setter) => Setters.Add(setter); + public void Add(SetterBase setter) => Setters.Add(setter); public void Add(IStyle style) => Children.Add(style); public event EventHandler? OwnerChanged; diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 97a8c6fe97..39a98bd48a 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -285,7 +285,7 @@ namespace Avalonia.Controls } } - void ISetterValue.Initialize(ISetter setter) + void ISetterValue.Initialize(SetterBase setter) { // ContextMenu can be assigned to the ContextMenu property in a setter. This overrides // the behavior defined in Control which requires controls to be wrapped in a