Skip to content

Commit daeb933

Browse files
StephaneDelcroixPureWeen
authored andcommitted
[C] Fix Slider and Stepper property order independence (#32939)
> [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description This PR ensures that the `Value` property of `Slider` and `Stepper` controls is correctly preserved regardless of the order in which `Minimum`, `Maximum`, and `Value` properties are set (either programmatically or via XAML bindings). ## Problem When using XAML data binding, the order in which properties are applied depends on XAML attribute order and binding evaluation timing. The previous implementation would clamp `Value` immediately when `Minimum` or `Maximum` changed, using the current (potentially default) range. This caused: - `Value=50` with `Min=10, Max=100` would get clamped to `1` (default max) if `Value` was set before `Maximum` - The user's intended value was lost and could never be restored ## Solution The fix introduces three private fields: - `_requestedValue`: stores the user's intended value before clamping - `_userSetValue`: tracks if the user explicitly set `Value` (vs. automatic recoercion) - `_isRecoercing`: prevents `_requestedValue` corruption during recoercion When `Minimum` or `Maximum` changes: 1. If the user explicitly set `Value`, restore `_requestedValue` (clamped to new range) 2. If not, just clamp the current value to the new range This allows `Value` to "spring back" to the user's intended value when the range expands to include it. ## Issues Fixed - Fixes #32903 - Slider Binding Initialization Order Causes Incorrect Value Assignment in XAML - Fixes #14472 - Slider is very broken, Value is a mess when setting Minimum - Fixes #18910 - Slider is buggy depending on order of properties - Fixes #12243 - Stepper Value is incorrectly clamped to default min/max when using bindableproperties in MVVM pattern - Closes #32907 - ## Testing Added comprehensive unit tests for both `Slider` and `Stepper`: - All 6 permutations of setting Min, Max, Value in different orders - Tests for `_requestedValue` preservation across multiple range changes - Tests for value clamping when only range changes (user didn't set Value) **Test Results:** 98 tests passed (39 Slider + 59 Stepper) ## Breaking Changes **Behavioral change in event ordering:** The order of `PropertyChanged` events for `Stepper` may change in edge cases where `Minimum`/`Maximum` changes trigger a `Value` change. Previously, `Value` changed before `Min`/`Max`; now it changes after. This is an implementation detail and should not affect correctly-written code. # Conflicts: # src/Controls/tests/Core.UnitTests/StepperUnitTests.cs
1 parent 2b1daa1 commit daeb933

4 files changed

Lines changed: 407 additions & 20 deletions

File tree

src/Controls/src/Core/Slider/Slider.cs

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,33 +10,64 @@ namespace Microsoft.Maui.Controls
1010
[DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
1111
public partial class Slider : View, ISliderController, IElementConfiguration<Slider>, ISlider
1212
{
13+
// Stores the value that was requested by the user, before clamping
14+
double _requestedValue = 0d;
15+
// Tracks if the user explicitly set Value (vs it being set by recoercion)
16+
bool _userSetValue = false;
17+
bool _isRecoercing = false;
18+
1319
/// <summary>Bindable property for <see cref="Minimum"/>.</summary>
14-
public static readonly BindableProperty MinimumProperty = BindableProperty.Create(nameof(Minimum), typeof(double), typeof(Slider), 0d, coerceValue: (bindable, value) =>
15-
{
16-
var slider = (Slider)bindable;
17-
slider.Value = slider.Value.Clamp((double)value, slider.Maximum);
18-
return value;
19-
});
20+
public static readonly BindableProperty MinimumProperty = BindableProperty.Create(
21+
nameof(Minimum), typeof(double), typeof(Slider), 0d,
22+
propertyChanged: (bindable, oldValue, newValue) =>
23+
{
24+
var slider = (Slider)bindable;
25+
slider.RecoerceValue();
26+
});
2027

2128
/// <summary>Bindable property for <see cref="Maximum"/>.</summary>
22-
public static readonly BindableProperty MaximumProperty = BindableProperty.Create(nameof(Maximum), typeof(double), typeof(Slider), 1d, coerceValue: (bindable, value) =>
23-
{
24-
var slider = (Slider)bindable;
25-
slider.Value = slider.Value.Clamp(slider.Minimum, (double)value);
26-
return value;
27-
});
29+
public static readonly BindableProperty MaximumProperty = BindableProperty.Create(
30+
nameof(Maximum), typeof(double), typeof(Slider), 1d,
31+
propertyChanged: (bindable, oldValue, newValue) =>
32+
{
33+
var slider = (Slider)bindable;
34+
slider.RecoerceValue();
35+
});
2836

2937
/// <summary>Bindable property for <see cref="Value"/>.</summary>
3038
public static readonly BindableProperty ValueProperty = BindableProperty.Create(nameof(Value), typeof(double), typeof(Slider), 0d, BindingMode.TwoWay, coerceValue: (bindable, value) =>
3139
{
3240
var slider = (Slider)bindable;
41+
// Only store the requested value if the user is setting it (not during recoercion)
42+
if (!slider._isRecoercing)
43+
{
44+
slider._requestedValue = (double)value;
45+
slider._userSetValue = true;
46+
}
3347
return ((double)value).Clamp(slider.Minimum, slider.Maximum);
3448
}, propertyChanged: (bindable, oldValue, newValue) =>
3549
{
3650
var slider = (Slider)bindable;
3751
slider.ValueChanged?.Invoke(slider, new ValueChangedEventArgs((double)oldValue, (double)newValue));
3852
});
3953

54+
void RecoerceValue()
55+
{
56+
_isRecoercing = true;
57+
try
58+
{
59+
// If the user explicitly set Value, try to restore the requested value within the new range
60+
if (_userSetValue)
61+
Value = _requestedValue;
62+
else
63+
Value = Value.Clamp(Minimum, Maximum);
64+
}
65+
finally
66+
{
67+
_isRecoercing = false;
68+
}
69+
}
70+
4071
/// <summary>Bindable property for <see cref="MinimumTrackColor"/>.</summary>
4172
public static readonly BindableProperty MinimumTrackColorProperty = BindableProperty.Create(nameof(MinimumTrackColor), typeof(Color), typeof(Slider), null);
4273

src/Controls/src/Core/Stepper/Stepper.cs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,41 @@ namespace Microsoft.Maui.Controls
1010
[DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
1111
public partial class Stepper : View, IElementConfiguration<Stepper>, IStepper
1212
{
13+
// Stores the value that was requested by the user, before clamping
14+
double _requestedValue = 0d;
15+
// Tracks if the user explicitly set Value (vs it being set by recoercion)
16+
bool _userSetValue = false;
17+
bool _isRecoercing = false;
18+
1319
/// <summary>Bindable property for <see cref="Maximum"/>.</summary>
1420
public static readonly BindableProperty MaximumProperty = BindableProperty.Create(nameof(Maximum), typeof(double), typeof(Stepper), 100.0,
1521
validateValue: (bindable, value) => (double)value >= ((Stepper)bindable).Minimum,
16-
coerceValue: (bindable, value) =>
22+
propertyChanged: (bindable, oldValue, newValue) =>
1723
{
1824
var stepper = (Stepper)bindable;
19-
stepper.Value = stepper.Value.Clamp(stepper.Minimum, (double)value);
20-
return value;
25+
stepper.RecoerceValue();
2126
});
2227

2328
/// <summary>Bindable property for <see cref="Minimum"/>.</summary>
2429
public static readonly BindableProperty MinimumProperty = BindableProperty.Create(nameof(Minimum), typeof(double), typeof(Stepper), 0.0,
2530
validateValue: (bindable, value) => (double)value <= ((Stepper)bindable).Maximum,
26-
coerceValue: (bindable, value) =>
31+
propertyChanged: (bindable, oldValue, newValue) =>
2732
{
2833
var stepper = (Stepper)bindable;
29-
stepper.Value = stepper.Value.Clamp((double)value, stepper.Maximum);
30-
return value;
34+
stepper.RecoerceValue();
3135
});
3236

3337
/// <summary>Bindable property for <see cref="Value"/>.</summary>
3438
public static readonly BindableProperty ValueProperty = BindableProperty.Create(nameof(Value), typeof(double), typeof(Stepper), 0.0, BindingMode.TwoWay,
3539
coerceValue: (bindable, value) =>
3640
{
3741
var stepper = (Stepper)bindable;
42+
// Only store the requested value if the user is setting it (not during recoercion)
43+
if (!stepper._isRecoercing)
44+
{
45+
stepper._requestedValue = (double)value;
46+
stepper._userSetValue = true;
47+
}
3848
return Math.Round(((double)value), stepper.digits).Clamp(stepper.Minimum, stepper.Maximum);
3949
},
4050
propertyChanged: (bindable, oldValue, newValue) =>
@@ -43,6 +53,23 @@ public partial class Stepper : View, IElementConfiguration<Stepper>, IStepper
4353
stepper.ValueChanged?.Invoke(stepper, new ValueChangedEventArgs((double)oldValue, (double)newValue));
4454
});
4555

56+
void RecoerceValue()
57+
{
58+
_isRecoercing = true;
59+
try
60+
{
61+
// If the user explicitly set Value, try to restore the requested value within the new range
62+
if (_userSetValue)
63+
Value = _requestedValue;
64+
else
65+
Value = Value.Clamp(Minimum, Maximum);
66+
}
67+
finally
68+
{
69+
_isRecoercing = false;
70+
}
71+
}
72+
4673
int digits = 4;
4774
//'-log10(increment) + 4' as rounding digits gives us 4 significant decimal digits after the most significant one.
4875
//If your increment uses more than 4 significant digits, you're holding it wrong.

src/Controls/tests/Core.UnitTests/SliderUnitTests.cs

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,172 @@ public void TestConstructor()
1919
Assert.Equal(50, slider.Value);
2020
}
2121

22+
// Tests for setting Min, Max, Value in all 6 possible orders
23+
// Order: Min, Max, Value
24+
[Theory]
25+
[InlineData(10, 100, 50)]
26+
[InlineData(0, 1, 0.5)]
27+
[InlineData(-100, 100, 0)]
28+
[InlineData(50, 150, 100)]
29+
public void SetProperties_MinMaxValue_Order(double min, double max, double value)
30+
{
31+
var slider = new Slider();
32+
slider.Minimum = min;
33+
slider.Maximum = max;
34+
slider.Value = value;
35+
36+
Assert.Equal(min, slider.Minimum);
37+
Assert.Equal(max, slider.Maximum);
38+
Assert.Equal(value, slider.Value);
39+
}
40+
41+
// Order: Min, Value, Max
42+
[Theory]
43+
[InlineData(10, 100, 50)]
44+
[InlineData(0, 1, 0.5)]
45+
[InlineData(-100, 100, 0)]
46+
[InlineData(50, 150, 100)]
47+
public void SetProperties_MinValueMax_Order(double min, double max, double value)
48+
{
49+
var slider = new Slider();
50+
slider.Minimum = min;
51+
slider.Value = value;
52+
slider.Maximum = max;
53+
54+
Assert.Equal(min, slider.Minimum);
55+
Assert.Equal(max, slider.Maximum);
56+
Assert.Equal(value, slider.Value);
57+
}
58+
59+
// Order: Max, Min, Value
60+
[Theory]
61+
[InlineData(10, 100, 50)]
62+
[InlineData(0, 1, 0.5)]
63+
[InlineData(-100, 100, 0)]
64+
[InlineData(50, 150, 100)]
65+
public void SetProperties_MaxMinValue_Order(double min, double max, double value)
66+
{
67+
var slider = new Slider();
68+
slider.Maximum = max;
69+
slider.Minimum = min;
70+
slider.Value = value;
71+
72+
Assert.Equal(min, slider.Minimum);
73+
Assert.Equal(max, slider.Maximum);
74+
Assert.Equal(value, slider.Value);
75+
}
76+
77+
// Order: Max, Value, Min
78+
[Theory]
79+
[InlineData(10, 100, 50)]
80+
[InlineData(0, 1, 0.5)]
81+
[InlineData(-100, 100, 0)]
82+
[InlineData(50, 150, 100)]
83+
public void SetProperties_MaxValueMin_Order(double min, double max, double value)
84+
{
85+
var slider = new Slider();
86+
slider.Maximum = max;
87+
slider.Value = value;
88+
slider.Minimum = min;
89+
90+
Assert.Equal(min, slider.Minimum);
91+
Assert.Equal(max, slider.Maximum);
92+
Assert.Equal(value, slider.Value);
93+
}
94+
95+
// Order: Value, Min, Max
96+
[Theory]
97+
[InlineData(10, 100, 50)]
98+
[InlineData(0, 1, 0.5)]
99+
[InlineData(-100, 100, 0)]
100+
[InlineData(50, 150, 100)]
101+
public void SetProperties_ValueMinMax_Order(double min, double max, double value)
102+
{
103+
var slider = new Slider();
104+
slider.Value = value;
105+
slider.Minimum = min;
106+
slider.Maximum = max;
107+
108+
Assert.Equal(min, slider.Minimum);
109+
Assert.Equal(max, slider.Maximum);
110+
Assert.Equal(value, slider.Value);
111+
}
112+
113+
// Order: Value, Max, Min
114+
[Theory]
115+
[InlineData(10, 100, 50)]
116+
[InlineData(0, 1, 0.5)]
117+
[InlineData(-100, 100, 0)]
118+
[InlineData(50, 150, 100)]
119+
public void SetProperties_ValueMaxMin_Order(double min, double max, double value)
120+
{
121+
var slider = new Slider();
122+
slider.Value = value;
123+
slider.Maximum = max;
124+
slider.Minimum = min;
125+
126+
Assert.Equal(min, slider.Minimum);
127+
Assert.Equal(max, slider.Maximum);
128+
Assert.Equal(value, slider.Value);
129+
}
130+
131+
// Tests that _requestedValue is preserved across multiple recoercions
132+
[Fact]
133+
public void RequestedValuePreservedAcrossMultipleRangeChanges()
134+
{
135+
var slider = new Slider();
136+
slider.Value = 50;
137+
slider.Minimum = -10;
138+
slider.Maximum = -1; // Value clamped to -1
139+
140+
Assert.Equal(-1, slider.Value);
141+
142+
slider.Maximum = -2; // Value should still be clamped, not corrupted
143+
144+
Assert.Equal(-2, slider.Value);
145+
146+
slider.Maximum = 100; // Now the original requested value (50) should be restored
147+
148+
Assert.Equal(50, slider.Value);
149+
}
150+
151+
[Fact]
152+
public void RequestedValuePreservedWhenMinimumChangesMultipleTimes()
153+
{
154+
var slider = new Slider();
155+
slider.Value = 5;
156+
slider.Maximum = 100;
157+
slider.Minimum = 10; // Value clamped to 10
158+
159+
Assert.Equal(10, slider.Value);
160+
161+
slider.Minimum = 20; // Value clamped to 20
162+
163+
Assert.Equal(20, slider.Value);
164+
165+
slider.Minimum = 0; // Original requested value (5) should be restored
166+
167+
Assert.Equal(5, slider.Value);
168+
}
169+
170+
[Fact]
171+
public void ValueClampedWhenOnlyRangeChanges()
172+
{
173+
var slider = new Slider(); // Value defaults to 0
174+
slider.Minimum = 10; // Value should clamp to 10
175+
slider.Maximum = 100;
176+
177+
Assert.Equal(10, slider.Value);
178+
179+
slider.Minimum = 5; // Value stays at 10 because 10 is within [5, 100]
180+
181+
Assert.Equal(10, slider.Value);
182+
183+
slider.Minimum = 15; // Value clamps to 15
184+
185+
Assert.Equal(15, slider.Value);
186+
}
187+
22188
[Fact]
23189
public void TestInvalidConstructor()
24190
{

0 commit comments

Comments
 (0)