Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating CommandBarFlyout keyboarding and UIA to account for AlwaysExpanded hiding the more button #5138

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Updating CommandBarFlyout keyboarding and UIA to account for AlwaysExpanded hiding the more button #5138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9fb9a7
Refactoring CommandBarFlyoutCommandBar's keyboard handling to properl…
llongley Jun 3, 2021
4313178
Fixing a test case for RS2.
llongley Jun 3, 2021
d61fcec
Updating to populate the accessible controls list only when its value…
llongley Jun 3, 2021
7ff8030
Merge remote-tracking branch 'origin/main' into user/llongley/FixComm…
llongley Jun 7, 2021
a4f0f52
Fixing a reversed boolean condition.
llongley Jun 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
280 changes: 109 additions & 171 deletions dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "CommandBarFlyoutCommandBar.h"
#include "CommandBarFlyoutCommandBarTemplateSettings.h"
#include "TypeLogging.h"
#include "Vector.h"

CommandBarFlyoutCommandBar::CommandBarFlyoutCommandBar()
{
Expand Down Expand Up @@ -119,6 +120,7 @@ CommandBarFlyoutCommandBar::CommandBarFlyoutCommandBar()
{
COMMANDBARFLYOUT_TRACE_VERBOSE(*this, TRACE_MSG_METH, METH_NAME, this);

PopulateAccessibleControls();
UpdateFlowsFromAndFlowsTo();
UpdateUI(!m_commandBarFlyoutIsOpening);
}
Expand All @@ -130,6 +132,7 @@ CommandBarFlyoutCommandBar::CommandBarFlyoutCommandBar()
COMMANDBARFLYOUT_TRACE_VERBOSE(*this, TRACE_MSG_METH, METH_NAME, this);

m_secondaryItemsRootSized = false;
PopulateAccessibleControls();
UpdateFlowsFromAndFlowsTo();
UpdateUI(!m_commandBarFlyoutIsOpening);
}
Expand Down Expand Up @@ -209,6 +212,7 @@ void CommandBarFlyoutCommandBar::OnApplyTemplate()
}

AttachEventHandlers();
PopulateAccessibleControls();
UpdateFlowsFromAndFlowsTo();
UpdateUI(false /* useTransitions */);
SetPresenterName(m_flyoutPresenter.get());
Expand Down Expand Up @@ -279,87 +283,7 @@ void CommandBarFlyoutCommandBar::AttachEventHandlers()
case winrt::VirtualKey::Down:
case winrt::VirtualKey::Up:
{
if (SecondaryCommands().Size() > 1)
{
winrt::Control focusedControl = nullptr;
int startIndex = 0;
int endIndex = static_cast<int>(SecondaryCommands().Size());
int deltaIndex = 1;
int loopCount = 0;

if (args.Key() == winrt::VirtualKey::Up)
{
deltaIndex = -1;
startIndex = endIndex - 1;
endIndex = -1;
}

do
{
// Give keyboard focus to the previous or next secondary command if possible
for (int index = startIndex; index != endIndex; index += deltaIndex)
{
auto secondaryCommand = SecondaryCommands().GetAt(index);

if (auto secondaryCommandAsControl = secondaryCommand.try_as<winrt::Control>())
{
if (secondaryCommandAsControl.FocusState() != winrt::FocusState::Unfocused)
{
focusedControl = secondaryCommandAsControl;
}
else if (focusedControl && IsControlFocusable(secondaryCommandAsControl, false /*checkTabStop*/) &&
focusedControl != secondaryCommandAsControl)
{
if (FocusControl(
secondaryCommandAsControl /*newFocus*/,
focusedControl /*oldFocus*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*updateTabStop*/))
{
args.Handled(true);
return;
}
}
}
}

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);
}
args.Handled(true);
OnKeyDown(args);
break;
}
}
Expand Down Expand Up @@ -523,7 +447,7 @@ void CommandBarFlyoutCommandBar::UpdateFlowsFromAndFlowsTo()

// If we have a more button and at least one focusable primary item, then
// we'll use the more button as the last element in our primary items list.
if (moreButton && m_currentPrimaryItemsEndElement)
if (moreButton && moreButton.Visibility() == winrt::Visibility::Visible && m_currentPrimaryItemsEndElement)
{
m_currentPrimaryItemsEndElement.set(moreButton);
}
Expand Down Expand Up @@ -797,7 +721,7 @@ void CommandBarFlyoutCommandBar::EnsureAutomationSetCountAndPosition()
}
}

