Skip to content

Commit

Permalink
[BreadcrumbBar] Add theme resource keys for customizing Chevron paddi…
Browse files Browse the repository at this point in the history
…ng and font weight (#4727)

* Add styles for dropdown flyout

* Fix ellipsis flyout widht

* Fix margins for breadcrumb items

* Remove flyout style retrieval and set it directly in xaml file

* First version of margin modification

* Fix margin bottom

* Fix focus margin height and fix padding for last item

* Stretch border for inline items and compress for dropdown items

* Fix PR comments

* Readd corner radius

* Add modifiable padding between chevrons

* Fix missing resource key

* Fix margin and padding

* Remove padding resource key for a different PR

* Move resources to theme resources

* Add test to verify current item doesnt raise invoke pattern and fix tests

* Remove extra keys

* Move margins to be rendered close to the buttons

* Add resource keys for font weight and chevron padding

* Clean xaml a little bit

* Remove other unused keys

* Hide element from Accessibility tree

* Remove hard coded breadcrumb names

* Fix accessibility control type and name

* Removed unused properties

* Remove extra code lines and set button to non focusable

* Remove hard coded naming for drop down items

* Fix comments from PR

* Remove unnecesary automation strings

* Fix tests

* small refactor to DropDown item retrieval method

* Fix DropDown item margin

* Remove extra theme resources keys

* Fix comments from design review

* Last fixes from focus rect
  • Loading branch information
almedina-ms authored Apr 9, 2021
1 parent 3d3fbb5 commit f4a2812
Show file tree
Hide file tree
Showing 11 changed files with 218 additions and 117 deletions.
5 changes: 0 additions & 5 deletions dev/Breadcrumb/BreadcrumbBar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ void BreadcrumbBar::OnElementPreparedEvent(const winrt::ItemsRepeater&, const wi
// Any other element just resets the visual properties
itemImpl->ResetVisualProperties();
}

winrt::AutomationProperties::SetName(item, s_breadcrumbItemAutomationName + winrt::to_hstring(itemIndex));
}
}
}
Expand All @@ -280,7 +278,6 @@ void BreadcrumbBar::OnElementIndexChangedEvent(const winrt::ItemsRepeater& sende
}

FocusElementAt(newIndex);
winrt::AutomationProperties::SetName(args.Element(), s_breadcrumbItemAutomationName + winrt::to_hstring(newIndex));
}
}

Expand All @@ -290,8 +287,6 @@ void BreadcrumbBar::OnElementClearingEvent(const winrt::ItemsRepeater&, const wi
{
const auto& itemImpl = winrt::get_self<BreadcrumbBarItem>(item);
itemImpl->ResetVisualProperties();

winrt::AutomationProperties::SetName(item, winrt::hstring());
}
}

Expand Down
3 changes: 0 additions & 3 deletions dev/Breadcrumb/BreadcrumbBar.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,4 @@ class BreadcrumbBar :

// Template Parts
static constexpr std::wstring_view s_itemsRepeaterPartName{ L"PART_ItemsRepeater"sv };

// Automation Names
static constexpr std::wstring_view s_breadcrumbItemAutomationName{ L"BreadcrumbBarItem"sv };
};
162 changes: 99 additions & 63 deletions dev/Breadcrumb/BreadcrumbBar.xaml

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions dev/Breadcrumb/BreadcrumbBarItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,6 @@ void BreadcrumbBarItem::UpdateFlyoutIndex(const winrt::UIElement& element, const
ellipsisDropDownItemImpl->SetIndex(itemCount - index);
}

hstring name = s_ellipsisItemAutomationName + winrt::to_hstring(index + 1);
winrt::AutomationProperties::SetName(element, name);

element.SetValue(winrt::AutomationProperties::PositionInSetProperty(), box_value(index + 1));
element.SetValue(winrt::AutomationProperties::SizeOfSetProperty(), box_value(itemCount));
}
Expand Down
1 change: 0 additions & 1 deletion dev/Breadcrumb/BreadcrumbBarItem.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,5 @@ class BreadcrumbBarItem :

