From 309c88f81cf23802d6406b452e4381fc9b4e9de1 Mon Sep 17 00:00:00 2001 From: Felix-Dev Date: Fri, 13 Nov 2020 22:09:16 +0100 Subject: [PATCH] TabView: Fix newly-added items not respecting TabWidthMode [compact] when using an ItemsSource collection (#3118) * Fix the following issues when populating the TabView via its ItemsSource API: * crash when clearing the ItemsSource * TabWidthMode [compact] being ignored in XAML Markup and for newly-added items * small comment improvement * Added two API tests and modified existing test. * Unrelated code which was commented out is run again. * Add API test. * Simplified API test. * Merge API tests. * Add API contract checks for corner radius. * Ensure that listview selected index is not outside of the item range. * Update TabView.cpp Co-authored-by: Ranjesh <28935693+ranjeshj@users.noreply.github.com> --- dev/TabView/APITests/TabViewTests.cs | 137 ++++++++++++++++++++------- dev/TabView/TabView.cpp | 12 +-- dev/TabView/TabView.xaml | 18 +++- dev/TabView/TabViewListView.cpp | 32 ++++++- dev/TabView/TabViewListView.h | 1 + 5 files changed, 148 insertions(+), 52 deletions(-) diff --git a/dev/TabView/APITests/TabViewTests.cs b/dev/TabView/APITests/TabViewTests.cs index 73250ce0d3..2c0488f9c0 100644 --- a/dev/TabView/APITests/TabViewTests.cs +++ b/dev/TabView/APITests/TabViewTests.cs @@ -13,6 +13,8 @@ using Windows.UI.Xaml.Automation.Peers; using Windows.UI.Xaml.Automation; using Windows.UI.Xaml.Automation.Provider; +using System.Collections.ObjectModel; +using Microsoft.UI.Xaml.Controls.Primitives; #if USING_TAEF using WEX.TestExecution; @@ -31,60 +33,99 @@ namespace Windows.UI.Xaml.Tests.MUXControls.ApiTests [TestClass] public class TabViewTests : ApiTestBase { + [TestMethod] + public void VerifyCompactTabWidthVisualStates_ItemsMode() + { + VerifyCompactTabWidthVisualStates(); + } [TestMethod] - public void VerifyCompactTabWidthVisualStates() + public void VerifyCompactTabWidthVisualStates_ItemsSourceMode() + { + VerifyCompactTabWidthVisualStates(isItemsSourceMode: true); + } + + private void VerifyCompactTabWidthVisualStates(bool isItemsSourceMode = false) { TabView tabView = null; RunOnUIThread.Execute(() => { tabView = new TabView(); - Content = tabView; + SetupTabViewItems(); - tabView.TabItems.Add(CreateTabViewItem("Item 0", Symbol.Add)); - tabView.TabItems.Add(CreateTabViewItem("Item 1", Symbol.AddFriend)); - tabView.TabItems.Add(CreateTabViewItem("Item 2")); + Log.Comment("Set TabWidthMode to compact"); + tabView.TabWidthMode = TabViewWidthMode.Compact; + Log.Comment("Select a tab. In TabWidthMode compact, a selected tab will not be in compact state. We verify that behavior in this test"); tabView.SelectedIndex = 0; - tabView.SelectedItem = tabView.TabItems[0]; - (tabView.SelectedItem as TabViewItem).IsSelected = true; - Verify.AreEqual("Item 0", (tabView.SelectedItem as TabViewItem).Header); - Content.UpdateLayout(); - }); - // Waiting for layout - IdleSynchronizer.Wait(); - RunOnUIThread.Execute(() => - { - // Now set tab width mode - tabView.TabWidthMode = TabViewWidthMode.Compact; + Content = tabView; }); IdleSynchronizer.Wait(); - // Check if switching to compact updates all items correctly RunOnUIThread.Execute(() => { - VerifyTabWidthVisualStates(tabView.TabItems, true); - tabView.TabItems.Add(CreateTabViewItem("Item 3")); - }); + Log.Comment("Verify a selected tab exists"); + VerifySelectedItem("Tab 0"); - IdleSynchronizer.Wait(); + Log.Comment("Verify the created TabView displays every tab in compact mode"); + VerifyTabWidthVisualStates(tabView, tabView.TabItems, true); - // Check if a newly added item has correct visual states - RunOnUIThread.Execute(() => - { - VerifyTabWidthVisualStates(tabView.TabItems, true); + Log.Comment("Verify that adding a new item creates a tab in compact mode"); + AddItem("Tab 3"); + Content.UpdateLayout(); + + VerifyTabWidthVisualStates(tabView, new List() { tabView.TabItems[tabView.TabItems.Count - 1] }, true); + + Log.Comment("Change the TabWidthMode to non compact and verify that every tab is no longer in compact mode"); tabView.TabWidthMode = TabViewWidthMode.Equal; + Content.UpdateLayout(); + + VerifyTabWidthVisualStates(tabView, tabView.TabItems, false); + + Log.Comment("Change the TabWidthMode to compact and verify that every tab is now in compact mode"); + tabView.TabWidthMode = TabViewWidthMode.Compact; + Content.UpdateLayout(); + + VerifyTabWidthVisualStates(tabView, tabView.TabItems, true); }); - IdleSynchronizer.Wait(); + void SetupTabViewItems() + { + if (isItemsSourceMode) + { + var tabItemsSource = new ObservableCollection() { "Tab 0", "Tab 1", "Tab 2" }; + tabView.TabItemsSource = tabItemsSource; + } + else + { + tabView.TabItems.Add(CreateTabViewItem("Tab 0", Symbol.Add)); + tabView.TabItems.Add(CreateTabViewItem("Tab 1", Symbol.AddFriend)); + tabView.TabItems.Add(CreateTabViewItem("Tab 2")); + } + } - // Switch back to non compact and check if every item has the correct visual state - RunOnUIThread.Execute(() => + void VerifySelectedItem(string expectedHeader) { - VerifyTabWidthVisualStates(tabView.TabItems, false); - }); + object selectedItemHeader = isItemsSourceMode + ? tabView.SelectedItem + : (tabView.SelectedItem as TabViewItem).Header; + + Verify.AreEqual(expectedHeader, selectedItemHeader); + } + + void AddItem(string header) + { + if (isItemsSourceMode) + { + ((ObservableCollection)tabView.TabItemsSource).Add(header); + } + else + { + tabView.TabItems.Add(CreateTabViewItem(header)); + } + } } [TestMethod] @@ -142,7 +183,7 @@ public void VerifyTabViewItemUIABehavior() RunOnUIThread.Execute(() => { var selectionItemProvider = GetProviderFromTVI(tvi0); - Verify.IsTrue(selectionItemProvider.IsSelected,"Item should be selected"); + Verify.IsTrue(selectionItemProvider.IsSelected, "Item should be selected"); selectionItemProvider = GetProviderFromTVI(tvi1); Verify.IsFalse(selectionItemProvider.IsSelected, "Item should not be selected"); @@ -150,7 +191,7 @@ public void VerifyTabViewItemUIABehavior() Log.Comment("Change selection through automationpeer"); selectionItemProvider.Select(); Verify.IsTrue(selectionItemProvider.IsSelected, "Item should have been selected"); - + selectionItemProvider = GetProviderFromTVI(tvi0); Verify.IsFalse(selectionItemProvider.IsSelected, "Item should not be selected anymore"); @@ -167,18 +208,43 @@ static ISelectionItemProvider GetProviderFromTVI(TabViewItem item) } } - private static void VerifyTabWidthVisualStates(IList items, bool isCompact) + [TestMethod] + public void VerifyTabViewWithoutTabsDoesNotCrash() + { + RunOnUIThread.Execute(() => + { + TabView tabView = new TabView(); + Content = tabView; + + // Creating a TabView without tabs should not crash the app. + Content.UpdateLayout(); + + var tabItemsSource = new ObservableCollection() { "Tab 1", "Tab 2" }; + tabView.TabItemsSource = tabItemsSource; + + // Clearing the ItemsSource should not crash the app. + Log.Comment("Clear the specified tab items source"); + tabItemsSource.Clear(); + }); + } + + private static void VerifyTabWidthVisualStates(TabView tabView, IList items, bool isCompact) { + var listView = VisualTreeUtils.FindVisualChildByName(tabView, "TabListView") as TabViewListView; + foreach (var item in items) { - var tabItem = item as TabViewItem; + var tabItem = item is TabViewItem + ? (TabViewItem)item + : listView.ContainerFromItem(item) as TabViewItem; + var rootGrid = VisualTreeHelper.GetChild(tabItem, 0) as FrameworkElement; foreach (var group in VisualStateManager.GetVisualStateGroups(rootGrid)) { if (group.Name == "TabWidthModes") { - if(tabItem.IsSelected || !isCompact) + if (tabItem.IsSelected || !isCompact) { Verify.AreEqual("StandardWidth", group.CurrentState.Name, "Verify that this tab item is rendering in standard width"); } @@ -188,7 +254,6 @@ private static void VerifyTabWidthVisualStates(IList items, bool isCompa } } } - } } diff --git a/dev/TabView/TabView.cpp b/dev/TabView/TabView.cpp index 18e7aa2ca7..57c380a8c5 100644 --- a/dev/TabView/TabView.cpp +++ b/dev/TabView/TabView.cpp @@ -635,11 +635,6 @@ void TabView::OnItemsChanged(winrt::IInspectable const& item) } else { - if (const auto newItem = TabItems().GetAt(args.Index()).try_as()) - { - newItem->OnTabViewWidthModeChanged(TabWidthMode()); - newItem->SetParentTabView(*this); - } UpdateTabWidths(); } } @@ -1003,7 +998,12 @@ void TabView::UpdateSelectedIndex() { if (auto&& listView = m_listView.get()) { - listView.SelectedIndex(SelectedIndex()); + const auto selectedIndex = SelectedIndex(); + // Ensure that the selected index is within range of the items + if (selectedIndex < static_cast(listView.Items().Size())) + { + listView.SelectedIndex(selectedIndex); + } } } diff --git a/dev/TabView/TabView.xaml b/dev/TabView/TabView.xaml index 2fb217076d..97591f97a6 100644 --- a/dev/TabView/TabView.xaml +++ b/dev/TabView/TabView.xaml @@ -5,6 +5,7 @@ xmlns:contract4Present="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractPresent(Windows.Foundation.UniversalApiContract,4)" xmlns:contract6Present="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractPresent(Windows.Foundation.UniversalApiContract,6)" xmlns:contract7Present="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractPresent(Windows.Foundation.UniversalApiContract,7)" + xmlns:contract7NotPresent="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractNotPresent(Windows.Foundation.UniversalApiContract,7)" xmlns:local="using:Microsoft.UI.Xaml.Controls" xmlns:primitives="using:Microsoft.UI.Xaml.Controls.Primitives"> @@ -133,7 +134,12 @@ - + - + @@ -244,7 +250,8 @@ BorderBrush="{TemplateBinding BorderBrush}" ContentTemplate="{TemplateBinding ContentTemplate}" Content="{TemplateBinding Content}" - CornerRadius="{TemplateBinding CornerRadius}" + contract7Present:CornerRadius="{TemplateBinding CornerRadius}" + contract7NotPresent:CornerRadius="{ThemeResource ControlCornerRadius}" ContentTransitions="{TemplateBinding ContentTransitions}" HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" Padding="{TemplateBinding Padding}" @@ -290,7 +297,7 @@