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

monaco-quick-input-service: set context key "inQuickOpen" #12377

Closed
wants to merge 2 commits into from

Conversation

marcdumais-work
Copy link
Contributor

What it does

Context Key ""inQuickOpen"" exists in vscode and its value (true or false) reflects whether the Quick Open, AKA Quick Input, is currently open/visible or not.

This context key can be useful in Theia too, e.g. it can be used in the "when" clause of commands registered by vscode extensions, or set programmatically whenever it's useful to know the state of the Quick Open UI.

How to test

I have provided a test in the browser suite. Confirm that CI passes.

Review checklist

Reminder for reviewers

Context Key "inQuickOpen" exists in vscode and its value (true or false) reflects
whether the Quick Open, AKA Quick input, is currently open/visible or not.

This context key can be useful in Theia too, e.g. it can be used in the "when"
clause of commands registered by vscode extensions, or programmatically whenever
it's useful to know the state of the Quick Open UI.

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work marcdumais-work added monaco issues related to monaco vscode issues related to VSCode compatibility labels Apr 4, 2023
Comment on lines 103 to 107
setContextKey(key: string | undefined): void {
if (key) {
this.contextKeyService.createKey<string>(key, undefined);
this.contextKey = this.contextKeyService.createKey<string>(key, undefined);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I believe this method is meant for callers, and it may be problematic for us to set the contextKey variable especially since we later rely on it for the inQuickOpen.

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 180 to 186
private initController(): void {
this.controller = new QuickInputController(this.getOptions());
this.setContextKey('inQuickOpen');
this.onShow(() => { this.contextKey.set(true); });
this.onHide(() => { this.contextKey.set(false); });
this.updateLayout();
}
Copy link
Member

Choose a reason for hiding this comment

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

In addition to the previous comment, I think we can simplify and use the following which is present in other areas of the framework, where we register listeners in the postConstruct:

+ protected inQuickOpen: ContextKey<boolean>;

 @postConstruct()
 protected init(): void {
     this.initContainer();
     this.initController();
 
+    this.inQuickOpen = this.contextKeyService.createKey<boolean>('inQuickOpen', false);
+    this.onShow(() => { this.inQuickOpen.set(true); });
+    this.onHide(() => { this.inQuickOpen.set(false); })

@marcdumais-work
Copy link
Contributor Author

Closing in favor of #12427 - thanks @FernandoAscencio !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
monaco issues related to monaco vscode issues related to VSCode compatibility
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants