From ce6ea8af41453b0c835e9a7ac400021708363c85 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Thu, 1 Apr 2021 14:52:55 -0700 Subject: [PATCH] Fix dropdown error & editor database dropdown validation (#14946) * Fix dropdown error & editor database dropdown validation * Set initial values * Update comment * hygiene * remove unused * Fix tests --- .../editableDropdown/browser/dropdown.ts | 31 ++++------ .../editableDropdown/browser/dropdownList.ts | 4 +- .../contrib/query/browser/queryActions.ts | 59 +++++++++++-------- .../query/test/browser/queryActions.test.ts | 1 + 4 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/sql/base/parts/editableDropdown/browser/dropdown.ts b/src/sql/base/parts/editableDropdown/browser/dropdown.ts index dd85538183..a4902126ab 100644 --- a/src/sql/base/parts/editableDropdown/browser/dropdown.ts +++ b/src/sql/base/parts/editableDropdown/browser/dropdown.ts @@ -13,7 +13,6 @@ import { IMessage, MessageType } from 'vs/base/browser/ui/inputbox/inputBox'; import { IListVirtualDelegate } from 'vs/base/browser/ui/list/list'; import { IListStyles, List } from 'vs/base/browser/ui/list/listWidget'; import { Color } from 'vs/base/common/color'; -import { onUnexpectedError } from 'vs/base/common/errors'; import { Emitter, Event } from 'vs/base/common/event'; import { KeyCode } from 'vs/base/common/keyCodes'; import { Disposable } from 'vs/base/common/lifecycle'; @@ -221,7 +220,7 @@ export class Dropdown extends Disposable implements IListVirtualDelegate })); this._input.onDidChange(e => { - if (this._dataSource.values?.length > 0) { + if (this._dataSource.values.length > 0) { this._dataSource.filter = e; if (this._isDropDownVisible) { this._updateDropDownList(); @@ -297,25 +296,21 @@ export class Dropdown extends Disposable implements IListVirtualDelegate } private _updateDropDownList(): void { - try { - this._selectList.splice(0, this._selectList.length, this._dataSource.filteredValues.map(v => { return { text: v }; })); - } catch (e) { - onUnexpectedError(e); - } + this._selectList.splice(0, this._selectList.length, this._dataSource.filteredValues.map(v => { return { text: v }; })); let width = this._inputContainer.clientWidth; - if (this._dataSource && this._dataSource.filteredValues) { - const longestOption = this._dataSource.filteredValues.reduce((previous, current) => { - return previous.length > current.length ? previous : current; - }, ''); - this._widthControlElement.innerText = longestOption; - const inputContainerWidth = DOM.getContentWidth(this._inputContainer); - const longestOptionWidth = DOM.getTotalWidth(this._widthControlElement); - width = clamp(longestOptionWidth, inputContainerWidth, 500); - } + // Find the longest option in the list and set our width to that (max 500px) + const longestOption = this._dataSource.filteredValues.reduce((previous, current) => { + return previous.length > current.length ? previous : current; + }, ''); + this._widthControlElement.innerText = longestOption; - const height = Math.min((this._dataSource.filteredValues?.length ?? 0) * this.getHeight(), this._options.maxHeight ?? 500); + const inputContainerWidth = DOM.getContentWidth(this._inputContainer); + const longestOptionWidth = DOM.getTotalWidth(this._widthControlElement); + width = clamp(longestOptionWidth, inputContainerWidth, 500); + + const height = Math.min(this._dataSource.filteredValues.length * this.getHeight(), this._options.maxHeight ?? 500); this._selectListContainer.style.width = `${width}px`; this._selectListContainer.style.height = `${height}px`; this._selectList.layout(height, width); @@ -361,7 +356,7 @@ export class Dropdown extends Disposable implements IListVirtualDelegate } private _inputValidator(value: string): IMessage | null { - if (!this._input.hasFocus() && !this._selectList.isDOMFocused() && this._dataSource.values && !this._dataSource.values.some(i => i === value)) { + if (!this._input.hasFocus() && this._input.isEnabled() && !this._selectList.isDOMFocused() && !this._dataSource.values.some(i => i === value)) { if (this._options.strictSelection && this._options.errorMessage) { return { content: this._options.errorMessage, diff --git a/src/sql/base/parts/editableDropdown/browser/dropdownList.ts b/src/sql/base/parts/editableDropdown/browser/dropdownList.ts index 5d6d231aac..734326a7ee 100644 --- a/src/sql/base/parts/editableDropdown/browser/dropdownList.ts +++ b/src/sql/base/parts/editableDropdown/browser/dropdownList.ts @@ -42,9 +42,9 @@ export class DropdownListRenderer implements IListRenderer { + this._dropdown.values = databaseNames; + }).catch(onUnexpectedError); + } + + /** + * Fetches the list of database names from the current editor connection + * @returns The list of database names + */ + private async getDatabaseNames(): Promise { if (!this._editor.input) { this.logService.error('editor input was null'); - return; + return []; } let uri = this._editor.input.uri; if (!uri) { - return; + return []; } - - this.connectionManagementService.listDatabases(uri) - .then(result => { - if (result && result.databaseNames) { - this._dropdown.values = result.databaseNames; - } - }); + try { + const result = await this.connectionManagementService.listDatabases(uri); + return result.databaseNames; + } catch (err) { + this.logService.error(`Error loading database names for query editor `, err); + } + return []; } - private updateConnection(databaseName: string) { + private updateConnection(databaseName: string): void { this._isConnected = true; this._currentDatabaseName = databaseName; if (this._isInAccessibilityMode) { - this._databaseSelectBox.enable(); - if (!this._editor.input) { - this.logService.error('editor input was null'); - return; - } - let uri = this._editor.input.uri; - if (!uri) { - return; - } - this.connectionManagementService.listDatabases(uri) - .then(result => { - if (result && result.databaseNames) { - this._databaseSelectBox.setOptions(result.databaseNames); - } + this.getDatabaseNames() + .then(databaseNames => { + this._databaseSelectBox.setOptions(databaseNames); this._databaseSelectBox.selectWithOptionName(databaseName); - }); + }).catch(onUnexpectedError); } else { - this._dropdown.enabled = true; + // Set the value immediately to the initial database so the user can see that, and then + // populate the list with just that value to avoid displaying an error while we load + // the full list of databases this._dropdown.value = databaseName; + this._dropdown.values = [databaseName]; + this._dropdown.enabled = true; + this.getDatabaseNames().then(databaseNames => { + this._dropdown.values = databaseNames; + }).catch(onUnexpectedError); } } diff --git a/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts b/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts index b3b4e042be..df9ee76202 100644 --- a/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts +++ b/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts @@ -68,6 +68,7 @@ suite('SQL QueryAction Tests', () => { queryModelService.setup(q => q.onRunQueryComplete).returns(() => Event.None); connectionManagementService = TypeMoq.Mock.ofType(TestConnectionManagementService); connectionManagementService.setup(q => q.onDisconnect).returns(() => Event.None); + connectionManagementService.setup(q => q.listDatabases(TypeMoq.It.isAny())).returns(() => Promise.resolve({ databaseNames: ['master', 'msdb', 'model'] })); const workbenchinstantiationService = workbenchInstantiationService(); const accessor = workbenchinstantiationService.createInstance(ServiceAccessor); const service = accessor.untitledTextEditorService;