Property view labels update when execution plan orientation changes (#19924)

* Updates properties view title when the orientation changes

* Property orientation labels change when the plan orientation changes.

* Renames top and bottom containers to secondary and primary

* Minor clean up

* Removes unnecessary class members

* Promotes constants to module level

* Moves function to module level

* Renames value1 and value2 to primary and secondary

* Replaces orientation string literal type with enum

* Resolves undefined orientation bug

* Removes unused orientation getter

* Updates property table columns only

* Uses orientation enum in editor view and properties view

* Clean up

src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonEditorView.ts

@@ -8,7 +8,7 @@ import * as TelemetryKeys from 'sql/platform/telemetry/common/telemetryKeys';
 import { SelectBox } from 'sql/base/browser/ui/selectBox/selectBox';
 import { ITaskbarContent, Taskbar } from 'sql/base/browser/ui/taskbar/taskbar';
 import { AzdataGraphView } from 'sql/workbench/contrib/executionPlan/browser/azdataGraphView';
-import { ExecutionPlanComparisonPropertiesView } from 'sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView';
+import { ExecutionPlanCompareOrientation, ExecutionPlanComparisonPropertiesView } from 'sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView';
 import { IExecutionPlanService } from 'sql/workbench/services/executionPlan/common/interfaces';
 import { IHorizontalSashLayoutProvider, ISashEvent, IVerticalSashLayoutProvider, Orientation, Sash } from 'vs/base/browser/ui/sash/sash';
 import { Action } from 'vs/base/common/actions';
@@ -56,7 +56,7 @@ export class ExecutionPlanComparisonEditorView {
 	private _sashContainer: HTMLElement;
 	private _horizontalSash: Sash;
 	private _verticalSash: Sash;
-	private _orientation: 'horizontal' | 'vertical' = 'horizontal';
+	private _orientation: ExecutionPlanCompareOrientation = ExecutionPlanCompareOrientation.Horizontal;
 
 	private _placeholderContainer: HTMLElement;
 	private _placeholderInfoboxContainer: HTMLElement;
@@ -452,7 +452,7 @@ export class ExecutionPlanComparisonEditorView {
 			this._topPlanContainer.style.minHeight = '200px';
 			this._topPlanContainer.style.minWidth = '';
 			this._topPlanContainer.style.flex = '1';
-			this._orientation = 'horizontal';
+			this._orientation = ExecutionPlanCompareOrientation.Horizontal;
 			this._toggleOrientationAction.class = splitScreenHorizontallyIconClassName;
 		} else {
 			this._sashContainer.style.width = '3px';
@@ -460,9 +460,11 @@ export class ExecutionPlanComparisonEditorView {
 			this.planSplitViewContainer.style.flexDirection = 'row';
 			this._topPlanContainer.style.minHeight = '';
 			this._topPlanContainer.style.minWidth = '200px';
-			this._orientation = 'vertical';
+			this._orientation = ExecutionPlanCompareOrientation.Vertical;
 			this._toggleOrientationAction.class = splitScreenVerticallyIconClassName;
 		}
+
+		this._propertiesView.orientation = this._orientation;
 		this._topPlanContainer.style.flex = '1';
 		this._bottomPlanContainer.style.flex = '1';
 	}

src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView.ts

@@ -16,10 +16,23 @@ import { executionPlanComparisonPropertiesDifferent, executionPlanComparisonProp
 import * as sqlExtHostType from 'sql/workbench/api/common/sqlExtHostTypes';
 import { TextWithIconColumn } from 'sql/base/browser/ui/table/plugins/textWithIconColumn';
 
+export enum ExecutionPlanCompareOrientation {
+	Horizontal = 'horizontal',
+	Vertical = 'vertical'
+}
+
+const topTitleColumnHeader = localize('nodePropertyViewNameValueColumnTopHeader', "Value (Top Plan)");
+const leftTitleColumnHeader = localize('nodePropertyViewNameValueColumnLeftHeader', "Value (Left Plan)");
+const rightTitleColumnHeader = localize('nodePropertyViewNameValueColumnRightHeader', "Value (Right Plan)");
+const bottomTitleColumnHeader = localize('nodePropertyViewNameValueColumnBottomHeader', "Value (Bottom Plan)");
+
 export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanPropertiesViewBase {
 	private _model: ExecutionPlanComparisonPropertiesViewModel;
-	private _topOperationNameContainer: HTMLElement;
+	private _primaryContainer: HTMLElement;
-	private _bottomOperationNameContainer: HTMLElement;
+	private _secondaryContainer: HTMLElement;
+	private _orientation: ExecutionPlanCompareOrientation = ExecutionPlanCompareOrientation.Horizontal;
+	private _primaryTarget: string;
+	private _secondaryTarget: string;
 
 	public constructor(
 		parentContainer: HTMLElement,
@@ -29,47 +42,86 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 		this._model = <ExecutionPlanComparisonPropertiesViewModel>{};
 		this._parentContainer.style.display = 'none';
 		const header = DOM.$('.compare-operation-name');
-		this._topOperationNameContainer = DOM.$('.compare-operation-name-text');
+		this._primaryContainer = DOM.$('.compare-operation-name-text');
-		header.appendChild(this._topOperationNameContainer);
+		header.appendChild(this._primaryContainer);
-		this._bottomOperationNameContainer = DOM.$('.compare-operation-name-text');
+		this._secondaryContainer = DOM.$('.compare-operation-name-text');
-		header.appendChild(this._bottomOperationNameContainer);
+		header.appendChild(this._secondaryContainer);
 		this.setHeader(header);
 	}
 
-
 	public setTopElement(e: InternalExecutionPlanElement): void {
 		this._model.topElement = e;
-		let target;
 		if ((<azdata.executionPlan.ExecutionPlanNode>e).name) {
-			target = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
+			this._primaryTarget = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
 		} else {
-			target = localize('executionPlanPropertiesEdgeOperationName', "Edge");
+			this._primaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge");
 		}
-		const titleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", target);
+
-		this._topOperationNameContainer.innerText = titleText;
+		let topTitleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", this._primaryTarget);
-		this._topOperationNameContainer.title = titleText;
+		this._primaryContainer.innerText = topTitleText;
-		this.addDataToTable();
+		this._primaryContainer.title = topTitleText;
+		this.refreshPropertiesTable();
 	}
 
 	public setBottomElement(e: InternalExecutionPlanElement): void {
 		this._model.bottomElement = e;
-		let target;
 		if ((<azdata.executionPlan.ExecutionPlanNode>e)?.name) {
-			target = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
+			this._secondaryTarget = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
 		} else {
-			target = localize('executionPlanPropertiesEdgeOperationName', "Edge");
+			this._secondaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge");
 		}
 
-		const titleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", target);
+		let bottomTitleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", this._secondaryTarget);
-		this._bottomOperationNameContainer.innerText = titleText;
+		this._secondaryContainer.innerText = bottomTitleText;
-		this._bottomOperationNameContainer.title = titleText;
+		this._secondaryContainer.title = bottomTitleText;
-		this.addDataToTable();
+		this.refreshPropertiesTable();
 	}
 
+	private updatePropertyContainerTitles(): void {
+		let primaryTitleText = '';
+		let secondaryTitleText = '';
+
+		if (this._orientation === ExecutionPlanCompareOrientation.Horizontal) {
+			primaryTitleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", this._primaryTarget);
+			secondaryTitleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", this._secondaryTarget);
+		}
+		else {
+			primaryTitleText = localize('executionPlanComparisonPropertiesLeftOperation', "Left operation: {0}", this._primaryTarget);
+			secondaryTitleText = localize('executionPlanComparisonPropertiesRightOperation', "Right operation: {0}", this._secondaryTarget);
+		}
+
+		this._primaryContainer.innerText = primaryTitleText;
+		this._primaryContainer.title = primaryTitleText;
+		this._secondaryContainer.innerText = secondaryTitleText;
+		this._secondaryContainer.title = secondaryTitleText;
+
+		this.updatePropertiesTableColumnHeaders();
+	}
+
+	public updatePropertiesTableColumnHeaders() {
+		const columns: Slick.Column<Slick.SlickData>[] = this.getPropertyTableColumns();
+
+		this.updateTableColumns(columns);
+	}
+
+	public refreshPropertiesTable() {
+		const columns: Slick.Column<Slick.SlickData>[] = this.getPropertyTableColumns();
+
+		let topProps = [];
+		let bottomProps = [];
+		if (this._model.topElement?.properties) {
+			topProps = this._model.topElement.properties;
+		}
+		if (this._model.bottomElement?.properties) {
+			bottomProps = this._model.bottomElement.properties;
+		}
+
+		this.populateTable(columns, this.convertPropertiesToTableRows(topProps, bottomProps, -1, 0));
+	}
+
+	private getPropertyTableColumns() {
+		const columns: Slick.Column<Slick.SlickData>[] = [];
 
-	public addDataToTable() {
-		const columns: Slick.Column<Slick.SlickData>[] = [
-		];
 		if (this._model.topElement) {
 			columns.push({
 				id: 'name',
@@ -82,8 +134,8 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 			});
 			columns.push({
 				id: 'value',
-				name: localize('nodePropertyViewNameValueColumnTopHeader', "Value (Top Plan)"),
+				name: getPropertyViewNameValueColumnTopHeaderForOrientation(this._orientation),
-				field: 'value1',
+				field: 'primary',
 				width: 150,
 				editor: Slick.Editors.Text,
 				headerCssClass: 'prop-table-header',
@@ -93,23 +145,14 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 		if (this._model.bottomElement) {
 			columns.push(new TextWithIconColumn({
 				id: 'value',
-				name: localize('nodePropertyViewNameValueColumnBottomHeader', "Value (Bottom Plan)"),
+				name: getPropertyViewNameValueColumnBottomHeaderForOrientation(this._orientation),
-				field: 'value2',
+				field: 'secondary',
 				width: 150,
 				headerCssClass: 'prop-table-header',
 			}).definition);
 		}
 
-		let topProps = [];
+		return columns;
-		let bottomProps = [];
-		if (this._model.topElement?.properties) {
-			topProps = this._model.topElement.properties;
-		}
-		if (this._model.bottomElement?.properties) {
-			bottomProps = this._model.bottomElement.properties;
-		}
-
-		this.populateTable(columns, this.convertPropertiesToTableRows(topProps, bottomProps, -1, 0));
 	}
 
 	public sortPropertiesAlphabetically(props: Map<string, TablePropertiesMapEntry>): Map<string, TablePropertiesMapEntry> {
@@ -224,17 +267,17 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 							break;
 					}
 				}
-				row['value1'] = {
+				row['primary'] = {
 					text: removeLineBreaks(v.topProp.displayValue, ' ')
 				};
-				row['value2'] = {
+				row['secondary'] = {
 					iconCssClass: diffIconClass,
 					title: removeLineBreaks(v.bottomProp.displayValue, ' ')
 				};
 				if ((topProp && !isString(topProp.value)) || (bottomProp && !isString(bottomProp.value))) {
 					row['name'].style = parentRowCellStyling;
-					row['value1'].style = parentRowCellStyling;
+					row['primary'].style = parentRowCellStyling;
-					row['value2'].iconCssClass += ` parent-row-styling`;
+					row['secondary'].iconCssClass += ` parent-row-styling`;
 				}
 				rows.push(row);
 				if (!isString(topProp.value) && !isString(bottomProp.value)) {
@@ -246,25 +289,25 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 				}
 			} else if (topProp && !bottomProp) {
 				row['displayOrder'] = v.topProp.displayOrder;
-				row['value1'] = {
+				row['primary'] = {
 					text: v.topProp.displayValue
 				};
 				rows.push(row);
 				if (!isString(topProp.value)) {
 					row['name'].style = parentRowCellStyling;
-					row['value1'].style = parentRowCellStyling;
+					row['primary'].style = parentRowCellStyling;
 					this.convertPropertiesToTableRows(topProp.value, undefined, rows.length - 1, indent + 2, rows);
 				}
 			} else if (!topProp && bottomProp) {
 				row['displayOrder'] = v.bottomProp.displayOrder;
-				row['value2'] = {
+				row['secondary'] = {
 					title: v.bottomProp.displayValue,
 					iconCssClass: diffIconClass
 				};
 				rows.push(row);
 				if (!isString(bottomProp.value)) {
 					row['name'].style = parentRowCellStyling;
-					row['value2'].iconCssClass += ` parent-row-styling`;
+					row['secondary'].iconCssClass += ` parent-row-styling`;
 					this.convertPropertiesToTableRows(undefined, bottomProp.value, rows.length - 1, indent + 2, rows);
 				}
 			}
@@ -272,6 +315,32 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 		});
 		return rows;
 	}
+
+	set orientation(value: ExecutionPlanCompareOrientation) {
+		if (this._orientation === value) {
+			return;
+		}
+		this._orientation = value;
+		this.updatePropertyContainerTitles();
+	}
+}
+
+function getPropertyViewNameValueColumnTopHeaderForOrientation(orientation: ExecutionPlanCompareOrientation): string {
+	if (orientation === ExecutionPlanCompareOrientation.Horizontal) {
+		return topTitleColumnHeader;
+	}
+	else {
+		return leftTitleColumnHeader;
+	}
+}
+
+function getPropertyViewNameValueColumnBottomHeaderForOrientation(orientation: ExecutionPlanCompareOrientation): string {
+	if (orientation === ExecutionPlanCompareOrientation.Horizontal) {
+		return bottomTitleColumnHeader;
+	}
+	else {
+		return rightTitleColumnHeader;
+	}
 }
 
 export interface ExecutionPlanComparisonPropertiesViewModel {

src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesView.ts

@@ -33,7 +33,7 @@ export class ExecutionPlanPropertiesView extends ExecutionPlanPropertiesViewBase
 
 	public set graphElement(element: azdata.executionPlan.ExecutionPlanNode | azdata.executionPlan.ExecutionPlanEdge) {
 		this._model.graphElement = element;
-		this.addDataToTable();
+		this.refreshPropertiesTable();
 	}
 
 	public sortPropertiesAlphabetically(props: azdata.executionPlan.ExecutionPlanGraphElementProperty[]): azdata.executionPlan.ExecutionPlanGraphElementProperty[] {
@@ -79,7 +79,7 @@ export class ExecutionPlanPropertiesView extends ExecutionPlanPropertiesViewBase
 		});
 	}
 
-	public addDataToTable(): void {
+	public refreshPropertiesTable(): void {
 		if (this._model.graphElement) {
 			const nodeName = (<azdata.executionPlan.ExecutionPlanNode>this._model.graphElement).name;
 			this._operationName.innerText = nodeName ? removeLineBreaks(nodeName) : localize('executionPlanPropertiesEdgeOperationName', "Edge"); //since edges do not have names like node, we set the operation name to 'Edge'

src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesViewBase.ts

@@ -156,7 +156,7 @@ export abstract class ExecutionPlanPropertiesViewBase implements IVerticalSashLa
 		return this._tableHeight;
 	}
 
-	public abstract addDataToTable();
+	public abstract refreshPropertiesTable();
 
 	public toggleVisibility(): void {
 		this._parentContainer.style.display = this._parentContainer.style.display === 'none' ? 'flex' : 'none';
@@ -169,6 +169,10 @@ export abstract class ExecutionPlanPropertiesViewBase implements IVerticalSashLa
 		this.resizeTable();
 	}
 
+	public updateTableColumns(columns: Slick.Column<Slick.SlickData>[]) {
+		this._tableComponent.columns = columns;
+	}
+
 	private resizeTable(): void {
 		const spaceOccupied = (this._titleBarContainer.getBoundingClientRect().height
 			+ this._headerContainer.getBoundingClientRect().height
@@ -207,7 +211,7 @@ export class SortPropertiesAlphabeticallyAction extends Action {
 
 	public override async run(context: ExecutionPlanPropertiesViewBase): Promise<void> {
 		context.sortType = PropertiesSortType.Alphabetical;
-		context.addDataToTable();
+		context.refreshPropertiesTable();
 	}
 }
 
@@ -221,7 +225,7 @@ export class SortPropertiesReverseAlphabeticallyAction extends Action {
 
 	public override async run(context: ExecutionPlanPropertiesViewBase): Promise<void> {
 		context.sortType = PropertiesSortType.ReverseAlphabetical;
-		context.addDataToTable();
+		context.refreshPropertiesTable();
 	}
 }
 
@@ -235,7 +239,7 @@ export class SortPropertiesByDisplayOrderAction extends Action {
 
 	public override async run(context: ExecutionPlanPropertiesViewBase): Promise<void> {
 		context.sortType = PropertiesSortType.DisplayOrder;
-		context.addDataToTable();
+		context.refreshPropertiesTable();
 	}
 }