From 2aa71dc9e0a57dff529fa3c08d3ba7b9989593c0 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Thu, 29 Jun 2023 09:52:25 -0700 Subject: [PATCH 1/5] fix #186352 --- .../accessibility/browser/accessibleView.ts | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index bbe750e99af..3b937a38a1b 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -5,7 +5,7 @@ import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { KeyCode } from 'vs/base/common/keyCodes'; -import { Disposable, toDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { IEditorConstructionOptions } from 'vs/editor/browser/config/editorConfiguration'; import { EditorExtensionsRegistry } from 'vs/editor/browser/editorExtensions'; @@ -92,10 +92,15 @@ class AccessibleView extends Disposable { } show(provider: IAccessibleContentProvider): void { + let view: IDisposable | undefined; const delegate: IContextViewDelegate = { getAnchor: () => this._editorContainer, render: (container) => { - return this._render(provider, container); + view = this._render(provider, container); + return view; + }, + onHide: () => { + view?.dispose(); } }; this._contextViewService.showContextView(delegate); @@ -121,28 +126,34 @@ class AccessibleView extends Disposable { model.setLanguage(provider.options.language); } container.appendChild(this._editorContainer); - this._keyListener = this._register(this._editorWidget.onKeyUp((e) => { - if (e.keyCode === KeyCode.Escape) { - this._contextViewService.hideContextView(); - // Delay to allow the context view to hide #186514 - setTimeout(() => provider.onClose(), 100); - this._keyListener?.dispose(); - } else if (e.keyCode === KeyCode.KeyD && this._configurationService.getValue(settingKey)) { - this._configurationService.updateValue(settingKey, false); - } else if (e.keyCode === KeyCode.KeyH && provider.options.readMoreUrl) { - const url: string = provider.options.readMoreUrl!; - alert(AccessibilityHelpNLS.openingDocs); - this._openerService.open(URI.parse(url)); - } - e.stopPropagation(); - provider.onKeyDown?.(e); - })); - this._register(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView())); - this._register(this._editorWidget.onDidContentSizeChange(() => this._layout())); this._editorWidget.updateOptions({ ariaLabel: provider.options.ariaLabel }); this._editorWidget.focus(); }); - return toDisposable(() => { }); + const disposableStore = new DisposableStore(); + disposableStore.add(this._editorWidget.onKeyUp((e) => { + if (e.keyCode === KeyCode.Escape) { + this._contextViewService.hideContextView(); + // Delay to allow the context view to hide #186514 + setTimeout(() => provider.onClose(), 100); + this._keyListener?.dispose(); + } else if (e.keyCode === KeyCode.KeyD && this._configurationService.getValue(settingKey)) { + this._configurationService.updateValue(settingKey, false); + } + e.stopPropagation(); + provider.onKeyDown?.(e); + })); + disposableStore.add(this._editorWidget.onKeyDown((e) => { + if (e.keyCode === KeyCode.KeyH && provider.options.readMoreUrl) { + const url: string = provider.options.readMoreUrl!; + alert(AccessibilityHelpNLS.openingDocs); + this._openerService.open(URI.parse(url)); + e.preventDefault(); + e.stopPropagation(); + } + })); + disposableStore.add(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView())); + disposableStore.add(this._editorWidget.onDidContentSizeChange(() => this._layout())); + return toDisposable(() => { disposableStore.dispose(); }); } private _layout(): void { From 72f94506d4624736129e30438f4d660bc6f80738 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Thu, 29 Jun 2023 10:09:36 -0700 Subject: [PATCH 2/5] Update src/vs/workbench/contrib/accessibility/browser/accessibleView.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> --- .../workbench/contrib/accessibility/browser/accessibleView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index c89ba5fda39..22ab3000dd0 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -163,7 +163,7 @@ class AccessibleView extends Disposable { })); disposableStore.add(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView())); disposableStore.add(this._editorWidget.onDidContentSizeChange(() => this._layout())); - return toDisposable(() => { disposableStore.dispose(); }); + return disposableStore; } private _layout(): void { From cfd8c45a8e58087b32d4ed0a41ce4a688c54b638 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Thu, 29 Jun 2023 10:11:23 -0700 Subject: [PATCH 3/5] don't explicitly dispose of view --- .../contrib/accessibility/browser/accessibleView.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index 22ab3000dd0..6f878bd8a81 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -96,18 +96,15 @@ class AccessibleView extends Disposable { } show(provider: IAccessibleContentProvider): void { - let view: IDisposable | undefined; const delegate: IContextViewDelegate = { getAnchor: () => this._editorContainer, render: (container) => { - view = this._render(provider, container); - return view; + return this._render(provider, container); }, onHide: () => { if (provider.options.type === AccessibleViewType.HelpMenu) { this._accessiblityHelpIsShown.reset(); } - view?.dispose(); } }; this._contextViewService.showContextView(delegate); @@ -163,7 +160,7 @@ class AccessibleView extends Disposable { })); disposableStore.add(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView())); disposableStore.add(this._editorWidget.onDidContentSizeChange(() => this._layout())); - return disposableStore; + return toDisposable(() => { disposableStore.dispose(); }); } private _layout(): void { From bfc73b6cfef9ba323f0ef520411ac718ab01000e Mon Sep 17 00:00:00 2001 From: meganrogge Date: Thu, 29 Jun 2023 10:18:08 -0700 Subject: [PATCH 4/5] rm unused var --- .../workbench/contrib/accessibility/browser/accessibleView.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index 6f878bd8a81..f10bfcb2d8d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -63,7 +63,6 @@ class AccessibleView extends Disposable { private _accessiblityHelpIsShown: IContextKey; get editorWidget() { return this._editorWidget; } private _editorContainer: HTMLElement; - private _keyListener: IDisposable | undefined; constructor( @IOpenerService private readonly _openerService: IOpenerService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @@ -142,7 +141,6 @@ class AccessibleView extends Disposable { this._contextViewService.hideContextView(); // Delay to allow the context view to hide #186514 setTimeout(() => provider.onClose(), 100); - this._keyListener?.dispose(); } else if (e.keyCode === KeyCode.KeyD && this._configurationService.getValue(settingKey)) { this._configurationService.updateValue(settingKey, false); } From 1e4479db85bd2f8d7876f8c6852e08f31ebaedfd Mon Sep 17 00:00:00 2001 From: meganrogge Date: Thu, 29 Jun 2023 11:11:43 -0700 Subject: [PATCH 5/5] redo change --- .../workbench/contrib/accessibility/browser/accessibleView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index f10bfcb2d8d..a84bf2907c4 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -158,7 +158,7 @@ class AccessibleView extends Disposable { })); disposableStore.add(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView())); disposableStore.add(this._editorWidget.onDidContentSizeChange(() => this._layout())); - return toDisposable(() => { disposableStore.dispose(); }); + return disposableStore; } private _layout(): void {