Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with selection being raised when selection did not change #2253

Merged
merged 7 commits into from
Apr 10, 2020
Merged

Fix issue with selection being raised when selection did not change #2253

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d39cbe2
Fix issue with selection being raised when selection did not change
marcelwgn Apr 8, 2020
cab3181
Update comments
marcelwgn Apr 8, 2020
7d1cbdc
Add handling for multiselect events being raised despite no change
marcelwgn Apr 8, 2020
4c0e9f3
CR refactors
marcelwgn Apr 8, 2020
24afb0f
Fix failing tests
marcelwgn Apr 8, 2020
6b1e4c7
Also handle deselects
marcelwgn Apr 9, 2020
02037f7
CR feedback
marcelwgn Apr 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions dev/Repeater/SelectionModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.

#include <pch.h>
Expand Down Expand Up @@ -585,7 +585,7 @@ void SelectionModel::OnSelectionChanged()

void SelectionModel::SelectImpl(int index, bool select)
{
if (!m_rootNode->IsSelected(index))
if (m_rootNode->IsSelected(index) != select)
{
if (m_singleSelect)
{
Expand Down Expand Up @@ -627,8 +627,8 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
{
if (auto const selectedIndex = SelectedIndex())
{
// If paths are equal, skip everything and do nothing
if (selectedIndex.CompareTo(index) == 0)
// If paths are equal and we want to select, skip everything and do nothing
if (selectedIndex.CompareTo(index) == 0 && select)
{
newSelection = false;
}
Expand All @@ -638,7 +638,8 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
// Selection is actually different from previous one, so update.
if (newSelection)
{
bool changedSelection = false;
// If we unselect something, raise event any way, otherwise changedSelection is false
bool changedSelection = !select;

if (m_singleSelect)
{
Expand All @@ -655,7 +656,7 @@ void SelectionModel::SelectWithPathImpl(const winrt::IndexPath& index, bool sele
{
if (!currentNode->IsSelected(childIndex))
{
// Node is not already selected, so we need to raise event
// Node has different value then we want to set, so lets update!
changedSelection = true;
}
selected = currentNode->Select(childIndex, select);
Expand Down