From 5fbea7a39c0dec7fb67068b9effaf44aacb1cf90 Mon Sep 17 00:00:00 2001 From: Lewis Sanchez <87730006+lewis-sanchez@users.noreply.github.com> Date: Thu, 7 Jul 2022 13:26:08 -0700 Subject: [PATCH] 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 --- .../executionPlanComparisonEditorView.ts | 10 +- .../executionPlanComparisonPropertiesView.ts | 161 +++++++++++++----- .../browser/executionPlanPropertiesView.ts | 4 +- .../executionPlanPropertiesViewBase.ts | 12 +- 4 files changed, 131 insertions(+), 56 deletions(-) diff --git a/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonEditorView.ts b/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonEditorView.ts index da489a823c..59db618562 100644 --- a/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonEditorView.ts +++ b/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'; } diff --git a/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView.ts b/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView.ts index 6f54ac390a..c1e315333a 100644 --- a/src/sql/workbench/contrib/executionPlan/browser/executionPlanComparisonPropertiesView.ts +++ b/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 _bottomOperationNameContainer: HTMLElement; + private _primaryContainer: 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 = {}; this._parentContainer.style.display = 'none'; const header = DOM.$('.compare-operation-name'); - this._topOperationNameContainer = DOM.$('.compare-operation-name-text'); - header.appendChild(this._topOperationNameContainer); - this._bottomOperationNameContainer = DOM.$('.compare-operation-name-text'); - header.appendChild(this._bottomOperationNameContainer); + this._primaryContainer = DOM.$('.compare-operation-name-text'); + header.appendChild(this._primaryContainer); + this._secondaryContainer = DOM.$('.compare-operation-name-text'); + header.appendChild(this._secondaryContainer); this.setHeader(header); } - public setTopElement(e: InternalExecutionPlanElement): void { this._model.topElement = e; - let target; if ((e).name) { - target = removeLineBreaks((e).name); + this._primaryTarget = removeLineBreaks((e).name); } else { - target = localize('executionPlanPropertiesEdgeOperationName', "Edge"); + this._primaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge"); } - const titleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", target); - this._topOperationNameContainer.innerText = titleText; - this._topOperationNameContainer.title = titleText; - this.addDataToTable(); + + let topTitleText = localize('executionPlanComparisonPropertiesTopOperation', "Top operation: {0}", this._primaryTarget); + this._primaryContainer.innerText = topTitleText; + this._primaryContainer.title = topTitleText; + this.refreshPropertiesTable(); } public setBottomElement(e: InternalExecutionPlanElement): void { this._model.bottomElement = e; - let target; if ((e)?.name) { - target = removeLineBreaks((e).name); + this._secondaryTarget = removeLineBreaks((e).name); } else { - target = localize('executionPlanPropertiesEdgeOperationName', "Edge"); + this._secondaryTarget = localize('executionPlanPropertiesEdgeOperationName', "Edge"); } - const titleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", target); - this._bottomOperationNameContainer.innerText = titleText; - this._bottomOperationNameContainer.title = titleText; - this.addDataToTable(); + let bottomTitleText = localize('executionPlanComparisonPropertiesBottomOperation', "Bottom operation: {0}", this._secondaryTarget); + this._secondaryContainer.innerText = bottomTitleText; + this._secondaryContainer.title = bottomTitleText; + 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[] = this.getPropertyTableColumns(); + + this.updateTableColumns(columns); + } + + public refreshPropertiesTable() { + const columns: Slick.Column[] = 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[] = []; - public addDataToTable() { - const columns: Slick.Column[] = [ - ]; 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)"), - field: 'value1', + name: getPropertyViewNameValueColumnTopHeaderForOrientation(this._orientation), + 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)"), - field: 'value2', + name: getPropertyViewNameValueColumnBottomHeaderForOrientation(this._orientation), + field: 'secondary', width: 150, headerCssClass: 'prop-table-header', }).definition); } - 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)); + return columns; } public sortPropertiesAlphabetically(props: Map): Map { @@ -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['value2'].iconCssClass += ` parent-row-styling`; + row['primary'].style = parentRowCellStyling; + 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 { diff --git a/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesView.ts b/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesView.ts index ae839a2072..7082f647d3 100644 --- a/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesView.ts +++ b/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 = (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' diff --git a/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesViewBase.ts b/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesViewBase.ts index 567e1c5000..3c4f2404a9 100644 --- a/src/sql/workbench/contrib/executionPlan/browser/executionPlanPropertiesViewBase.ts +++ b/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[]) { + 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 { context.sortType = PropertiesSortType.Alphabetical; - context.addDataToTable(); + context.refreshPropertiesTable(); } } @@ -221,7 +225,7 @@ export class SortPropertiesReverseAlphabeticallyAction extends Action { public override async run(context: ExecutionPlanPropertiesViewBase): Promise { context.sortType = PropertiesSortType.ReverseAlphabetical; - context.addDataToTable(); + context.refreshPropertiesTable(); } } @@ -235,7 +239,7 @@ export class SortPropertiesByDisplayOrderAction extends Action { public override async run(context: ExecutionPlanPropertiesViewBase): Promise { context.sortType = PropertiesSortType.DisplayOrder; - context.addDataToTable(); + context.refreshPropertiesTable(); } }