diff --git a/dev/Repeater/APITests/ItemTemplateTests.cs b/dev/Repeater/APITests/ItemTemplateTests.cs index 0800b765a4..92c249da02 100644 --- a/dev/Repeater/APITests/ItemTemplateTests.cs +++ b/dev/Repeater/APITests/ItemTemplateTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. using Common; @@ -70,7 +70,6 @@ public void ValidateRecycling() ItemTemplate = elementFactory, }; - var context = (ElementFactoryGetArgs)RepeaterTestHooks.CreateRepeaterElementFactoryGetArgs(); context.Parent = repeater; var clearContext = (ElementFactoryRecycleArgs)RepeaterTestHooks.CreateRepeaterElementFactoryRecycleArgs(); @@ -565,6 +564,39 @@ public void ValidateTemplateSwitchingRefreshesElements(Layout layout) } } + [TestMethod] + public void ValidateNullItemTemplateAndContainerInItems() + { + RunOnUIThread.Execute(() => + { + const int numItems = 10; + ItemsRepeater repeater = null; + Content = CreateAndInitializeRepeater + ( + itemsSource: Enumerable.Range(0, numItems).Select(i => new Button() { Content = i }), // ItemsSource is UIElements + elementFactory: null, // No ItemTemplate + layout: new StackLayout(), + repeater: ref repeater + ); + + Content.UpdateLayout(); + + Verify.AreEqual(numItems, VisualTreeHelper.GetChildrenCount(repeater)); + + for (int i = 0; i < numItems; i++) + { + var element = (Button)repeater.TryGetElement(i); + Verify.AreEqual(i, element.Content); + } + + Button button0 = (Button)repeater.TryGetElement(0); + Verify.IsNotNull(button0.Parent); + + repeater.ItemsSource = null; + Verify.IsNull(button0.Parent); + }); + } + private ItemsRepeaterScrollHost CreateAndInitializeRepeater( object itemsSource, Layout layout, diff --git a/dev/Repeater/APITests/SelectionModelTests.cs b/dev/Repeater/APITests/SelectionModelTests.cs index 89c19f6983..688edffc34 100644 --- a/dev/Repeater/APITests/SelectionModelTests.cs +++ b/dev/Repeater/APITests/SelectionModelTests.cs @@ -69,7 +69,7 @@ public void ValidateSelectionChangedEvent() selectionModel.Source = Enumerable.Range(0, 10).ToList(); int selectionChangedFiredCount = 0; - selectionModel.SelectionChanged += delegate (SelectionModel sender, SelectionModelSelectionChangedEventArgs args) + selectionModel.SelectionChanged += delegate (SelectionModel sender, SelectionModelSelectionChangedEventArgs args) { selectionChangedFiredCount++; ValidateSelection(selectionModel, new List() { Path(4) }, new List() { Path() }); @@ -127,7 +127,7 @@ public void ValidateOneLevelMultipleSelection() Path(6) }, new List() { Path() }); - + SetAnchorIndex(selectionModel, 4); SelectRangeFromAnchor(selectionModel, 5, false /* select */); ValidateSelection(selectionModel, @@ -280,12 +280,16 @@ private void ValidateNestedMultipleSelection(bool handleChildrenRequested) RunOnUIThread.Execute(() => { SelectionModel selectionModel = new SelectionModel(); + List sourcePaths = new List(); + Log.Comment("Setting the source"); selectionModel.Source = CreateNestedData(3 /* levels */ , 2 /* groupsAtLevel */, 4 /* countAtLeaf */); if (handleChildrenRequested) { selectionModel.ChildrenRequested += (SelectionModel sender, SelectionModelChildrenRequestedEventArgs args) => { + Log.Comment("ChildrenRequestedIndexPath:" + args.SourceIndex); + sourcePaths.Add(args.SourceIndex); args.Children = args.Source is IEnumerable ? args.Source : null; }; } @@ -304,6 +308,36 @@ private void ValidateNestedMultipleSelection(bool handleChildrenRequested) var endPath = Path(1, 1, 1, 0); SelectRangeFromAnchor(selectionModel, endPath, true /* select */); + + if (handleChildrenRequested) + { + // Validate SourceIndices. + var expectedSourceIndices = new List() + { + Path(), + Path(1), + Path(1, 0), + Path(1), + Path(1, 0, 1), + Path(1, 0, 1), + Path(1, 0, 1), + Path(1, 0, 1), + Path(1, 1), + Path(1, 1), + Path(1, 1, 0), + Path(1, 1, 0), + Path(1, 1, 0), + Path(1, 1, 0), + Path(1, 1, 1) + }; + + Verify.AreEqual(expectedSourceIndices.Count, sourcePaths.Count); + for (int i = 0; i < expectedSourceIndices.Count; i++) + { + Verify.IsTrue(AreEqual(expectedSourceIndices[i], sourcePaths[i])); + } + } + ValidateSelection(selectionModel, new List() { @@ -1080,6 +1114,24 @@ private object GetData(SelectionModel selectionModel, IndexPath indexPath) return data; } + private bool AreEqual(IndexPath a, IndexPath b) + { + if (a.GetSize() != b.GetSize()) + { + return false; + } + + for (int i = 0; i < a.GetSize(); i++) + { + if (a.GetAt(i) != b.GetAt(i)) + { + return false; + } + } + + return true; + } + private List GetIndexPathsInSource(object source) { List paths = new List(); diff --git a/dev/Repeater/ItemsRepeater.idl b/dev/Repeater/ItemsRepeater.idl index 78e0bf5456..a99fec74b2 100644 --- a/dev/Repeater/ItemsRepeater.idl +++ b/dev/Repeater/ItemsRepeater.idl @@ -550,6 +550,7 @@ runtimeclass SelectionModelSelectionChangedEventArgs runtimeclass SelectionModelChildrenRequestedEventArgs { Object Source { get; }; + IndexPath SourceIndex { get; }; Object Children { get; set; }; } diff --git a/dev/Repeater/SelectionModel.cpp b/dev/Repeater/SelectionModel.cpp index 45e5f54499..9e9bb0ae11 100644 --- a/dev/Repeater/SelectionModel.cpp +++ b/dev/Repeater/SelectionModel.cpp @@ -517,7 +517,7 @@ void SelectionModel::OnSelectionInvalidatedDueToCollectionChange() OnSelectionChanged(); } -winrt::IInspectable SelectionModel::ResolvePath(const winrt::IInspectable& data) +winrt::IInspectable SelectionModel::ResolvePath(const winrt::IInspectable& data, const std::weak_ptr& sourceNode) { winrt::IInspectable resolved = nullptr; // Raise ChildrenRequested event if there is a handler @@ -525,15 +525,18 @@ winrt::IInspectable SelectionModel::ResolvePath(const winrt::IInspectable& data) { if (!m_childrenRequestedEventArgs) { - m_childrenRequestedEventArgs = tracker_ref(this, winrt::make(data)); + m_childrenRequestedEventArgs = tracker_ref(this, winrt::make(data, sourceNode)); } else { - winrt::get_self(m_childrenRequestedEventArgs.get())->Initialize(data); + winrt::get_self(m_childrenRequestedEventArgs.get())->Initialize(data, sourceNode); } m_getChildrenEventSource(*this, m_childrenRequestedEventArgs.get()); resolved = m_childrenRequestedEventArgs.get().Children(); + + // Clear out the values in the args so that it cannot be used after the event handler call. + winrt::get_self(m_childrenRequestedEventArgs.get())->Initialize(nullptr, std::weak_ptr() /* empty weakptr */); } else { diff --git a/dev/Repeater/SelectionModel.h b/dev/Repeater/SelectionModel.h index f37f96f2ef..5a36fc1c1b 100644 --- a/dev/Repeater/SelectionModel.h +++ b/dev/Repeater/SelectionModel.h @@ -88,7 +88,7 @@ class SelectionModel : void OnPropertyChanged(winrt::hstring const& propertyName); #pragma endregion - winrt::IInspectable ResolvePath(const winrt::IInspectable& data); + winrt::IInspectable ResolvePath(const winrt::IInspectable& data, const std::weak_ptr& sourceNode); void OnSelectionInvalidatedDueToCollectionChange(); std::shared_ptr SharedLeafNode() { return m_leafNode; } @@ -120,4 +120,4 @@ class SelectionModel : // use just one instance of a leaf node to avoid creating a bunch of these. std::shared_ptr m_leafNode; -}; \ No newline at end of file +}; diff --git a/dev/Repeater/SelectionModelChildrenRequestedEventArgs.cpp b/dev/Repeater/SelectionModelChildrenRequestedEventArgs.cpp index 5a1ff0df7a..1ce5719a58 100644 --- a/dev/Repeater/SelectionModelChildrenRequestedEventArgs.cpp +++ b/dev/Repeater/SelectionModelChildrenRequestedEventArgs.cpp @@ -4,20 +4,38 @@ #include "pch.h" #include "common.h" #include "ItemsRepeater.common.h" +#include "SelectionNode.h" #include "SelectionModelChildrenRequestedEventArgs.h" -SelectionModelChildrenRequestedEventArgs::SelectionModelChildrenRequestedEventArgs(const winrt::IInspectable& source) +SelectionModelChildrenRequestedEventArgs::SelectionModelChildrenRequestedEventArgs(const winrt::IInspectable& source, const std::weak_ptr& sourceNode) { - Initialize(source); + Initialize(source, sourceNode); } #pragma region ISelectionModelChildrenRequestedEventArgs winrt::IInspectable SelectionModelChildrenRequestedEventArgs::Source() { + auto node = m_sourceNode.lock(); + if (!node) + { + throw winrt::hresult_error(E_FAIL, L"Source can only be accesed in the ChildrenRequested event handler."); + } + return m_source.get(); } +winrt::IndexPath SelectionModelChildrenRequestedEventArgs::SourceIndex() +{ + auto node = m_sourceNode.lock(); + if (!node) + { + throw winrt::hresult_error(E_FAIL, L"SourceIndex can only be accesed in the ChildrenRequested event handler."); + } + + return node->IndexPath(); +} + winrt::IInspectable SelectionModelChildrenRequestedEventArgs::Children() { return m_children.get(); @@ -30,8 +48,9 @@ void SelectionModelChildrenRequestedEventArgs::Children(winrt::IInspectable cons #pragma endregion -void SelectionModelChildrenRequestedEventArgs::Initialize(const winrt::IInspectable& source) +void SelectionModelChildrenRequestedEventArgs::Initialize(const winrt::IInspectable& source, const std::weak_ptr& sourceNode) { m_source.set(source); + m_sourceNode = sourceNode; m_children.set(nullptr); } diff --git a/dev/Repeater/SelectionModelChildrenRequestedEventArgs.h b/dev/Repeater/SelectionModelChildrenRequestedEventArgs.h index 71bd05b09f..988e8d0e96 100644 --- a/dev/Repeater/SelectionModelChildrenRequestedEventArgs.h +++ b/dev/Repeater/SelectionModelChildrenRequestedEventArgs.h @@ -9,17 +9,19 @@ class SelectionModelChildrenRequestedEventArgs : public ReferenceTracker { public: - SelectionModelChildrenRequestedEventArgs(const winrt::IInspectable& data); + SelectionModelChildrenRequestedEventArgs(const winrt::IInspectable& data, const std::weak_ptr& sourceNode); #pragma region ISelectionModelChildrenRequestedEventArgs winrt::IInspectable Source(); + winrt::IndexPath SourceIndex(); winrt::IInspectable Children(); void Children(winrt::IInspectable const& value); #pragma endregion - void Initialize(const winrt::IInspectable& source); + void Initialize(const winrt::IInspectable& source, const std::weak_ptr& sourceNode); private: tracker_ref m_source{ this }; tracker_ref m_children{ this }; -}; \ No newline at end of file + std::weak_ptr m_sourceNode; +}; diff --git a/dev/Repeater/SelectionNode.cpp b/dev/Repeater/SelectionNode.cpp index 220143ce1c..2c4da66c07 100644 --- a/dev/Repeater/SelectionNode.cpp +++ b/dev/Repeater/SelectionNode.cpp @@ -6,6 +6,7 @@ #include "ItemsRepeater.common.h" #include "SelectionNode.h" #include "SelectionModel.h" +#include "IndexPath.h" SelectionNode::SelectionNode(SelectionModel* manager, SelectionNode* parent) : m_manager(manager), m_parent(parent), m_source(manager), m_dataSource(manager) @@ -77,6 +78,26 @@ void SelectionNode::AnchorIndex(int value) m_anchorIndex = value; } +winrt::IndexPath SelectionNode::IndexPath() +{ + std::vector path; + auto parent = m_parent; + auto child = this; + while (parent != nullptr) + { + auto childNodes = parent->m_childrenNodes; + auto it = std::find_if(childNodes.cbegin(), childNodes.cend(), [&child](const auto& item) {return item.get() == child;}); + const auto index = static_cast(distance(childNodes.cbegin(), it)); + assert(index >= 0); + // we are walking up to the parent, so the path will be backwards + path.insert(path.begin(), index); + child = parent; + parent = parent->m_parent; + } + + return winrt::make<::IndexPath>(path); +} + // For a genuine tree view, we dont know which node is leaf until we // actually walk to it, so currently the tree builds up to the leaf. I don't // create a bunch of leaf node instances - instead i use the same instance m_leafNode to avoid @@ -105,7 +126,7 @@ std::shared_ptr SelectionNode::GetAt(int index, bool realizeChild auto childData = m_dataSource.get().GetAt(index); if (childData != nullptr) { - auto resolvedChild = m_manager->ResolvePath(childData); + auto resolvedChild = m_manager->ResolvePath(childData, weak_from_this()); if (resolvedChild != nullptr) { child = std::make_shared(m_manager, this /* parent */); diff --git a/dev/Repeater/SelectionNode.h b/dev/Repeater/SelectionNode.h index 309f9b76f9..b9f1c95cbf 100644 --- a/dev/Repeater/SelectionNode.h +++ b/dev/Repeater/SelectionNode.h @@ -17,7 +17,7 @@ enum SelectionState // a nested scenario. This would map to one ItemsSourceView/Collection. This node reacts // to collection changes and keeps the selected indices up to date. // This can either be a leaf node or a non leaf node. -class SelectionNode final +class SelectionNode final: public std::enable_shared_from_this { public: SelectionNode(SelectionModel* manager, SelectionNode* parent); @@ -48,6 +48,7 @@ class SelectionNode final bool SelectRange(const IndexRange& range, bool select); SelectionState EvaluateIsSelectedBasedOnChildrenNodes(); static winrt::IReference ConvertToNullableBool(SelectionState isSelected); + winrt::IndexPath IndexPath(); private: void HookupCollectionChangedHandler(); diff --git a/dev/Repeater/ViewManager.cpp b/dev/Repeater/ViewManager.cpp index 4be98a35e4..1edac71fc5 100644 --- a/dev/Repeater/ViewManager.cpp +++ b/dev/Repeater/ViewManager.cpp @@ -118,6 +118,19 @@ void ViewManager::ClearElementToElementFactory(const winrt::UIElement& element) context.Element(nullptr); context.Parent(nullptr); } + else + { + // No ItemTemplate to recycle to, remove the element from the children collection. + auto children = m_owner->Children(); + unsigned int childIndex = 0; + bool found = children.IndexOf(element, childIndex); + if (!found) + { + throw winrt::hresult_error(E_FAIL, L"ItemsRepeater's child not found in its Children collection."); + } + + children.RemoveAt(childIndex); + } auto virtInfo = ItemsRepeater::GetVirtualizationInfo(element); virtInfo->MoveOwnershipToElementFactory();