if (moreButton)
if (moreButton && moreButton.Visibility() == winrt::Visibility::Visible)
{
// Accounting for the MoreButton
sizeOfSet++;
Expand All @@ -822,7 +746,7 @@ void CommandBarFlyoutCommandBar::EnsureAutomationSetCountAndPosition()
}
}

if (moreButton)
if (moreButton && moreButton.Visibility() == winrt::Visibility::Visible)
{
winrt::AutomationProperties::SetSizeOfSet(moreButton, sizeOfSet);
winrt::AutomationProperties::SetPositionInSet(moreButton, sizeOfSet);
Expand Down Expand Up @@ -868,6 +792,52 @@ void CommandBarFlyoutCommandBar::EnsureFocusedPrimaryCommand()
}
}

void CommandBarFlyoutCommandBar::PopulateAccessibleControls()
{
COMMANDBARFLYOUT_TRACE_VERBOSE(*this, TRACE_MSG_METH, METH_NAME, this);

// The primary commands and the more button are the only controls accessible
// using left and right arrow keys. All of the commands are accessible using
// the up and down arrow keys.
if (!m_horizontallyAccessibleControls)
{
MUX_ASSERT(!m_verticallyAccessibleControls);

m_horizontallyAccessibleControls = winrt::make<Vector<winrt::Control>>();
m_verticallyAccessibleControls = winrt::make<Vector<winrt::Control>>();
}
else
{
MUX_ASSERT(m_verticallyAccessibleControls);

m_horizontallyAccessibleControls.Clear();
m_verticallyAccessibleControls.Clear();
}

for (winrt::ICommandBarElement const& command : PrimaryCommands())
{
if (auto const& commandAsControl = command.try_as<winrt::Control>())
{
m_horizontallyAccessibleControls.Append(commandAsControl);
m_verticallyAccessibleControls.Append(commandAsControl);
}
}

if (auto const& moreButton = m_moreButton.get())
{
m_horizontallyAccessibleControls.Append(moreButton);
m_verticallyAccessibleControls.Append(moreButton);
}

for (winrt::ICommandBarElement const& command : SecondaryCommands())
{
if (auto const& commandAsControl = command.try_as<winrt::Control>())
{
m_verticallyAccessibleControls.Append(commandAsControl);
}
}
}

