diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts index 4e861b4190..0809280416 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts @@ -302,7 +302,9 @@ export class ScalarCardDataTable { } } -function makeValueSortable(value: number | string | null | undefined) { +function makeValueSortable( + value: number | string | boolean | null | undefined +) { if ( Number.isNaN(value) || value === 'NaN' || diff --git a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html index a1adbebef2..fc47374151 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html @@ -29,8 +29,16 @@ [header]="header" [sortingInfo]="sortingInfo" [hparamsEnabled]="true" - [controlsEnabled]="header.type !== ColumnHeaderType.COLOR" - > +
+ +
+ @@ -42,12 +50,17 @@ [header]="header" [datum]="dataRow[header.name]" > -
- -
+ +
+ +
+
+ +
+ diff --git a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts index 00707140a1..bbfcc953f3 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts @@ -41,15 +41,38 @@ export class RunsDataTable { @Output() sortDataBy = new EventEmitter(); @Output() orderColumns = new EventEmitter(); + @Output() onSelectionToggle = new EventEmitter(); + @Output() onAllSelectionToggle = new EventEmitter(); getHeaders() { - return this.headers.concat([ + return [ { - name: 'color', + name: 'selected', displayName: '', - type: ColumnHeaderType.COLOR, + type: ColumnHeaderType.CUSTOM, enabled: true, }, - ]); + ].concat( + this.headers.concat([ + { + name: 'color', + displayName: '', + type: ColumnHeaderType.COLOR, + enabled: true, + }, + ]) + ); + } + + getRunIds() { + return this.data.map((row) => row.id); + } + + allRowsSelected() { + return this.data.every((row) => row.selected); + } + + someRowsSelected() { + return this.data.some((row) => row.selected); } } diff --git a/tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts index a35b3c4163..2791a318f3 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts @@ -18,6 +18,7 @@ import {TestBed} from '@angular/core/testing'; import {RunsDataTable} from './runs_data_table'; import {DataTableModule} from '../../../widgets/data_table/data_table_module'; import {MatIconTestingModule} from '../../../testing/mat_icon_module'; +import {MatCheckboxModule} from '@angular/material/checkbox'; import { SortingOrder, SortingInfo, @@ -39,6 +40,8 @@ import {ContentCellComponent} from '../../../widgets/data_table/content_cell_com [sortingInfo]="sortingInfo" (sortDataBy)="sortDataBy($event)" (orderColumns)="orderColumns($event)" + (onSelectionToggle)="onSelectionToggle($event)" + (onAllSelectionToggle)="onAllSelectionToggle($event)" > `, }) @@ -49,9 +52,14 @@ class TestableComponent { @Input() headers!: ColumnHeader[]; @Input() data!: TableData[]; @Input() sortingInfo!: SortingInfo; + + @Input() onSelectionToggle!: (runId: string) => void; + @Input() onAllSelectionToggle!: (runIds: string[]) => void; } describe('runs_data_table', () => { + let onSelectionToggleSpy: jasmine.Spy; + let onAllSelectionToggleSpy: jasmine.Spy; function createComponent(input: { data?: TableData[]; headers?: ColumnHeader[]; @@ -88,13 +96,19 @@ describe('runs_data_table', () => { order: SortingOrder.ASCENDING, }; + onSelectionToggleSpy = jasmine.createSpy(); + fixture.componentInstance.onSelectionToggle = onSelectionToggleSpy; + + onAllSelectionToggleSpy = jasmine.createSpy(); + fixture.componentInstance.onAllSelectionToggle = onAllSelectionToggleSpy; + fixture.detectChanges(); return fixture; } beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [DataTableModule, MatIconTestingModule], + imports: [DataTableModule, MatIconTestingModule, MatCheckboxModule], declarations: [TestableComponent, RunsDataTable], }).compileComponents(); }); @@ -106,20 +120,21 @@ describe('runs_data_table', () => { ).toBeTruthy(); }); - it('projects enabled headers plus color column', () => { + it('projects enabled headers plus color and selected column', () => { const fixture = createComponent({}); const dataTable = fixture.debugElement.query( By.directive(DataTableComponent) ); const headers = dataTable.queryAll(By.directive(HeaderCellComponent)); - expect(headers.length).toBe(3); - expect(headers[0].componentInstance.header.name).toEqual('run'); - expect(headers[1].componentInstance.header.name).toEqual('other_header'); - expect(headers[2].componentInstance.header.name).toEqual('color'); + expect(headers.length).toBe(4); + expect(headers[0].componentInstance.header.name).toEqual('selected'); + expect(headers[1].componentInstance.header.name).toEqual('run'); + expect(headers[2].componentInstance.header.name).toEqual('other_header'); + expect(headers[3].componentInstance.header.name).toEqual('color'); }); - it('projects content for each enabled header and color column', () => { + it('projects content for each enabled header, selected, and color column', () => { const fixture = createComponent({ data: [{id: 'runid', run: 'run name', color: 'red', other_header: 'foo'}], }); @@ -128,10 +143,11 @@ describe('runs_data_table', () => { ); const cells = dataTable.queryAll(By.directive(ContentCellComponent)); - expect(cells.length).toBe(3); - expect(cells[0].componentInstance.header.name).toEqual('run'); - expect(cells[1].componentInstance.header.name).toEqual('other_header'); - expect(cells[2].componentInstance.header.name).toEqual('color'); + expect(cells.length).toBe(4); + expect(cells[0].componentInstance.header.name).toEqual('selected'); + expect(cells[1].componentInstance.header.name).toEqual('run'); + expect(cells[2].componentInstance.header.name).toEqual('other_header'); + expect(cells[3].componentInstance.header.name).toEqual('color'); }); it('disables controls for color header', () => { @@ -148,4 +164,80 @@ describe('runs_data_table', () => { expect(colorHeader.componentInstance.controlsEnabled).toBe(false); }); + + it('disables controls for selected header', () => { + const fixture = createComponent({}); + + const dataTable = fixture.debugElement.query( + By.directive(DataTableComponent) + ); + const headers = dataTable.queryAll(By.directive(HeaderCellComponent)); + + const selectedHeader = headers.find( + (h) => h.componentInstance.header.name === 'selected' + )!; + + expect(selectedHeader.componentInstance.controlsEnabled).toBe(false); + }); + + it('adds checkbox to selected column', () => { + const fixture = createComponent({}); + + const dataTable = fixture.debugElement.query( + By.directive(DataTableComponent) + ); + const headers = dataTable.queryAll(By.directive(HeaderCellComponent)); + const cells = dataTable.queryAll(By.directive(ContentCellComponent)); + + const selectedHeader = headers.find( + (h) => h.componentInstance.header.name === 'selected' + )!; + + const selectedContentCells = cells.filter((cell) => { + return cell.componentInstance.header.name === 'selected'; + }); + + expect(selectedHeader.query(By.css('mat-checkbox'))).toBeTruthy(); + selectedContentCells.forEach((cell) => { + expect(cell.query(By.css('mat-checkbox'))).toBeTruthy(); + }); + }); + + it('emits onAllSelectionToggle event when selected header checkbox is clicked', () => { + const fixture = createComponent({}); + + const dataTable = fixture.debugElement.query( + By.directive(DataTableComponent) + ); + const headers = dataTable.queryAll(By.directive(HeaderCellComponent)); + + const selectedHeader = headers.find( + (h) => h.componentInstance.header.name === 'selected' + )!; + + const selectedCheckbox = selectedHeader.query(By.css('mat-checkbox')); + + selectedCheckbox.nativeElement.dispatchEvent(new MouseEvent('click')); + + expect(onAllSelectionToggleSpy).toHaveBeenCalledWith(['runid']); + }); + + it('emits onSelectionToggle event when selected content checkbox is clicked', () => { + const fixture = createComponent({}); + + const dataTable = fixture.debugElement.query( + By.directive(DataTableComponent) + ); + const cells = dataTable.queryAll(By.directive(ContentCellComponent)); + + const selectedContentCells = cells.filter((cell) => { + return cell.componentInstance.header.name === 'selected'; + }); + + const firstCheckbox = selectedContentCells[0].query(By.css('mat-checkbox')); + + firstCheckbox.nativeElement.dispatchEvent(new Event('change')); + + expect(onSelectionToggleSpy).toHaveBeenCalledWith('runid'); + }); }); diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts index bcb58f745f..185ee83694 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts @@ -221,7 +221,7 @@ function matchFilter( [regexFilter]="regexFilter$ | async" [sortOption]="sortOption$ | async" [usePagination]="usePagination" - (onSelectionToggle)="onRunSelectionToggle($event)" + (onSelectionToggle)="onRunItemSelectionToggle($event)" (onSelectionDblClick)="onRunSelectionDblClick($event)" (onPageSelectionToggle)="onPageSelectionToggle($event)" (onPaginationChange)="onPaginationChange($event)" @@ -239,6 +239,8 @@ function matchFilter( [sortingInfo]="sortingInfo$ | async" (sortDataBy)="sortDataBy($event)" (orderColumns)="orderColumns($event)" + (onSelectionToggle)="onRunSelectionToggle($event)" + (onAllSelectionToggle)="onAllSelectionToggle($event)" > `, host: { @@ -579,15 +581,18 @@ export class RunsTableContainer implements OnInit, OnDestroy { return combineLatest([ this.store.select(getRuns, {experimentId}), this.store.select(getRunColorMap), + this.store.select(getCurrentRouteRunSelection), this.runsColumns$, this.runToHParamValues$, ]).pipe( - map(([runs, colorMap, runsColumns, runToHParamValues]) => { + map(([runs, colorMap, selectionMap, runsColumns, runToHParamValues]) => { return runs.map((run) => { const tableData: TableData = { id: run.id, color: colorMap[run.id], + selected: Boolean(selectionMap?.get(run.id)), }; + runsColumns.forEach((column) => { switch (column.type) { case ColumnHeaderType.RUN: @@ -646,7 +651,7 @@ export class RunsTableContainer implements OnInit, OnDestroy { ); } - onRunSelectionToggle(item: RunTableItem) { + onRunItemSelectionToggle(item: RunTableItem) { this.store.dispatch( runSelectionToggled({ runId: item.run.id, @@ -654,6 +659,14 @@ export class RunsTableContainer implements OnInit, OnDestroy { ); } + onRunSelectionToggle(id: string) { + this.store.dispatch( + runSelectionToggled({ + runId: id, + }) + ); + } + onRunSelectionDblClick(item: RunTableItem) { // Note that a user's double click will trigger both 'change' and 'dblclick' // events so onRunSelectionToggle() will also be called and we will fire @@ -674,6 +687,14 @@ export class RunsTableContainer implements OnInit, OnDestroy { ); } + onAllSelectionToggle(runIds: string[]) { + this.store.dispatch( + runPageSelectionToggled({ + runIds, + }) + ); + } + // When `usePagination` is false, page selection affects the single page, // containing all items. onPageSelectionToggle(event: {items: RunTableItem[]}) { diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts index 5285a91692..d2728fe65f 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts @@ -3194,7 +3194,7 @@ describe('runs_table', () => { ).toBeFalsy(); }); - it('passes run name and color to data table', () => { + it('passes run name, selected value, and color to data table', () => { // To make sure we only return the runs when called with the right props. const selectSpy = spyOn(store, 'select').and.callThrough(); selectSpy @@ -3221,6 +3221,14 @@ describe('runs_table', () => { book2: '#111', }); + store.overrideSelector( + getCurrentRouteRunSelection, + new Map([ + ['book1', true], + ['book2', false], + ]) + ); + const fixture = createComponent(['book']); fixture.detectChanges(); const runsDataTable = fixture.debugElement.query( @@ -3228,11 +3236,42 @@ describe('runs_table', () => { ); expect(runsDataTable.componentInstance.data).toEqual([ - {id: 'book1', color: '#000', run: "The Philosopher's Stone"}, - {id: 'book2', color: '#111', run: 'The Chamber Of Secrets'}, + { + id: 'book1', + color: '#000', + run: "The Philosopher's Stone", + selected: true, + }, + { + id: 'book2', + color: '#111', + run: 'The Chamber Of Secrets', + selected: false, + }, ]); }); + it('passes selected value of false if run is not in selectionMap', () => { + // To make sure we only return the runs when called with the right props. + const selectSpy = spyOn(store, 'select').and.callThrough(); + selectSpy + .withArgs(getRuns, {experimentId: 'book'}) + .and.returnValue(of([buildRun({id: 'book1'})])); + + store.overrideSelector( + getCurrentRouteRunSelection, + new Map([['otherbook', true]]) + ); + + const fixture = createComponent(['book']); + fixture.detectChanges(); + const runsDataTable = fixture.debugElement.query( + By.directive(RunsDataTable) + ); + + expect(runsDataTable.componentInstance.data[0].selected).toEqual(false); + }); + it('passes hparam values to data table', () => { const run1 = buildRun({id: 'book1', name: "The Philosopher's Stone"}); const run2 = buildRun({id: 'book2', name: 'The Chamber Of Secrets'}); @@ -3277,10 +3316,8 @@ describe('runs_table', () => { By.directive(RunsDataTable) ); - expect(runsDataTable.componentInstance.data).toEqual([ - {id: 'book1', color: '#000', batch_size: 1}, - {id: 'book2', color: '#111', batch_size: 2}, - ]); + expect(runsDataTable.componentInstance.data[0].batch_size).toEqual(1); + expect(runsDataTable.componentInstance.data[1].batch_size).toEqual(2); }); }); }); diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.scss b/tensorboard/webapp/widgets/data_table/data_table_component.scss index 2303675c27..aa4d727d3c 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.scss +++ b/tensorboard/webapp/widgets/data_table/data_table_component.scss @@ -27,10 +27,6 @@ $_accent: map-get(mat.get-color-config($tb-theme), accent); display: table-row; } - .col { - display: table-cell; - } - .header { background-color: mat.get-color-from-palette($tb-background, background); position: sticky; diff --git a/tensorboard/webapp/widgets/data_table/types.ts b/tensorboard/webapp/widgets/data_table/types.ts index f17dcbb072..46b67b9103 100644 --- a/tensorboard/webapp/widgets/data_table/types.ts +++ b/tensorboard/webapp/widgets/data_table/types.ts @@ -39,6 +39,7 @@ export enum ColumnHeaderType { MEAN = 'MEAN', RAW_CHANGE = 'RAW_CHANGE', HPARAM = 'HPARAM', + CUSTOM = 'CUSTOM', } export interface ColumnHeader { @@ -68,7 +69,7 @@ export interface SortingInfo { * DataTable. It will have a value for each required ColumnHeader for a given * run. */ -export type TableData = Record & { +export type TableData = Record & { id: string; };