Fixes incorrect operation label when plans are oriented vertically and node selection changes. (#20630)

* Fixes incorrect label when plans are vertical

* Code review changes.

* Adds helper methods to get localized strings

* Helper functions moved to module level

* Renames fields from top and bottom to primary and secondary

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

@@ -208,7 +208,7 @@ export class ExecutionPlanComparisonEditorView {
 				c.style.display = 'none';
 			});
 			this._topPlanDiagramContainers[e.index].style.display = '';
-			this._propertiesView.setTopElement(this._topPlanDiagramModels[e.index].root);
+			this._propertiesView.setPrimaryElement(this._topPlanDiagramModels[e.index].root);
 			this._topPlanRecommendations.recommendations = this._topPlanDiagramModels[e.index].recommendations;
 			this._activeTopPlanIndex = e.index;
 
@@ -233,7 +233,7 @@ export class ExecutionPlanComparisonEditorView {
 				c.style.display = 'none';
 			});
 			this._bottomPlanDiagramContainers[e.index].style.display = '';
-			this._propertiesView.setTopElement(this._bottomPlanDiagramModels[e.index].root);
+			this._propertiesView.setPrimaryElement(this._bottomPlanDiagramModels[e.index].root);
 			this._bottomPlanRecommendations.recommendations = this._bottomPlanDiagramModels[e.index].recommendations;
 			this._activeBottomPlanIndex = e.index;
 
