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
microsoft · May 6, 2020 · 6ab6d30 · 6ab6d30
1 parent a34eaf2
commit 6ab6d30
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 4 deletions.
diff --git 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
@@ -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");