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

DialogService: three butons and a checkbox #80403

Closed
isidorn opened this issue Sep 5, 2019 · 6 comments · Fixed by #80703
Closed

DialogService: three butons and a checkbox #80403

isidorn opened this issue Sep 5, 2019 · 6 comments · Fixed by #80703
Assignees
Labels
dialogs Issues with native and custom dialogs feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded

Comments

@isidorn
Copy link
Contributor

isidorn commented Sep 5, 2019

Currently the dialogService only allows me to have a checkbox if I do a confirm. Due to that I can only have two buttons and a checkbox.
It is not possible to have multiple buttons and a checkbox. I would expect this to be possible if the confirm would accept multiple buttons, or if the show would support the checkbox.

fyi @bpasero

@isidorn isidorn added the feature-request Request for new features or functionality label Sep 5, 2019
@bpasero
Copy link
Member

bpasero commented Sep 5, 2019

Caution though, the show signature currently does not give any indication if the checkbox was clicked by the user or not, so it would need to provide this info.

@bpasero bpasero added the dialogs Issues with native and custom dialogs label Sep 5, 2019
@isidorn
Copy link
Contributor Author

isidorn commented Sep 5, 2019

@bpasero agree fully. We would need to change the return value

Also fyi I need this for #68084
@sbatten If you do not have time to add this, let me know if you are ok with me adding this APi to the dialogService.

@sbatten
Copy link
Member

sbatten commented Sep 5, 2019

I'm okay with this change. As for being careful, when updating references, the progress service will need special attention to make sure we don't break reconnection

@isidorn isidorn added this to the September 2019 milestone Sep 6, 2019
@isidorn isidorn self-assigned this Sep 6, 2019
@isidorn
Copy link
Contributor Author

isidorn commented Sep 10, 2019

@sbatten what do you prefer that I do:

  1. change the confirm API to allow multiple buttons
  2. change the show to allow checkboxes and to return a richer result?

@sbatten
Copy link
Member

sbatten commented Sep 10, 2019

@isidorn my opinion is that show is intended for a more complete experience and confirm is for fairly boolean response. I think that show can be expanded to fit that.

@isidorn
Copy link
Contributor Author

isidorn commented Sep 19, 2019

Will be verified as part of #68084, thus adding verified label.

@isidorn isidorn added the verified Verification succeeded label Sep 19, 2019
@isidorn isidorn added the verification-needed Verification of issue is requested label Sep 30, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dialogs Issues with native and custom dialogs feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants