Also handle deselects

Author: marcelwgn

dev/Repeater/APITests/SelectionModelTests.cs

@@ -1039,6 +1039,59 @@ void SelectionModel_SelectionChanged(SelectionModel sender, SelectionModelSelect
             }
         }
 
+        [TestMethod]
+        public void AlreadyDeselectedDoesNotRaiseEvent()
+        {
+            var testName = "Deselect(int32 index), single select";
+
+            RunOnUIThread.Execute(() =>
+            {
+                var list = Enumerable.Range(0, 10).ToList();
+
+                var selectionModel = new SelectionModel() {
+                    Source = list,
+                    SingleSelect = true
+                };
+
+                // Single select index
+                selectionModel.SelectionChanged += SelectionModel_SelectionChanged;
+                selectionModel.Deselect(0);
+
+                selectionModel = new SelectionModel() {
+                    Source = list,
+                    SingleSelect = true
+                };
+                // Single select indexpath
+                testName = "DeselectAt(IndexPath index), single select";
+                selectionModel.SelectionChanged += SelectionModel_SelectionChanged;
+                selectionModel.DeselectAt(IndexPath.CreateFrom(1));
+
+                // multi select index
+                selectionModel = new SelectionModel() {
+                    Source = list
+                };
+                testName = "Deselect(int32 index), multiselect";
+                selectionModel.SelectionChanged += SelectionModel_SelectionChanged;
+                selectionModel.Deselect(1);
+                selectionModel.Deselect(2);
+
+                selectionModel = new SelectionModel() {
+                    Source = list
+                };
+
+                // multi select indexpath
+                testName = "DeselectAt(IndexPath index), multiselect";
+                selectionModel.SelectionChanged += SelectionModel_SelectionChanged;
+                selectionModel.DeselectAt(IndexPath.CreateFrom(1));
+                selectionModel.DeselectAt(IndexPath.CreateFrom(2));
+            });
+
+            void SelectionModel_SelectionChanged(SelectionModel sender, SelectionModelSelectionChangedEventArgs args)
+            {
+                throw new Exception("SelectionChangedEvent was raised, but shouldn't have been raised as selection did not change. Tested method: " + testName);
+            }
+        }
+
         private void Select(SelectionModel manager, int index, bool select)
         {
             Log.Comment((select ? "Selecting " : "DeSelecting ") + index);

dev/Repeater/SelectionModel.cpp

@@ -633,13 +633,19 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
                 newSelection = false;
             }
         }
+        else
+        {
+            // If we are in single select and selectedIndex is null, deselecting is not a new change.
+            // Selecting something is a new change, so set flag to appropriate value here.
+            newSelection = select;
+        }
     }
 
     // Selection is actually different from previous one, so update.
     if (newSelection)
     {
         // If we unselect something, raise event any way, otherwise changedSelection is false
-        bool changedSelection = !select;
+        bool changedSelection = false;
 
         if (m_singleSelect)
         {
@@ -654,7 +660,7 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
             {
                 if (depth == path.GetSize() - 1)
                 {
-                    if (!currentNode->IsSelected(childIndex))
+                    if (currentNode->IsSelected(childIndex) != select)
                     {
                         // Node has different value then we want to set, so lets update!
                         changedSelection = true;
@@ -669,6 +675,11 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
             AnchorIndex(index);
         }
 
+        // The walk tree operation can change the indices, and the next time it get's read,
+        // we would throw an exception. That's what we are preventing with next two lines
+        m_selectedIndicesCached = nullptr;
+        m_selectedItemsCached = nullptr;
+
         if (raiseSelectionChanged && changedSelection)
         {
             OnSelectionChanged();