Skip to content

Commit

Permalink
Syncing content from committish release/1.4.2
Browse files Browse the repository at this point in the history
  • Loading branch information
reunion-maestro-bot committed Oct 24, 2023
1 parent 702b954 commit 7198c4e
Show file tree
Hide file tree
Showing 22 changed files with 583 additions and 52 deletions.
19 changes: 19 additions & 0 deletions controls/dev/CommandBarFlyout/CommandBarFlyout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@
#include "CommandBarFlyoutCommandBar.h"
#include "Vector.h"
#include "RuntimeProfiler.h"
#include "velocity.h"
#include <FrameworkUdk/Containment.h>

#include "CommandBarFlyout.properties.cpp"

// Bug 46607274: [1.4 servicing] Microsoft.UI.Xaml.Controls.dll!CommandBarFlyoutCommandBar::EnsureLocalizedControlTypes impacting context menu performance
#define WINAPPSDK_CHANGEID_46607274 46607274

// Change to 'true' to turn on debugging outputs in Output window
bool CommandBarFlyoutTrace::s_IsDebugOutputEnabled{ false };
bool CommandBarFlyoutTrace::s_IsVerboseDebugOutputEnabled{ false };
Expand Down Expand Up @@ -307,6 +312,20 @@ winrt::Control CommandBarFlyout::CreatePresenter()
{
auto commandBar = winrt::make_self<CommandBarFlyoutCommandBar>();

if (WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_46607274>())
{
// Localized string resource lookup is more expensive on MRTCore. Do the lookup ahead of time and reuse it for all
// the CommandBarFlyoutCommandBar::EnsureLocalizedControlTypes calls in response to PrimaryCommands/SecondCommands
// changed events.
commandBar->CacheLocalizedStringResources();
}
auto const scopeGuard = WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_46607274>()
? wil::scope_exit(std::function<void()>([commandBar]()
{
commandBar->ClearLocalizedStringResourceCache();
}))
: wil::scope_exit(std::function<void()>([](){}));

SharedHelpers::CopyVector(m_primaryCommands, commandBar->PrimaryCommands());
SharedHelpers::CopyVector(m_secondaryCommands, commandBar->SecondaryCommands());

Expand Down
34 changes: 32 additions & 2 deletions controls/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -895,17 +895,47 @@ void CommandBarFlyoutCommandBar::EnsureLocalizedControlTypes()
}
}

void CommandBarFlyoutCommandBar::CacheLocalizedStringResources()
{
m_localizedCommandBarFlyoutAppBarButtonControlType = ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarButtonLocalizedControlType);
m_localizedCommandBarFlyoutAppBarToggleButtonControlType = ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarToggleButtonLocalizedControlType);
m_areLocalizedStringResourcesCached = true;
}

void CommandBarFlyoutCommandBar::ClearLocalizedStringResourceCache()
{
m_areLocalizedStringResourcesCached = false;
m_localizedCommandBarFlyoutAppBarButtonControlType.clear();
m_localizedCommandBarFlyoutAppBarToggleButtonControlType.clear();
}

