Skip to content

Commit

Permalink
Fix the sliders in a horizontally oriented color picker going the inc…
Browse files Browse the repository at this point in the history 
…orrect direction with arrow keys. (#5934)

* Fix the sliders in a horizontally oriented color picker going the incorrect direction with arrow keys.

* Add tests and fix control up/down behavior.
  • Loading branch information
@StephenLPeters
StephenLPeters authored Sep 21, 2021
1 parent ed60605 commit 74fa22c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
14 changes: 6 additions & 8 deletions dev/ColorPicker/ColorPickerSlider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,10 @@ void ColorPickerSlider::OnApplyTemplate()

void ColorPickerSlider::OnKeyDown(winrt::KeyRoutedEventArgs const& args)
{
if ((Orientation() == winrt::Orientation::Horizontal &&
args.Key() != winrt::VirtualKey::Left &&
args.Key() != winrt::VirtualKey::Right) ||
(Orientation() == winrt::Orientation::Vertical &&
args.Key() != winrt::VirtualKey::Up &&
args.Key() != winrt::VirtualKey::Down))
if (args.Key() != winrt::VirtualKey::Left &&
args.Key() != winrt::VirtualKey::Right &&
args.Key() != winrt::VirtualKey::Up &&
args.Key() != winrt::VirtualKey::Down)
{
__super::OnKeyDown(args);
return;
Expand Down Expand Up @@ -102,9 +100,9 @@ void ColorPickerSlider::OnKeyDown(winrt::KeyRoutedEventArgs const& args)
const bool shouldInvertHorizontalDirection = FlowDirection() == winrt::FlowDirection::RightToLeft && !IsDirectionReversed();

const IncrementDirection direction =
((args.Key() == winrt::VirtualKey::Left && !shouldInvertHorizontalDirection) ||
((args.Key() == winrt::VirtualKey::Left && !shouldInvertHorizontalDirection) ||
(args.Key() == winrt::VirtualKey::Right && shouldInvertHorizontalDirection) ||
args.Key() == winrt::VirtualKey::Up) ?
args.Key() == winrt::VirtualKey::Down) ?
IncrementDirection::Lower :
IncrementDirection::Higher;

Expand Down
60 changes: 43 additions & 17 deletions dev/ColorPicker/InteractionTests/ColorPickerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,12 +648,28 @@ public void CanUseKeyboardToInteractColorSpectrumRTL()
public void CanUseKeyboardToInteractWithSlidersLTR()
{
CanUseKeyboardToInteractWithSliders(isRTL: false);
CanUseKeyboardToInteractWithSliders(isRTL: false, shouldTryUpAndDown: true);
}

[TestMethod]
public void CanUseKeyboardToInteractWithSlidersLTRHorizontalOrientation()
{
CanUseKeyboardToInteractWithSliders(isRTL: false, shouldOrientationBeHorizontal: true);
CanUseKeyboardToInteractWithSliders(isRTL: false, shouldTryUpAndDown: true, shouldOrientationBeHorizontal: true);
}

[TestMethod]
public void CanUseKeyboardToInteractSlidersRTL()
{
CanUseKeyboardToInteractWithSliders(isRTL: true);
CanUseKeyboardToInteractWithSliders(isRTL: false, shouldTryUpAndDown: true);
}

[TestMethod]
public void CanUseKeyboardToInteractWithSlidersRTLHorizontalOrientation()
{
CanUseKeyboardToInteractWithSliders(isRTL: true, shouldOrientationBeHorizontal: true);
CanUseKeyboardToInteractWithSliders(isRTL: true, shouldTryUpAndDown: true, shouldOrientationBeHorizontal: true);
}

[TestMethod]
Expand Down Expand Up @@ -801,11 +817,14 @@ public void CanUseKeyboardToInteractWithColorSpectrum(bool isRTL)
}
}

public void CanUseKeyboardToInteractWithSliders(bool isRTL)
public void CanUseKeyboardToInteractWithSliders(bool isRTL, bool shouldTryUpAndDown = false, bool shouldOrientationBeHorizontal = false)
{
using (var setup = SetupColorPickerTest(TestOptions.EnableAlpha | TestOptions.DisableColorSpectrumLoadWait))
{
SetIsRTL(isRTL);
SetIsHorizontalOrientation(shouldOrientationBeHorizontal);
Key increaseKey = shouldTryUpAndDown ? Key.Up : isRTL ? Key.Left : Key.Right;
Key decreaseKey = shouldTryUpAndDown ? Key.Down : isRTL ? Key.Right : Key.Left;

bool colorNamesAvailable = ApiInformation.IsMethodPresent("Windows.UI.ColorHelper", "ToDisplayName");

Expand All @@ -821,33 +840,33 @@ public void CanUseKeyboardToInteractWithSliders(bool isRTL)
VerifyElementIsFocused(ThirdDimensionAutomationId);

Log.Comment("Keyboard to the right and left first. We expect this to change the value by 1 each time (+/- 2.5 to the max RGB channel), but not wrap.");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left);
KeyboardHelper.PressKey(decreaseKey);
VerifyColorIsSelected(252, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0);
VerifyColorNameIsSelected("Red");
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right);
KeyboardHelper.PressKey(increaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0);
VerifyColorNameIsSelected("Red");
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right);
KeyboardHelper.PressKey(increaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0);
VerifyColorNameIsSelected("Red");

