diff --git a/src/sql/workbench/browser/modal/modal.ts b/src/sql/workbench/browser/modal/modal.ts index 4bac8afa71..0c1c2eb888 100644 --- a/src/sql/workbench/browser/modal/modal.ts +++ b/src/sql/workbench/browser/modal/modal.ts @@ -7,7 +7,7 @@ import { attachButtonStyler } from 'vs/platform/theme/common/styler'; import { Color } from 'vs/base/common/color'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { mixin } from 'vs/base/common/objects'; -import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import * as DOM from 'vs/base/browser/dom'; import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { generateUuid } from 'vs/base/common/uuid'; @@ -83,6 +83,7 @@ export abstract class Modal extends Disposable implements IThemable { protected _useDefaultMessageBoxLocation: boolean = true; protected _messageElement: HTMLElement; protected _modalOptions: IModalOptions; + protected readonly disposableStore = this._register(new DisposableStore()); private _detailsButtonContainer: HTMLElement; private _messageIcon: HTMLElement; private _messageSeverity: HTMLElement; @@ -117,10 +118,6 @@ export abstract class Modal extends Disposable implements IThemable { private _leftFooter: HTMLElement; private _rightFooter: HTMLElement; private _footerButtons: Button[]; - - private _keydownListener: IDisposable; - private _resizeListener: IDisposable; - private _backButton: Button; private _modalShowingContext: IContextKey>; @@ -359,7 +356,7 @@ export abstract class Modal extends Disposable implements IThemable { DOM.append(this.layoutService.container, this._bodyContainer); this.setInitialFocusedElement(); - this._keydownListener = DOM.addDisposableListener(document, DOM.EventType.KEY_DOWN, (e: KeyboardEvent) => { + this.disposableStore.add(DOM.addDisposableListener(document, DOM.EventType.KEY_DOWN, (e: KeyboardEvent) => { let context = this._modalShowingContext.get()!; if (context[context.length - 1] === this._staticKey) { let event = new StandardKeyboardEvent(e); @@ -373,10 +370,10 @@ export abstract class Modal extends Disposable implements IThemable { this.handleForwardTab(e); } } - }); - this._resizeListener = DOM.addDisposableListener(window, DOM.EventType.RESIZE, (e: Event) => { + })); + this.disposableStore.add(DOM.addDisposableListener(window, DOM.EventType.RESIZE, (e: Event) => { this.layout(DOM.getTotalHeight(this._modalBodySection)); - }); + })); this.layout(DOM.getTotalHeight(this._modalBodySection)); this._telemetryService.createActionEvent(TelemetryKeys.TelemetryView.Shell, TelemetryKeys.ModalDialogOpened) @@ -395,8 +392,7 @@ export abstract class Modal extends Disposable implements IThemable { protected hide() { this._modalShowingContext.get()!.pop(); this._bodyContainer.remove(); - this._keydownListener.dispose(); - this._resizeListener.dispose(); + this.disposableStore.clear(); this._telemetryService.createActionEvent(TelemetryKeys.TelemetryView.Shell, TelemetryKeys.ModalDialogClosed) .withAdditionalProperties({ name: this._name }) .send(); @@ -416,7 +412,7 @@ export abstract class Modal extends Disposable implements IThemable { */ protected addFooterButton(label: string, onSelect: () => void, orientation: 'left' | 'right' = 'right'): Button { let footerButton = DOM.$('.footer-button'); - let button = new Button(footerButton); + let button = this._register(new Button(footerButton)); button.label = label; button.onDidClick(() => onSelect()); // @todo this should be registered to dispose but that brakes some dialogs if (orientation === 'left') { @@ -618,7 +614,6 @@ export abstract class Modal extends Disposable implements IThemable { public dispose() { super.dispose(); - this._keydownListener.dispose(); this._footerButtons = []; } } diff --git a/src/sql/workbench/browser/modal/optionsDialog.ts b/src/sql/workbench/browser/modal/optionsDialog.ts index 7fba5d8b89..a6d6da95c3 100644 --- a/src/sql/workbench/browser/modal/optionsDialog.ts +++ b/src/sql/workbench/browser/modal/optionsDialog.ts @@ -163,7 +163,8 @@ export class OptionsDialog extends Modal { for (let i = 0; i < options.length; i++) { let option: azdata.ServiceOption = options[i]; let rowContainer = DialogHelper.appendRow(container, option.displayName, 'optionsDialog-label', 'optionsDialog-input'); - OptionsDialogHelper.createOptionElement(option, rowContainer, this._optionValues, this._optionElements, this._contextViewService, (name) => this.onOptionLinkClicked(name)); + const optionElement = OptionsDialogHelper.createOptionElement(option, rowContainer, this._optionValues, this._optionElements, this._contextViewService, (name) => this.onOptionLinkClicked(name)); + this.disposableStore.add(optionElement.optionWidget); } } @@ -175,12 +176,12 @@ export class OptionsDialog extends Modal { switch (option.valueType) { case ServiceOptionType.category: case ServiceOptionType.boolean: - this._register(styler.attachSelectBoxStyler(widget, this._themeService)); + this.disposableStore.add(styler.attachSelectBoxStyler(widget, this._themeService)); break; case ServiceOptionType.string: case ServiceOptionType.password: case ServiceOptionType.number: - this._register(styler.attachInputBoxStyler(widget, this._themeService)); + this.disposableStore.add(styler.attachInputBoxStyler(widget, this._themeService)); } } } @@ -224,8 +225,8 @@ export class OptionsDialog extends Modal { } public close() { - this.dispose(); this.hide(); + this._optionElements = {}; this._onCloseEvent.fire(); } @@ -260,10 +261,6 @@ export class OptionsDialog extends Modal { public dispose(): void { super.dispose(); - for (let optionName in this._optionElements) { - let widget: Widget = this._optionElements[optionName].optionWidget; - widget.dispose(); - delete this._optionElements[optionName]; - } + this._optionElements = {}; } } diff --git a/src/sql/workbench/browser/modal/optionsDialogHelper.ts b/src/sql/workbench/browser/modal/optionsDialogHelper.ts index fd75d5bd15..e30fcfdc4f 100644 --- a/src/sql/workbench/browser/modal/optionsDialogHelper.ts +++ b/src/sql/workbench/browser/modal/optionsDialogHelper.ts @@ -21,7 +21,7 @@ export interface IOptionElement { } export function createOptionElement(option: azdata.ServiceOption, rowContainer: HTMLElement, options: { [name: string]: any }, - optionsMap: { [optionName: string]: IOptionElement }, contextViewService: IContextViewService, onFocus: (name) => void): void { + optionsMap: { [optionName: string]: IOptionElement }, contextViewService: IContextViewService, onFocus: (name) => void): IOptionElement { let possibleInputs: string[] = []; let optionValue = getOptionValueAndCategoryValues(option, options, possibleInputs); let optionWidget: any; @@ -63,8 +63,10 @@ export function createOptionElement(option: azdata.ServiceOption, rowContainer: } inputElement = findElement(rowContainer, 'input'); } - optionsMap[option.name] = { optionWidget: optionWidget, option: option, optionValue: optionValue }; + const optionElement = { optionWidget: optionWidget, option: option, optionValue: optionValue }; + optionsMap[option.name] = optionElement; inputElement.onfocus = () => onFocus(option.name); + return optionElement; } export function getOptionValueAndCategoryValues(option: azdata.ServiceOption, options: { [optionName: string]: any }, possibleInputs: string[]): any {