Skip to content

Commit

Permalink
Fix issue with selection being raised when selection did not change
Browse files Browse the repository at this point in the history
  • Loading branch information
@Kinnara
Kinnara committed Apr 12, 2020
1 parent b9907ca commit 4d07d6f
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 26 deletions.
93 changes: 67 additions & 26 deletions ModernWpf.Controls/Repeater/SelectionModel/SelectionModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,18 +548,21 @@ void OnSelectionChanged()

void SelectImpl(int index, bool select)
{
if (m_singleSelect)
if (m_rootNode.IsSelected(index) != select)
{
ClearSelection(true /*resetAnchor*/, false /* raiseSelectionChanged */);
}
if (m_singleSelect)
{
ClearSelection(true /*resetAnchor*/, false /* raiseSelectionChanged */);
}

var selected = m_rootNode.Select(index, select);
if (selected)
{
AnchorIndex = new IndexPath(index);
}
var selected = m_rootNode.Select(index, select);
if (selected)
{
AnchorIndex = new IndexPath(index);
}

OnSelectionChanged();
OnSelectionChanged();
}
}

void SelectWithGroupImpl(int groupIndex, int itemIndex, bool select)
Expand All @@ -581,34 +584,72 @@ void SelectWithGroupImpl(int groupIndex, int itemIndex, bool select)

void SelectWithPathImpl(IndexPath index, bool select, bool raiseSelectionChanged)
{
bool selected = false;
bool newSelection = true;

// Handle single select differently as comparing indexpaths is faster
if (m_singleSelect)
{
ClearSelection(true /*restAnchor*/, false /* raiseSelectionChanged */);
var selectedIndex = SelectedIndex;
if (selectedIndex != null)
{
// If paths are equal and we want to select, skip everything and do nothing
if (select && selectedIndex.CompareTo(index) == 0)
{
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;
}
}

SelectionTreeHelper.TraverseIndexPath(
m_rootNode,
index,
true, /* realizeChildren */
// Selection is actually different from previous one, so update.
if (newSelection)
{
bool selected = false;
// If we unselect something, raise event any way, otherwise changedSelection is false
bool changedSelection = false;

(currentNode, path, depth, childIndex) =>
if (m_singleSelect)
{
if (depth == path.GetSize() - 1)
ClearSelection(true /*resetAnchor*/, false /* raiseSelectionChanged */);
}

SelectionTreeHelper.TraverseIndexPath(
m_rootNode,
index,
true, /* realizeChildren */
(currentNode, path, depth, childIndex) =>
{
selected = currentNode.Select(childIndex, select);
if (depth == path.GetSize() - 1)
{
if (currentNode.IsSelected(childIndex) != select)
{
// Node has different value then we want to set, so lets update!
changedSelection = true;
}
selected = currentNode.Select(childIndex, select);
}
}
);

if (selected)
{
AnchorIndex = index;
}
);

if (selected)
{
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 = null;
m_selectedItemsCached = null;

if (raiseSelectionChanged)
{
OnSelectionChanged();
if (raiseSelectionChanged && changedSelection)
{
OnSelectionChanged();
}
}
}

Expand Down
112 changes: 112 additions & 0 deletions test/ModernWpfTestApp/ApiTests/RepeaterTests/SelectionModelTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,118 @@ public void SelectRangeRegressionTest()
});
}

[TestMethod]
public void AlreadySelectedDoesNotRaiseEvent()
{
var testName = "Select(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.Select(0);
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.Select(0);
selectionModel = new SelectionModel() {
Source = list,
SingleSelect = true
};
// Single select indexpath
testName = "SelectAt(IndexPath index), single select";
selectionModel.SelectAt(IndexPath.CreateFrom(1));
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.SelectAt(IndexPath.CreateFrom(1));
// multi select index
selectionModel = new SelectionModel() {
Source = list
};
selectionModel.Select(1);
selectionModel.Select(2);
testName = "Select(int32 index), multiselect";
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.Select(1);
selectionModel.Select(2);
selectionModel = new SelectionModel() {
Source = list
};
// multi select indexpath
selectionModel.SelectAt(IndexPath.CreateFrom(1));
selectionModel.SelectAt(IndexPath.CreateFrom(2));
testName = "SelectAt(IndexPath index), multiselect";
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.SelectAt(IndexPath.CreateFrom(1));
selectionModel.SelectAt(IndexPath.CreateFrom(2));
});

void ThrowIfRaisedSelectionChanged(SelectionModel sender, SelectionModelSelectionChangedEventArgs args)
{
throw new Exception("SelectionChangedEvent was raised, but shouldn't have been raised as selection did not change. Tested method: " + testName);
}
}

[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 += ThrowIfRaisedSelectionChanged;
selectionModel.Deselect(0);
selectionModel = new SelectionModel() {
Source = list,
SingleSelect = true
};
// Single select indexpath
testName = "DeselectAt(IndexPath index), single select";
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.DeselectAt(IndexPath.CreateFrom(1));
// multi select index
selectionModel = new SelectionModel() {
Source = list
};
testName = "Deselect(int32 index), multiselect";
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.Deselect(1);
selectionModel.Deselect(2);
selectionModel = new SelectionModel() {
Source = list
};
// multi select indexpath
testName = "DeselectAt(IndexPath index), multiselect";
selectionModel.SelectionChanged += ThrowIfRaisedSelectionChanged;
selectionModel.DeselectAt(IndexPath.CreateFrom(1));
selectionModel.DeselectAt(IndexPath.CreateFrom(2));
});

void ThrowIfRaisedSelectionChanged(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);
Expand Down

0 comments on commit 4d07d6f

Please sign in to comment.