Skip to content

fix #85938: prevent duplicate selections - #86286

Merged
Isidor Nikolic (isidorn) merged 1 commit into
microsoft:masterfrom
jzyrobert:85938
Dec 4, 2019
Merged

fix #85938: prevent duplicate selections#86286
Isidor Nikolic (isidorn) merged 1 commit into
microsoft:masterfrom
jzyrobert:85938

Conversation

@jzyrobert

Copy link
Copy Markdown
Contributor

This PR fixes #85938 (and #85880 ?)
Isidor Nikolic (@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

Copy link
Copy Markdown

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

@isidorn Isidor Nikolic (isidorn) left a comment

Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

Isidor Nikolic (@isidorn) done

@isidorn

Copy link
Copy Markdown

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 Isidor Nikolic (isidorn) added this to the November 2019 milestone Dec 4, 2019
@isidorn Isidor Nikolic (isidorn) added the file-explorer Explorer widget issues label Dec 4, 2019
@isidorn

Isidor Nikolic (isidorn) commented Dec 4, 2019

Copy link
Copy Markdown

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

@isidorn
Isidor Nikolic (isidorn) merged commit 7f04538 into microsoft:master Dec 4, 2019
@jzyrobert

Copy link
Copy Markdown
Contributor Author

Isidor Nikolic (@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