From d5c495f05a70567d664cb528fad9ed040c032597 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Fri, 24 Mar 2023 09:57:32 -0700 Subject: [PATCH] Address Secure enclaves feedback to show required indicator optionally (#22428) --- extensions/mssql/package.json | 6 ++- extensions/mssql/package.nls.json | 8 ++-- src/sql/azdata.proposed.d.ts | 7 ++++ .../workbench/browser/modal/dialogHelper.ts | 24 ++++++++++-- .../workbench/browser/modal/optionsDialog.ts | 39 ++++++++++++++++--- .../connection/browser/connectionWidget.ts | 20 ++++++++-- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/extensions/mssql/package.json b/extensions/mssql/package.json index 9e603ea505..1194cac88b 100644 --- a/extensions/mssql/package.json +++ b/extensions/mssql/package.json @@ -1048,7 +1048,8 @@ "dependentOptionActions": [ { "optionName": "attestationProtocol", - "action": "hide" + "action": "hide", + "required": true } ] } @@ -1089,7 +1090,8 @@ "dependentOptionActions": [ { "optionName": "enclaveAttestationUrl", - "action": "hide" + "action": "hide", + "required": true } ] } diff --git a/extensions/mssql/package.nls.json b/extensions/mssql/package.nls.json index 45621c6ad7..19b4537565 100644 --- a/extensions/mssql/package.nls.json +++ b/extensions/mssql/package.nls.json @@ -112,14 +112,14 @@ "mssql.connectionOptions.currentLanguage.description": "The SQL Server language record name", "mssql.connectionOptions.columnEncryptionSetting.displayName": "Always Encrypted", "mssql.connectionOptions.columnEncryptionSetting.description": "Enables or disables Always Encrypted for the connection", - "mssql.connectionOptions.secureEnclaves.displayName": "Secure Enclaves", - "mssql.connectionOptions.secureEnclaves.description": "Enables or disables Secure Enclaves for the connection", - "mssql.connectionOptions.enclaveAttestationProtocol.displayName": "Attestation Protocol", + "mssql.connectionOptions.secureEnclaves.displayName": "Secure enclaves", + "mssql.connectionOptions.secureEnclaves.description": "Enables or disables Secure enclaves for the connection", + "mssql.connectionOptions.enclaveAttestationProtocol.displayName": "Attestation protocol", "mssql.connectionOptions.enclaveAttestationProtocol.description": "Specifies a protocol for attesting a server-side enclave used with Always Encrypted with secure enclaves", "mssql.connectionOptions.enclaveAttestationProtocol.categoryValues.AAS": "Azure Attestation", "mssql.connectionOptions.enclaveAttestationProtocol.categoryValues.HGS": "Host Guardian Service", "mssql.connectionOptions.enclaveAttestationProtocol.categoryValues.None": "None", - "mssql.connectionOptions.enclaveAttestationUrl.displayName": "Enclave Attestation URL", + "mssql.connectionOptions.enclaveAttestationUrl.displayName": "Attestation URL", "mssql.connectionOptions.enclaveAttestationUrl.description": "Specifies an endpoint for attesting a server-side enclave used with Always Encrypted with secure enclaves", "mssql.connectionOptions.encrypt.displayName": "Encrypt", "mssql.connectionOptions.encrypt.description": "When 'Mandatory' or 'Strict', SQL Server uses SSL encryption for all data sent between the client and server if the server has a certificate installed. When set to 'Strict', SQL Server uses TDS 8.0 for all data transfer between the client and server. 'Strict' is supported on SQL Server 2022 onwards.", diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index 5dde4e7f69..6fb4cf5689 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -614,6 +614,13 @@ declare module 'azdata' { * Action to be taken, Supported values: 'show', 'hide'. */ action: string; + + /** + * Whether or not the option should be set to required when visible. Defaults to false. + * NOTE: Since this is dynamically defined, option values are not updated on 'show' and validation is not performed. + * When set to true, providers must handle property validation. + */ + required?: boolean; } // Object Explorer interfaces -------------------------------- diff --git a/src/sql/workbench/browser/modal/dialogHelper.ts b/src/sql/workbench/browser/modal/dialogHelper.ts index 9c5c05cabe..ad50a1f49f 100644 --- a/src/sql/workbench/browser/modal/dialogHelper.ts +++ b/src/sql/workbench/browser/modal/dialogHelper.ts @@ -10,6 +10,8 @@ import * as types from 'vs/base/common/types'; import * as azdata from 'azdata'; import { wrapStringWithNewLine } from 'sql/workbench/common/sqlWorkbenchUtils'; +export const requiredIndicatorSpan = 'span.required-indicator'; + export function appendRow(container: HTMLElement, label: string, labelClass: string, cellContainerClass: string, rowContainerClass?: string | Array, showRequiredIndicator: boolean = false, title?: string, titleMaxWidth?: number): HTMLElement { let rowContainer = append(container, $('tr')); if (rowContainerClass) { @@ -29,10 +31,7 @@ export function appendRow(container: HTMLElement, label: string, labelClass: str append(labelContainer, $('div')).innerText = label; if (showRequiredIndicator) { - const indicator = append(labelContainer, $('span.required-indicator')); - indicator.innerText = '*'; - indicator.style.color = 'red'; - indicator.style.marginLeft = '5px'; + appendRequiredIndicator(labelContainer); } let inputCellContainer = append(rowContainer, $(`td.${cellContainerClass}`)); @@ -81,3 +80,20 @@ export function getCategoryName(categories: azdata.CategoryValue[], categoryDisp }); return categoryName; } + +export function appendRequiredIndicator(labelContainer: HTMLElement): HTMLElement { + const indicator = append(labelContainer, $(requiredIndicatorSpan)); + indicator.innerText = '*'; + indicator.style.color = 'red'; + indicator.style.marginLeft = '5px'; + return indicator; +} + +export function getOptionContainerByName(parentContainer: HTMLElement, optionName: string): HTMLElement | undefined { + for (let i = 0; i < parentContainer.childElementCount; i++) { + if (parentContainer.children.item(i).classList.contains(`option-${optionName}`)) { + return parentContainer.children.item(i).children.item(0).children.item(0) as HTMLElement; + } + } + return undefined; +} diff --git a/src/sql/workbench/browser/modal/optionsDialog.ts b/src/sql/workbench/browser/modal/optionsDialog.ts index 58d841f56f..77b6e27ea4 100644 --- a/src/sql/workbench/browser/modal/optionsDialog.ts +++ b/src/sql/workbench/browser/modal/optionsDialog.ts @@ -125,7 +125,7 @@ export class OptionsDialog extends Modal { private fillInOptions(container: HTMLElement, options: azdata.ServiceOption[]): void { 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', `option-${option.name}`); + let rowContainer = DialogHelper.appendRow(container, option.displayName, 'optionsDialog-label', 'optionsDialog-input', `option-${option.name}`, option.isRequired); const optionElement = OptionsDialogHelper.createOptionElement(option, rowContainer, this._optionValues, this._optionElements, this._contextViewService, (name) => this.onOptionLinkClicked(name)); this.disposableStore.add(optionElement.optionWidget); } @@ -217,7 +217,7 @@ export class OptionsDialog extends Modal { /** * Registers on selection change event for connection options configured with 'onSelectionChange' property. */ - private registerOnSelectionChangeEvents(options: { [name: string]: any }, container: HTMLElement): void { + private registerOnSelectionChangeEvents(optionValues: { [name: string]: any }, container: HTMLElement): void { //Register on selection change event for all advanced options for (let optionName in this._optionElements) { let widget: Widget = this._optionElements[optionName].optionWidget; @@ -237,10 +237,23 @@ export class OptionsDialog extends Modal { let widget: AdsWidget | undefined = this._findWidget(collections, optionAction.optionName); if (widget) { createCSSRule(`.hide-${widget.id} .option-${widget.id}`, `display: none;`); - this._onValueChangeEvent(container, selectedValue, event.values, widget, defaultValue, optionAction.action); + this._onValueChangeEvent(container, selectedValue, event.values, widget, defaultValue, optionAction); } }); })); + event?.dependentOptionActions?.forEach((optionAction) => { + if (this._optionValues[optionAction.optionName] && optionAction.required) { + let element = DialogHelper.getOptionContainerByName(container, optionAction.optionName) as HTMLElement; + // Append required indicator when not present. + if (element && element.childElementCount === 1) { + DialogHelper.appendRequiredIndicator(element); + } + } + // Force selection event if option value is available. + if (this.optionValues[option.name]) { + widget.selectWithOptionName(this.optionValues[option.name], false, true); + } + }); }); // Clear selection change actions once event is registered. option.onSelectionChange = undefined; @@ -253,10 +266,17 @@ export class OptionsDialog extends Modal { } private _onValueChangeEvent(container: HTMLElement, selectedValue: string, acceptedValues: string[], - widget: AdsWidget, defaultValue: string, action: string): void { - if ((acceptedValues.includes(selectedValue.toLocaleLowerCase()) && action === Actions.Show) - || (!acceptedValues.includes(selectedValue.toLocaleLowerCase()) && action === Actions.Hide)) { + widget: AdsWidget, defaultValue: string, optionAction: azdata.DependentOptionAction): void { + if ((acceptedValues.includes(selectedValue.toLocaleLowerCase()) && optionAction.action === Actions.Show) + || (!acceptedValues.includes(selectedValue.toLocaleLowerCase()) && optionAction.action === Actions.Hide)) { container.classList.remove(`hide-${widget.id}`); + if (optionAction.required) { + let element = DialogHelper.getOptionContainerByName(container, optionAction.optionName) as HTMLElement; + // Append required indicator when not present. + if (element && element.childElementCount === 1) { + DialogHelper.appendRequiredIndicator(element); + } + } } else { // Support more Widget classes here as needed. if (widget instanceof SelectBox) { @@ -264,6 +284,13 @@ export class OptionsDialog extends Modal { } else if (widget instanceof InputBox) { widget.value = defaultValue; } + + // Reset required indicator if present. + let element = DialogHelper.getOptionContainerByName(container, optionAction.optionName); + if (element && element!.hasChildNodes && element.childElementCount > 1) { + element!.children.item(1).remove(); + } + container.classList.add(`hide-${widget.id}`); widget.hideMessage(); } diff --git a/src/sql/workbench/services/connection/browser/connectionWidget.ts b/src/sql/workbench/services/connection/browser/connectionWidget.ts index bc88df3d88..ffb59c510e 100644 --- a/src/sql/workbench/services/connection/browser/connectionWidget.ts +++ b/src/sql/workbench/services/connection/browser/connectionWidget.ts @@ -326,7 +326,7 @@ export class ConnectionWidget extends lifecycle.Disposable { let widget: AdsWidget | undefined = this._findWidget(collections, optionAction.optionName); if (widget) { createCSSRule(`.hide-${widget.id} .option-${widget.id}`, `display: none;`); - this._onValueChangeEvent(selectedValue, event.values, widget, defaultValue, optionAction.action); + this._onValueChangeEvent(selectedValue, event.values, widget, defaultValue, optionAction); } }); })); @@ -351,10 +351,16 @@ export class ConnectionWidget extends lifecycle.Disposable { } private _onValueChangeEvent(selectedValue: string, acceptedValues: string[], - widget: AdsWidget, defaultValue: string, action: string): void { - if ((acceptedValues.includes(selectedValue.toLocaleLowerCase()) && action === Actions.Show) - || (!acceptedValues.includes(selectedValue.toLocaleLowerCase()) && action === Actions.Hide)) { + widget: AdsWidget, defaultValue: string, optionAction: azdata.DependentOptionAction): void { + if ((acceptedValues.includes(selectedValue.toLocaleLowerCase()) && optionAction.action === Actions.Show) + || (!acceptedValues.includes(selectedValue.toLocaleLowerCase()) && optionAction.action === Actions.Hide)) { this._tableContainer.classList.remove(`hide-${widget.id}`); + if (optionAction.required) { + let element = DialogHelper.getOptionContainerByName(this._tableContainer, optionAction.optionName); + if (element) { + DialogHelper.appendRequiredIndicator(element); + } + } } else { // Support more Widget classes here as needed. if (widget instanceof SelectBox) { @@ -362,6 +368,12 @@ export class ConnectionWidget extends lifecycle.Disposable { } else if (widget instanceof InputBox) { widget.value = defaultValue; } + + // Reset required indicator. + let element = DialogHelper.getOptionContainerByName(this._tableContainer, optionAction.optionName); + if (element && element!.hasChildNodes && element.childElementCount > 1) { + element!.children.item(1).remove(); + } this._tableContainer.classList.add(`hide-${widget.id}`); widget.hideMessage(); }