void CommandBarFlyoutCommandBar::OnKeyDown(
winrt::KeyRoutedEventArgs const& args)
{
Expand Down Expand Up @@ -920,110 +890,78 @@ void CommandBarFlyoutCommandBar::OnKeyDown(
const bool isDown = args.Key() == winrt::VirtualKey::Down;
const bool isUp = args.Key() == winrt::VirtualKey::Up;

auto moreButton = m_moreButton.get();

if (isDown &&
moreButton &&
moreButton.FocusState() != winrt::FocusState::Unfocused &&
SecondaryCommands().Size() > 0)
// To avoid code duplication, we'll use the key directionality to determine
// both which control list to use and in which direction to iterate through
// it to find the next control to focus. Then we'll do that iteration
// to focus the next control.
auto const& accessibleControls{ isUp || isDown ? m_verticallyAccessibleControls : m_horizontallyAccessibleControls };
int const startIndex = isLeft || isUp ? accessibleControls.Size() - 1 : 0;
int const endIndex = isLeft || isUp ? -1 : accessibleControls.Size();
int const deltaIndex = isLeft || isUp ? -1 : 1;
bool const shouldLoop = isUp || isDown;
winrt::Control focusedControl{ nullptr };
int focusedControlIndex = -1;

for (int i = startIndex;
// We'll stop looping at the end index unless we're looping,
// in which case we want to wrap back around to the start index.
(i != endIndex || shouldLoop) ||
// If we found a focused control but have looped all the way back around,
// then there wasn't another control to focus, so we should quit.
(focusedControlIndex > 0 && i == focusedControlIndex);
i += deltaIndex)
{
// When on the MoreButton, give keyboard focus to the first focusable secondary command
// First ensure the secondary commands flyout is open
if (!IsOpen())
{
IsOpen(true);
}

if (FocusCommand(
SecondaryCommands() /*commands*/,
nullptr /*moreButton*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*firstCommand*/,
SharedHelpers::IsRS3OrHigher() /*ensureTabStopUniqueness*/))
// If we've reached the end index, that means we want to loop.
// We'll wrap around to the start index.
if (i == endIndex)
{
args.Handled(true);
}
}

if (!args.Handled() && PrimaryCommands().Size() > 0)
{
winrt::Control focusedControl = nullptr;
int startIndex = 0;
int endIndex = static_cast<int>(PrimaryCommands().Size());
int deltaIndex = 1;
MUX_ASSERT(shouldLoop);

if (isLeft || isUp)
{
deltaIndex = -1;
startIndex = endIndex - 1;
endIndex = -1;

if (moreButton && moreButton.FocusState() != winrt::FocusState::Unfocused)
if (focusedControl)
{
focusedControl = moreButton;
i = startIndex;
}
}

// Give focus to the previous or next command if possible
for (int index = startIndex; index != endIndex; index += deltaIndex)
{
auto primaryCommand = PrimaryCommands().GetAt(index);

if (auto primaryCommandAsControl = primaryCommand.try_as<winrt::Control>())
else
{
if (primaryCommandAsControl.FocusState() != winrt::FocusState::Unfocused)
{
focusedControl = primaryCommandAsControl;
}
else if (focusedControl &&
IsControlFocusable(primaryCommandAsControl, false /*checkTabStop*/) &&
FocusControl(
primaryCommandAsControl /*newFocus*/,
focusedControl /*oldFocus*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*updateTabStop*/))
{
args.Handled(true);
break;
}
// If no focused control was found after going through the entire list of controls,
// then we have nowhere for focus to go. Let's early-out in that case.
break;
}
}

if (!args.Handled())
auto const& control = accessibleControls.GetAt(i);

// If we've yet to find the focused control, we'll keep looking for it.
// Otherwise, we'll try to focus the next control after it.
if (!focusedControl)
{
if ((isRight || isDown) &&
focusedControl &&
moreButton &&
IsControlFocusable(moreButton, false /*checkTabStop*/))
if (control.FocusState() != winrt::FocusState::Unfocused)
{
// When on last primary command, give keyboard focus to the MoreButton
if (FocusControl(
moreButton /*newFocus*/,
focusedControl /*oldFocus*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*updateTabStop*/))
{
args.Handled(true);
}
focusedControl = control;
focusedControlIndex = i;
}
else if (isUp && SecondaryCommands().Size() > 0)
}
else if (IsControlFocusable(control, false /*checkTabStop*/))
{
// If the control we're trying to focus is in the secondary command list,
// then we'll make sure that that list is open before trying to focus the control.
if (auto const& controlAsCommandBarElement = control.try_as<winrt::ICommandBarElement>())
{
// When on first primary command, give keyboard focus to the last focusable secondary command
// First ensure the secondary commands flyout is open
if (!IsOpen())
uint32_t index = 0;
if (SecondaryCommands().IndexOf(controlAsCommandBarElement, index) && !IsOpen())
{
IsOpen(true);
}
}

if (FocusCommand(
SecondaryCommands() /*commands*/,
nullptr /*moreButton*/,
winrt::FocusState::Keyboard /*focusState*/,
false /*firstCommand*/,
SharedHelpers::IsRS3OrHigher() /*ensureTabStopUniqueness*/))
{
args.Handled(true);
}
if (FocusControl(
accessibleControls.GetAt(i) /*newFocus*/,
focusedControl /*oldFocus*/,
winrt::FocusState::Keyboard /*focusState*/,
true /*updateTabStop*/))
{
args.Handled(true);
break;
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions dev/CommandBarFlyout/CommandBarFlyoutCommandBar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class CommandBarFlyoutCommandBar :
void UpdateTemplateSettings();
void EnsureAutomationSetCountAndPosition();
void EnsureFocusedPrimaryCommand();
void PopulateAccessibleControls();

void SetPresenterName(winrt::FlyoutPresenter const& presenter);

Expand Down Expand Up @@ -113,4 +114,7 @@ class CommandBarFlyoutCommandBar :
winrt::Storyboard::Completed_revoker m_expandedDownToCollapsedStoryboardRevoker{};
winrt::Storyboard::Completed_revoker m_collapsedToExpandedUpStoryboardRevoker{};
winrt::Storyboard::Completed_revoker m_collapsedToExpandedDownStoryboardRevoker{};

winrt::IVector<winrt::Control> m_horizontallyAccessibleControls{ nullptr };
winrt::IVector<winrt::Control> m_verticallyAccessibleControls{ nullptr };
};
Loading