From 61ceb72ceafd5a3aa2ff072ce1f54b177f5300af Mon Sep 17 00:00:00 2001 From: Amir Omidi Date: Fri, 11 Sep 2020 13:44:19 -0700 Subject: [PATCH] Change tables to make them work for our scenario (#12193) * Change tables to make them work for our scenario * Comments & deprecate API * Disable selections by default --- .../machine-learning/src/test/views/utils.ts | 1 + extensions/notebook/src/test/common.ts | 1 + .../managePackagesDialog.test.ts | 1 + .../src/dialogs/publishDatabaseDialog.ts | 2 +- .../assessmentResults/sqlDatabasesTree.ts | 59 ++++++++-- src/sql/azdata.d.ts | 5 +- src/sql/azdata.proposed.d.ts | 28 ++++- .../workbench/api/common/extHostModelView.ts | 24 ++++ .../browser/modelComponents/componentBase.ts | 11 ++ .../declarativeTable.component.html | 61 ++++++---- .../declarativeTable.component.ts | 105 +++++++++++++++--- .../media/declarativeTable.css | 4 + 12 files changed, 253 insertions(+), 49 deletions(-) diff --git a/extensions/machine-learning/src/test/views/utils.ts b/extensions/machine-learning/src/test/views/utils.ts index 711461abf1..58aa31785b 100644 --- a/extensions/machine-learning/src/test/views/utils.ts +++ b/extensions/machine-learning/src/test/views/utils.ts @@ -104,6 +104,7 @@ export function createViewContext(): ViewTestContext { }); let declarativeTable: () => azdata.DeclarativeTableComponent = () => Object.assign({}, componentBase, { onDataChanged: undefined!, + onRowSelected: undefined!, data: [], columns: [] }); diff --git a/extensions/notebook/src/test/common.ts b/extensions/notebook/src/test/common.ts index c2ae75d67d..9f48dfaea9 100644 --- a/extensions/notebook/src/test/common.ts +++ b/extensions/notebook/src/test/common.ts @@ -346,6 +346,7 @@ class TestDeclarativeTableComponent extends TestComponentBase implements azdata. super(); } onDataChanged: vscode.Event = this.onClick.event; + onRowSelected: vscode.Event = this.onClick.event; data: any[][]; columns: azdata.DeclarativeTableColumn[]; } diff --git a/extensions/notebook/src/test/managePackages/managePackagesDialog.test.ts b/extensions/notebook/src/test/managePackages/managePackagesDialog.test.ts index d82d28ab6f..db625f57ff 100644 --- a/extensions/notebook/src/test/managePackages/managePackagesDialog.test.ts +++ b/extensions/notebook/src/test/managePackages/managePackagesDialog.test.ts @@ -180,6 +180,7 @@ describe('Manage Package Dialog', () => { }); let declarativeTable: () => azdata.DeclarativeTableComponent = () => Object.assign({}, componentBase, { onDataChanged: undefined!, + onRowSelected: undefined!, data: [], columns: [] }); diff --git a/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts b/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts index ca9d49e0b5..b6c90824d3 100644 --- a/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts +++ b/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts @@ -416,7 +416,7 @@ export class PublishDatabaseDialog { table.onDataChanged(() => { this.sqlCmdVars = {}; - table.data.forEach((row) => { + table.data?.forEach((row) => { (>this.sqlCmdVars)[row[0]] = row[1]; }); diff --git a/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts b/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts index 8405c4cb20..ac0c5eee1b 100644 --- a/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts +++ b/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts @@ -16,32 +16,77 @@ export class SqlDatabaseTree extends AssessmentDialogComponent { private createTableComponent(view: azdata.ModelView): azdata.DeclarativeTableComponent { - const table = view.modelBuilder.declarativeTable().withProperties( + const style: azdata.CssStyles = { + 'border': 'none', + 'text-align': 'left' + }; + + const table = view.modelBuilder.declarativeTable().withProps( { + selectEffect: true, columns: [ + { + displayName: '', + valueType: azdata.DeclarativeDataType.boolean, + width: 5, + isReadOnly: false, + showCheckAll: true, + headerCssStyles: style, + ariaLabel: 'Database Migration Check' // TODO localize + }, { displayName: 'Database', // TODO localize valueType: azdata.DeclarativeDataType.string, width: 50, isReadOnly: true, - showCheckAll: true + headerCssStyles: style }, { displayName: '', // Incidents valueType: azdata.DeclarativeDataType.string, width: 5, isReadOnly: true, - showCheckAll: false + headerCssStyles: style, + ariaLabel: 'Issue Count' // TODO localize } ], - data: [ - ['DB1', '1'], - ['DB2', '0'] + dataValues: [ + [ + { + value: false, + style + }, + { + value: 'DB1', + style + }, + { + value: 1, + style + } + ], + [ + { + value: true, + style + }, + { + value: 'DB2', + style + }, + { + value: 2, + style + } + ] ], - width: '200px' } ); + table.component().onRowSelected(({ row }) => { + console.log(row); + }); + return table.component(); } } diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index 97a76c58ba..2c6d704bd3 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -3306,7 +3306,10 @@ declare module 'azdata' { } export interface DeclarativeTableProperties { - data: any[][]; + /** + * @deprecated Use dataValues instead. + */ + data?: any[][]; columns: DeclarativeTableColumn[]; } diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index 620b9aed21..a06d1462da 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -203,17 +203,26 @@ declare module 'azdata' { } export interface DeclarativeTableColumn { - headerCssStyles?: { [key: string]: string }; - rowCssStyles?: { [key: string]: string }; + headerCssStyles?: CssStyles; + rowCssStyles?: CssStyles; ariaLabel?: string; showCheckAll?: boolean; isChecked?: boolean; } + export enum DeclarativeDataType { component = 'component' } + export type DeclarativeTableRowSelectedEvent = { + row: number + }; + + export interface DeclarativeTableComponent extends Component, DeclarativeTableProperties { + onRowSelected: vscode.Event; + } + /* * Add optional azureAccount for connectionWidget. */ @@ -295,6 +304,21 @@ declare module 'azdata' { } export interface DeclarativeTableProperties extends ComponentProperties { + /** + * dataValues will only be used if data is an empty array + */ + dataValues?: DeclarativeTableCellValue[][]; + + /** + * Should the table react to user selections + */ + selectEffect?: boolean; // Defaults to false + } + + export interface DeclarativeTableCellValue { + value: string | number | boolean; + ariaLabel?: string; + style?: CssStyles } export interface ComponentProperties { diff --git a/src/sql/workbench/api/common/extHostModelView.ts b/src/sql/workbench/api/common/extHostModelView.ts index 0fbbe18ccb..d10f11b195 100644 --- a/src/sql/workbench/api/common/extHostModelView.ts +++ b/src/sql/workbench/api/common/extHostModelView.ts @@ -1462,15 +1462,26 @@ class DeclarativeTableWrapper extends ComponentWrapper implements azdata.Declara super(proxy, handle, ModelComponentTypes.DeclarativeTable, id); this.properties = {}; this._emitterMap.set(ComponentEventType.onDidChange, new Emitter()); + this._emitterMap.set(ComponentEventType.onDidClick, new Emitter()); + } public get data(): any[][] { return this.properties['data']; } + public set data(v: any[][]) { this.setProperty('data', v); } + public get dataValues(): azdata.DeclarativeTableCellValue[][] { + return this.properties['dataValues']; + } + + public set dataValues(v: azdata.DeclarativeTableCellValue[][]) { + this.setProperty('dataValues', v); + } + public get columns(): azdata.DeclarativeTableColumn[] { return this.properties['columns']; } @@ -1484,10 +1495,23 @@ class DeclarativeTableWrapper extends ComponentWrapper implements azdata.Declara return emitter && emitter.event; } + public get onRowSelected(): vscode.Event { + let emitter = this._emitterMap.get(ComponentEventType.onDidClick); + return emitter && emitter.event; + } + protected notifyPropertyChanged(): Thenable { return this._proxy.$setProperties(this._handle, this._id, this.getPropertiesForMainThread()); } + public get selectEffect(): boolean | undefined { + return this.properties['selectEffect']; + } + + public set selectEffect(v: boolean | undefined) { + this.setProperty('selectEffect', v); + } + public toComponentShape(): IComponentShape { // Overridden to ensure we send the correct properties mapping. return { diff --git a/src/sql/workbench/browser/modelComponents/componentBase.ts b/src/sql/workbench/browser/modelComponents/componentBase.ts index e13e4fea80..fda779102c 100644 --- a/src/sql/workbench/browser/modelComponents/componentBase.ts +++ b/src/sql/workbench/browser/modelComponents/componentBase.ts @@ -372,6 +372,17 @@ export abstract class ContainerBase { + if (current) { + return Object.assign(previous, current); + } + return previous; + }, {}); + + return x; + } + protected onItemsUpdated(): void { } diff --git a/src/sql/workbench/browser/modelComponents/declarativeTable.component.html b/src/sql/workbench/browser/modelComponents/declarativeTable.component.html index e0fa7f2e3a..bb74d930cc 100644 --- a/src/sql/workbench/browser/modelComponents/declarativeTable.component.html +++ b/src/sql/workbench/browser/modelComponents/declarativeTable.component.html @@ -1,26 +1,45 @@ - +
- + - - - - - - - - + + + + + + + -
- {{column.displayName}} - - + {{column.displayName}} + +
- - - - - {{cellData}} - -
+ + + + + + + + + {{cellData.value}} + + + +
+ + diff --git a/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts b/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts index cfb0c40096..4759e28586 100644 --- a/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts +++ b/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts @@ -30,12 +30,13 @@ export enum DeclarativeDataType { selector: 'modelview-declarativeTable', templateUrl: decodeURI(require.toUrl('./declarativeTable.component.html')) }) -export default class DeclarativeTableComponent extends ContainerBase implements IComponent, OnDestroy, AfterViewInit { +export default class DeclarativeTableComponent extends ContainerBase implements IComponent, OnDestroy, AfterViewInit { @Input() descriptor: IComponentDescriptor; @Input() modelStore: IModelStore; - private data: any[][] = []; + private _data: azdata.DeclarativeTableCellValue[][] = []; private columns: azdata.DeclarativeTableColumn[] = []; + private _selectedRow: number; constructor( @Inject(forwardRef(() => ChangeDetectorRef)) changeRef: ChangeDetectorRef, @@ -77,7 +78,12 @@ export default class DeclarativeTableComponent extends ContainerBase implem public isChecked(rowIdx: number, colIdx: number): boolean { let cellData = this.data[rowIdx][colIdx]; - return cellData; + if (cellData?.value === false) { + return false; + } + // Disabling it to check for null and undefined. + // eslint-disable-next-line eqeqeq + return cellData != undefined; } public onInputBoxChanged(e: string, rowIdx: number, colIdx: number): void { @@ -86,10 +92,11 @@ export default class DeclarativeTableComponent extends ContainerBase implem public onCheckBoxChanged(e: boolean, rowIdx: number, colIdx: number): void { this.onCellDataChanged(e, rowIdx, colIdx); + // If all of the rows in that column are now checked, let's update the header. if (this.columns[colIdx].showCheckAll) { if (e) { for (let rowIdx = 0; rowIdx < this.data.length; rowIdx++) { - if (!this.data[rowIdx][colIdx]) { + if (this.data[rowIdx][colIdx].value === false) { return; } } @@ -102,20 +109,20 @@ export default class DeclarativeTableComponent extends ContainerBase implem public onHeaderCheckBoxChanged(e: boolean, colIdx: number): void { this.columns[colIdx].isChecked = e; this.data.forEach((row, rowIdx) => { - if (row[colIdx] !== e) { + if (row[colIdx].value !== e) { this.onCellDataChanged(e, rowIdx, colIdx); } }); this._changeRef.detectChanges(); } - public trackByFnCols(index: number, item: any): any { + public trackByFnCols(index: number, _item: any): number { return index; } public onSelectBoxChanged(e: ISelectData | string, rowIdx: number, colIdx: number): void { - let column: azdata.DeclarativeTableColumn = this.columns[colIdx]; + if (column.categoryValues) { if (typeof e === 'string') { let category = find(column.categoryValues, c => c.displayName === e); @@ -130,8 +137,8 @@ export default class DeclarativeTableComponent extends ContainerBase implem } } - private onCellDataChanged(newValue: any, rowIdx: number, colIdx: number): void { - this.data[rowIdx][colIdx] = newValue; + private onCellDataChanged(newValue: string | number | boolean | any, rowIdx: number, colIdx: number): void { + this.data[rowIdx][colIdx].value = newValue; this.setPropertyFromUI((props, value) => props.data = value, this.data); let newCellData: azdata.TableCell = { row: rowIdx, @@ -177,11 +184,11 @@ export default class DeclarativeTableComponent extends ContainerBase implem let column: azdata.DeclarativeTableColumn = this.columns[colIdx]; let cellData = this.data[rowIdx][colIdx]; if (cellData && column.categoryValues) { - let category = find(column.categoryValues, v => v.name === cellData); + let category = find(column.categoryValues, v => v.name === cellData.value); if (category) { return category.displayName; } else if (this.isEditableSelectBox(colIdx)) { - return cellData; + return String(cellData.value); } else { return undefined; } @@ -192,7 +199,19 @@ export default class DeclarativeTableComponent extends ContainerBase implem public getAriaLabel(rowIdx: number, colIdx: number): string { const cellData = this.data[rowIdx][colIdx]; - return this.isLabel(colIdx) ? (cellData && cellData !== '' ? cellData : localize('blankValue', "blank")) : ''; + if (this.isLabel(colIdx)) { + if (cellData) { + if (cellData.ariaLabel) { + return cellData.ariaLabel; + } else if (cellData.value) { + return String(cellData.value); + } + } else { + return localize('blankValue', "blank"); + } + } + + return ''; } public getItemDescriptor(componentId: string): IComponentDescriptor { @@ -206,12 +225,34 @@ export default class DeclarativeTableComponent extends ContainerBase implem this.layout(); } - public setProperties(properties: { [key: string]: any; }): void { - const newData = properties.data ?? []; + private static ACCEPTABLE_VALUES = new Set(['number', 'string', 'boolean']); + public setProperties(properties: azdata.DeclarativeTableProperties): void { + const basicData: any[][] = properties.data ?? []; + const complexData: azdata.DeclarativeTableCellValue[][] = properties.dataValues; + let finalData: azdata.DeclarativeTableCellValue[][]; + + finalData = basicData.map(row => { + return row.map((value): azdata.DeclarativeTableCellValue => { + if (DeclarativeTableComponent.ACCEPTABLE_VALUES.has(typeof (value))) { + return { + value: value + }; + } else { + return { + value: JSON.stringify(value) + }; + } + }); + }); + + if (finalData.length <= 0) { + finalData = complexData; + } + this.columns = properties.columns ?? []; // check whether the data property is changed before actually setting the properties. - const isDataPropertyChanged = !arrayEquals(this.data, newData ?? [], (a, b) => { + const isDataPropertyChanged = !arrayEquals(this.data, finalData ?? [], (a, b) => { return arrayEquals(a, b); }); @@ -221,15 +262,45 @@ export default class DeclarativeTableComponent extends ContainerBase implem // so that the events can be passed upwards through the control hierarchy. if (isDataPropertyChanged) { this.clearContainer(); - this.data = newData; + this._data = finalData; this.data?.forEach(row => { for (let i = 0; i < row.length; i++) { if (this.isComponent(i)) { - this.addToContainer(this.getItemDescriptor(row[i] as string), undefined); + this.addToContainer(this.getItemDescriptor(row[i].value as string), undefined); } } }); } super.setProperties(properties); } + + public get data(): azdata.DeclarativeTableCellValue[][] { + return this._data; + } + + public isRowSelected(row: number): boolean { + // Only react when the user wants you to + if (this.getProperties().selectEffect !== true) { + return false; + } + return this._selectedRow === row; + } + + public onCellClick(row: number) { + // Only react when the user wants you to + if (this.getProperties().selectEffect !== true) { + return; + } + if (!this.isRowSelected(row)) { + this._selectedRow = row; + this._changeRef.detectChanges(); + + this.fireEvent({ + eventType: ComponentEventType.onDidClick, + args: { + row + } + }); + } + } } diff --git a/src/sql/workbench/browser/modelComponents/media/declarativeTable.css b/src/sql/workbench/browser/modelComponents/media/declarativeTable.css index 9eba631d91..0be2c7ec1c 100644 --- a/src/sql/workbench/browser/modelComponents/media/declarativeTable.css +++ b/src/sql/workbench/browser/modelComponents/media/declarativeTable.css @@ -26,3 +26,7 @@ width: 100%; text-align: center; } + +.declarative-table-row.selected { + background-color: rgb(0, 120, 215); +}