highlight problematic property in the designer when error is selected (#18512)

* navigate to property when selecting error message

* use list component

* highlight problematic property

* remove unnecessary call

* comment

* comment

src/sql/workbench/browser/designer/designer.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import {
-	DesignerComponentInput, DesignerEditType, DesignerTab, DesignerEdit, DesignerEditPath, DesignerViewModel, DesignerDataPropertyInfo,
+	DesignerComponentInput, DesignerEditType, DesignerTab, DesignerEdit, DesignerPropertyPath, DesignerViewModel, DesignerDataPropertyInfo,
 	DesignerTableComponentRowData, DesignerTableProperties, InputBoxProperties, DropDownProperties, CheckBoxProperties,
 	DesignerEditProcessedEventArgs, DesignerStateChangedEventArgs, DesignerAction, DesignerUIState, ScriptProperty, DesignerRootObjectPath
 }
@@ -38,6 +38,10 @@ import { INotificationService } from 'vs/platform/notification/common/notificati
 import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
 import { DesignerMessagesTabPanelView } from 'sql/workbench/browser/designer/designerMessagesTabPanelView';
 import { DesignerScriptEditorTabPanelView } from 'sql/workbench/browser/designer/designerScriptEditorTabPanelView';
+import { DesignerPropertyPathValidator } from 'sql/workbench/browser/designer/designerPropertyPathValidator';
+import { IThemeService } from 'vs/platform/theme/common/themeService';
+import { listActiveSelectionBackground, listActiveSelectionForeground } from 'vs/platform/theme/common/colorRegistry';
+import { alert } from 'vs/base/browser/ui/aria/aria';
 
 export interface IDesignerStyle {
 	tabbedPanelStyles?: ITabbedPanelStyles;
@@ -52,7 +56,7 @@ export interface IDesignerStyle {
 
 export type DesignerUIComponent = InputBox | Checkbox | Table<Slick.SlickData> | SelectBox;
 
-export type CreateComponentsFunc = (container: HTMLElement, components: DesignerDataPropertyInfo[], parentPath: DesignerEditPath) => DesignerUIComponent[];
+export type CreateComponentsFunc = (container: HTMLElement, components: DesignerDataPropertyInfo[], parentPath: DesignerPropertyPath) => DesignerUIComponent[];
 export type SetComponentValueFunc = (definition: DesignerDataPropertyInfo, component: DesignerUIComponent, data: DesignerViewModel) => void;
 
 const TableRowHeight = 25;
@@ -92,14 +96,15 @@ export class Designer extends Disposable implements IThemable {
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@IContextViewService private readonly _contextViewProvider: IContextViewService,
 		@INotificationService private readonly _notificationService: INotificationService,
-		@IDialogService private readonly _dialogService: IDialogService) {
+		@IDialogService private readonly _dialogService: IDialogService,
+		@IThemeService private readonly _themeService: IThemeService) {
 		super();
 		this._tableCellEditorFactory = new TableCellEditorFactory(
 			{
 				valueGetter: (item, column): string => {
 					return item[column.field].value;
 				},
-				valueSetter: (parentPath: DesignerEditPath, row: number, item: DesignerTableComponentRowData, column: Slick.Column<Slick.SlickData>, value: string): void => {
+				valueSetter: (parentPath: DesignerPropertyPath, row: number, item: DesignerTableComponentRowData, column: Slick.Column<Slick.SlickData>, value: string): void => {
 					this.handleEdit({
 						type: DesignerEditType.Update,
 						path: [...parentPath, row, column.field],
@@ -138,7 +143,10 @@ export class Designer extends Disposable implements IThemable {
 			onDidChange: Event.None
 		}, Sizing.Distribute);
 		this._scriptTabbedPannel = new TabbedPanel(this._editorContainer);
-		this._messagesView = new DesignerMessagesTabPanelView();
+		this._messagesView = this._instantiationService.createInstance(DesignerMessagesTabPanelView);
+		this._register(this._messagesView.onMessageSelected((path) => {
+			this.selectProperty(path);
+		}));
 		this._scriptEditorView = new DesignerScriptEditorTabPanelView(this._instantiationService);
 		this._scriptTabbedPannel.pushTab({
 			title: localize('designer.scriptTabTitle', "Scripts"),
@@ -311,6 +319,9 @@ export class Designer extends Disposable implements IThemable {
 	private handleEditProcessedEvent(args: DesignerEditProcessedEventArgs): void {
 		const edit = args.edit;
 		this._supressEditProcessing = true;
+		if (!args.result.isValid) {
+			alert(localize('designer.errorCountAlert', "{0} validation errors found.", args.result.errors.length));
+		}
 		try {
 			this.updateComponentValues();
 			if (edit.type === DesignerEditType.Add) {
@@ -407,7 +418,7 @@ export class Designer extends Disposable implements IThemable {
 		return rows * TableRowHeight + TableHeaderRowHeight;
 	}
 
-	private updatePropertiesPane(objectPath: DesignerEditPath): void {
+	private updatePropertiesPane(objectPath: DesignerPropertyPath): void {
 		let type: string;
 		let components: DesignerDataPropertyInfo[];
 		let objectViewModel: DesignerViewModel;
@@ -469,6 +480,74 @@ export class Designer extends Disposable implements IThemable {
 		this._messagesView.updateMessages(this._input.validationErrors);
 	}
 
+	private selectProperty(path: DesignerPropertyPath): void {
+		if (!DesignerPropertyPathValidator.validate(path, this._input.viewModel)) {
+			return;
+		}
+
+		// Find top level property
+		let found = false;
+		if (this._input.view.components) {
+			for (const component of this._input.view.components) {
+				if (path[0] === component.propertyName) {
+					found = true;
+					break;
+				}
+			}
+		}
+		if (this._input.view.tabs) {
+			for (const tab of this._input.view.tabs) {
+				if (tab) {
+					for (const component of tab.components) {
+						if (path[0] === component.propertyName) {
+							this._contentTabbedPanel.showTab(tab.title);
+							found = true;
+							break;
+						}
+					}
+				}
+				if (found) {
+					break;
+				}
+			}
+		}
+
+		if (found) {
+			const propertyInfo = this._componentMap.get(<string>path[0]);
+			if (propertyInfo.defintion.componentType !== 'table') {
+				propertyInfo.component.focus();
+				return;
+			} else {
+				const tableComponent = <Table<Slick.SlickData>>propertyInfo.component;
+				const targetRow = <number>path[1];
+				const targetCell = 0;
+				tableComponent.setActiveCell(targetRow, targetCell);
+				tableComponent.grid.scrollCellIntoView(targetRow, targetCell, false);
+				if (path.length > 2) {
+					const relativePath = path.slice(2);
+					this._propertiesPane.selectProperty(relativePath);
+				}
+			}
+			this.highlightActiveElement();
+		}
+	}
+
+	private highlightActiveElement(): void {
+		const bgColor = this._themeService.getColorTheme().getColor(listActiveSelectionBackground);
+		const color = this._themeService.getColorTheme().getColor(listActiveSelectionForeground);
+		const currentElement = document.activeElement as HTMLElement;
+		if (currentElement) {
+			const originalBGColor = currentElement.style.backgroundColor;
+			const originalColor = currentElement.style.color;
+			currentElement.style.backgroundColor = bgColor.toString();
+			currentElement.style.color = color.toString();
+			setTimeout(() => {
+				currentElement.style.color = originalColor;
+				currentElement.style.backgroundColor = originalBGColor;
+			}, 500);
+		}
+	}
+
 	private handleEdit(edit: DesignerEdit): void {
 		if (this._supressEditProcessing) {
 			return;
@@ -553,7 +632,7 @@ export class Designer extends Disposable implements IThemable {
 		components: DesignerDataPropertyInfo[],
 		componentMap: Map<string, { defintion: DesignerDataPropertyInfo, component: DesignerUIComponent }>,
 		groupHeaders: HTMLElement[],
-		parentPath: DesignerEditPath,
+		parentPath: DesignerPropertyPath,
 		area: DesignerUIArea): DesignerUIComponent[] {
 		const uiComponents = [];
 		const groupNames = [];
@@ -589,7 +668,7 @@ export class Designer extends Disposable implements IThemable {
 
 	private createComponent(container: HTMLElement,
 		componentDefinition: DesignerDataPropertyInfo,
-		parentPath: DesignerEditPath,
+		parentPath: DesignerPropertyPath,
 		componentMap: Map<string, { defintion: DesignerDataPropertyInfo, component: DesignerUIComponent }>,
 		view: DesignerUIArea): DesignerUIComponent {
 		const propertyPath = [...parentPath, componentDefinition.propertyName];
@@ -625,6 +704,7 @@ export class Designer extends Disposable implements IThemable {
 				const dropdownContainer = container.appendChild(DOM.$(''));
 				const dropdownProperties = componentDefinition.componentProperties as DropDownProperties;
 				const dropdown = new SelectBox(dropdownProperties.values as string[] || [], undefined, this._contextViewProvider, undefined);
+				dropdown.setAriaLabel(componentDefinition.componentProperties?.title);
 				dropdown.render(dropdownContainer);
 				dropdown.selectElem.style.height = '25px';
 				dropdown.onDidSelect((e) => {
@@ -679,6 +759,7 @@ export class Designer extends Disposable implements IThemable {
 					addRowButton.icon = {
 						id: `add-row-button new codicon`
 					};
+					addRowButton.ariaLabel = localize('designer.newRowButtonAriaLabel', "Add new row to '{0}' table", tableProperties.ariaLabel);
 					this._buttons.push(addRowButton);
 				}
 				const tableContainer = container.appendChild(DOM.$('.full-row'));
@@ -695,7 +776,8 @@ export class Designer extends Disposable implements IThemable {
 						}
 					},
 					rowHeight: TableRowHeight,
-					headerRowHeight: TableHeaderRowHeight
+					headerRowHeight: TableHeaderRowHeight,
+					editorLock: new Slick.EditorLock()
 				});
 				table.ariaLabel = tableProperties.ariaLabel;
 				const columns = tableProperties.columns.map(propName => {
@@ -819,13 +901,15 @@ export class Designer extends Disposable implements IThemable {
 
 	private getUIState(): DesignerUIState {
 		return {
-			activeTabId: this._contentTabbedPanel.activeTabId
+			activeContentTabId: this._contentTabbedPanel.activeTabId,
+			activeScriptTabId: this._scriptTabbedPannel.activeTabId
 		};
 	}
 
 	private restoreUIState(): void {
 		if (this._input.designerUIState) {
-			this._contentTabbedPanel.showTab(this._input.designerUIState.activeTabId);
+			this._contentTabbedPanel.showTab(this._input.designerUIState.activeContentTabId);
+			this._scriptTabbedPannel.showTab(this._input.designerUIState.activeScriptTabId);
 		}
 	}
 }

src/sql/workbench/browser/designer/designerMessagesTabPanelView.ts

@@ -6,25 +6,118 @@
 import { IPanelView } from 'sql/base/browser/ui/panel/panel';
 import { Disposable } from 'vs/base/common/lifecycle';
 import * as DOM from 'vs/base/browser/dom';
-import { DesignerValidationError } from 'sql/workbench/browser/designer/interfaces';
+import { DesignerPropertyPath, DesignerValidationError } from 'sql/workbench/browser/designer/interfaces';
+import { Emitter, Event } from 'vs/base/common/event';
+import { IListAccessibilityProvider, List } from 'vs/base/browser/ui/list/listWidget';
+import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
+import { localize } from 'vs/nls';
+import { IColorTheme, ICssStyleCollector, IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService';
+import { attachListStyler } from 'vs/platform/theme/common/styler';
+import { problemsErrorIconForeground } from 'vs/platform/theme/common/colorRegistry';
+import { Codicon } from 'vs/base/common/codicons';
 
 export class DesignerMessagesTabPanelView extends Disposable implements IPanelView {
 	private _container: HTMLElement;
+	private _onMessageSelected = new Emitter<DesignerPropertyPath>();
+	private _messageList: List<DesignerValidationError>;
+
+	public readonly onMessageSelected: Event<DesignerPropertyPath> = this._onMessageSelected.event;
+
+	constructor(@IThemeService private _themeService: IThemeService) {
+		super();
+	}
 
 	render(container: HTMLElement): void {
 		this._container = container.appendChild(DOM.$('.messages-container'));
+		this._messageList = new List<DesignerValidationError>('designerMessageList', this._container, new DesignerMessageListDelegate(), [new TableFilterListRenderer()], {
+			multipleSelectionSupport: false,
+			keyboardSupport: true,
+			mouseSupport: true,
+			accessibilityProvider: new DesignerMessagesListAccessibilityProvider()
+		});
+		this._register(this._messageList.onDidChangeSelection((e) => {
+			if (e.elements && e.elements.length === 1) {
+				this._onMessageSelected.fire(e.elements[0].propertyPath);
+			}
+		}));
+		this._register(attachListStyler(this._messageList, this._themeService));
 	}
 
 	layout(dimension: DOM.Dimension): void {
+		this._messageList.layout(dimension.height, dimension.width);
 	}
 
 	updateMessages(errors: DesignerValidationError[]) {
-		if (this._container) {
-			DOM.clearNode(this._container);
-			errors?.forEach(error => {
-				const messageItem = this._container.appendChild(DOM.$('.message-item.codicon.error'));
-				messageItem.innerText = error.message;
-			});
+		if (this._messageList) {
+			this._messageList.splice(0, this._messageList.length, errors);
 		}
 	}
 }
+
+registerThemingParticipant((theme: IColorTheme, collector: ICssStyleCollector) => {
+	const errorForegroundColor = theme.getColor(problemsErrorIconForeground);
+	if (errorForegroundColor) {
+		collector.addRule(`
+		.designer-component .messages-container .message-item .message-icon {
+			color: ${errorForegroundColor};
+		}
+		`);
+	}
+});
+
+const DesignerMessageListTemplateId = 'DesignerMessageListTemplate';
+class DesignerMessageListDelegate implements IListVirtualDelegate<DesignerValidationError> {
+	getHeight(element: DesignerValidationError): number {
+		return 25;
+	}
+
+	getTemplateId(element: DesignerValidationError): string {
+		return DesignerMessageListTemplateId;
+	}
+}
+
+interface DesignerMessageListItemTemplate {
+	messageText: HTMLDivElement;
+}
+
+class TableFilterListRenderer implements IListRenderer<DesignerValidationError, DesignerMessageListItemTemplate> {
+	renderTemplate(container: HTMLElement): DesignerMessageListItemTemplate {
+		const data: DesignerMessageListItemTemplate = Object.create(null);
+		const messageItem = container.appendChild(DOM.$('.message-item'));
+		messageItem.appendChild(DOM.$(`.message-icon${Codicon.error.cssSelector}`));
+		data.messageText = messageItem.appendChild(DOM.$('.message-text'));
+		return data;
+	}
+
+	renderElement(element: DesignerValidationError, index: number, templateData: DesignerMessageListItemTemplate, height: number): void {
+		templateData.messageText.innerText = element.message;
+	}
+
+	disposeElement?(element: DesignerValidationError, index: number, templateData: DesignerMessageListItemTemplate, height: number): void {
+	}
+
+	public disposeTemplate(templateData: DesignerMessageListItemTemplate): void {
+	}
+
+	public get templateId(): string {
+		return DesignerMessageListTemplateId;
+	}
+}
+
+class DesignerMessagesListAccessibilityProvider implements IListAccessibilityProvider<DesignerValidationError> {
+	getAriaLabel(element: DesignerValidationError): string {
+		return element.message;
+	}
+
+	getWidgetAriaLabel(): string {
+		return localize('designer.MessageListAriaLabel', "Errors");
+	}
+
+	getWidgetRole() {
+		return 'listbox';
+	}
+
+	getRole(element: DesignerValidationError): string {
+		return 'option';
+	}
+}

src/sql/workbench/browser/designer/designerPropertiesPane.ts

@@ -3,14 +3,15 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { Table } from 'sql/base/browser/ui/table/table';
 import { CreateComponentsFunc, DesignerUIComponent, SetComponentValueFunc } from 'sql/workbench/browser/designer/designer';
-import { DesignerViewModel, DesignerDataPropertyInfo, DesignerEditPath } from 'sql/workbench/browser/designer/interfaces';
+import { DesignerViewModel, DesignerDataPropertyInfo, DesignerPropertyPath } from 'sql/workbench/browser/designer/interfaces';
 import * as DOM from 'vs/base/browser/dom';
 import { equals } from 'vs/base/common/objects';
 import { localize } from 'vs/nls';
 
 export interface ObjectInfo {
-	path: DesignerEditPath;
+	path: DesignerPropertyPath;
 	type: string;
 	components: DesignerDataPropertyInfo[];
 	viewModel: DesignerViewModel;
@@ -19,7 +20,7 @@ export interface ObjectInfo {
 export class DesignerPropertiesPane {
 	private _titleElement: HTMLElement;
 	private _contentElement: HTMLElement;
-	private _objectPath: DesignerEditPath;
+	private _objectPath: DesignerPropertyPath;
 	private _componentMap = new Map<string, { defintion: DesignerDataPropertyInfo, component: DesignerUIComponent }>();
 	private _groupHeaders: HTMLElement[] = [];
 
@@ -48,7 +49,7 @@ export class DesignerPropertiesPane {
 		return this._componentMap;
 	}
 
-	public get objectPath(): DesignerEditPath {
+	public get objectPath(): DesignerPropertyPath {
 		return this._objectPath;
 	}
 
@@ -94,4 +95,22 @@ export class DesignerPropertiesPane {
 		});
 		this._descriptionContainer.style.display = 'none';
 	}
+
+	public selectProperty(path: DesignerPropertyPath): void {
+		const componentInfo = this.componentMap.get(<string>path[0]);
+		if (componentInfo.defintion.componentType !== 'table') {
+			componentInfo.component.focus();
+			return;
+		}
+
+		const table = componentInfo.component as Table<Slick.SlickData>;
+		const row = path[1] as number;
+		let cell = 0;
+		if (path.length === 3) {
+			const colName = path[2] as string;
+			cell = table.columns.findIndex(c => c.field === colName);
+		}
+		table.setActiveCell(row, cell);
+		table.grid.scrollCellIntoView(row, cell, false);
+	}
 }

src/sql/workbench/browser/designer/designerPropertyPathValidator.ts

@@ -0,0 +1,64 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { DesignerPropertyPath, DesignerTableProperties, DesignerViewModel } from 'sql/workbench/browser/designer/interfaces';
+
+export class DesignerPropertyPathValidator {
+
+	/**
+	 * Validate the path property, detail of the path can be found in the azdata typings file.
+	 * @param path path of the property.
+	 * @param viewModel the view model.
+	 * @returns Whether the path is valid.
+	 */
+	static validate(path: DesignerPropertyPath, viewModel: DesignerViewModel): boolean {
+		/**
+		 * Path specification for all supported scenarios:
+		 * 1. 'Add' scenario
+		 *     a. ['propertyName1']. Example: add a column to the columns property: ['columns'].
+		 *     b. ['propertyName1',index-1,'propertyName2']. Example: add a column mapping to the first foreign key: ['foreignKeys',0,'mappings'].
+		 * 2. 'Update' scenario
+		 *     a. ['propertyName1']. Example: update the name of the table: ['name'].
+		 *     b. ['propertyName1',index-1,'propertyName2']. Example: update the name of a column: ['columns',0,'name'].
+		 *     c. ['propertyName1',index-1,'propertyName2',index-2,'propertyName3']. Example: update the source column of an entry in a foreign key's column mapping table: ['foreignKeys',0,'mappings',0,'source'].
+		 * 3. 'Remove' scenario
+		 *     a. ['propertyName1',index-1]. Example: remove a column from the columns property: ['columns',0'].
+		 *     b. ['propertyName1',index-1,'propertyName2',index-2]. Example: remove a column mapping from a foreign key's column mapping table: ['foreignKeys',0,'mappings',0].
+		 */
+		if (!path || path.length === 0 || path.length > 5) {
+			return false;
+		}
+
+		for (let index = 0; index < path.length; index++) {
+			const expectingNumber = (index % 2) !== 0;
+			if (expectingNumber && typeof path[index] !== 'number') {
+				return false;
+			}
+
+			if (!expectingNumber && typeof path[index] !== 'string') {
+				return false;
+			}
+		}
+		let currentObject = viewModel;
+		for (let index = 0; index < path.length;) {
+			const propertyName = <string>path[index];
+			if (Object.keys(currentObject).indexOf(propertyName) === -1) {
+				return false;
+			}
+			if (index === path.length - 1) {
+				break;
+			}
+			index++;
+			const tableData = <DesignerTableProperties>currentObject[propertyName];
+			const objectIndex = <number>path[index];
+			if (!tableData.data || tableData.data.length - 1 < objectIndex) {
+				return false;
+			}
+			currentObject = tableData.data[objectIndex];
+			index++;
+		}
+		return true;
+	}
+}

src/sql/workbench/browser/designer/interfaces.ts

@@ -81,7 +81,8 @@ export interface DesignerComponentInput {
 }
 
 export interface DesignerUIState {
-	activeTabId: PanelTabIdentifier;
+	activeContentTabId: PanelTabIdentifier;
+	activeScriptTabId: PanelTabIdentifier;
 }
 
 export type DesignerAction = 'publish' | 'initialize' | 'processEdit' | 'generateScript' | 'generateReport';
@@ -180,7 +181,6 @@ export interface DesignerTableProperties extends ComponentProperties {
 	 * Whether user can add new rows to the table. The default value is true.
 	 */
 	canAddRows?: boolean;
-
 	/**
 	 * Whether user can remove rows from the table. The default value is true.
 	 */
@@ -208,14 +208,14 @@ export enum DesignerEditType {
 
 export interface DesignerEdit {
 	type: DesignerEditType;
-	path: DesignerEditPath;
+	path: DesignerPropertyPath;
 	value?: any;
 }
 
-export type DesignerEditPath = (string | number)[];
-export const DesignerRootObjectPath: DesignerEditPath = [];
+export type DesignerPropertyPath = (string | number)[];
+export const DesignerRootObjectPath: DesignerPropertyPath = [];
 
-export type DesignerValidationError = { message: string, property?: DesignerEditPath };
+export type DesignerValidationError = { message: string, propertyPath?: DesignerPropertyPath };
 
 export interface DesignerEditResult {
 	isValid: boolean;

src/sql/workbench/browser/designer/media/designer.css

@@ -30,19 +30,25 @@
 }
 
 .designer-component .messages-container {
-	overflow: scroll;
+	overflow: hidden;
 	height: 100%;
 	width: 100%;
 }
 
 .designer-component .messages-container .message-item {
-	padding: 0px 5px 0px 25px;
-	background-position: 5px center;
-	background-size: 16px 16px;
-	user-select: text;
+	display: flex;
+}
+
+.designer-component .messages-container .message-item .message-icon {
+	margin: 0px 6px;
+	flex: 0 0 auto;
 	line-height: 25px;
 }
 
+.designer-component .messages-container .message-item .message-text {
+	flex: 1 1 auto;
+}
+
 .designer-component .tabbed-panel-container {
 	flex: 1 1 auto;
 	overflow: hidden;