// Automation Names
static constexpr std::wstring_view s_ellipsisFlyoutAutomationName{ L"EllipsisFlyout"sv };
static constexpr std::wstring_view s_ellipsisItemAutomationName{ L"EllipsisItem"sv };
static constexpr std::wstring_view s_ellipsisItemsRepeaterAutomationName{ L"EllipsisItemsRepeater"sv };
};
6 changes: 6 additions & 0 deletions dev/Breadcrumb/BreadcrumbBarItemAutomationPeer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "pch.h"
#include "common.h"
#include "ResourceAccessor.h"
#include "BreadcrumbBarItem.h"
#include "BreadcrumbBarItemAutomationPeer.h"
#include "BreadcrumbBarItemAutomationPeer.properties.cpp"
Expand All @@ -13,6 +14,11 @@ BreadcrumbBarItemAutomationPeer::BreadcrumbBarItemAutomationPeer(winrt::Breadcru
}

// IAutomationPeerOverrides
hstring BreadcrumbBarItemAutomationPeer::GetLocalizedControlTypeCore()
{
return ResourceAccessor::GetLocalizedStringResource(SR_BreadcrumbBarItemLocalizedControlType);
}

winrt::IInspectable BreadcrumbBarItemAutomationPeer::GetPatternCore(winrt::PatternInterface const& patternInterface)
{
if (patternInterface == winrt::PatternInterface::Invoke)
Expand Down
1 change: 1 addition & 0 deletions dev/Breadcrumb/BreadcrumbBarItemAutomationPeer.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class BreadcrumbBarItemAutomationPeer :
BreadcrumbBarItemAutomationPeer(winrt::BreadcrumbBarItem const& owner);

// IAutomationPeerOverrides
hstring GetLocalizedControlTypeCore();
winrt::IInspectable GetPatternCore(winrt::PatternInterface const& patternInterface);
hstring GetClassNameCore();
winrt::AutomationControlType GetAutomationControlTypeCore();
Expand Down
36 changes: 24 additions & 12 deletions dev/Breadcrumb/BreadcrumbBar_themeresources.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,16 @@
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">

<x:String x:Key="BreadcrumbBarChevronLeftToRight">&#xE76C;</x:String>
<x:String x:Key="BreadcrumbBarChevronRightToLeft">&#xE76B;</x:String>

<x:Double x:Key="BreadcrumbBarEllipsisFlyoutThemeMinHeight">40</x:Double>
<Thickness x:Key="BreadcrumbBarEllipsisDropDownItemMargin">5,3,5,3</Thickness>
<Thickness x:Key="BreadcrumbBarEllipsisDropDownItemPadding">11,7,11,9</Thickness>
<Thickness x:Key="BreadcrumbBarEllipsisFlyoutPresenterThemePadding">0,2,0,2</Thickness>

<Thickness x:Key="BreadcrumbBarItemDefaultPadding">4,0,3,0</Thickness>
<Thickness x:Key="BreadcrumbBarItemDefaultMargin">0,3,0,4</Thickness>
<Thickness x:Key="BreadcrumbBarItemLastItemMargin">0,3,1,4</Thickness>

<ResourceDictionary.ThemeDictionaries>

<ResourceDictionary x:Key="Default">
<x:String x:Key="BreadcrumbBarChevronLeftToRight">&#xE974;</x:String>
<x:String x:Key="BreadcrumbBarChevronRightToLeft">&#xE973;</x:String>

<Thickness x:Key="BreadcrumbBarChevronPadding">2,0</Thickness>

<FontWeight x:Key="BreadcrumbBarItemFontWeight">Normal</FontWeight>

<StaticResource x:Key="BreadcrumbBarNormalForegroundBrush" ResourceKey="TextFillColorPrimaryBrush" />
<StaticResource x:Key="BreadcrumbBarHoverForegroundBrush" ResourceKey="TextFillColorSecondaryBrush" />
<StaticResource x:Key="BreadcrumbBarPressedForegroundBrush" ResourceKey="TextFillColorTertiaryBrush" />
Expand Down Expand Up @@ -47,9 +42,17 @@
<Thickness x:Key="BreadcrumbBarEllipsisFlyoutPresenterBorderThemeThickness">1</Thickness>

<StaticResource x:Key="BreadcrumbBarItemThemeFontSize" ResourceKey="ControlContentThemeFontSize" />
<x:Double x:Key="BreadcrumbBarChevronFontSize">12</x:Double>
</ResourceDictionary>

<ResourceDictionary x:Key="Light">
<x:String x:Key="BreadcrumbBarChevronLeftToRight">&#xE974;</x:String>
<x:String x:Key="BreadcrumbBarChevronRightToLeft">&#xE973;</x:String>

<Thickness x:Key="BreadcrumbBarChevronPadding">2,0</Thickness>

<FontWeight x:Key="BreadcrumbBarItemFontWeight">Normal</FontWeight>

<StaticResource x:Key="BreadcrumbBarNormalForegroundBrush" ResourceKey="TextFillColorPrimaryBrush" />
<StaticResource x:Key="BreadcrumbBarHoverForegroundBrush" ResourceKey="TextFillColorSecondaryBrush" />
<StaticResource x:Key="BreadcrumbBarPressedForegroundBrush" ResourceKey="TextFillColorTertiaryBrush" />
Expand Down Expand Up @@ -80,9 +83,17 @@
<Thickness x:Key="BreadcrumbBarEllipsisFlyoutPresenterBorderThemeThickness">1</Thickness>

<StaticResource x:Key="BreadcrumbBarItemThemeFontSize" ResourceKey="ControlContentThemeFontSize" />
<x:Double x:Key="BreadcrumbBarChevronFontSize">12</x:Double>
</ResourceDictionary>

<ResourceDictionary x:Key="HighContrast">
<x:String x:Key="BreadcrumbBarChevronLeftToRight">&#xE974;</x:String>
<x:String x:Key="BreadcrumbBarChevronRightToLeft">&#xE973;</x:String>

<Thickness x:Key="BreadcrumbBarChevronPadding">2,0</Thickness>

<FontWeight x:Key="BreadcrumbBarItemFontWeight">Normal</FontWeight>

<StaticResource x:Key="BreadcrumbBarNormalForegroundBrush" ResourceKey="SystemColorHighlightColor" />
<StaticResource x:Key="BreadcrumbBarHoverForegroundBrush" ResourceKey="SystemColorButtonTextColor" />
<StaticResource x:Key="BreadcrumbBarPressedForegroundBrush" ResourceKey="SystemColorHighlightColor" />
Expand Down Expand Up @@ -112,6 +123,7 @@
<Thickness x:Key="BreadcrumbBarEllipsisFlyoutPresenterBorderThemeThickness">2</Thickness>

<StaticResource x:Key="BreadcrumbBarItemThemeFontSize" ResourceKey="ControlContentThemeFontSize" />
<x:Double x:Key="BreadcrumbBarChevronFontSize">12</x:Double>
</ResourceDictionary>
</ResourceDictionary.ThemeDictionaries>
</ResourceDictionary>
113 changes: 83 additions & 30 deletions dev/Breadcrumb/InteractionTests/BreadcrumbBarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
using System.Runtime.CompilerServices;
using System.Threading;
using System.Collections.ObjectModel;
using Microsoft.Windows.Apps.Test.Automation.Text;
using System.Drawing;
using Windows.UI.Xaml.Controls.Primitives;
using System.Runtime.InteropServices;

namespace Windows.UI.Xaml.Tests.MUXControls.InteractionTests
{
Expand Down Expand Up @@ -123,6 +127,30 @@ public void NoInteractionTest()
}
}