@@ -345,7 +345,7 @@ export class ExecutionPlanComparisonEditorView {
 				this._topPlanContainer.appendChild(graphContainer);
 				const diagram = this._instantiationService.createInstance(AzdataGraphView, graphContainer, e);
 				diagram.onElementSelected(e => {
-					this._propertiesView.setTopElement(e);
+					this._propertiesView.setPrimaryElement(e);
 					const id = e.id.replace(`element-`, '');
 					if (this._topSimilarNode.has(id)) {
 						const similarNode = this._topSimilarNode.get(id);
@@ -364,7 +364,7 @@ export class ExecutionPlanComparisonEditorView {
 				graphContainer.style.display = 'none';
 			});
 			this._topPlanDropdown.select(preSelectIndex);
-			this._propertiesView.setTopElement(executionPlanGraphs[0].root);
+			this._propertiesView.setPrimaryElement(executionPlanGraphs[0].root);
 			this._propertiesAction.enabled = true;
 			this._zoomInAction.enabled = true;
 			this._zoomOutAction.enabled = true;
@@ -385,7 +385,7 @@ export class ExecutionPlanComparisonEditorView {
 				this._bottomPlanContainer.appendChild(graphContainer);
 				const diagram = this._instantiationService.createInstance(AzdataGraphView, graphContainer, e);
 				diagram.onElementSelected(e => {
-					this._propertiesView.setBottomElement(e);
+					this._propertiesView.setSecondaryElement(e);
 					const id = e.id.replace(`element-`, '');
 					if (this._bottomSimilarNode.has(id)) {
 						const similarNode = this._bottomSimilarNode.get(id);
@@ -404,7 +404,7 @@ export class ExecutionPlanComparisonEditorView {
 				graphContainer.style.display = 'none';
 			});
 			this._bottomPlanDropdown.select(preSelectIndex);
-			this._propertiesView.setBottomElement(executionPlanGraphs[0].root);
+			this._propertiesView.setSecondaryElement(executionPlanGraphs[0].root);
 			this._addExecutionPlanAction.enabled = false;
 			this._searchNodeActionForAddedPlan.enabled = true;
 		}

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

@@ -23,6 +23,22 @@ export enum ExecutionPlanCompareOrientation {
 	Vertical = 'vertical'
 }
 
+function getTopOperationLabel(target: string): string {
+	return localize('nodePropertyViewTopOperation', 'Top operation: {0}', target);
+}
+
+function getBottomOperationLabel(target: string): string {
+	return localize('nodePropertyViewBottomOperation', 'Bottom operation: {0}', target);
+}
+
+function getLeftOperationLabel(target: string): string {
+	return localize('nodePropertyViewLeftOperation', 'Left operation: {0}', target);
+}
+
+function getRightOperationLabel(target: string): string {
+	return localize('nodePropertyViewRightOperation', 'Right operation: {0}', target);
+}
+
 const topTitleColumnHeader = localize('nodePropertyViewNameValueColumnTopHeader', "Value (Top Plan)");
 const leftTitleColumnHeader = localize('nodePropertyViewNameValueColumnLeftHeader', "Value (Left Plan)");
 const rightTitleColumnHeader = localize('nodePropertyViewNameValueColumnRightHeader', "Value (Right Plan)");
@@ -53,46 +69,44 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 		this.setHeader(header);
 	}
 
-	public setTopElement(e: InternalExecutionPlanElement): void {
+	public setPrimaryElement(e: InternalExecutionPlanElement): void {
-		this._model.topElement = e;
+		this._model.primaryElement = e;
 		if ((<azdata.executionPlan.ExecutionPlanNode>e).name) {
 			this._primaryTarget = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
 		} else {
 			this._primaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge");
 		}
 
-		let topTitleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", this._primaryTarget);
+		const primaryTitleText = this._orientation === ExecutionPlanCompareOrientation.Horizontal
-		this._primaryContainer.innerText = topTitleText;
+			? getTopOperationLabel(this._primaryTarget)
-		this._primaryContainer.title = topTitleText;
+			: getLeftOperationLabel(this._primaryTarget);
+
+		this._primaryContainer.innerText = primaryTitleText;
+		this._primaryContainer.title = primaryTitleText;
 		this.refreshPropertiesTable();
 	}
 
-	public setBottomElement(e: InternalExecutionPlanElement): void {
+	public setSecondaryElement(e: InternalExecutionPlanElement): void {
-		this._model.bottomElement = e;
+		this._model.secondaryElement = e;
 		if ((<azdata.executionPlan.ExecutionPlanNode>e)?.name) {
 			this._secondaryTarget = removeLineBreaks((<azdata.executionPlan.ExecutionPlanNode>e).name);
 		} else {
 			this._secondaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge");
 		}
 
-		let bottomTitleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", this._secondaryTarget);
+		const secondaryTitleText = this._orientation === ExecutionPlanCompareOrientation.Horizontal
-		this._secondaryContainer.innerText = bottomTitleText;
+			? getBottomOperationLabel(this._secondaryTarget)
-		this._secondaryContainer.title = bottomTitleText;
+			: getRightOperationLabel(this._secondaryTarget);
+
+		this._secondaryContainer.innerText = secondaryTitleText;
+		this._secondaryContainer.title = secondaryTitleText;
 		this.refreshPropertiesTable();
 	}
 
 	private updatePropertyContainerTitles(): void {
-		let primaryTitleText = '';
+		const [primaryTitleText, secondaryTitleText] = this._orientation === ExecutionPlanCompareOrientation.Horizontal
-		let secondaryTitleText = '';
+			? [getTopOperationLabel(this._primaryTarget), getBottomOperationLabel(this._secondaryTarget)]
-
+			: [getLeftOperationLabel(this._primaryTarget), getRightOperationLabel(this._secondaryTarget)];
-		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;
@@ -111,22 +125,22 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 	public refreshPropertiesTable() {
 		const columns: Slick.Column<Slick.SlickData>[] = this.getPropertyTableColumns();
 
-		let topProps = [];
+		let primaryProps = [];
-		let bottomProps = [];
+		let secondaryProps = [];
-		if (this._model.topElement?.properties) {
+		if (this._model.primaryElement?.properties) {
-			topProps = this._model.topElement.properties;
+			primaryProps = this._model.primaryElement.properties;
 		}
-		if (this._model.bottomElement?.properties) {
+		if (this._model.secondaryElement?.properties) {
-			bottomProps = this._model.bottomElement.properties;
+			secondaryProps = this._model.secondaryElement.properties;
 		}
 
-		this.populateTable(columns, this.convertPropertiesToTableRows(topProps, bottomProps, -1, 0));
+		this.populateTable(columns, this.convertPropertiesToTableRows(primaryProps, secondaryProps, -1, 0));
 	}
 
 	private getPropertyTableColumns() {
 		const columns: Slick.Column<Slick.SlickData>[] = [];
 
-		if (this._model.topElement) {
+		if (this._model.primaryElement) {
 			columns.push({
 				id: 'name',
 				name: localize('nodePropertyViewNameNameColumnHeader', "Name"),
@@ -144,7 +158,7 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 				formatter: textFormatter
 			});
 		}
-		if (this._model.bottomElement) {
+		if (this._model.secondaryElement) {
 			columns.push(new TextWithIconColumn({
 				id: 'value2',
 				name: getPropertyViewNameValueColumnBottomHeaderForOrientation(this._orientation),
@@ -198,28 +212,28 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 		}));
 	}
 
-	private convertPropertiesToTableRows(topNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], bottomNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], parentIndex: number, indent: number, rows: { [key: string]: string }[] = []): { [key: string]: string }[] {
+	private convertPropertiesToTableRows(primaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], secondaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], parentIndex: number, indent: number, rows: { [key: string]: string }[] = []): { [key: string]: string }[] {
 		let propertiesMap: Map<string, TablePropertiesMapEntry> = new Map();
 
-		if (topNode) {
+		if (primaryNode) {
-			topNode.forEach(p => {
+			primaryNode.forEach(p => {
 				propertiesMap.set(p.name, {
-					topProp: p,
+					primaryProp: p,
-					bottomProp: undefined,
+					secondaryProp: undefined,
 					displayOrder: p.displayOrder,
 					name: p.name
 				});
 			});
 		}
 
-		if (bottomNode) {
+		if (secondaryNode) {
-			bottomNode.forEach(p => {
+			secondaryNode.forEach(p => {
 				if (propertiesMap.has(p.name)) {
-					propertiesMap.get(p.name).bottomProp = p;
+					propertiesMap.get(p.name).secondaryProp = p;
 				} else {
 					propertiesMap.set(p.name, {
-						topProp: undefined,
+						primaryProp: undefined,
-						bottomProp: p,
+						secondaryProp: p,
 						displayOrder: p.displayOrder,
 						name: p.name
 					});
@@ -246,18 +260,18 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 			};
 			row['parent'] = parentIndex;
 
-			const topProp = v.topProp;
+			const primaryProp = v.primaryProp;
-			const bottomProp = v.bottomProp;
+			const secondaryProp = v.secondaryProp;
 			let diffIconClass = '';
-			if (topProp && bottomProp) {
+			if (primaryProp && secondaryProp) {
-				row['displayOrder'] = v.topProp.displayOrder;
+				row['displayOrder'] = v.primaryProp.displayOrder;
-				if (v.topProp.displayValue !== v.bottomProp.displayValue) {
+				if (v.primaryProp.displayValue !== v.secondaryProp.displayValue) {
-					switch (v.topProp.dataType) {
+					switch (v.primaryProp.dataType) {
 						case sqlExtHostType.executionPlan.ExecutionPlanGraphElementPropertyDataType.Boolean:
 							diffIconClass = executionPlanComparisonPropertiesDifferent;
 							break;
 						case sqlExtHostType.executionPlan.ExecutionPlanGraphElementPropertyDataType.Number:
-							diffIconClass = (parseFloat(v.topProp.displayValue) > parseFloat(v.bottomProp.displayValue)) ? executionPlanComparisonPropertiesDownArrow : executionPlanComparisonPropertiesUpArrow;
+							diffIconClass = (parseFloat(v.primaryProp.displayValue) > parseFloat(v.secondaryProp.displayValue)) ? executionPlanComparisonPropertiesDownArrow : executionPlanComparisonPropertiesUpArrow;
 							break;
 						case sqlExtHostType.executionPlan.ExecutionPlanGraphElementPropertyDataType.String:
 							diffIconClass = executionPlanComparisonPropertiesDifferent;
@@ -268,47 +282,47 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
 					}
 				}
 				row['primary'] = {
-					text: removeLineBreaks(v.topProp.displayValue, ' ')
+					text: removeLineBreaks(v.primaryProp.displayValue, ' ')
 				};
 				row['secondary'] = {
 					iconCssClass: diffIconClass,
-					title: removeLineBreaks(v.bottomProp.displayValue, ' ')
+					title: removeLineBreaks(v.secondaryProp.displayValue, ' ')
 				};
-				if ((topProp && !isString(topProp.value)) || (bottomProp && !isString(bottomProp.value))) {
+				if ((primaryProp && !isString(primaryProp.value)) || (secondaryProp && !isString(secondaryProp.value))) {
 					row['name'].iconCssClass += ` parent-row-styling`;
 					row['primary'].iconCssClass += ` parent-row-styling`;
 					row['secondary'].iconCssClass += ` parent-row-styling`;
 				}
 				rows.push(row);
-				if (!isString(topProp.value) && !isString(bottomProp.value)) {
+				if (!isString(primaryProp.value) && !isString(secondaryProp.value)) {
-					this.convertPropertiesToTableRows(topProp.value, bottomProp.value, rows.length - 1, indent + 2, rows);
+					this.convertPropertiesToTableRows(primaryProp.value, secondaryProp.value, rows.length - 1, indent + 2, rows);
-				} else if (isString(topProp?.value) && !isString(bottomProp.value)) {
+				} else if (isString(primaryProp?.value) && !isString(secondaryProp.value)) {
-					this.convertPropertiesToTableRows(undefined, bottomProp.value, rows.length - 1, indent + 2, rows);
+					this.convertPropertiesToTableRows(undefined, secondaryProp.value, rows.length - 1, indent + 2, rows);
-				} else if (!isString(topProp.value) && !isString(bottomProp.value)) {
+				} else if (!isString(primaryProp.value) && !isString(secondaryProp.value)) {
-					this.convertPropertiesToTableRows(topProp.value, undefined, rows.length - 1, indent + 2, rows);
+					this.convertPropertiesToTableRows(primaryProp.value, undefined, rows.length - 1, indent + 2, rows);
 				}
-			} else if (topProp && !bottomProp) {
+			} else if (primaryProp && !secondaryProp) {
-				row['displayOrder'] = v.topProp.displayOrder;
+				row['displayOrder'] = v.primaryProp.displayOrder;
 				row['primary'] = {
-					text: v.topProp.displayValue
+					text: v.primaryProp.displayValue
 				};
 				rows.push(row);
-				if (!isString(topProp.value)) {
+				if (!isString(primaryProp.value)) {
 					row['name'].iconCssClass += ` parent-row-styling`;
 					row['primary'].iconCssClass += ` parent-row-styling`;
-					this.convertPropertiesToTableRows(topProp.value, undefined, rows.length - 1, indent + 2, rows);
+					this.convertPropertiesToTableRows(primaryProp.value, undefined, rows.length - 1, indent + 2, rows);
 				}
-			} else if (!topProp && bottomProp) {
+			} else if (!primaryProp && secondaryProp) {
-				row['displayOrder'] = v.bottomProp.displayOrder;
+				row['displayOrder'] = v.secondaryProp.displayOrder;
 				row['secondary'] = {
-					title: v.bottomProp.displayValue,
+					title: v.secondaryProp.displayValue,
 					iconCssClass: diffIconClass
 				};
 				rows.push(row);
-				if (!isString(bottomProp.value)) {
+				if (!isString(secondaryProp.value)) {
 					row['name'].iconCssClass += ` parent-row-styling`;
 					row['secondary'].iconCssClass += ` parent-row-styling`;
-					this.convertPropertiesToTableRows(undefined, bottomProp.value, rows.length - 1, indent + 2, rows);
+					this.convertPropertiesToTableRows(undefined, secondaryProp.value, rows.length - 1, indent + 2, rows);
 				}
 			}
 
@@ -345,13 +359,13 @@ function getPropertyViewNameValueColumnBottomHeaderForOrientation(orientation: E
 }
 
 export interface ExecutionPlanComparisonPropertiesViewModel {
-	topElement: InternalExecutionPlanElement,
+	primaryElement: InternalExecutionPlanElement,
-	bottomElement: InternalExecutionPlanElement
+	secondaryElement: InternalExecutionPlanElement
 }
 
 interface TablePropertiesMapEntry {
-	topProp: azdata.executionPlan.ExecutionPlanGraphElementProperty,
+	primaryProp: azdata.executionPlan.ExecutionPlanGraphElementProperty,
-	bottomProp: azdata.executionPlan.ExecutionPlanGraphElementProperty,
+	secondaryProp: azdata.executionPlan.ExecutionPlanGraphElementProperty,
 	displayOrder: number,
 	name: string
 }