void CommandBarFlyoutCommandBar::SetKnownCommandLocalizedControlTypes(winrt::ICommandBarElement const& command)
{
COMMANDBARFLYOUT_TRACE_VERBOSE(*this, TRACE_MSG_METH, METH_NAME, this);

if (auto const& appBarButton = command.try_as<winrt::AppBarButton>())
{
winrt::AutomationProperties::SetLocalizedControlType(appBarButton, ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarButtonLocalizedControlType));
if (m_areLocalizedStringResourcesCached)
{
MUX_ASSERT(!m_localizedCommandBarFlyoutAppBarButtonControlType.empty());
winrt::AutomationProperties::SetLocalizedControlType(appBarButton, m_localizedCommandBarFlyoutAppBarButtonControlType);
}
else
{
winrt::AutomationProperties::SetLocalizedControlType(appBarButton, ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarButtonLocalizedControlType));
}
}
else if (auto const& appBarToggleButton = command.try_as<winrt::AppBarToggleButton>())
{
winrt::AutomationProperties::SetLocalizedControlType(appBarToggleButton, ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarToggleButtonLocalizedControlType));
if (m_areLocalizedStringResourcesCached)
{
MUX_ASSERT(!m_localizedCommandBarFlyoutAppBarToggleButtonControlType.empty());
winrt::AutomationProperties::SetLocalizedControlType(appBarToggleButton, m_localizedCommandBarFlyoutAppBarToggleButtonControlType);
}
else
{
winrt::AutomationProperties::SetLocalizedControlType(appBarToggleButton, ResourceAccessor::GetLocalizedStringResource(SR_CommandBarFlyoutAppBarToggleButtonLocalizedControlType));
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions controls/dev/CommandBarFlyout/CommandBarFlyoutCommandBar.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class CommandBarFlyoutCommandBar :

void OnPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);

void CacheLocalizedStringResources();
void ClearLocalizedStringResourceCache();

private:
void AttachEventHandlers();
void DetachEventHandlers();
Expand Down Expand Up @@ -127,4 +130,11 @@ class CommandBarFlyoutCommandBar :
// The one built into Popup is too high up in the Visual tree to be animated by a custom animation.
winrt::ContentExternalBackdropLink m_backdropLink{ nullptr };
winrt::ContentExternalBackdropLink m_overflowPopupBackdropLink{ nullptr };

// Localized string caches. Looking these up from MRTCore is expensive, so we don't want to put the lookups in a
// loop. Instead, look them up once, cache them, use the cached values, then clear the cache. The values in these
// caches are only valid after CacheLocalizedStringResources and before ClearLocalizedStringResourceCache.
bool m_areLocalizedStringResourcesCached{ false };
winrt::hstring m_localizedCommandBarFlyoutAppBarButtonControlType{};
winrt::hstring m_localizedCommandBarFlyoutAppBarToggleButtonControlType{};
};
52 changes: 52 additions & 0 deletions controls/dev/NavigationView/NavigationView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
#include "NavigationViewItemExpandingEventArgs.h"
#include "NavigationViewItemCollapsedEventArgs.h"
#include "InspectingDataSource.h"
#include "velocity.h"
#include <FrameworkUdk/Containment.h>

// Bug 46697123: [1.4 servicing] - Tab/Shift+Tab Navigation is very very confusing with List Views
#define WINAPPSDK_CHANGEID_46697123 46697123