[TestMethod]
[TestProperty("TestSuite", "A")]
public void CheckCurrentItemDoesNotRaiseInvokeEvent()
{
// This is a sanity test that verifies that the BreadcrumbBar control exists and the basic setup for a test is correct
using (var setup = new TestSetupHelper("BreadcrumbBar Tests"))
{
UIObject breadcrumb = RetrieveBreadcrumbControl();
var breadcrumbItems = breadcrumb.Children;

Verify.AreEqual(2, breadcrumbItems.Count, "The breadcrumb should contain 2 items: 1 item and an ellipsis");

var currentItem = breadcrumbItems[1];

var currentBreadcrumbBarItem = ConvertTo<BreadcrumbBarItem>(currentItem);
Verify.IsNotNull(currentBreadcrumbBarItem, "UIElement should be a BreadcrumbBarItem");

currentBreadcrumbBarItem.Click();

VerifyLastClickedItemIs("");
Verify.AreEqual("Root", GetCurrentItemText(currentBreadcrumbBarItem));
}
}

[TestMethod]
[TestProperty("TestSuite", "A")]
public void AddItemsAndCompressBreadcrumbTest()
Expand All @@ -134,7 +162,7 @@ public void AddItemsAndCompressBreadcrumbTest()
{
var breadcrumb = SetUpTest();

VerifyBreadcrumbBarItemsContain(breadcrumb.Children, new string[] { "Root", "Node A", "Node A_2", "Node A_2_3", "Node A_2_3_1" });
VerifyBreadcrumbBarItemsContain(breadcrumb.Children, new string[] { "Root", "Node A", "Node A_2", "Node A_2_3", "Node A_2_3_1" }, true);

Verify.AreEqual(2, breadcrumb.Children.Count, "The breadcrumb should contain 2 items: the root and an ellipsis");
}
Expand Down Expand Up @@ -178,9 +206,9 @@ public void InvokeEllipsisItemTest()
// Click on the Ellipsis BreadcrumbBarItem
InvokeEllipsisItem(breadcrumb);

