From 24e8c20511880e00a67b27449f5f8a93bb7bd8dd Mon Sep 17 00:00:00 2001 From: Matt Irvine Date: Fri, 27 Apr 2018 16:29:18 -0700 Subject: [PATCH] Simplify button logic and enable button updates for custom dialogs (#1283) --- .../platform/dialog/customDialogService.ts | 16 +++++-- src/sql/platform/dialog/dialogModal.ts | 20 ++++++--- src/sql/platform/dialog/dialogPane.ts | 5 +++ src/sql/platform/dialog/dialogTypes.ts | 45 ++++++++++++++----- src/sql/platform/dialog/media/dialogModal.css | 2 +- src/sql/sqlops.proposed.d.ts | 43 ++++++++---------- .../workbench/api/common/sqlExtHostTypes.ts | 1 + .../api/node/extHostModelViewDialog.ts | 32 ++++++------- .../api/node/mainThreadModelViewDialog.ts | 9 ++-- .../workbench/api/node/sqlExtHost.api.impl.ts | 6 +++ .../api/mainThreadModelViewDialog.test.ts | 12 +++-- 11 files changed, 119 insertions(+), 72 deletions(-) diff --git a/src/sql/platform/dialog/customDialogService.ts b/src/sql/platform/dialog/customDialogService.ts index a44a3ec02f..5146e1df80 100644 --- a/src/sql/platform/dialog/customDialogService.ts +++ b/src/sql/platform/dialog/customDialogService.ts @@ -15,11 +15,21 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti const defaultOptions: IModalOptions = { hasBackButton: true, isWide: false }; export class CustomDialogService { + private _dialogModals = new Map(); + constructor( @IInstantiationService private _instantiationService: IInstantiationService) { } public showDialog(dialog: Dialog, options?: IModalOptions): void { - let optionsDialog = this._instantiationService.createInstance(DialogModal, dialog, 'CustomDialog', options || defaultOptions); - optionsDialog.render(); - optionsDialog.open(); + let dialogModal = this._instantiationService.createInstance(DialogModal, dialog, 'CustomDialog', options || defaultOptions); + this._dialogModals.set(dialog, dialogModal); + dialogModal.render(); + dialogModal.open(); + } + + public closeDialog(dialog: Dialog): void { + let dialogModal = this._dialogModals.get(dialog); + if (dialogModal) { + dialogModal.cancel(); + } } } diff --git a/src/sql/platform/dialog/dialogModal.ts b/src/sql/platform/dialog/dialogModal.ts index 98d9514608..379ae71f73 100644 --- a/src/sql/platform/dialog/dialogModal.ts +++ b/src/sql/platform/dialog/dialogModal.ts @@ -61,25 +61,33 @@ export class DialogModal extends Modal { if (this._dialog.customButtons) { this._dialog.customButtons.forEach(button => { let buttonElement = this.addDialogButton(button); - buttonElement.enabled = button.enabled; - attachButtonStyler(buttonElement, this._themeService); + this.updateButtonElement(buttonElement, button); }); } this._cancelButton = this.addDialogButton(this._dialog.cancelButton, () => this.cancel()); - this._cancelButton.enabled = this._dialog.cancelButton.enabled; + this.updateButtonElement(this._cancelButton, this._dialog.cancelButton); this._doneButton = this.addDialogButton(this._dialog.okButton, () => this.done()); - this._doneButton.enabled = this._dialog.okButton.enabled; - attachButtonStyler(this._cancelButton, this._themeService); - attachButtonStyler(this._doneButton, this._themeService); + this.updateButtonElement(this._doneButton, this._dialog.okButton); } private addDialogButton(button: DialogButton, onSelect: () => void = () => undefined): Button { let buttonElement = this.addFooterButton(button.label, onSelect); + buttonElement.enabled = button.enabled; button.registerClickEvent(buttonElement.onDidClick); + button.onUpdate(() => { + this.updateButtonElement(buttonElement, button); + }); + attachButtonStyler(buttonElement, this._themeService); return buttonElement; } + private updateButtonElement(buttonElement: Button, dialogButton: DialogButton) { + buttonElement.label = dialogButton.label; + buttonElement.enabled = dialogButton.enabled; + dialogButton.hidden ? buttonElement.element.classList.add('dialogModal-hidden') : buttonElement.element.classList.remove('dialogModal-hidden'); + } + protected renderBody(container: HTMLElement): void { new Builder(container).div({ class: 'dialogModal-body' }, (bodyBuilder) => { this._body = bodyBuilder.getHTMLElement(); diff --git a/src/sql/platform/dialog/dialogPane.ts b/src/sql/platform/dialog/dialogPane.ts index c8aec8b9e2..33689a22e7 100644 --- a/src/sql/platform/dialog/dialogPane.ts +++ b/src/sql/platform/dialog/dialogPane.ts @@ -92,4 +92,9 @@ export class DialogPane extends Disposable implements IThemable { this._body.style.backgroundColor = styles.dialogBodyBackground ? styles.dialogBodyBackground.toString() : undefined; this._body.style.color = styles.dialogForeground ? styles.dialogForeground.toString() : undefined; } + + public dispose() { + super.dispose(); + this._moduleRef.destroy(); + } } diff --git a/src/sql/platform/dialog/dialogTypes.ts b/src/sql/platform/dialog/dialogTypes.ts index b256952ef8..f86d08fff3 100644 --- a/src/sql/platform/dialog/dialogTypes.ts +++ b/src/sql/platform/dialog/dialogTypes.ts @@ -17,8 +17,6 @@ export class DialogTab implements sqlops.window.modelviewdialog.DialogTab { this.content = content; } } - - public updateContent(): void { } } export class Dialog implements sqlops.window.modelviewdialog.Dialog { @@ -35,21 +33,48 @@ export class Dialog implements sqlops.window.modelviewdialog.Dialog { this.content = content; } } - - public open(): void { } - public close(): void { } - public updateContent(): void { } } export class DialogButton implements sqlops.window.modelviewdialog.Button { - public label: string; - public enabled: boolean; + private _label: string; + private _enabled: boolean; + private _hidden: boolean; private _onClick: Emitter = new Emitter(); public readonly onClick: Event = this._onClick.event; + private _onUpdate: Emitter = new Emitter(); + public readonly onUpdate: Event = this._onUpdate.event; constructor(label: string, enabled: boolean) { - this.label = label; - this.enabled = enabled; + this._label = label; + this._enabled = enabled; + this._hidden = false; + } + + public get label(): string { + return this._label; + } + + public set label(label: string) { + this._label = label; + this._onUpdate.fire(); + } + + public get enabled(): boolean { + return this._enabled; + } + + public set enabled(enabled: boolean) { + this._enabled = enabled; + this._onUpdate.fire(); + } + + public get hidden(): boolean { + return this._hidden; + } + + public set hidden(hidden: boolean) { + this._hidden = hidden; + this._onUpdate.fire(); } /** diff --git a/src/sql/platform/dialog/media/dialogModal.css b/src/sql/platform/dialog/media/dialogModal.css index 3fc5b41fed..e2572ae07d 100644 --- a/src/sql/platform/dialog/media/dialogModal.css +++ b/src/sql/platform/dialog/media/dialogModal.css @@ -23,6 +23,6 @@ height: 100%; } -.dialogModal-pane.dialogModal-hidden { +.dialogModal-hidden { display: none; } diff --git a/src/sql/sqlops.proposed.d.ts b/src/sql/sqlops.proposed.d.ts index 895ee4c6c4..435441465b 100644 --- a/src/sql/sqlops.proposed.d.ts +++ b/src/sql/sqlops.proposed.d.ts @@ -287,6 +287,16 @@ declare module 'sqlops' { */ export function createButton(label: string): Button; + /** + * Opens the given dialog if it is not already open + */ + export function openDialog(dialog: Dialog): void; + + /** + * Closes the given dialog if it is open + */ + export function closeDialog(dialog: Dialog): void; + // Model view dialog classes export interface Dialog { /** @@ -297,7 +307,6 @@ declare module 'sqlops' { /** * The content of the dialog. If multiple tabs are given they will be displayed with tabs * If a string is given, it should be the ID of the dialog's model view content - * TODO mairvine 4/18/18: use a model view content type */ content: string | DialogTab[], @@ -315,51 +324,35 @@ declare module 'sqlops' { * Any additional buttons that should be displayed */ customButtons: Button[]; - - /** - * Opens the dialog - */ - open(): void; - - /** - * Closes the dialog - */ - close(): void; - - /** - * Updates the dialog on screen to reflect changes to the buttons or content - */ - updateContent(): void; } export interface DialogTab { /** * The title of the tab */ - title: string, + title: string; /** * A string giving the ID of the tab's model view content - * TODO mairvine 4/18/18: use a model view content type */ content: string; - - /** - * Updates the dialog on screen to reflect changes to the content - */ - updateContent(): void; } export interface Button { /** * The label displayed on the button */ - label: string, + label: string; /** * Whether the button is enabled */ - enabled: boolean, + enabled: boolean; + + /** + * Whether the button is hidden + */ + hidden: boolean; /** * Raised when the button is clicked diff --git a/src/sql/workbench/api/common/sqlExtHostTypes.ts b/src/sql/workbench/api/common/sqlExtHostTypes.ts index 24f98ad7f1..05f8dd2a9d 100644 --- a/src/sql/workbench/api/common/sqlExtHostTypes.ts +++ b/src/sql/workbench/api/common/sqlExtHostTypes.ts @@ -115,4 +115,5 @@ export interface IModelViewTabDetails { export interface IModelViewButtonDetails { label: string; enabled: boolean; + hidden: boolean; } diff --git a/src/sql/workbench/api/node/extHostModelViewDialog.ts b/src/sql/workbench/api/node/extHostModelViewDialog.ts index 87e48b3c37..a019f4bc6c 100644 --- a/src/sql/workbench/api/node/extHostModelViewDialog.ts +++ b/src/sql/workbench/api/node/extHostModelViewDialog.ts @@ -26,18 +26,6 @@ class DialogImpl implements sqlops.window.modelviewdialog.Dialog { this.okButton = this._extHostModelViewDialog.createButton(nls.localize('dialogOkLabel', 'Done')); this.cancelButton = this._extHostModelViewDialog.createButton(nls.localize('dialogCancelLabel', 'Cancel')); } - - public open(): void { - this._extHostModelViewDialog.open(this); - } - - public close(): void { - this._extHostModelViewDialog.close(this); - } - - public updateContent(): void { - this._extHostModelViewDialog.updateDialogContent(this); - } } class TabImpl implements sqlops.window.modelviewdialog.DialogTab { @@ -45,21 +33,19 @@ class TabImpl implements sqlops.window.modelviewdialog.DialogTab { public content: string; constructor(private _extHostModelViewDialog: ExtHostModelViewDialog) { } - - public updateContent(): void { - this._extHostModelViewDialog.updateTabContent(this); - } } class ButtonImpl implements sqlops.window.modelviewdialog.Button { private _label: string; private _enabled: boolean; + private _hidden: boolean; private _onClick = new Emitter(); public onClick = this._onClick.event; constructor(private _extHostModelViewDialog: ExtHostModelViewDialog) { this._enabled = true; + this._hidden = false; } public get label(): string { @@ -80,6 +66,15 @@ class ButtonImpl implements sqlops.window.modelviewdialog.Button { this._extHostModelViewDialog.updateButton(this); } + public get hidden(): boolean { + return this._hidden; + } + + public set hidden(hidden: boolean) { + this._hidden = hidden; + this._extHostModelViewDialog.updateButton(this); + } + public getOnClickCallback(): () => void { return () => this._onClick.fire(); } @@ -154,7 +149,7 @@ export class ExtHostModelViewDialog implements ExtHostModelViewDialogShape { let handle = this.getDialogHandle(dialog); let tabs = dialog.content; if (tabs && typeof tabs !== 'string') { - tabs.forEach(tab => tab.updateContent()); + tabs.forEach(tab => this.updateTabContent(tab)); } if (dialog.customButtons) { dialog.customButtons.forEach(button => this.updateButton(button)); @@ -182,7 +177,8 @@ export class ExtHostModelViewDialog implements ExtHostModelViewDialogShape { let handle = this.getButtonHandle(button); this._proxy.$setButtonDetails(handle, { label: button.label, - enabled: button.enabled + enabled: button.enabled, + hidden: button.hidden }); } diff --git a/src/sql/workbench/api/node/mainThreadModelViewDialog.ts b/src/sql/workbench/api/node/mainThreadModelViewDialog.ts index 7d3c7eba45..6a198ebeb2 100644 --- a/src/sql/workbench/api/node/mainThreadModelViewDialog.ts +++ b/src/sql/workbench/api/node/mainThreadModelViewDialog.ts @@ -22,7 +22,7 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape constructor( context: IExtHostContext, - @IInstantiationService instatiationService: IInstantiationService + @IInstantiationService instatiationService: IInstantiationService, ) { this._proxy = context.getProxy(SqlExtHostContext.ExtHostModelViewDialog); this._dialogService = new CustomDialogService(instatiationService); @@ -40,7 +40,7 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape public $close(handle: number): Thenable { let dialog = this.getDialog(handle); - dialog.close(); + this._dialogService.closeDialog(dialog); return Promise.resolve(); } @@ -66,7 +66,6 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape dialog.customButtons = details.customButtons.map(buttonHandle => this.getButton(buttonHandle)); } - dialog.updateContent(); return Promise.resolve(); } @@ -79,8 +78,6 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape tab.title = details.title; tab.content = details.content; - - tab.updateContent(); return Promise.resolve(); } @@ -88,11 +85,13 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape let button = this._buttons.get(handle); if (!button) { button = new DialogButton(details.label, details.enabled); + button.hidden = details.hidden; button.onClick(() => this.onButtonClick(handle)); this._buttons.set(handle, button); } else { button.label = details.label; button.enabled = details.enabled; + button.hidden = details.hidden; } return Promise.resolve(); diff --git a/src/sql/workbench/api/node/sqlExtHost.api.impl.ts b/src/sql/workbench/api/node/sqlExtHost.api.impl.ts index 4f14c4bec9..7b15f1335b 100644 --- a/src/sql/workbench/api/node/sqlExtHost.api.impl.ts +++ b/src/sql/workbench/api/node/sqlExtHost.api.impl.ts @@ -293,6 +293,12 @@ export function createApiFactory( }, createButton(label: string): sqlops.window.modelviewdialog.Button { return extHostModelViewDialog.createButton(label); + }, + openDialog(dialog: sqlops.window.modelviewdialog.Dialog) { + return extHostModelViewDialog.open(dialog); + }, + closeDialog(dialog: sqlops.window.modelviewdialog.Dialog) { + return extHostModelViewDialog.close(dialog); } }; diff --git a/src/sqltest/workbench/api/mainThreadModelViewDialog.test.ts b/src/sqltest/workbench/api/mainThreadModelViewDialog.test.ts index 6d4128ab29..325f2317a6 100644 --- a/src/sqltest/workbench/api/mainThreadModelViewDialog.test.ts +++ b/src/sqltest/workbench/api/mainThreadModelViewDialog.test.ts @@ -55,19 +55,23 @@ suite('MainThreadModelViewDialog Tests', () => { // Set up the dialog details button1Details = { label: 'button1', - enabled: false + enabled: false, + hidden: false }; button2Details = { label: 'button2', - enabled: true + enabled: true, + hidden: false }; okButtonDetails = { label: 'ok_label', - enabled: true + enabled: true, + hidden: false }; cancelButtonDetails = { label: 'cancel_label', - enabled: true + enabled: true, + hidden: false }; tab1Details = { title: 'tab1',