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

Change to use async API for modal dialogs #39536

Closed
bpasero opened this issue Dec 4, 2017 · 0 comments · Fixed by #40211
Closed

Change to use async API for modal dialogs #39536

bpasero opened this issue Dec 4, 2017 · 0 comments · Fixed by #40211
Assignees
Labels
debt Code quality issues on-testplan workbench-electron Electron-VS Code issues
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Dec 4, 2017

Electron provides sync and async dialog API (file pickers and message boxes). Today we mainly use the sync variant of the API which means that the JavaScript thread is stopped while the dialog is showing. However, for the usages of the dialogs with checkbox, we must use the async version because otherwise the result of the checkbox is not returned.

We should consider switching to the async version of the API to have a consistent usage pattern. However, there are bugs and issues that make this hard:

In order to get this right imho we need to:

  • make all dialog API promise based
  • switch to the async version of all dialogs
  • have a central place on the main thread to handle the case of multiple dialogs opening at the same time. we must ensure that at any time only 1 dialog is active and make sure that other dialog open requests are buffered and processed one after the other

Improves: #17552
Fixes: #9262
Fixes: #39729

@bpasero bpasero added debt Code quality issues workbench-electron Electron-VS Code issues labels Dec 4, 2017
@bpasero bpasero added this to the On Deck milestone Dec 4, 2017
@bpasero bpasero self-assigned this Dec 4, 2017
@bpasero bpasero modified the milestones: On Deck, December 2017/January 2018 Dec 14, 2017
@bpasero bpasero changed the title Revisit sync vs async dialog usage Change to use async API for modal dialogs Dec 20, 2017
@bpasero bpasero mentioned this issue Dec 20, 2017
3 tasks
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues on-testplan workbench-electron Electron-VS Code issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant