-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Display commands when necessary #9986
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Display `Commands` related to `Interactive Window` and `Notebooks` only when necessary. |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,8 @@ import { | |
INotebookServerOptions, | ||
InterruptResult, | ||
IStatusProvider, | ||
IThemeFinder | ||
IThemeFinder, | ||
WebViewViewChangeEventArgs | ||
} from '../types'; | ||
import { WebViewHost } from '../webViewHost'; | ||
import { InteractiveWindowMessageListener } from './interactiveWindowMessageListener'; | ||
|
@@ -389,7 +390,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
}); | ||
} | ||
|
||
protected onViewStateChanged(_visible: boolean, active: boolean) { | ||
protected onViewStateChanged(args: WebViewViewChangeEventArgs) { | ||
// Only activate if the active editor is empty. This means that | ||
// vscode thinks we are actually supposed to have focus. It would be | ||
// nice if they would more accurrately tell us this, but this works for now. | ||
|
@@ -398,7 +399,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
// it's been activated. However if there's no active text editor and we're active, we | ||
// can safely attempt to give ourselves focus. This won't actually give us focus if we aren't | ||
// allowed to have it. | ||
if (active && !this.viewState.active) { | ||
if (args.current.active && !args.previous.active) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed this so that the state is update immediately. |
||
this.activating().ignoreErrors(); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,7 +56,8 @@ import { | |
INotebookImporter, | ||
INotebookServerOptions, | ||
IStatusProvider, | ||
IThemeFinder | ||
IThemeFinder, | ||
WebViewViewChangeEventArgs | ||
} from '../types'; | ||
|
||
// tslint:disable-next-line:no-require-imports no-var-requires | ||
|
@@ -495,12 +496,12 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { | |
} | ||
} | ||
|
||
protected async onViewStateChanged(visible: boolean, active: boolean) { | ||
super.onViewStateChanged(visible, active); | ||
protected async onViewStateChanged(args: WebViewViewChangeEventArgs) { | ||
super.onViewStateChanged(args); | ||
|
||
// Update our contexts | ||
const interactiveContext = new ContextKey(EditorContexts.HaveNative, this.commandManager); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
should probably rename this. It's not the 'interactiveContext', but the 'nativeContext' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because CI sucks big time, I'll do it in the next PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in another PR that I'll be submitting shortly |
||
interactiveContext.set(visible && active).catch(); | ||
interactiveContext.set(args.current.visible && args.current.active).catch(); | ||
this._onDidChangeViewState.fire(); | ||
} | ||
|
||
|
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.
Added some
or
commands as its not possible to haveor
conditions in the commands.