From 627d64dffe1c015b3f50af27ee5ef9cc17501a27 Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Sun, 21 Apr 2024 18:08:34 +0900 Subject: [PATCH 1/6] Memoizes column header functions --- .../webapp/metrics/views/card_renderer/BUILD | 1 + .../scalar_card_data_table.ng.html | 4 +- .../card_renderer/scalar_card_data_table.ts | 27 +++++++---- .../webapp/runs/views/runs_table/BUILD | 1 + .../views/runs_table/runs_data_table.ng.html | 4 +- .../runs/views/runs_table/runs_data_table.ts | 47 +++++++++---------- 6 files changed, 47 insertions(+), 37 deletions(-) diff --git a/tensorboard/webapp/metrics/views/card_renderer/BUILD b/tensorboard/webapp/metrics/views/card_renderer/BUILD index 0790e5e8f3..288f052416 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/BUILD +++ b/tensorboard/webapp/metrics/views/card_renderer/BUILD @@ -286,6 +286,7 @@ tf_ng_module( ":utils", "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/runs:types", + "//tensorboard/webapp/util:memoize", "//tensorboard/webapp/widgets/card_fob:types", "//tensorboard/webapp/widgets/data_table", "//tensorboard/webapp/widgets/data_table:types", diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html index 880ce02d9c..f787fb2a85 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html @@ -30,7 +30,7 @@ (loadAllColumns)="loadAllColumns.emit()" > - + - + ).concat( + [ + { + name: 'color', + displayName: '', + type: ColumnHeaderType.COLOR, + enabled: true, + }, + ], + headers + ); } getMinPointInRange( diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index 719cb0bd5b..f425ef4b20 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -107,6 +107,7 @@ tf_ng_module( "//tensorboard/webapp/types:ui", "//tensorboard/webapp/util:colors", "//tensorboard/webapp/util:matcher", + "//tensorboard/webapp/util:memoize", "//tensorboard/webapp/widgets/custom_modal", "//tensorboard/webapp/widgets/data_table", "//tensorboard/webapp/widgets/data_table:filter_dialog", 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 83688127bd..41cf95f2a0 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 @@ -41,7 +41,7 @@ (loadAllColumns)="loadAllColumns.emit()" > - + - + (); @Output() loadAllColumns = new EventEmitter(); - // These columns must be stored and reused to stop needless re-rendering of - // the content and headers in these columns. This has been known to cause - // problems with the controls in these columns, specifically the color picker. - beforeColumns = [ - { - name: 'selected', - displayName: '', - type: ColumnHeaderType.CUSTOM, - enabled: true, - }, - ]; + // Columns must be memoized to stop needless re-rendering of the content and headers in these + // columns. This has been known to cause problems with the controls in these columns, + // specifically the add button. + extendHeaders = memoize(this.internalExtendHeaders); - afterColumns = [ - { - name: 'color', - displayName: '', - type: ColumnHeaderType.COLOR, - enabled: true, - }, - ]; - - getHeaders() { + private internalExtendHeaders(headers: ColumnHeader[]) { return ([] as Array).concat( - this.beforeColumns, - this.headers, - this.afterColumns + [ + { + name: 'selected', + displayName: '', + type: ColumnHeaderType.CUSTOM, + enabled: true, + }, + ], + headers, + [ + { + name: 'color', + displayName: '', + type: ColumnHeaderType.COLOR, + enabled: true, + }, + ] ); } From 24d55042f337276c3ec9039ac5600f9d7619e364 Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Sun, 21 Apr 2024 18:41:05 +0900 Subject: [PATCH 2/6] Fixes test --- .../runs/views/runs_table/runs_data_table_test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 f2b5432481..6f5d9c411b 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 @@ -220,25 +220,25 @@ describe('runs_data_table', () => { }); }); - // If the call to getHeaders produces a new color header on each call, then + // If the call to extendHeaders produces a new color header on each call, then // the content gets re-rendered when the color picker opens. This causes the // color picker modal to close immediately. This test ensures that we keep // the same reference to the color header and continue to use that reference // instead of creating a new one on each call. - it('getHeaders does not produce new color header on each call', () => { + it('extendHeaders does not produce new color header on each call', () => { const fixture = createComponent({}); - const runsDataTable = fixture.debugElement.query( By.directive(RunsDataTable) ); + const {headers} = runsDataTable.componentInstance; expect( runsDataTable.componentInstance - .getHeaders() + .extendHeaders(headers) .find((header: ColumnHeader) => header.name === 'color') ).toBe( runsDataTable.componentInstance - .getHeaders() + .extendHeaders(headers) .find((header: ColumnHeader) => header.name === 'color') ); }); From e11681dd158fe0e35bc7a25dea5d44d389dd69d5 Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Mon, 22 Apr 2024 14:14:29 +0900 Subject: [PATCH 3/6] Moves expand button to layout container and fixes sidebar scroll --- .../webapp/core/views/layout_container.scss | 72 +++++++++++++++---- .../webapp/core/views/layout_container.ts | 28 +++++++- tensorboard/webapp/core/views/layout_test.ts | 2 +- .../runs_selector/runs_selector_component.ts | 7 ++ .../views/runs_table/runs_data_table.ng.html | 13 ---- .../views/runs_table/runs_data_table.scss | 54 ++------------ .../runs/views/runs_table/runs_data_table.ts | 2 - .../views/runs_table/runs_table_container.ts | 9 --- 8 files changed, 95 insertions(+), 92 deletions(-) diff --git a/tensorboard/webapp/core/views/layout_container.scss b/tensorboard/webapp/core/views/layout_container.scss index 57a197257e..8b437f2510 100644 --- a/tensorboard/webapp/core/views/layout_container.scss +++ b/tensorboard/webapp/core/views/layout_container.scss @@ -24,18 +24,34 @@ limitations under the License. .sidebar { max-width: 80vw; + position: relative; } .resizer, -.expand { +.expand-collapsed-sidebar { @include tb-theme-foreground-prop(border-color, border); box-sizing: border-box; flex: 0 0; justify-self: stretch; } -.expand { +.expand-collapsed-sidebar { width: 20px; + align-items: center; + background: transparent; + border-style: solid; + border-width: 0 1px 0 0; + color: inherit; + contain: content; + cursor: pointer; + display: flex; + justify-self: stretch; + padding: 0; + + mat-icon { + transform: rotate(-90deg); + transform-origin: center; + } } .resizer { @@ -63,20 +79,46 @@ limitations under the License. } } -.expand { - align-items: center; - background: transparent; - border-style: solid; - border-width: 0 1px 0 0; - color: inherit; - contain: content; - cursor: pointer; +.full-screen-toggle { + opacity: 0; + position: absolute; + height: 100%; + // Ensure the button is on the right side then add 2px for the drag target. + left: calc(100% + 2px); + top: 0; + z-index: 1; display: flex; - justify-self: stretch; - padding: 0; + align-items: center; - mat-icon { - transform: rotate(-90deg); - transform-origin: center; + &:hover { + opacity: 0.8; + } + + &.full-screen { + left: unset; + right: 0; + } + + .full-screen-btn { + $_arrow_size: 16px; + $_arrow_button_size: calc($_arrow_size + 4px); + + background-color: gray; + padding: 0; + min-width: $_arrow_button_size; + width: $_arrow_button_size; + + &.expand { + border-radius: 0 $_arrow_button_size $_arrow_button_size 0; + } + + &.collapse { + border-radius: $_arrow_button_size 0 0 $_arrow_button_size; + } + + .expand-collapse-icon { + font-size: $_arrow_size; + margin-right: 0; // Removes default + } } } diff --git a/tensorboard/webapp/core/views/layout_container.ts b/tensorboard/webapp/core/views/layout_container.ts index e90276da5b..f48e6303f8 100644 --- a/tensorboard/webapp/core/views/layout_container.ts +++ b/tensorboard/webapp/core/views/layout_container.ts @@ -22,7 +22,7 @@ import {Store} from '@ngrx/store'; import {fromEvent, Observable, Subject} from 'rxjs'; import {combineLatestWith, filter, map, takeUntil} from 'rxjs/operators'; import {MouseEventButtons} from '../../util/dom'; -import {sideBarWidthChanged} from '../actions'; +import {runsTableFullScreenToggled, sideBarWidthChanged} from '../actions'; import {State} from '../state'; import { getRunsTableFullScreen, @@ -34,7 +34,7 @@ import { template: ` +
{ let dispatchedActions: Action[] = []; const byCss = { - EXPANDER: By.css('.expand'), + EXPANDER: By.css('.expand-collapsed-sidebar'), RESIZER: By.css('.resizer'), SIDEBAR_CONTAINER: By.css('nav'), LAYOUT: By.directive(LayoutContainer), diff --git a/tensorboard/webapp/runs/views/runs_selector/runs_selector_component.ts b/tensorboard/webapp/runs/views/runs_selector/runs_selector_component.ts index eb230a3546..db7d189a72 100644 --- a/tensorboard/webapp/runs/views/runs_selector/runs_selector_component.ts +++ b/tensorboard/webapp/runs/views/runs_selector/runs_selector_component.ts @@ -25,6 +25,13 @@ import {RunsTableColumn} from '../runs_table/types'; `, styles: [ ` + :host { + display: block; + height: 100%; + width: 100%; + overflow: auto; + } + runs-table { height: 100%; } 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 41cf95f2a0..1b3dc0416a 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 @@ -106,16 +106,3 @@
-
- -
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss b/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss index 246b95cbe2..acfce22fa1 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss @@ -18,12 +18,11 @@ $_circle-size: 20px; $_arrow_size: 16px; :host { - width: 100%; -} + min-width: 100%; -:host { - overflow-y: auto; - width: 100%; + & ::ng-deep tb-data-table .data-table { + @include tb-theme-foreground-prop(border-top, border, 1px solid); + } } .color-container { @@ -72,49 +71,7 @@ tb-data-table-header-cell:last-of-type { width: 40px; } -.full-screen-toggle { - opacity: 0; - position: absolute; - height: 100%; - // Ensure the button is on the right side then add 2px for the drag target. - left: calc(100% + 2px); - top: 0; - z-index: 1; - display: flex; - align-items: center; - - &:hover { - opacity: 0.8; - } - - &.full-screen { - left: unset; - right: 0; - } - - .full-screen-btn { - background-color: gray; - padding: 0; - min-width: $_arrow_size; - width: $_arrow_size; - - &.expand { - border-radius: 0 $_arrow_size $_arrow_size 0; - } - - &.collapse { - border-radius: $_arrow_size 0 0 $_arrow_size; - } - - .expand-collapse-icon { - font-size: $_arrow_size; - width: $_arrow_size; - } - } -} - .filter-row { - @include tb-theme-foreground-prop(border-bottom, border, 1px solid); display: flex; align-items: center; height: 48px; @@ -125,6 +82,3 @@ tb-data-table-header-cell:last-of-type { flex-grow: 1; } } -.table-container { - overflow-x: auto; -} 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 95a1a2f4aa..57e75098a4 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts @@ -43,7 +43,6 @@ export class RunsDataTable { @Input() sortingInfo!: SortingInfo; @Input() experimentIds!: string[]; @Input() regexFilter!: string; - @Input() isFullScreen!: boolean; @Input() selectableColumns!: ColumnHeader[]; @Input() numColumnsLoaded!: number; @Input() numColumnsToLoad!: number; @@ -57,7 +56,6 @@ export class RunsDataTable { @Output() onSelectionToggle = new EventEmitter(); @Output() onAllSelectionToggle = new EventEmitter(); @Output() onRegexFilterChange = new EventEmitter(); - @Output() toggleFullScreen = new EventEmitter(); @Output() onRunColorChange = new EventEmitter<{ runId: string; newColor: string; 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 375b9c9391..bac141cbc5 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts @@ -45,7 +45,6 @@ import { getRuns, getRunSelectorRegexFilter, getRunsLoadState, - getRunsTableFullScreen, getRunsTableSortingInfo, getGroupedRunsTableHeaders, } from '../../../selectors'; @@ -73,7 +72,6 @@ import { getFilteredRenderableRuns, getSelectableColumns, } from '../../../metrics/views/main_view/common_selectors'; -import {runsTableFullScreenToggled} from '../../../core/actions'; import {sortTableDataItems} from './sorting_utils'; const getRunsLoading = createSelector< @@ -102,7 +100,6 @@ const getRunsLoading = createSelector< [sortingInfo]="sortingInfo$ | async" [experimentIds]="experimentIds" [regexFilter]="regexFilter$ | async" - [isFullScreen]="runsTableFullScreen$ | async" [loading]="loading$ | async" (sortDataBy)="sortDataBy($event)" (orderColumns)="orderColumns($event)" @@ -111,7 +108,6 @@ const getRunsLoading = createSelector< (onRunColorChange)="onRunColorChange($event)" (onRegexFilterChange)="onRegexFilterChange($event)" (onSelectionDblClick)="onRunSelectionDblClick($event)" - (toggleFullScreen)="toggleFullScreen()" (addColumn)="addColumn($event)" (removeColumn)="removeColumn($event)" (addFilter)="addHparamFilter($event)" @@ -147,7 +143,6 @@ export class RunsTableContainer implements OnInit, OnDestroy { regexFilter$ = this.store.select(getRunSelectorRegexFilter); runsColumns$ = this.store.select(getGroupedRunsTableHeaders); - runsTableFullScreen$ = this.store.select(getRunsTableFullScreen); selectableColumns$ = this.store.select(getSelectableColumns); numColumnsLoaded$ = this.store.select( hparamsSelectors.getNumDashboardHparamsLoaded @@ -328,10 +323,6 @@ export class RunsTableContainer implements OnInit, OnDestroy { this.store.dispatch(runColorChanged({runId, newColor})); } - toggleFullScreen() { - this.store.dispatch(runsTableFullScreenToggled()); - } - addColumn({column, nextTo, side}: AddColumnEvent) { this.store.dispatch( hparamsActions.dashboardHparamColumnAdded({ From 47f993debfeebc0b60e2d592a983668b0d8eac89 Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:48:57 +0900 Subject: [PATCH 4/6] Adds add column in data table --- .../scalar_card_data_table.ng.html | 1 + .../card_renderer/scalar_card_data_table.ts | 4 +- .../views/runs_table/runs_data_table.ng.html | 1 + .../views/runs_table/runs_data_table.scss | 13 ---- .../data_table/data_table_component.ng.html | 20 +++--- .../data_table/data_table_component.scss | 63 ++++++++++++++++--- .../data_table/data_table_component.ts | 4 ++ .../webapp/widgets/data_table/types.ts | 5 ++ 8 files changed, 81 insertions(+), 30 deletions(-) diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html index f787fb2a85..a2a49f9530 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html @@ -22,6 +22,7 @@ [selectableColumns]="selectableColumns" [numColumnsLoaded]="numColumnsLoaded" [hasMoreColumnsToLoad]="numColumnsLoaded === numColumnsToLoad" + [addColumnSize]="AddColumnSize.SMALL" (sortDataBy)="sortDataBy.emit($event)" (orderColumns)="onOrderColumns($event)" (addColumn)="addColumn.emit($event)" 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 6764b0a3b5..97602aaecf 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 @@ -41,6 +41,7 @@ import { FilterAddedEvent, ReorderColumnEvent, AddColumnEvent, + AddColumnSize, } from '../../../widgets/data_table/types'; import {isDatumVisible} from './utils'; import {memoize} from '../../../util/memoize'; @@ -73,7 +74,8 @@ export class ScalarCardDataTable { @Output() addFilter = new EventEmitter(); @Output() loadAllColumns = new EventEmitter(); - ColumnHeaderType = ColumnHeaderType; + readonly ColumnHeaderType = ColumnHeaderType; + readonly AddColumnSize = AddColumnSize; // Columns must be memoized to stop needless re-rendering of the content and headers in these // columns. This has been known to cause problems with the controls in these columns, 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 1b3dc0416a..9845b75c60 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 @@ -33,6 +33,7 @@ [hasMoreColumnsToLoad]="numColumnsLoaded === numColumnsToLoad" [columnFilters]="columnFilters" [loading]="loading" + [shouldAddBorders]="true" (sortDataBy)="sortDataBy.emit($event)" (orderColumns)="orderColumns.emit($event)" (addColumn)="addColumn.emit($event)" diff --git a/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss b/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss index acfce22fa1..31534ca835 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss @@ -15,14 +15,9 @@ limitations under the License. @use '@angular/material' as mat; @import 'tensorboard/webapp/theme/tb_theme'; $_circle-size: 20px; -$_arrow_size: 16px; :host { min-width: 100%; - - & ::ng-deep tb-data-table .data-table { - @include tb-theme-foreground-prop(border-top, border, 1px solid); - } } .color-container { @@ -56,14 +51,6 @@ tb-data-table-header-cell { padding: 0 4px; vertical-align: middle; @include tb-theme-foreground-prop(border-bottom, border, 1px solid); - - &:last-child { - @include tb-theme-foreground-prop(border-right, border, 1px solid); - } -} - -tb-data-table-header-cell:last-of-type { - @include tb-theme-foreground-prop(border-right, border, 1px solid); } .table-column-selected, diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html index 9fea50b51f..065c58184b 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html +++ b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html @@ -43,16 +43,23 @@ > -
-
- +
+
+
+
+ +
+ +
+
+
-
diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.scss b/tensorboard/webapp/widgets/data_table/data_table_component.scss index db88c5d54f..83bbd46dfa 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.scss +++ b/tensorboard/webapp/widgets/data_table/data_table_component.scss @@ -17,23 +17,35 @@ limitations under the License. $_accent: map-get(mat.get-color-config($tb-theme), accent); +.data-table-wrapper { + display: flex; + + &.should-add-borders { + .left-section, + .right-section { + @include tb-theme-foreground-prop(border-top, border, 1px solid); + } + + .add-button-column { + @include tb-theme-foreground-prop(border-left, border, 1px solid); + } + } +} + .data-table { font-size: 13px; display: table; width: 100%; - .header { - display: table-row; - z-index: 1; - } - .header { background-color: mat.get-color-from-palette($tb-background, background); + display: table-row; + font-weight: bold; position: sticky; text-align: left; top: 0; - font-weight: bold; vertical-align: bottom; + z-index: 1; &:hover { cursor: pointer; @@ -55,7 +67,40 @@ $_accent: map-get(mat.get-color-config($tb-theme), accent); justify-content: center; } -.add-button-cell { - display: table-cell; - width: 40px; +.left-section { + flex-grow: 1; +} + +.right-section { + background-color: mat.get-color-from-palette($tb-background, background); + position: sticky; + right: -1px; // Prevent fractional width from creating a gap. + z-index: 1; + + @include tb-dark-theme { + background-color: map-get($tb-dark-background, background); + } + + .add-button-column { + width: 40px; + height: 100%; + + .add-button { + position: sticky; + top: 0; + } + } +} + +.right-section .add-button-column.small-add-button { + display: flex; + justify-content: center; + width: 24px; + + .add-button { + // Override very high specificity with !important + // Ref: https://github.com/tensorflow/tensorboard/blob/f2e8bd82a533c175f6dc5be3da65344d719ae0f6/tensorboard/webapp/theme/_tb_theme.template.scss#L320 + width: 20px !important; + height: 20px !important; + } } diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ts b/tensorboard/webapp/widgets/data_table/data_table_component.ts index afc9722202..f4e1f108e1 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.ts +++ b/tensorboard/webapp/widgets/data_table/data_table_component.ts @@ -38,6 +38,7 @@ import { ReorderColumnEvent, Side, AddColumnEvent, + AddColumnSize, } from './types'; import {HeaderCellComponent} from './header_cell_component'; import {Subscription} from 'rxjs'; @@ -67,6 +68,8 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { @Input() hasMoreColumnsToLoad!: boolean; @Input() columnFilters!: Map; @Input() loading: boolean = false; + @Input() shouldAddBorders: boolean = false; + @Input() addColumnSize: AddColumnSize = AddColumnSize.DEFAULT; @ContentChildren(HeaderCellComponent) headerCells!: QueryList; @@ -102,6 +105,7 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { readonly SortingOrder = SortingOrder; readonly Side = Side; + readonly AddColumnSize = AddColumnSize; constructor( private readonly customModal: CustomModal, diff --git a/tensorboard/webapp/widgets/data_table/types.ts b/tensorboard/webapp/widgets/data_table/types.ts index 70efa4bd02..b2d3e29cf0 100644 --- a/tensorboard/webapp/widgets/data_table/types.ts +++ b/tensorboard/webapp/widgets/data_table/types.ts @@ -136,3 +136,8 @@ export enum ColumnGroup { HPARAM = 'HPARAM', OTHER = 'OTHER', } + +export enum AddColumnSize { + DEFAULT = 'DEFAULT', + SMALL = 'SMALL', +} From b38870237b7f4a55a7f43a83178c03218cb4595c Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Mon, 22 Apr 2024 18:12:00 +0900 Subject: [PATCH 5/6] Fixes test --- .../webapp/widgets/data_table/data_table_component.ng.html | 7 +++++-- tensorboard/webapp/widgets/data_table/data_table_test.ts | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html index 065c58184b..32200bbff9 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html +++ b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html @@ -49,10 +49,13 @@
- +
-
+
{ it('does not show add button when there are no selectable columns', () => { const fixture = createComponent({}); - expect(fixture.debugElement.query(By.css('.add-column-button'))).toBeNull(); + expect(fixture.debugElement.query(ADD_BUTTON_PREDICATE)).toBeNull(); }); it('renders column selector when + button is clicked', () => { @@ -612,7 +614,7 @@ describe('data table', () => { expect( fixture.debugElement.query(By.directive(ColumnSelectorComponent)) ).toBeNull(); - const addBtn = fixture.debugElement.query(By.css('.add-column-btn')); + const addBtn = fixture.debugElement.query(ADD_BUTTON_PREDICATE); addBtn.nativeElement.click(); expect( fixture.debugElement.query(By.directive(ColumnSelectorComponent)) From 24d62c4df7207ea767db23564d457c08344224f6 Mon Sep 17 00:00:00 2001 From: Hoonji <736199+hoonji@users.noreply.github.com> Date: Wed, 24 Apr 2024 23:26:42 +0900 Subject: [PATCH 6/6] Addresses review comments --- tensorboard/webapp/theme/_tb_theme.template.scss | 6 ++++-- .../webapp/widgets/data_table/data_table_component.ng.html | 2 +- .../webapp/widgets/data_table/data_table_component.scss | 6 ++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tensorboard/webapp/theme/_tb_theme.template.scss b/tensorboard/webapp/theme/_tb_theme.template.scss index 6bef55d34f..46a748a128 100644 --- a/tensorboard/webapp/theme/_tb_theme.template.scss +++ b/tensorboard/webapp/theme/_tb_theme.template.scss @@ -312,14 +312,16 @@ $tb-dark-theme: map_merge( button.mat-mdc-button-base { --tb-icon-width: 24px; --tb-icon-height: 24px; + --tb-icon-button-width: 40px; + --tb-icon-button-height: 40px; --mdc-text-button-label-text-tracking: normal; --mdc-filled-button-label-text-tracking: normal; --mdc-outlined-button-label-text-tracking: normal; --mdc-protected-button-label-text-tracking: normal; &[mat-icon-button].mat-mdc-icon-button { - width: 40px; - height: 40px; + width: var(--tb-icon-button-width); + height: var(--tb-icon-button-height); display: inline-flex; justify-content: center; align-items: center; diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html index 32200bbff9..c641da6dd3 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html +++ b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html @@ -58,7 +58,7 @@ >