Skip to content

Commit

Permalink
Update up/down arrow keys' behavior for CommandBarFlyoutCommandBar (#805
Browse files Browse the repository at this point in the history 
)

* Fixing CommandBarFlyoutCommandBar's behavior of Up/Down keys:
- Down arrow from last primary command moves focus to More button (all releases)
- Down arrow from last secondary command moves focus to first primary command (on RS3+)
- Up arrow from first secondary command moves focus to More button (on RS3+)

* Removing dead code.

* Using Down/Up keys to move among primary commands logically, as opposed to physically.
  • Loading branch information
@RBrid @jevansaks
RBrid authored and jevansaks committed Jun 7, 2019
1 parent f8ee9ad commit f5568fc
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 35 deletions.
44 changes: 39 additions & 5 deletions dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,42 @@ void CommandBarFlyoutCommandBar::AttachEventHandlers()
}
}
}
loopCount++; // Looping again is needed when the last command has focus

if (loopCount == 0 && PrimaryCommands().Size() > 0)
{
auto moreButton = m_moreButton.get();

if (deltaIndex == 1 &&
FocusCommand(
PrimaryCommands() /*commands*/,
moreButton /*moreButton*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*firstCommand*/,
true /*ensureTabStopUniqueness*/))
{
// Being on the last secondary command, keyboard focus was given to the first primary command
args.Handled(true);
return;
}
else if (deltaIndex == -1 &&
focusedControl &&
moreButton &&
IsControlFocusable(moreButton, false /*checkTabStop*/) &&
FocusControl(
moreButton /*newFocus*/,
focusedControl /*oldFocus*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*updateTabStop*/))
{
// Being on the first secondary command, keyboard focus was given to the MoreButton
args.Handled(true);
return;
}
}

loopCount++; // Looping again when focus could not be given to a MoreButton going up or primary command going down.
}
while (loopCount < 2 && focusedControl && deltaIndex == 1);
while (loopCount < 2 && focusedControl);
}
args.Handled(true);
break;
Expand Down Expand Up @@ -774,11 +807,12 @@ void CommandBarFlyoutCommandBar::OnKeyDown(
bool isRightToLeft = m_primaryItemsRoot && m_primaryItemsRoot.get().FlowDirection() == winrt::FlowDirection::RightToLeft;
bool isLeft = (args.Key() == winrt::VirtualKey::Left && !isRightToLeft) || (args.Key() == winrt::VirtualKey::Right && isRightToLeft);
bool isRight = (args.Key() == winrt::VirtualKey::Right && !isRightToLeft) || (args.Key() == winrt::VirtualKey::Left && isRightToLeft);
bool isUp = (args.Key() == winrt::VirtualKey::Up && !isRightToLeft) || (args.Key() == winrt::VirtualKey::Down && isRightToLeft);
bool isDown = args.Key() == winrt::VirtualKey::Down;
bool isUp = args.Key() == winrt::VirtualKey::Up;

auto moreButton = m_moreButton.get();

if (args.Key() == winrt::VirtualKey::Down &&
if (isDown &&
moreButton &&
moreButton.FocusState() != winrt::FocusState::Unfocused &&
SecondaryCommands().Size() > 0)
Expand Down Expand Up @@ -847,7 +881,7 @@ void CommandBarFlyoutCommandBar::OnKeyDown(

if (!args.Handled())
{
if (isRight &&
if ((isRight || isDown) &&
focusedControl &&
moreButton &&
IsControlFocusable(moreButton, false /*checkTabStop*/))
Expand Down
108 changes: 78 additions & 30 deletions dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,13 @@ public void VerifyTabNavigationBetweenPrimaryAndSecondaryCommands()
[TestMethod]
public void VerifyLeftAndRightNavigationBetweenPrimaryCommands()
{
VerifyLeftAndRightNavigationBetweenPrimaryCommands(false /*inRTL*/);
VerifyLeftAndRightNavigationBetweenPrimaryCommands(true /*inRTL*/);
VerifyLeftAndRightNavigationBetweenPrimaryCommands(inRTL: false, useUpDownKeys: false);
VerifyLeftAndRightNavigationBetweenPrimaryCommands(inRTL: true, useUpDownKeys: false);
VerifyLeftAndRightNavigationBetweenPrimaryCommands(inRTL: false, useUpDownKeys: true);
VerifyLeftAndRightNavigationBetweenPrimaryCommands(inRTL: true, useUpDownKeys: true);
}

private void VerifyLeftAndRightNavigationBetweenPrimaryCommands(bool inRTL)
private void VerifyLeftAndRightNavigationBetweenPrimaryCommands(bool inRTL, bool useUpDownKeys)
{
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone2))
{
Expand All @@ -259,39 +261,59 @@ private void VerifyLeftAndRightNavigationBetweenPrimaryCommands(bool inRTL)
Wait.ForIdle();
}

string rightStr = inRTL ? "Left" : "Right";
string leftStr = inRTL ? "Right" : "Left";
Key rightKey = inRTL ? Key.Left : Key.Right;
Key leftKey = inRTL ? Key.Right : Key.Left;
string rightStr;
string leftStr;
Key rightKey;
Key leftKey;

if (useUpDownKeys)
{
// Down and Up keys are used to move logically within the primary commands: Up to move to the previous command and Down to move to the next command.
rightStr = "Down";
leftStr = "Up";
rightKey = Key.Down;
leftKey = Key.Up;
}
else
{
// Left and Right keys are used to move physically within the primary commands: Left to move to the left command and Right to move to the right command.
rightStr = inRTL ? "Left" : "Right";
leftStr = inRTL ? "Right" : "Left";
rightKey = inRTL ? Key.Left : Key.Right;
leftKey = inRTL ? Key.Right : Key.Left;
}

Log.Comment("Tap on a button to show the CommandBarFlyout.");
InputHelper.Tap(showCommandBarFlyoutButton);

Log.Comment("Press " + rightStr + " key to move focus to second primary command: Copy.");
Log.Comment($"Press {rightStr} key to move focus to second primary command: Copy.");
KeyboardHelper.PressKey(rightKey);
Wait.ForIdle();

Button copyButton1 = FindElement.ById<Button>("CopyButton1");
var copyButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(copyButtonElement.Current.AutomationId, copyButton1.AutomationId);

Log.Comment("Press " + leftStr + " key to move focus back to first primary command: Cut.");
Log.Comment($"Press {leftStr} key to move focus back to first primary command: Cut.");
KeyboardHelper.PressKey(leftKey);
Wait.ForIdle();

Button cutButton1 = FindElement.ById<Button>("CutButton1");
var cutButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(cutButtonElement.Current.AutomationId, cutButton1.AutomationId);

Log.Comment("Press " + leftStr + " key and remain on first primary command: Cut.");
KeyboardHelper.PressKey(leftKey);
Wait.ForIdle();
if (!useUpDownKeys)
{
Log.Comment($"Press {leftStr} key and remain on first primary command: Cut.");
KeyboardHelper.PressKey(leftKey);
Wait.ForIdle();

cutButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(cutButtonElement.Current.AutomationId, cutButton1.AutomationId);
cutButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(cutButtonElement.Current.AutomationId, cutButton1.AutomationId);
}

Log.Comment("Press " + rightStr + " key to move focus to MoreButton.");
for (int i = 0; i <= 6; i++)
Log.Comment($"Press {rightStr} key to move focus to MoreButton.");
for (int i = 0; i <= 5; i++)
{
KeyboardHelper.PressKey(rightKey);
Wait.ForIdle();
Expand All @@ -301,12 +323,15 @@ private void VerifyLeftAndRightNavigationBetweenPrimaryCommands(bool inRTL)
var moreButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);

Log.Comment("Press " + rightStr + " key and remain on MoreButton.");
KeyboardHelper.PressKey(rightKey);
Wait.ForIdle();
if (!useUpDownKeys)
{
Log.Comment($"Press {rightStr} key and remain on MoreButton.");
KeyboardHelper.PressKey(rightKey);
Wait.ForIdle();

moreButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);
moreButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);
}
}
}