Log.Comment("Now hold control and keyboard to the right and left again. We expect this to jump to the next named color each time, but not wrap.");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left, ModifierKey.Control);
KeyboardHelper.PressKey(decreaseKey, ModifierKey.Control);
VerifyColorIsSelected(colorNamesAvailable ? 102 : 230, colorNamesAvailable ? 15 : 0, colorNamesAvailable ? 20 : 0);
VerifyColorNameIsSelected("Dark red");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left, ModifierKey.Control);
KeyboardHelper.PressKey(decreaseKey, ModifierKey.Control);
VerifyColorIsSelected(colorNamesAvailable ? 15 : 204, colorNamesAvailable ? 2 : 0, colorNamesAvailable ? 3 : 0);
VerifyColorNameIsSelected("Black");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left, ModifierKey.Control);
KeyboardHelper.PressKey(decreaseKey, ModifierKey.Control);
VerifyColorIsSelected(colorNamesAvailable ? 0 : 179, 0, 0);
VerifyColorNameIsSelected("Black");
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right, ModifierKey.Control);
KeyboardHelper.PressKey(increaseKey, ModifierKey.Control);
VerifyColorIsSelected(colorNamesAvailable ? 105 : 204, colorNamesAvailable ? 16 : 0, colorNamesAvailable ? 20 : 0);
VerifyColorNameIsSelected("Dark red");
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right, ModifierKey.Control);
KeyboardHelper.PressKey(increaseKey, ModifierKey.Control);
VerifyColorIsSelected(colorNamesAvailable ? 214 : 230, colorNamesAvailable ? 32 : 0, colorNamesAvailable ? 41 : 0);
VerifyColorNameIsSelected("Red");
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right, ModifierKey.Control);
KeyboardHelper.PressKey(increaseKey, ModifierKey.Control);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0);
VerifyColorNameIsSelected("Red");

Expand All @@ -856,21 +875,21 @@ public void CanUseKeyboardToInteractWithSliders(bool isRTL)
VerifyElementIsFocused(AlphaSliderAutomationId);

Log.Comment("Keyboard to the right and left first. We expect this to change the alpha by 2.5 each time, but not wrap.");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left);
KeyboardHelper.PressKey(decreaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 252);
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right);
KeyboardHelper.PressKey(increaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 255);
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right);
KeyboardHelper.PressKey(increaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 255);

Log.Comment("Now hold control and keyboard to the right and left again. We expect this to change the alpha by 25 each time, but not wrap. We also expect us to snap to multiples of 10 if we're between them.");
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left, ModifierKey.Control);
KeyboardHelper.PressKey(decreaseKey, ModifierKey.Control);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 230);
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right);
KeyboardHelper.PressKey(increaseKey);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 232);
KeyboardHelper.PressKey(isRTL ? Key.Right : Key.Left, ModifierKey.Control);
KeyboardHelper.PressKey(decreaseKey, ModifierKey.Control);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 230);
KeyboardHelper.PressKey(isRTL ? Key.Left : Key.Right, ModifierKey.Control);
KeyboardHelper.PressKey(increaseKey, ModifierKey.Control);
VerifyColorIsSelected(255, colorNamesAvailable ? 38 : 0, colorNamesAvailable ? 49 : 0, 255);
}
}
Expand Down Expand Up @@ -1496,6 +1515,13 @@ private void SetIsRTL(bool isRTL)
}
}

private void SetIsHorizontalOrientation(bool shouldBeHorizontal)
{
ComboBox isRTLCheckBox = new ComboBox(FindElement.ById("OrientationComboBox"));
isRTLCheckBox.SelectItemById(shouldBeHorizontal ? "OrientationHorizontal" : "OrientationVertical");
Wait.ForIdle();
}

private void SetMinSaturation(double minSaturation)
{
Log.Comment("Setting MinSaturation to {0}.", minSaturation);
Expand Down

0 comments on commit 74fa22c

Please sign in to comment.