// General items
static constexpr auto c_togglePaneButtonName = L"TogglePaneButton"sv;
Expand Down Expand Up @@ -2885,6 +2890,53 @@ void NavigationView::OnRepeaterGettingFocus(const winrt::IInspectable& sender, c
args.Handled(true);
}
}
else if (WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_46697123>() &&
!isFocusOutsideCurrentRootRepeater)
{
// Tab or Shift-Tab from within the same root repeater should move focus outside it
auto navigationViewItemBase = args.NewFocusedElement().try_as<winrt::NavigationViewItemBase>();
if (navigationViewItemBase &&
(args.Direction() == winrt::FocusNavigationDirection::Previous ||
args.Direction() == winrt::FocusNavigationDirection::Next))
{
// We need to find the next tab stop outside the root repeater.
// winrt::FocusManager::FindNextElement will at first give us NavigationViewItems
// within the root repeater. This is why we need to set IsTabStop = false for
// the found the elements within the root repeater so that FindNextElement eventually
// finds an element outside the root repeater.
std::vector<winrt::NavigationViewItemBase> containersWithTabFocusOff{};

const winrt::FindNextElementOptions options;
options.SearchRoot(this->XamlRoot().Content());

auto nextElement = winrt::FocusManager::FindNextElement(args.Direction(), options);
while (auto nvib = nextElement.try_as<winrt::NavigationViewItemBase>())
{
// Verify the item is in the root repeater we are trying to leave
if (newRootItemsRepeater == GetParentRootItemsRepeaterForContainer(nvib))
{
nvib.IsTabStop(false);
// Keep track of the container so that we can re-enable tab stop once focus leaves
containersWithTabFocusOff.push_back(nvib);
nextElement = winrt::FocusManager::FindNextElement(args.Direction(), options);
}
else
{
break;
}
}

for (auto nvib : containersWithTabFocusOff)
{
nvib.IsTabStop(true);
}

if (args.TrySetNewFocusedElement(nextElement))
{
args.Handled(true);
}
}
}
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion controls/dev/NavigationView/NavigationView.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@
<controls:ItemsRepeaterScrollHost HorizontalAlignment="Stretch" VerticalAlignment="Stretch">
<ScrollViewer x:Name="MenuItemsScrollViewer" TabNavigation="Local" VerticalScrollBarVisibility="Auto">
<!-- Left nav ItemsRepeater -->
<controls:ItemsRepeater x:Name="MenuItemsHost" AutomationProperties.Name="{TemplateBinding AutomationProperties.Name}" AutomationProperties.AccessibilityView="Content">
<controls:ItemsRepeater
x:Name="MenuItemsHost"
AutomationProperties.Name="{TemplateBinding AutomationProperties.Name}"
AutomationProperties.AccessibilityView="Content">
<controls:ItemsRepeater.Layout>
<controls:StackLayout />
</controls:ItemsRepeater.Layout>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,160 @@ string GetSelectedItem()
}
}

[TestMethod]
public void TabNavigationHierarchicalTest()
{
var testScenarios = RegressionTestScenario.BuildLeftNavRegressionTestScenarios();
foreach (var testScenario in testScenarios)
{
using (var setup = new TestSetupHelper(new[] { "NavigationView Tests", "HierarchicalNavigationView Markup Test" }))
{
Button togglePaneButton = new Button(FindElement.ById("TogglePaneButton"));
Button getSelectItemButton = new Button(FindElement.ByName("GetSelectedItemLabelButton"));

VerifyTabNavigationWithoutMenuItemSelected();
VerifyTabNavigationWithMenuItemSelected();
VerifyTabNavigationWithSettingsItemVisible();

void VerifyTabNavigationWithoutMenuItemSelected()
{
Log.Comment("Verify Tab navigation without a selected menu item");

getSelectItemButton.Invoke();
Wait.ForIdle();

Verify.AreEqual("No Item Selected", GetSelectedItem());

UIObject menuItem1 = FindElement.ByName("Menu Item 1");
UIObject menuItem29 = FindElement.ByName("Menu Item 29 (Selectable)");

// Set focus on the pane's toggle button.
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to Menu Item 1.");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(menuItem1.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while Menu Item 1 has focus moves to 'Get Selected Item Label' Button item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(getSelectItemButton.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to Menu Item 29");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(menuItem29.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
}

void VerifyTabNavigationWithMenuItemSelected()
{
Log.Comment("Verify Tab navigation with a selected menu item");

Log.Comment("Expand Menu Item 15.");
UIObject menuItem15 = FindElement.ByName("Menu Item 15");
InputHelper.LeftClick(menuItem15);

Log.Comment("Select Menu Item 16.");
UIObject menuItem16 = FindElement.ByName("Menu Item 16");
InputHelper.LeftClick(menuItem16);

getSelectItemButton.Invoke();
Wait.ForIdle();

Verify.AreEqual("Menu Item 16", GetSelectedItem());

// Set focus on the pane's toggle button.
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to Menu Item 16.");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(menuItem16.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while Menu Item 16 has focus moves to 'Get Selected Item Label' Button item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(getSelectItemButton.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to Menu Item 16");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(menuItem16.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);

Log.Comment("Verify that pressing tab from parent of item will move focus to 'Get Selected Item Label' Button item");
KeyboardHelper.PressKey(Key.Down, ModifierKey.None, 3);
Wait.ForIdle();
Verify.IsTrue(menuItem15.HasKeyboardFocus);

KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(getSelectItemButton.HasKeyboardFocus);
}

void VerifyTabNavigationWithSettingsItemVisible()
{
Log.Comment("Verify tab navigation with settings item visible.");

Log.Comment("Check IsSettingsVisible");
CheckBox checkBoxIsSettingsVisible = FindElement.ByName<CheckBox>("Settings Item Visibility");
checkBoxIsSettingsVisible.Check();
Wait.ForIdle();

UIObject settingsItem = FindElement.ByName("Settings");

getSelectItemButton.Invoke();
Wait.ForIdle();

Verify.AreEqual("Menu Item 16", GetSelectedItem());

UIObject menuItem16 = FindElement.ByName("Menu Item 16");

// Set focus on the pane's toggle button.
togglePaneButton.SetFocus();
Wait.ForIdle();

Log.Comment("Verify that pressing tab while TogglePaneButton has focus moves to Menu Item 16.");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(menuItem16.HasKeyboardFocus);

Log.Comment("Verify that pressing tab while Menu Item 16 has focus moves to Settings item");
KeyboardHelper.PressKey(Key.Tab);
Wait.ForIdle();
Verify.IsTrue(settingsItem.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to Menu Item 16");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(menuItem16.HasKeyboardFocus);

Log.Comment("Verify that pressing SHIFT+tab will move focus to the TogglePaneButton");
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift, 1);
Wait.ForIdle();
Verify.IsTrue(togglePaneButton.HasKeyboardFocus);
}
}

string GetSelectedItem()
{
return new TextBlock(FindElement.ByName("SelectedItemLabel")).DocumentText;
}
}
}