Expand Down Expand Up @@ -353,15 +378,16 @@ public void VerifyUpAndDownNavigationBetweenPrimaryAndSecondaryCommands()
var underlineButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(underlineButtonElement.Current.AutomationId, underlineButton1.AutomationId);

Log.Comment("Press Down key and remain on last primary command: Underline.");
Log.Comment("Press Down key to move focus to MoreButton.");
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();

underlineButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(underlineButtonElement.Current.AutomationId, underlineButton1.AutomationId);
Button moreButton = FindElement.ById<Button>("MoreButton");
var moreButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);

Log.Comment("Press Up key to move focus to first primary command: Cut.");
for (int i = 0; i <= 4; i++)
for (int i = 0; i <= 5; i++)
{
KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
Expand Down Expand Up @@ -389,12 +415,34 @@ public void VerifyUpAndDownNavigationBetweenPrimaryAndSecondaryCommands()
var undoButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(undoButtonElement.Current.AutomationId, undoButton1.AutomationId);

Log.Comment("Press Up key and remain on first secondary command: Undo.");
KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone3))
{
Log.Comment("Press Up key and remain on first secondary command: Undo.");
KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();

undoButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(undoButtonElement.Current.AutomationId, undoButton1.AutomationId);
undoButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(undoButtonElement.Current.AutomationId, undoButton1.AutomationId);
}
else
{
Log.Comment("Press Up key to move focus to MoreButton.");
KeyboardHelper.PressKey(Key.Up);
Wait.ForIdle();

moreButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(moreButtonElement.Current.AutomationId, moreButton.AutomationId);

Log.Comment("Press Down key to move focus to first primary command through all secondary commands: Cut.");
for (int i = 0; i <= 4; i++)
{
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();
}

cutButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(cutButtonElement.Current.AutomationId, cutButton1.AutomationId);
}
}
}

Expand Down

0 comments on commit f5568fc

Please sign in to comment.