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.

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

OE clicking and awaiting the expansion calls #6761

Merged
merged 4 commits into from
Aug 21, 2019

Conversation

aaomidi
Copy link
Contributor

@aaomidi aaomidi commented Aug 15, 2019

Before this we call the expand/refresh/expandAll codepoints without awaiting their response. This was causing random issues and race conditions with our code depending on how fast you clicked.

@aaomidi aaomidi requested a review from anthonydresser August 15, 2019 10:04
}
this._lastClicked = specificSelection;
this._lastClicked = selection;
}

this._clickTimer = setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make more sense to just move the setTiemout to the if mouse event since for other selection types (keyboard) we don't want to use this timeout method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm yeah. Do we want to treat a keyboard selection as a full double click?

}
this._lastClicked = specificSelection;
this._lastClicked = selection;
}

this._clickTimer = setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

vscode also has a class for wrapping timeouts, https://github.com/microsoft/azuredatastudio/blob/master/src/vs/base/common/async.ts#L496 if you want to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oo this looks good!

@aaomidi aaomidi merged commit b731c56 into master Aug 21, 2019
@aaomidi aaomidi deleted the fix/oe_expansion_selection branch August 22, 2019 03:22
aaomidi added a commit that referenced this pull request Aug 22, 2019
kevcunnane pushed a commit that referenced this pull request Aug 22, 2019
aaomidi added a commit that referenced this pull request Aug 22, 2019
aaomidi added a commit that referenced this pull request Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants