Skip to content

fix #85938: prevent duplicate selections#86286

Merged
isidorn merged 1 commit intomicrosoft:masterfrom
jzyrobert:85938
Dec 4, 2019
Merged

fix #85938: prevent duplicate selections#86286
isidorn merged 1 commit intomicrosoft:masterfrom
jzyrobert:85938

Conversation

@jzyrobert
Copy link
Contributor

This PR fixes #85938 (and #85880 ?)
@isidorn

When creating a new empty folder, upon focusing on it, duplicate selections can occur where selection already contains the object, but oldSelection has the id and will push it again.

Will this being a set cause any issues if the objects are not in the same order as before?

@isidorn isidorn self-assigned this Dec 4, 2019
@isidorn
Copy link
Collaborator

isidorn commented Dec 4, 2019

Looks great! Can you just use an array and indexOf instead of a Set.
That sounds safer to me, so we preserve the order. Thanks

Copy link
Collaborator

@isidorn isidorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented in the issue, do not use set but keep working with arrays.

@jzyrobert
Copy link
Contributor Author

@isidorn done

@isidorn
Copy link
Collaborator

isidorn commented Dec 4, 2019

Looks great. Just remove the usage of distinct from the abstractTree since it is no longer needed and now I get a compile error.

@isidorn isidorn added this to the November 2019 milestone Dec 4, 2019
@isidorn isidorn added the file-explorer Explorer widget issues label Dec 4, 2019
@isidorn
Copy link
Collaborator

isidorn commented Dec 4, 2019

Tested it out and it is great work. Thanks a lot, I will remove the unused import.
🍻

@isidorn isidorn merged commit 7f04538 into microsoft:master Dec 4, 2019
@jzyrobert
Copy link
Contributor Author

@isidorn Thanks, I should have spotted the unused import

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

file-explorer Explorer widget issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Second delete folder operation does not work in Insiders 1.41.0

2 participants