-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: configure tasks from open task quick pick #13367
fix: configure tasks from open task quick pick #13367
Conversation
@@ -182,6 +182,7 @@ export class QuickOpenTask implements QuickAccessProvider { | |||
picker.matchOnDescription = true; | |||
picker.ignoreFocusOut = false; | |||
picker.items = this.items; | |||
picker.onDidTriggerItemButton(({ item }) => this.onDidTriggerGearIcon(item as QuickPickItem)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd that we have to case here, but not below. Any idea why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two methods have different signatures 😄
Here the item can be a separator as well, even if it won't happen (separators won't have buttons that will trigger this method). However, to make this nicer I updated the code and added a check here, instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the question: is it true that one time the button item can be a separator as well or is that a mistake in the typing of "picker.onDidTriggerItemButton()"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tsmaeder! No, the item passed to the button cannot be a separator. The separator only has a label and no buttons. I restricted the type in the interface for this method. Ideally, we would have a dedicated type for a separator picker, that does not have this method at all, but I think that refactoring is out of scope for this PR.
ffa8af6
to
db5f7e0
Compare
This comment was marked as spam.
This comment was marked as spam.
The gear icon in the task quick pick should open the task configuration. Fixes eclipse-theia#13086 Contributed on behalf of STMicroelectronics Signed-off-by: Alexandra Buzila <[email protected]>
db5f7e0
to
9b48eb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:lgtm:
What it does
The gear icon in the task quick pick should open the task configuration.
Fixes #13086
Contributed on behalf of STMicroelectronics
How to test
tasks.json
configuration file should openOr:
npm
tasks.json
configuration file should openReview checklist
Reminder for reviewers