Fix the select box issue (#9508)

* Fix the select box issue

* map => forEach

* Fix tests and a product issue

* Maybe another potential fix

* Fix a few more issues

* Change backendValue to value

* localization

* Use Array.isArray

extensions/mssql/package.json

@@ -687,11 +687,11 @@
           "objectType": null,
           "categoryValues": [
             {
-              "displayName": null,
+              "displayName": "%mssql.disabled%",
               "name": "Disabled"
             },
             {
-              "displayName": null,
+              "displayName": "%mssql.enabled%",
               "name": "Enabled"
             }
           ],

extensions/mssql/package.nls.json

@@ -36,6 +36,9 @@
 
 	"title.showLogFile": "Show Log File",
 
+	"mssql.disabled": "Disabled",
+	"mssql.enabled": "Enabled",
+
 	"mssql.configuration.title": "MSSQL configuration",
 	"mssql.query.displayBitAsNumber": "Should BIT columns be displayed as numbers (1 or 0)? If false, BIT columns will be displayed as 'true' or 'false'",
 	"mssql.query.maxXmlCharsToStore": "Number of XML characters to store after running a query",

src/sql/base/browser/ui/selectBox/selectBox.ts

@@ -20,6 +20,10 @@ import { SelectBoxList } from 'vs/base/browser/ui/selectBox/selectBoxCustom';
 const $ = dom.$;
 
 
+export interface SelectOptionItemSQL extends ISelectOptionItem {
+	value: string; // THIS IS REQUIRED, this is the value that will actually be returned on SelectBox#values()
+}
+
 export interface ISelectBoxStyles extends vsISelectBoxStyles {
 	disabledSelectBackground?: Color;
 	disabledSelectForeground?: Color;
@@ -36,7 +40,7 @@ export interface ISelectBoxStyles extends vsISelectBoxStyles {
 
 export class SelectBox extends vsSelectBox {
 	private _optionsDictionary: Map<string, number>;
-	private _dialogOptions: string[];
+	private _dialogOptions: SelectOptionItemSQL[];
 	private _selectedOption: string;
 	private _selectBoxOptions?: ISelectBoxOptions;
 	private enabledSelectBackground?: Color;
@@ -60,20 +64,30 @@ export class SelectBox extends vsSelectBox {
 
 	private element?: HTMLElement;
 
-	constructor(options: string[], selectedOption: string, contextViewProvider: IContextViewProvider, container?: HTMLElement, selectBoxOptions?: ISelectBoxOptions) {
+	constructor(options: SelectOptionItemSQL[] | string[], selectedOption: string, contextViewProvider: IContextViewProvider, container?: HTMLElement, selectBoxOptions?: ISelectBoxOptions) {
-		super(options.map(option => { return { text: option }; }), 0, contextViewProvider, undefined, selectBoxOptions);
+		let optionItems: SelectOptionItemSQL[];
+		if (Array.isArray<string>(options)) {
+			optionItems = (options as string[]).map(o => {
+				return { text: o, value: o } as SelectOptionItemSQL;
+			});
+		} else {
+			optionItems = options;
+		}
+
+		super(optionItems, 0, contextViewProvider, undefined, selectBoxOptions);
 		this._optionsDictionary = new Map<string, number>();
 		for (let i = 0; i < options.length; i++) {
-			this._optionsDictionary.set(options[i], i);
+			this._optionsDictionary.set(optionItems[i].value, i);
 		}
 		const option = this._optionsDictionary.get(selectedOption);
 		if (option) {
 			super.select(option);
 		}
 		this._selectedOption = selectedOption;
-		this._dialogOptions = options;
+		this._dialogOptions = optionItems;
 		this._register(this.onDidSelect(newInput => {
-			this._selectedOption = newInput.selected;
+			const selected = optionItems[newInput.index];
+			this._selectedOption = selected.value;
 		}));
 
 		this.enabledSelectBackground = this.selectBackground;
@@ -147,23 +161,23 @@ export class SelectBox extends vsSelectBox {
 	public select(index: number): void {
 		super.select(index);
 		if (this._dialogOptions !== undefined) {
-			this._selectedOption = this._dialogOptions[index];
+			this._selectedOption = this._dialogOptions[index]?.value;
 		}
 	}
 
 	public setOptions(options: string[] | ISelectOptionItem[], selected?: number): void {
-		let stringOptions: string[];
+		let selectOptions: SelectOptionItemSQL[];
 		if (options.length > 0 && typeof options[0] !== 'string') {
-			stringOptions = (options as ISelectOptionItem[]).map(option => option.text);
+			selectOptions = options as SelectOptionItemSQL[];
 		} else {
-			stringOptions = options as string[];
+			selectOptions = (options as string[]).map(o => { return { text: o, value: o } as SelectOptionItemSQL; });
 		}
 		this._optionsDictionary = new Map<string, number>();
-		for (let i = 0; i < stringOptions.length; i++) {
+		for (let i = 0; i < selectOptions.length; i++) {
-			this._optionsDictionary.set(stringOptions[i], i);
+			this._optionsDictionary.set(selectOptions[i].value, i);
 		}
-		this._dialogOptions = stringOptions;
+		this._dialogOptions = selectOptions;
-		super.setOptions(stringOptions.map(option => { return { text: option }; }), selected);
+		super.setOptions(selectOptions, selected);
 	}
 
 	public get value(): string {
@@ -171,7 +185,7 @@ export class SelectBox extends vsSelectBox {
 	}
 
 	public get values(): string[] {
-		return this._dialogOptions;
+		return this._dialogOptions.map(s => s.value);
 	}
 
 	public enable(): void {

src/sql/workbench/browser/modal/optionsDialogHelper.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as DialogHelper from './dialogHelper';
-import { SelectBox } from 'sql/base/browser/ui/selectBox/selectBox';
+import { SelectBox, SelectOptionItemSQL } from 'sql/base/browser/ui/selectBox/selectBox';
 import { MessageType } from 'vs/base/browser/ui/inputbox/inputBox';
 import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { InputBox } from 'sql/base/browser/ui/inputBox/inputBox';
@@ -22,7 +22,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): IOptionElement {
-	let possibleInputs: string[] = [];
+	let possibleInputs: SelectOptionItemSQL[] = [];
 	let optionValue = getOptionValueAndCategoryValues(option, options, possibleInputs);
 	let optionWidget: any;
 	let inputElement: HTMLElement;
@@ -69,7 +69,7 @@ export function createOptionElement(option: azdata.ServiceOption, rowContainer:
 	return optionElement;
 }
 
-export function getOptionValueAndCategoryValues(option: azdata.ServiceOption, options: { [optionName: string]: any }, possibleInputs: string[]): any {
+export function getOptionValueAndCategoryValues(option: azdata.ServiceOption, options: { [optionName: string]: any }, possibleInputs: SelectOptionItemSQL[]): any {
 	let optionValue = option.defaultValue;
 	if (options[option.name] !== undefined) {
 		// if the value type is boolean, the option value can be either boolean or string
@@ -87,18 +87,18 @@ export function getOptionValueAndCategoryValues(option: azdata.ServiceOption, op
 	if (option.valueType === ServiceOptionType.boolean || option.valueType === ServiceOptionType.category) {
 		// If the option is not required, the empty string should be add at the top of possible choices
 		if (!option.isRequired) {
-			possibleInputs.push('');
+			possibleInputs.push({ text: '', value: '' });
 		}
 
 		if (option.valueType === ServiceOptionType.boolean) {
-			possibleInputs.push(trueInputValue, falseInputValue);
+			possibleInputs.push({ text: trueInputValue, value: trueInputValue }, { text: falseInputValue, value: falseInputValue });
 		} else {
-			option.categoryValues.map(c => possibleInputs.push(c.name));
+			option.categoryValues.forEach(c => possibleInputs.push({ text: c.displayName, value: c.name }));
 		}
 
 		// If the option value is not set and default value is null, the option value should be set to the first possible input.
 		if (optionValue === null || optionValue === undefined) {
-			optionValue = possibleInputs[0];
+			optionValue = possibleInputs[0].text;
 		}
 	}
 	return optionValue;

src/sql/workbench/test/browser/modal/optionsDialogHelper.test.ts

@@ -10,9 +10,10 @@ import * as TypeMoq from 'typemoq';
 import * as assert from 'assert';
 import { ServiceOptionType } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { $ } from 'vs/base/browser/dom';
+import { SelectOptionItemSQL } from 'sql/base/browser/ui/selectBox/selectBox';
 
 suite('Advanced options helper tests', () => {
-	let possibleInputs: string[];
+	let possibleInputs: SelectOptionItemSQL[];
 	let options: { [name: string]: any };
 	let categoryOption: azdata.ServiceOption;
 	let booleanOption: azdata.ServiceOption;
@@ -35,8 +36,8 @@ suite('Advanced options helper tests', () => {
 			description: 'Declares the application workload type when connecting to a server',
 			groupName: 'Initialization',
 			categoryValues: [
-				{ displayName: 'ReadWrite', name: 'ReadWrite' },
+				{ displayName: 'ReadWrite', name: 'RW' },
-				{ displayName: 'ReadOnly', name: 'ReadOnly' }
+				{ displayName: 'ReadOnly', name: 'RO' }
 			],
 			defaultValue: null,
 			isRequired: false,
@@ -110,9 +111,12 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, 'ReadWrite');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'ReadWrite');
+		assert.equal(possibleInputs[1].text, 'ReadWrite');
-		assert.equal(possibleInputs[2], 'ReadOnly');
+		assert.equal(possibleInputs[2].text, 'ReadOnly');
+		assert.equal(possibleInputs[0].value, '');
+		assert.equal(possibleInputs[1].value, 'RW');
+		assert.equal(possibleInputs[2].value, 'RO');
 	});
 
 	test('create default and required category options should set the option value and possible inputs correctly', () => {
@@ -122,8 +126,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, 'ReadWrite');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'ReadWrite');
+		assert.equal(possibleInputs[0].text, 'ReadWrite');
-		assert.equal(possibleInputs[1], 'ReadOnly');
+		assert.equal(possibleInputs[1].text, 'ReadOnly');
 	});
 
 	test('create no default and not required category options should set the option value and possible inputs correctly', () => {
@@ -133,9 +137,9 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, '');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'ReadWrite');
+		assert.equal(possibleInputs[1].text, 'ReadWrite');
-		assert.equal(possibleInputs[2], 'ReadOnly');
+		assert.equal(possibleInputs[2].text, 'ReadOnly');
 	});
 
 	test('create no default but required category options should set the option value and possible inputs correctly', () => {
@@ -145,8 +149,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, 'ReadWrite');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'ReadWrite');
+		assert.equal(possibleInputs[0].text, 'ReadWrite');
-		assert.equal(possibleInputs[1], 'ReadOnly');
+		assert.equal(possibleInputs[1].text, 'ReadOnly');
 	});
 
 	test('create not required category options with option value should set the option value and possible inputs correctly', () => {
@@ -157,9 +161,9 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, 'ReadOnly');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'ReadWrite');
+		assert.equal(possibleInputs[1].text, 'ReadWrite');
-		assert.equal(possibleInputs[2], 'ReadOnly');
+		assert.equal(possibleInputs[2].text, 'ReadOnly');
 	});
 
 	test('create required category options with option value should set the option value and possible inputs correctly', () => {
@@ -170,8 +174,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(categoryOption, options, possibleInputs);
 		assert.equal(optionValue, 'ReadOnly');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'ReadWrite');
+		assert.equal(possibleInputs[0].text, 'ReadWrite');
-		assert.equal(possibleInputs[1], 'ReadOnly');
+		assert.equal(possibleInputs[1].text, 'ReadOnly');
 	});
 
 	test('create default but not required boolean options should set the option value and possible inputs correctly', () => {
@@ -181,9 +185,9 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, 'False');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'True');
+		assert.equal(possibleInputs[1].text, 'True');
-		assert.equal(possibleInputs[2], 'False');
+		assert.equal(possibleInputs[2].text, 'False');
 	});
 
 	test('create default and required boolean options should set the option value and possible inputs correctly', () => {
@@ -193,8 +197,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, 'False');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'True');
+		assert.equal(possibleInputs[0].text, 'True');
-		assert.equal(possibleInputs[1], 'False');
+		assert.equal(possibleInputs[1].text, 'False');
 	});
 
 	test('create no default and not required boolean options should set the option value and possible inputs correctly', () => {
@@ -204,9 +208,9 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, '');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'True');
+		assert.equal(possibleInputs[1].text, 'True');
-		assert.equal(possibleInputs[2], 'False');
+		assert.equal(possibleInputs[2].text, 'False');
 	});
 
 	test('create no default but required boolean options should set the option value and possible inputs correctly', () => {
@@ -216,8 +220,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, 'True');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'True');
+		assert.equal(possibleInputs[0].text, 'True');
-		assert.equal(possibleInputs[1], 'False');
+		assert.equal(possibleInputs[1].text, 'False');
 	});
 
 	test('create not required boolean options with option value should set the option value and possible inputs correctly', () => {
@@ -228,9 +232,9 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, 'True');
 		assert.equal(possibleInputs.length, 3);
-		assert.equal(possibleInputs[0], '');
+		assert.equal(possibleInputs[0].text, '');
-		assert.equal(possibleInputs[1], 'True');
+		assert.equal(possibleInputs[1].text, 'True');
-		assert.equal(possibleInputs[2], 'False');
+		assert.equal(possibleInputs[2].text, 'False');
 	});
 
 	test('create required boolean options with option value should set the option value and possible inputs correctly', () => {
@@ -241,8 +245,8 @@ suite('Advanced options helper tests', () => {
 		let optionValue = OptionsDialogHelper.getOptionValueAndCategoryValues(booleanOption, options, possibleInputs);
 		assert.equal(optionValue, 'False');
 		assert.equal(possibleInputs.length, 2);
-		assert.equal(possibleInputs[0], 'True');
+		assert.equal(possibleInputs[0].text, 'True');
-		assert.equal(possibleInputs[1], 'False');
+		assert.equal(possibleInputs[1].text, 'False');
 	});
 
 	test('create default number options should set the option value and possible inputs correctly', () => {