[TestMethod]
public void CanDoSelectionChangedOfItemTemplate()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,15 @@
</Page.Resources>

<Grid>
<muxcontrols:NavigationView x:Name="navview" MenuItemsSource="{x:Bind categories, Mode=OneWay}" MenuItemTemplate="{StaticResource NavigationViewMenuItem}"
ItemInvoked="{x:Bind ClickedItem}" Expanding="OnItemExpanding" Collapsed="OnItemCollapsed" SelectionChanged="OnSelectionChanged" PaneDisplayMode="Left">
<muxcontrols:NavigationView
x:Name="navview"
MenuItemsSource="{x:Bind categories, Mode=OneWay}"
MenuItemTemplate="{StaticResource NavigationViewMenuItem}"
ItemInvoked="{x:Bind ClickedItem}"
Expanding="OnItemExpanding"
Collapsed="OnItemCollapsed"
SelectionChanged="OnSelectionChanged"
PaneDisplayMode="Left">
<StackPanel Margin="10,10,0,0" Spacing="5">
<TextBlock x:Name="SelectedItemLabel" AutomationProperties.Name="SelectedItemLabel" Text="uninitialized"/>
<Button Content="Get Selected Item Label" x:Name="GetSelectedItemLabelButton" AutomationProperties.Name="GetSelectedItemLabelButton" Click="PrintSelectedItem"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<TextBlock x:Name="IsChildSelectedLabel" AutomationProperties.Name="IsChildSelectedLabel" Text="uninitialized"/>
<Button AutomationProperties.Name="PrintTopLevelIsChildSelectedItemsButton" Content="Print TopLevel IsChildSelected Items" Click="PrintTopLevelIsChildSelectedItems"/>
<Button Content="Select Second Item" Click="SelectSecondItem"/>
<CheckBox Content="Settings Item Visibility" x:Name="CheckBoxIsSettingsVisible" Checked="OnCheckedIsSettingsVisible" Unchecked="OnUncheckedIsSettingsVisible"/>
<!-- Components that get updated when item is expanded or collapsed -->
<StackPanel x:Name="ExpandCollapseRelatedComponents" Margin="0,10,0,10">
<Grid x:Name="ExpandingEventStates">
Expand Down
Loading

0 comments on commit 7198c4e

Please sign in to comment.