VerifyDropDownItemContainsText("EllipsisItem1", "Node A_2");
VerifyDropDownItemContainsText("EllipsisItem2", "Node A");
VerifyDropDownItemContainsText("EllipsisItem3", "Root");
VerifyDropDownItemContainsText("Node A_2");
VerifyDropDownItemContainsText("Node A");
VerifyDropDownItemContainsText("Root");

Log.Comment("Verify only 3 ellipsis items exist");
var ellipsisItem4 = FindElement.ByName("EllipsisItem4");
Expand All @@ -201,7 +229,7 @@ public void InvokeDropDownItemTest()
// Click on the Ellipsis BreadcrumbBarItem
InvokeEllipsisItem(breadcrumb);

var ellipsisItemNodeA = VerifyDropDownItemContainsText("EllipsisItem2", "Node A");
var ellipsisItemNodeA = GetDropDownItemByName("Node A");
ellipsisItemNodeA.Invoke();
Thread.Sleep(500);

Expand Down Expand Up @@ -373,28 +401,28 @@ public void KeyboardNavigationFlyoutItemsTest()

// Here we should verify that the first element in the flyout has focus and we can move up/down

var dropDownItem = GetDropDownItemByName("EllipsisItem1");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem1 BreadcrumbBarItem should have focus");
var dropDownItem = GetDropDownItemByName("Node A_2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A_2' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Down);

dropDownItem = GetDropDownItemByName("EllipsisItem2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem2 BreadcrumbBarItem should have focus");
dropDownItem = GetDropDownItemByName("Node A");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Down);

dropDownItem = GetDropDownItemByName("EllipsisItem3");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem3 BreadcrumbBarItem should have focus");
dropDownItem = GetDropDownItemByName("Root");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Root' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Up);

dropDownItem = GetDropDownItemByName("EllipsisItem2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem2 BreadcrumbBarItem should have focus");
dropDownItem = GetDropDownItemByName("Node A");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Up);

dropDownItem = GetDropDownItemByName("EllipsisItem1");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem1 BreadcrumbBarItem should have focus");
dropDownItem = GetDropDownItemByName("Node A_2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A_2' BreadcrumbBarItem should have focus");
}
}

Expand Down Expand Up @@ -448,13 +476,14 @@ public void KeyboardNavigationDropDownItemInvokationTest()
Thread.Sleep(1000);

// Here we should verify that the first element in the flyout has focus and we can move up/down
var dropDownItem = GetDropDownItemByName("EllipsisItem1");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem1 BreadcrumbBarItem should have focus");
// The flyout items should contain: "Node A_2", "Node A" and "Root"
var dropDownItem = GetDropDownItemByName("Node A_2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A_2' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Down);

dropDownItem = GetDropDownItemByName("EllipsisItem2");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "EllipsisItem2 BreadcrumbBarItem should have focus");
dropDownItem = GetDropDownItemByName("Node A");
Verify.IsTrue(dropDownItem.HasKeyboardFocus, "'Node A' BreadcrumbBarItem should have focus");

KeyboardHelper.PressKey(Key.Enter);

Expand Down Expand Up @@ -484,7 +513,7 @@ public void VerifyMulticlickCrash()

InvokeEllipsisItem(breadcrumb);

var ellipsisItemNodeA = VerifyDropDownItemContainsText("EllipsisItem2", "Root");
var ellipsisItemNodeA = VerifyDropDownItemContainsText("Root");
ellipsisItemNodeA.Invoke();
Thread.Sleep(500);

Expand All @@ -511,7 +540,7 @@ public void VerifyFlyoutRecycleCrash()

