From 6b549696c5c239faed16bcccfb596c214ec43c85 Mon Sep 17 00:00:00 2001 From: Abbie Petchtes Date: Mon, 14 May 2018 11:25:13 -0700 Subject: [PATCH] Bug/keyboard issue in manange linked account (#1400) * add message and add account button when the linked account is empty * fix account dialog tests * address comment --- .../accountDialog/accountDialog.ts | 109 +++++++++++++++--- .../accountDialog/media/accountDialog.css | 62 ++++++---- src/sql/platform/views/fixedListView.ts | 7 ++ .../accountDialogController.test.ts | 1 + 4 files changed, 139 insertions(+), 40 deletions(-) diff --git a/src/sql/parts/accountManagement/accountDialog/accountDialog.ts b/src/sql/parts/accountManagement/accountDialog/accountDialog.ts index 2a2af12832..e78988b01d 100644 --- a/src/sql/parts/accountManagement/accountDialog/accountDialog.ts +++ b/src/sql/parts/accountManagement/accountDialog/accountDialog.ts @@ -34,20 +34,28 @@ import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 's import { FixedListView } from 'sql/platform/views/fixedListView'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +export interface IProviderViewUiComponent { + view: FixedListView; + addAccountAction: AddAccountAction; +} + export class AccountDialog extends Modal { public static ACCOUNTLIST_HEIGHT = 77; public viewModel: AccountViewModel; // MEMBER VARIABLES //////////////////////////////////////////////////// - private _providerViews: { [providerId: string]: FixedListView } = {}; + private _providerViews: { [providerId: string]: IProviderViewUiComponent } = {}; private _closeButton: Button; + private _addAccountButton: Button; private _delegate: AccountListDelegate; private _accountRenderer: AccountListRenderer; private _actionRunner: ActionRunner; private _splitView: SplitView; private _container: HTMLElement; + private _splitViewContainer: HTMLElement; + private _noaccountViewContainer: HTMLElement; // EVENTING //////////////////////////////////////////////////////////// private _onAddAccountErrorEmitter: Emitter; @@ -89,6 +97,14 @@ export class AccountDialog extends Modal { this.viewModel.addProviderEvent(arg => { self.addProvider(arg); }); this.viewModel.removeProviderEvent(arg => { self.removeProvider(arg); }); this.viewModel.updateAccountListEvent(arg => { self.updateProviderAccounts(arg); }); + + // Load the initial contents of the view model + this.viewModel.initialize() + .then(addedProviders => { + for (let addedProvider of addedProviders) { + self.addProvider(addedProvider); + } + }); } // MODAL OVERRIDE METHODS ////////////////////////////////////////////// @@ -104,26 +120,35 @@ export class AccountDialog extends Modal { attachModalDialogStyler(this, this._themeService); this._closeButton = this.addFooterButton(localize('accountDialog.close', 'Close'), () => this.close()); this.registerListeners(); - - // Load the initial contents of the view model - this.viewModel.initialize() - .then(addedProviders => { - for (let addedProvider of addedProviders) { - self.addProvider(addedProvider); - } - }); } protected renderBody(container: HTMLElement) { this._container = container; - let viewBody = DOM.$('div.account-view'); - DOM.append(container, viewBody); - this._splitView = new SplitView(viewBody); + this._splitViewContainer = DOM.$('div.account-view'); + DOM.append(container, this._splitViewContainer); + this._splitView = new SplitView(this._splitViewContainer); + + this._noaccountViewContainer = DOM.$('div.no-account-view'); + let noAccountTitle = DOM.append(this._noaccountViewContainer, DOM.$('.no-account-view-label')); + let noAccountLabel = localize('accountDialog.noAccountLabel', 'There is no linked account. Please add an acount.'); + noAccountTitle.innerHTML = noAccountLabel; + + // Show the add account button for the first provider + // Todo: If we have more than 1 provider, need to show all add account buttons for all providers + let buttonSection = DOM.append(this._noaccountViewContainer, DOM.$('div.button-section')); + this._addAccountButton = new Button(buttonSection); + this._addAccountButton.label = localize('accountDialog.addConnection', 'Add an account'); + this._register(this._addAccountButton.onDidClick(() => { + (Object.values(this._providerViews)[0]).addAccountAction.run(); + })); + + DOM.append(container, this._noaccountViewContainer); } private registerListeners(): void { // Theme styler this._register(attachButtonStyler(this._closeButton, this._themeService)); + this._register(attachButtonStyler(this._addAccountButton, this._themeService)); } /* Overwrite escape key behavior */ @@ -143,12 +168,48 @@ export class AccountDialog extends Modal { public open() { this.show(); + if (!this.isEmptyLinkedAccount()) { + this.showSplitView(); + } else { + this._splitViewContainer.hidden = true; + this._noaccountViewContainer.hidden = false; + this._addAccountButton.focus(); + } + + } + + private showSplitView() { + this._splitViewContainer.hidden = false; + this._noaccountViewContainer.hidden = true; + let views = this._splitView.getViews(); + if (views && views.length > 0) { + let firstView = views[0]; + if (firstView instanceof FixedListView) { + firstView.list.setSelection([0]); + firstView.list.domFocus(); + } + } + } + + private isEmptyLinkedAccount(): boolean { + for (var providerId in this._providerViews) { + var listView = this._providerViews[providerId].view; + if (listView && listView.list.length > 0) { + return false; + } + } + return true; } public dispose(): void { super.dispose(); for (let key in this._providerViews) { - this._providerViews[key].dispose(); + if (this._providerViews[key].addAccountAction) { + this._providerViews[key].addAccountAction.dispose(); + } + if (this._providerViews[key].view) { + this._providerViews[key].view.dispose(); + } delete this._providerViews[key]; } } @@ -199,21 +260,26 @@ export class AccountDialog extends Modal { // Set the initial items of the list providerView.updateList(newProvider.initialAccounts); + + if (newProvider.initialAccounts.length > 0 && this._splitViewContainer.hidden) { + this.showSplitView(); + } + this.layout(); - // Store the view for the provider - this._providerViews[newProvider.addedProvider.id] = providerView; + // Store the view for the provider and action + this._providerViews[newProvider.addedProvider.id] = { view: providerView, addAccountAction: addAccountAction }; } private removeProvider(removedProvider: sqlops.AccountProviderMetadata) { // Skip removing the provider if it doesn't exist let providerView = this._providerViews[removedProvider.id]; - if (!providerView) { + if (!providerView || !providerView.view) { return; } // Remove the list view from the split view - this._splitView.removeView(providerView); + this._splitView.removeView(providerView.view); this._splitView.layout(DOM.getContentHeight(this._container)); // Remove the list view from our internal map @@ -223,10 +289,15 @@ export class AccountDialog extends Modal { private updateProviderAccounts(args: UpdateAccountListEventParams) { let providerMapping = this._providerViews[args.providerId]; - if (!providerMapping) { + if (!providerMapping || !providerMapping.view) { return; } - providerMapping.updateList(args.accountList); + providerMapping.view.updateList(args.accountList); + + if (args.accountList.length > 0 && this._splitViewContainer.hidden) { + this.showSplitView(); + } + this.layout(); } } diff --git a/src/sql/parts/accountManagement/accountDialog/media/accountDialog.css b/src/sql/parts/accountManagement/accountDialog/media/accountDialog.css index 979b0ce262..2abe6f2c98 100644 --- a/src/sql/parts/accountManagement/accountDialog/media/accountDialog.css +++ b/src/sql/parts/accountManagement/accountDialog/media/accountDialog.css @@ -2,9 +2,49 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +.no-account-view { + font-size: 12px; + padding: 15px; +} -.account-view .monaco-split-view .split-view-view .header { +.no-account-view .no-account-view-label { + padding-bottom: 15px; +} + +.account-view .header { + position: relative; + line-height: 22px; + font-size: 11px; + font-weight: bold; + text-transform: uppercase; + padding-left: 20px; padding-right: 12px; + overflow: hidden; + display: flex; +} + +.account-view .header .title { + display: flex; + justify-content: flex-start; + flex: 1 1 auto; +} + +.account-view .header .actions { + display: flex; + justify-content: flex-end; +} + +.account-view .header .actions .action-item .action-label { + width: 30px; + background-size: 16px; + background-position: center center; + background-repeat: no-repeat; + margin-right: 0; + height: 22px; +} + +.account-view .header .count-badge-wrapper { + justify-content: flex-end; } .account-view .provider-view .list-row { @@ -58,24 +98,4 @@ .account-view .provider-view .monaco-list .monaco-list-row.selected .list-row .actions-container, .account-view .provider-view .monaco-list .monaco-list-row.focused .list-row .actions-container{ display: block; -} - -.account-view .split-view-view .header .title { - display: flex; - justify-content: flex-start; - flex: 1 1 auto; -} - -.account-view .split-view-view .header .actions { - display: flex; - justify-content: flex-end; -} - -.account-view .split-view-view .header .actions .action-item .action-label { - width: 30px; - background-size: 16px; - background-position: center center; - background-repeat: no-repeat; - margin-right: 0; - height: 22px; } \ No newline at end of file diff --git a/src/sql/platform/views/fixedListView.ts b/src/sql/platform/views/fixedListView.ts index b77601ab17..20c019c1dc 100644 --- a/src/sql/platform/views/fixedListView.ts +++ b/src/sql/platform/views/fixedListView.ts @@ -67,6 +67,10 @@ export class FixedListView extends CollapsibleView { this.setFixed(this.fixedSize); } + public get list(): List { + return this._list; + } + public listContentHeight(): number { return this._list.contentHeight; } @@ -86,6 +90,9 @@ export class FixedListView extends CollapsibleView { protected changeState(state: CollapsibleState): void { super.changeState(state); this.setFixed(this.fixedSize); + if (this.list) { + this.list.getHTMLElement().hidden = (state === CollapsibleState.COLLAPSED); + } } /** diff --git a/src/sqltest/parts/accountManagement/accountDialogController.test.ts b/src/sqltest/parts/accountManagement/accountDialogController.test.ts index 65c2c11b98..5ead263296 100644 --- a/src/sqltest/parts/accountManagement/accountDialogController.test.ts +++ b/src/sqltest/parts/accountManagement/accountDialogController.test.ts @@ -78,6 +78,7 @@ function createInstantiationService(addAccountFailureEmitter?: Emitter): mockAccountViewModel.setup(x => x.addProviderEvent).returns(() => mockEvent.event); mockAccountViewModel.setup(x => x.removeProviderEvent).returns(() => mockEvent.event); mockAccountViewModel.setup(x => x.updateAccountListEvent).returns(() => mockEvent.event); + mockAccountViewModel.setup(x => x.initialize()).returns(() => Promise.resolve([])); // Create a mocked out instantiation service let instantiationService = TypeMoq.Mock.ofType(InstantiationService, TypeMoq.MockBehavior.Strict);