Skip to content

Commit

Permalink
Changing CommandBarFlyout to not take keyboard focus when opened in t…
Browse files Browse the repository at this point in the history 
…ransient mode (#6844)

CommandBarFlyout was changed to unconditionally give keyboard focus to the first focusable command when it opens. This breaks the assumption when opening in transient mode that the flyout won't take keyboard focus in that mode. We should only give keyboard focus when opening in standard mode.

Also added a test to make sure that this doesn't regress in the future.
  • Loading branch information
@llongley
llongley authored Mar 18, 2022
1 parent 01eea31 commit 0926753
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 23 deletions.
54 changes: 31 additions & 23 deletions dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,41 +25,49 @@ CommandBarFlyoutCommandBar::CommandBarFlyoutCommandBar()

UpdateUI(!m_commandBarFlyoutIsOpening);

// Programmatically focus the first primary command if any, else programmatically focus the first secondary command if any.
auto commands = PrimaryCommands().Size() > 0 ? PrimaryCommands() : (SecondaryCommands().Size() > 0 ? SecondaryCommands() : nullptr);

if (commands)
if (auto owningFlyout = m_owningFlyout.get())
{
const bool usingPrimaryCommands = commands == PrimaryCommands();
const bool ensureTabStopUniqueness = usingPrimaryCommands || SharedHelpers::IsRS3OrHigher();
const auto firstCommandAsFrameworkElement = commands.GetAt(0).try_as<winrt::FrameworkElement>();

if (firstCommandAsFrameworkElement)
// We only want to focus an initial element if we're opening in standard mode -
// in transient mode, we don't want to be taking focus.
if (owningFlyout.ShowMode() == winrt::FlyoutShowMode::Standard)
{
if (SharedHelpers::IsFrameworkElementLoaded(firstCommandAsFrameworkElement))
{
FocusCommand(
commands,
usingPrimaryCommands ? m_moreButton.get() : nullptr /*moreButton*/,
winrt::FocusState::Programmatic /*focusState*/,
true /*firstCommand*/,
ensureTabStopUniqueness);
}
else
// Programmatically focus the first primary command if any, else programmatically focus the first secondary command if any.
auto commands = PrimaryCommands().Size() > 0 ? PrimaryCommands() : (SecondaryCommands().Size() > 0 ? SecondaryCommands() : nullptr);

if (commands)
{
m_firstItemLoadedRevoker = firstCommandAsFrameworkElement.Loaded(winrt::auto_revoke,
const bool usingPrimaryCommands = commands == PrimaryCommands();
const bool ensureTabStopUniqueness = usingPrimaryCommands || SharedHelpers::IsRS3OrHigher();
const auto firstCommandAsFrameworkElement = commands.GetAt(0).try_as<winrt::FrameworkElement>();

if (firstCommandAsFrameworkElement)
{
[this, commands, usingPrimaryCommands, ensureTabStopUniqueness](winrt::IInspectable const& sender, auto const&)
if (SharedHelpers::IsFrameworkElementLoaded(firstCommandAsFrameworkElement))
{
FocusCommand(
commands,
usingPrimaryCommands ? m_moreButton.get() : nullptr /*moreButton*/,
winrt::FocusState::Programmatic /*focusState*/,
true /*firstCommand*/,
ensureTabStopUniqueness);
m_firstItemLoadedRevoker.revoke();
}
});
else
{
m_firstItemLoadedRevoker = firstCommandAsFrameworkElement.Loaded(winrt::auto_revoke,
{
[this, commands, usingPrimaryCommands, ensureTabStopUniqueness](winrt::IInspectable const& sender, auto const&)
{
FocusCommand(
commands,
usingPrimaryCommands ? m_moreButton.get() : nullptr /*moreButton*/,
winrt::FocusState::Programmatic /*focusState*/,
true /*firstCommand*/,
ensureTabStopUniqueness);
m_firstItemLoadedRevoker.revoke();
}
});
}
}
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions dev/CommandBarFlyout/InteractionTests/CommandBarFlyoutTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ public void VerifyTabNavigationBetweenPrimaryAndSecondaryCommands()
Log.Comment("Tap on a button to show the CommandBarFlyout.");
InputHelper.Tap(showCommandBarFlyoutButton);

FocusHelper.SetFocus(FindElement.ById("CutButton1"));

Log.Comment("Press Tab key to move focus to first secondary command: Undo.");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Expand Down Expand Up @@ -311,6 +313,8 @@ private void VerifyLeftAndRightNavigationBetweenPrimaryCommands(bool inRTL, bool
Log.Comment("Tap on a button to show the CommandBarFlyout.");
InputHelper.Tap(showCommandBarFlyoutButton);

FocusHelper.SetFocus(FindElement.ById("CutButton1"));

Log.Comment($"Press {rightStr} key to move focus to second primary command: Copy.");
KeyboardHelper.PressKey(rightKey);
Wait.ForIdle();
Expand Down Expand Up @@ -376,6 +380,8 @@ public void VerifyUpAndDownNavigationBetweenPrimaryAndSecondaryCommands()
Log.Comment("Tap on a button to show the CommandBarFlyout.");
InputHelper.Tap(showCommandBarFlyoutButton);

FocusHelper.SetFocus(FindElement.ById("CutButton1"));

Log.Comment("Press Down key to move focus to second primary command: Copy.");
KeyboardHelper.PressKey(Key.Down);
Wait.ForIdle();
Expand Down Expand Up @@ -487,6 +493,8 @@ public void VerifyPrimaryCommandsAutomationSet()
Log.Comment("Tap on a button to show the CommandBarFlyout.");
InputHelper.Tap(showCommandBarFlyoutButton);

FocusHelper.SetFocus(FindElement.ById("CutButton1"));

Button cutButton1 = FindElement.ById<Button>("CutButton1");
var cutButtonElement = AutomationElement.FocusedElement;
Verify.AreEqual(cutButtonElement.Current.AutomationId, cutButton1.AutomationId);
Expand Down Expand Up @@ -557,6 +565,8 @@ public void VerifyFlowsToAndFromConnectsPrimaryAndSecondaryCommands()
FindElement.ById<Button>("MoreButton").InvokeAndWait();
}

FocusHelper.SetFocus(FindElement.ById("CutButton1"));

Log.Comment("Retrieving the more button and undo button's automation element objects.");

// Moving to the MoreButton to retrieve it
Expand Down Expand Up @@ -959,6 +969,8 @@ public void VerifyDynamicSecondaryCommandLabel()
Wait.ForIdle();
Verify.AreEqual(ToggleState.On, isFlyoutOpenCheckBox.ToggleState);

FocusHelper.SetFocus(FindElement.ById("UndoButton6"));

Button undoButton6 = FindElement.ById<Button>("UndoButton6");
Verify.IsNotNull(undoButton6);

Expand Down
48 changes: 48 additions & 0 deletions dev/CommandBarFlyout/InteractionTests/TextCommandBarFlyoutTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,54 @@ public void VerifyTextCommandBarRemainsOpenWithItems()
}
}

[TestMethod]
public void ValidateSelectionFlyoutDoesNotTakeFocus()
{
using (var setup = new TextCommandBarFlyoutTestSetupHelper())
{
// RichEditBox only implements the Text pattern, which the "TextBlock" MITA type exposes.
var richEditBox = new TextBlock(FindElement.ById("RichEditBox"));
var fillWithTextButton = new Button(FindElement.ById("RichEditBoxFillWithTextButton"));

Log.Comment("Give focus to the RichEditBox.");
FocusHelper.SetFocus(richEditBox);

Log.Comment("Enter enough text to guarantee that double-tapping the RichEditBox will select text.");
fillWithTextButton.InvokeAndWait();

Log.Comment("Double-click to select the text and bring up the selection menu. The CommandBarFlyout should appear, but should not take focus.");
InputHelper.LeftDoubleClick(richEditBox);
Wait.ForIdle();

var boldButton = FindElement.ByName("Bold");
Verify.IsNotNull(boldButton);
Verify.IsFalse(boldButton.HasKeyboardFocus);

Log.Comment("Press backspace to delete the selected text. This should work because the RichEditBox should still have focus.");
KeyboardHelper.PressKey(Key.Backspace);
Wait.ForIdle();

Verify.AreEqual(string.Empty, richEditBox.DocumentText);

Log.Comment("Enter text again.");
fillWithTextButton.InvokeAndWait();

Log.Comment("Double-click to select the text and bring up the selection menu. The CommandBarFlyout should appear, but should not take focus.");
InputHelper.LeftDoubleClick(richEditBox);
Wait.ForIdle();

boldButton = FindElement.ByName("Bold");
Verify.IsNotNull(boldButton);
Verify.IsFalse(boldButton.HasKeyboardFocus);

Log.Comment("Press the A key to overwrite the selected text.");
richEditBox.SendKeys("a");
Wait.ForIdle();

Verify.AreEqual("a", richEditBox.DocumentText);
}
}

private void OpenFlyoutOn(string textControlName, bool asTransient)
{
Log.Comment("Opening text control flyout on the {0} in {1} mode.", textControlName, asTransient ? "transient" : "standard");
Expand Down
3 changes: 3 additions & 0 deletions dev/CommandBarFlyout/TestUI/TextCommandBarFlyoutPage.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<ColumnDefinition />
<ColumnDefinition Width="Auto" />
<ColumnDefinition Width="Auto" />
<ColumnDefinition Width="Auto" />
</Grid.ColumnDefinitions>
<TextBox x:Name="TextBox1" AutomationProperties.AutomationId="TextBox" Text="Lorem ipsum ergo sum" Margin="10" />
<TextBlock x:Name="TextBlock1" AutomationProperties.AutomationId="TextBlock" Text="Lorem ipsum ergo sum" Margin="10" IsTextSelectionEnabled="True" Grid.Row="1" />
Expand All @@ -54,6 +55,8 @@
<Button x:Name="TextBlockClearSelectionButton" AutomationProperties.AutomationId="TextBlockClearSelectionButton" Content="Clear selection" Click="OnTextBlockClearSelectionClicked" Grid.Row="1" Grid.Column="2" />
<Button x:Name="RichEditBoxClearSelectionButton" AutomationProperties.AutomationId="RichEditBoxClearSelectionButton" Content="Clear selection" Click="OnRichEditBoxClearSelectionClicked" Grid.Row="2" Grid.Column="2" />
<Button x:Name="RichTextBlockClearSelectionButton" AutomationProperties.AutomationId="RichTextBlockClearSelectionButton" Content="Clear selection" Click="OnRichTextBlockClearSelectionClicked" Grid.Row="3" Grid.Column="2" />
<Button x:Name="TextBoxFillWithTextButton" AutomationProperties.AutomationId="TextBoxFillWithTextButton" Content="Fill with text" Click="OnTextBoxFillWithTextClicked" Grid.Column="3" />
<Button x:Name="RichEditBoxFillWithTextButton" AutomationProperties.AutomationId="RichEditBoxFillWithTextButton" Content="Fill with text" Click="OnRichEditBoxFillWithTextClicked" Grid.Row="2" Grid.Column="3" />
</Grid>
<TextBlock Text="Show TextCommandBarFlyout in transient mode on..." HorizontalAlignment="Center" />
<VariableSizedWrapGrid Orientation="Horizontal">
Expand Down
10 changes: 10 additions & 0 deletions dev/CommandBarFlyout/TestUI/TextCommandBarFlyoutPage.xaml.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,16 @@ private void OnRichEditBoxClearSelectionClicked(object sender, object args)
RichEditBox1.Document.Selection.Collapse(true);
}

private void OnTextBoxFillWithTextClicked(object sender, object args)
{
TextBox1.Text = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
}

private void OnRichEditBoxFillWithTextClicked(object sender, object args)
{
RichEditBox1.Document.SetText(TextSetOptions.None, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
}

private void OnRichTextBlockClearSelectionClicked(object sender, object args)
{
RichTextBlock1.Select(RichTextBlock1.ContentStart, RichTextBlock1.ContentStart);
Expand Down
66 changes: 66 additions & 0 deletions test/testinfra/MUXTestInfra/Common/InputHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,20 @@ public static void LeftClick(UIObject obj, double offsetX, double offsetY)
Wait.ForIdle();
}

public static void LeftDoubleClick(UIObject obj)
{
Log.Comment("Left double-click on {0}.", obj.GetIdentifier());
obj.DoubleClick(PointerButtons.Primary);
Wait.ForIdle();
}

public static void LeftDoubleClick(UIObject obj, double offsetX, double offsetY)
{
Log.Comment("Left double-click on {0}, at ({1}, {2}).", obj.GetIdentifier(), offsetX, offsetY);
obj.DoubleClick(PointerButtons.Primary, offsetX, offsetY);
Wait.ForIdle();
}

public static void RightClick(UIObject obj, int offsetX = 0, int offsetY = 0)
{
if (offsetX == 0 && offsetY == 0)
Expand Down Expand Up @@ -295,6 +309,58 @@ public static void TapAndHold(UIObject obj, uint durationMs)
Wait.ForIdle();
}

public static void DoubleTap(UIObject obj)
{
try
{
Log.Comment("Double-tap on {0}.", obj.GetIdentifier());
using (var waiter = GetWaiterForInputEvent(obj, InputEvent.Tap))
{
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone5))
{
Log.Warning("Touch input is not available on OS versions less than RS5. Falling back to mouse input.");
obj.DoubleClick(PointerButtons.Primary);
}
else
{
obj.DoubleTap();
}
}
}
catch (Exception e)
{
Log.Warning("Exception while double-tapping: " + e.Message);
}

Wait.ForIdle();
}

public static void DoubleTap(UIObject obj, double offsetX, double offsetY)
{
try
{
Log.Comment("Double-tap on {0} at ({1}, {2}).", obj.GetIdentifier(), offsetX, offsetY);
using (var waiter = GetWaiterForInputEvent(obj, InputEvent.Tap))
{
if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone5))
{
Log.Warning("Touch input is not available on OS versions less than RS5. Falling back to mouse input.");
obj.DoubleClick(PointerButtons.Primary, offsetX, offsetY);
}
else
{
obj.DoubleTap(offsetX, offsetY);
}
}
}
catch (Exception e)
{
Log.Warning("Exception while double-tapping: " + e.Message);
}

Wait.ForIdle();
}

public static void Pan(UIObject obj, int distance, Direction direction, uint holdDuration = DefaultPanHoldDuration, float panAcceleration = DefaultPanAcceleration, uint dragDuration = DefaultDragDuration, bool waitForIdle = true)
{
Log.Comment("Pan on {0} for {1} pixels in the {2} direction, after holding {3} ms.",
Expand Down

0 comments on commit 0926753

Please sign in to comment.