Skip to content

Commit 5385ebc

Browse files
authored
Merge pull request microsoft#186650 from microsoft/merogge/help-dispose
use disposable store for accessible view listeners, prevent repeated handling
2 parents bb5c4d5 + 1e4479d commit 5385ebc

File tree

1 file changed

+25
-21
lines changed

1 file changed

+25
-21
lines changed

src/vs/workbench/contrib/accessibility/browser/accessibleView.ts

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
77
import { KeyCode } from 'vs/base/common/keyCodes';
8-
import { Disposable, toDisposable } from 'vs/base/common/lifecycle';
8+
import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
99
import { URI } from 'vs/base/common/uri';
1010
import { IEditorConstructionOptions } from 'vs/editor/browser/config/editorConfiguration';
1111
import { EditorExtensionsRegistry } from 'vs/editor/browser/editorExtensions';
@@ -63,7 +63,6 @@ class AccessibleView extends Disposable {
6363
private _accessiblityHelpIsShown: IContextKey<boolean>;
6464
get editorWidget() { return this._editorWidget; }
6565
private _editorContainer: HTMLElement;
66-
private _keyListener: IDisposable | undefined;
6766
constructor(
6867
@IOpenerService private readonly _openerService: IOpenerService,
6968
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@@ -133,28 +132,33 @@ class AccessibleView extends Disposable {
133132
model.setLanguage(provider.options.language);
134133
}
135134
container.appendChild(this._editorContainer);
136-
this._keyListener = this._register(this._editorWidget.onKeyUp((e) => {
137-
if (e.keyCode === KeyCode.Escape) {
138-
this._contextViewService.hideContextView();
139-
// Delay to allow the context view to hide #186514
140-
setTimeout(() => provider.onClose(), 100);
141-
this._keyListener?.dispose();
142-
} else if (e.keyCode === KeyCode.KeyD && this._configurationService.getValue(settingKey)) {
143-
this._configurationService.updateValue(settingKey, false);
144-
} else if (e.keyCode === KeyCode.KeyH && provider.options.readMoreUrl) {
145-
const url: string = provider.options.readMoreUrl!;
146-
alert(AccessibilityHelpNLS.openingDocs);
147-
this._openerService.open(URI.parse(url));
148-
}
149-
e.stopPropagation();
150-
provider.onKeyDown?.(e);
151-
}));
152-
this._register(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView()));
153-
this._register(this._editorWidget.onDidContentSizeChange(() => this._layout()));
154135
this._editorWidget.updateOptions({ ariaLabel: provider.options.ariaLabel });
155136
this._editorWidget.focus();
156137
});
157-
return toDisposable(() => { });
138+
const disposableStore = new DisposableStore();
139+
disposableStore.add(this._editorWidget.onKeyUp((e) => {
140+
if (e.keyCode === KeyCode.Escape) {
141+
this._contextViewService.hideContextView();
142+
// Delay to allow the context view to hide #186514
143+
setTimeout(() => provider.onClose(), 100);
144+
} else if (e.keyCode === KeyCode.KeyD && this._configurationService.getValue(settingKey)) {
145+
this._configurationService.updateValue(settingKey, false);
146+
}
147+
e.stopPropagation();
148+
provider.onKeyDown?.(e);
149+
}));
150+
disposableStore.add(this._editorWidget.onKeyDown((e) => {
151+
if (e.keyCode === KeyCode.KeyH && provider.options.readMoreUrl) {
152+
const url: string = provider.options.readMoreUrl!;
153+
alert(AccessibilityHelpNLS.openingDocs);
154+
this._openerService.open(URI.parse(url));
155+
e.preventDefault();
156+
e.stopPropagation();
157+
}
158+
}));
159+
disposableStore.add(this._editorWidget.onDidBlurEditorText(() => this._contextViewService.hideContextView()));
160+
disposableStore.add(this._editorWidget.onDidContentSizeChange(() => this._layout()));
161+
return disposableStore;
158162
}
159163

160164
private _layout(): void {

0 commit comments

Comments
 (0)