InvokeEllipsisItem(breadcrumb);

var ellipsisItemNodeA_2_3 = VerifyDropDownItemContainsText("EllipsisItem1", "Node A_2_3");
var ellipsisItemNodeA_2_3 = GetDropDownItemByName("Node A_2_3");
ellipsisItemNodeA_2_3.Invoke();
Thread.Sleep(500);

Expand Down Expand Up @@ -725,7 +754,7 @@ private UIObject RetrieveRTLCheckBox(bool mustClickCheckBox = false)
return rtlCheckbox;
}

private void VerifyBreadcrumbBarItemsContain(UICollection<UIObject> breadcrumbItems, string[] expectedItemValues)
private void VerifyBreadcrumbBarItemsContain(UICollection<UIObject> breadcrumbItems, string[] expectedItemValues, bool firstItemIsCurrentItem = false)
{
// WARNING: this method clicks on each breadcrumb so once the verification has finished,
// only the ellipsis item and 'Root' should exist
Expand All @@ -735,8 +764,10 @@ private void VerifyBreadcrumbBarItemsContain(UICollection<UIObject> breadcrumbIt
Verify.IsTrue(breadcrumbItems.Count > expectedItemValues.Length,
"The expected values count should at least be one less than the BreadcrumbBarItems count");

bool mustVerifyItemAsLastItem = firstItemIsCurrentItem;

// To verify the existence of the expected nodes we click on each of them and verify agains the strings
// in LastClickedItemIndex and LastClickedItem textboxes.
// in LastClickedItemIndex and LastClickedItem textboxes.
for (int i = expectedItemValues.Length - 1; i >= 0; --i)
{
var currentItem = breadcrumbItems[i + 1];
Expand All @@ -747,26 +778,43 @@ private void VerifyBreadcrumbBarItemsContain(UICollection<UIObject> breadcrumbIt

currentBreadcrumbBarItem.Click();

VerifyLastClickedItemIndexIs(i);
VerifyLastClickedItemIs(expectedItemValues[i]);
if (mustVerifyItemAsLastItem)
{
Verify.AreEqual(expectedItemValues[i], GetCurrentItemText(currentBreadcrumbBarItem));
mustVerifyItemAsLastItem = false;
}
else
{
VerifyLastClickedItemIndexIs(i);
VerifyLastClickedItemIs(expectedItemValues[i]);
}
}
}

private Button VerifyDropDownItemContainsText(string dropDownItemName, string expectedText)
private Button VerifyDropDownItemContainsText(string expectedText)
{
Log.Comment("Retrieve the ellipsis item: " + expectedText);
var dropDownItem = GetDropDownItemByName(dropDownItemName);

var dropDownItem = GetDropDownItemByName(expectedText);
VerifyDropDownItemContainsText(dropDownItem, expectedText);

return dropDownItem;
}

private Button GetDropDownItemByName(string dropDownItemName)
{
var dropDownItem = FindElement.ByName<Button>(dropDownItemName);
Verify.IsNotNull(dropDownItem, dropDownItemName + " not found");
return dropDownItem;
var ellipsisFlyout = FindElement.ByName("EllipsisFlyout");

foreach (var child in ellipsisFlyout.Children)
{
if (child.Name == dropDownItemName)
{
return ConvertTo<Button>(child);
}
}

Verify.Fail(dropDownItemName + " not found");
return null;
}

private void VerifyDropDownItemContainsText(Button dropDownItem, string expectedEllipsisItemText)
Expand Down Expand Up @@ -803,5 +851,10 @@ private int GetLastClickedItemIndex()
var lastItemTextBlock = FindElement.ByName<TextBlock>("LastClickedItemIndex");
return Int32.Parse(lastItemTextBlock.DocumentText);
}

private string GetCurrentItemText(BreadcrumbBarItem currentItem)
{
return currentItem.Children[0].GetText();
}
}
}
4 changes: 4 additions & 0 deletions dev/Breadcrumb/Strings/en-us/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,8 @@
<value>More</value>
<comment>The automation name for the BreadcrumbBar ellipsis button.</comment>
</data>
<data name="BreadcrumbBarItemLocalizedControlType" xml:space="preserve">
<value>breadcrumb bar item</value>
<comment>A simple description of the control for UIA</comment>
</data>
</root>
Loading

0 comments on commit f4a2812

Please sign in to comment.