RangeSelector: Moving past second handle

[stephtr]

Fixes #4029

When grabbing one thumb of the RangeSelector, its movement was restricted by the position of the other thumb; this PR makes it move beyond, automatically moving also the second thumb.

PR Type

Feature

What is the current behavior?

When grabbing one thumb of the RangeSelector, its movement was restricted by the position of the other thumb.

What is the new behavior?

When moving for example the RangeStart thumb beyond the RangeEnd one, the latter gets also moved. This is also more consistent with the keyboard arrow key behaviour when operating the control.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

Other information

One might also think about getting rid of the min and max argument of DragThumb; SyncThumbs, when being called from RangeSelector.Input.Key is the only one relying on those arguments, even though I don't know, whether that's actually necessary.

[ghost]

Thanks stephtr for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[michael-hawker]

FYI @RosarioPulella you've been looking at RangeSelector a lot, so want to take an initial look?

[Rosuavio]

Yep, I was thinking maybe we should hold this until the UI tests #4007 are merged, and see if this breaks any of them and if we can add more UI test. I will try to get the UI tests done soon.

[Kyaa-dost]

Thanks, @stephtr for the work. As of now, it looks good to me 🚀 Will test again once Rosario's PR is merged.

[Rosuavio]

@stephtr so my UITest PR had to go more in the direction of fixing the UITests infra. I have yet to add more significant tests for the control, but there is a space to do that now. While I am going to add more tests for this control, do you want to add any more specifically around this behavior?

Since there are not many other UITests, if you need help with writing UITests just let me know. A few tests right now will serve as good examples for the rest of the community.

[stephtr]

I don't know whether this behavior wouldn't be too specific (and anyway not specified) for being included in UI tests. If you wish, I could give it a try somewhen in the next few days.

[michael-hawker]

@stephtr never hurts to have more tests. It also means that it helps protect your contribution from regression if other logic in the control is updated or refactored. That way your feature can be tested and ensure that it's still working now and in the future!

[michael-hawker]

@stephtr wanted to check if you were still planning to add a test or if we should finish a review? Thanks!

[stephtr]

I gave it a look, but besides not being able to run the existing tests for the RangeSelector, due to lacking time and not being sufficiently familiar with programmatically spoofing the drag events, I wasn't able to do it (yet).

[michael-hawker]

@RosarioPulella just want to take a look at this one when you're back?

[Rosuavio]

With everything going on I was not able to to make UITests for this, but from my manual testing, it works well. We might want to consider making this behavior configurable in the future though, I think that having the handles stop each other might be desired in some cases.

[michael-hawker]

Thanks @stephtr for the fix! Sorry it took a while to get merged in.