ux improvement for range selector #964
Conversation
|
Hi @liakamp, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
| @@ -8,7 +8,7 @@ | |||
| <Setter Property="Template"> | |||
| <Setter.Value> | |||
| <ControlTemplate TargetType="controls:RangeSelector"> | |||
| <Grid Height="24"> | |||
| <Grid Height="40"> | |||
There was a problem hiding this comment.
Hello unfortunately this is a breaking change.
People who developed applications using this control don't expect this to change suddenly.
|
I see this as a bug. If it is difficult to use via touch, then it needs to be fixed. It should be more aligned with the Slider control which has a MinHeight of 44. And it you want to change it you need to restyle the control. I tested this today and can confirm that it's difficult to use the control on a touch device |
|
I agree with the difficulty but I won't flag it as bug. It is better with than without for sure. To avoid issue, we can either think about having a Boolean to control this behavior for instance |
|
I also think it is a bug, but @deltakosh I understand your concerns. I could add a dependency property, something like "IsTouchOptimized" to control this height, but still at some point shouldn't this be default behavior? |
|
For sure. But one of the promise of the toolkit is that you can trust it to build your app. Every time a braking change is introduced we break this promise. For now I would not like to as this one is easy to fix by changing the style. But we can at least introduced the DP and when moving to 2.0 just flag it as deprecated and fix the control for good Thoughts? |
|
@deltakosh you are right. I have once used a library that broke at some point, it was frustrating. I will add the DP and then it will be easy to set this as default in 2.0 or any other release, when the time is right. |
|
Thank you! |
|
@skendrot thanks for the info, I will also set this to 44 instead of 40 like the native Slider |
liakamp
changed the title
| { | ||
| _outOfRangeContentContainer.Height = _minThumb.Height = _maxThumb.Height = 24; | ||
| _minThumb.Width = _maxThumb.Width = 8; | ||
| _minThumb.Margin = _maxThumb.Margin = new Thickness(-8, 0, 0, 0); |
There was a problem hiding this comment.
I could not think of a more elegant solution here, there is no relation between the two cases (44-> 24, 20->8), any thoughts?
|
I added the DP, default is false so no breaking changes if someone already has it |
|
|
||
| if (IsTouchOptimized) | ||
| { | ||
| _outOfRangeContentContainer.Height = _minThumb.Height = _maxThumb.Height = 44; |
There was a problem hiding this comment.
just check if these properties are not null
The RangeSlider on a touchscreen (mobile/tablet) was extremely difficult to use, because you had to drag the two thumbs who were very small for touch. I added a grid containing the thumb border in the thumb template, a bit bigger with a transparent background. Now it is a lot easier to manipulate the thumbs using touch.
Also I modified the slider's sample page a bit because now the control requires more height and I had to adjust the alignment of the min/max indicators.