From 6ab6d3065fce22f21d22240f0af808a1c8c16081 Mon Sep 17 00:00:00 2001 From: Felix-Dev Date: Wed, 6 May 2020 21:56:32 +0200 Subject: [PATCH] SelectionModel: Fix for missing SelectionChanged event raise (#2359) * Fix for missing SelectionChanged event raise * Add API test * Update some comments * Address test feedback * corrected unintended check deletion * add check fail message to improve clarity * change test name --- dev/Repeater/APITests/SelectionModelTests.cs | 74 ++++++++++++++++++++ dev/Repeater/SelectionModel.cpp | 10 +-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/dev/Repeater/APITests/SelectionModelTests.cs b/dev/Repeater/APITests/SelectionModelTests.cs index 5b86b8f31d..53933f5756 100644 --- a/dev/Repeater/APITests/SelectionModelTests.cs +++ b/dev/Repeater/APITests/SelectionModelTests.cs @@ -1134,6 +1134,80 @@ void ThrowIfRaisedSelectionChanged(SelectionModel sender, SelectionModelSelectio } } + [TestMethod] + public void ValidateSelectionModeChangeFromMultipleToSingle() + { + RunOnUIThread.Execute(() => + { + SelectionModel selectionModel = new SelectionModel(); + selectionModel.Source = Enumerable.Range(0, 10).ToList(); + + // First test: switching from multiple to single selection mode with just one selected item + selectionModel.Select(4); + + selectionModel.SingleSelect = true; + + // Verify that the item at index 4 is still selected + Verify.IsTrue(selectionModel.SelectedIndex.CompareTo(Path(4)) == 0, "Item at index 4 should have still been selected"); + + // Second test: this time switching from multiple to single selection mode with more than one selected item + selectionModel.SingleSelect = false; + selectionModel.Select(5); + selectionModel.Select(6); + + // Now switch to single selection mode + selectionModel.SingleSelect = true; + + // Verify that + // - only one item is currently selected + // - the currently selected item is the item with the lowest index in the Multiple selection list + Verify.AreEqual(1, selectionModel.SelectedIndices.Count, + "Exactly one item should have been selected now after we switched from Multiple to Single selection mode"); + Verify.IsTrue(selectionModel.SelectedIndices[0].CompareTo(selectionModel.SelectedIndex) == 0, + "SelectedIndex and SelectedIndices should have been identical"); + Verify.IsTrue(selectionModel.SelectedIndex.CompareTo(Path(4)) == 0, "The currently selected item should have been the first item in the Multiple selection list"); + }); + } + + [TestMethod] + public void ValidateSelectionModeChangeFromMultipleToSingleSelectionChangedEvent() + { + RunOnUIThread.Execute(() => + { + SelectionModel selectionModel = new SelectionModel(); + selectionModel.Source = Enumerable.Range(0, 10).ToList(); + + // First test: switching from multiple to single selection mode with just one selected item + selectionModel.Select(4); + + int selectionChangedFiredCount = 0; + selectionModel.SelectionChanged += IncreaseCountIfRaisedSelectionChanged; + + // Now switch to single selection mode + selectionModel.SingleSelect = true; + + // Verify that no SelectionChanged event was raised + Verify.AreEqual(0, selectionChangedFiredCount, "SelectionChanged event should have not been raised as only one item was selected"); + + // Second test: this time switching from multiple to single selection mode with more than one selected item + selectionModel.SelectionChanged -= IncreaseCountIfRaisedSelectionChanged; + selectionModel.SingleSelect = false; + selectionModel.Select(5); + selectionModel.SelectionChanged += IncreaseCountIfRaisedSelectionChanged; + + // Now switch to single selection mode + selectionModel.SingleSelect = true; + + // Verify that the SelectionChanged event was raised + Verify.AreEqual(1, selectionChangedFiredCount, "SelectionChanged event should have been raised as the selection changed"); + + void IncreaseCountIfRaisedSelectionChanged(SelectionModel sender, SelectionModelSelectionChangedEventArgs args) + { + selectionChangedFiredCount++; + } + }); + } + private void Select(SelectionModel manager, int index, bool select) { Log.Comment((select ? "Selecting " : "DeSelecting ") + index); diff --git a/dev/Repeater/SelectionModel.cpp b/dev/Repeater/SelectionModel.cpp index 9caeefb3ac..33d155b6d0 100644 --- a/dev/Repeater/SelectionModel.cpp +++ b/dev/Repeater/SelectionModel.cpp @@ -60,15 +60,17 @@ void SelectionModel::SingleSelect(bool value) { m_singleSelect = value; auto selectedIndices = SelectedIndices(); - if (value && selectedIndices && selectedIndices.Size() > 0) + + // Only update selection and raise SelectionChanged event when: + // - we switch from SelectionMode::Multiple to SelectionMode::Single and + // - more than one item was selected at the time of the switch + if (value && selectedIndices && selectedIndices.Size() > 1) { // We want to be single select, so make sure there is only // one selected item. auto firstSelectionIndexPath = selectedIndices.GetAt(0); ClearSelection(true /* resetAnchor */, false /*raiseSelectionChanged */); - SelectWithPathImpl(firstSelectionIndexPath, true /* select */, false /* raiseSelectionChanged */); - // Setting SelectedIndex will raise SelectionChanged event. - SelectedIndex(firstSelectionIndexPath); + SelectWithPathImpl(firstSelectionIndexPath, true /* select */, true /* raiseSelectionChanged */); } RaisePropertyChanged(L"SingleSelect");