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/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_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/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..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 @@ -41,7 +41,7 @@ (loadAllColumns)="loadAllColumns.emit()" > - + - +
-
- -
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..2f1e9610c1 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.scss @@ -15,15 +15,13 @@ limitations under the License. @use '@angular/material' as mat; @import 'tensorboard/webapp/theme/tb_theme'; $_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 +70,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 +81,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 d4cf74814f..57e75098a4 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_data_table.ts @@ -30,6 +30,7 @@ import { ReorderColumnEvent, AddColumnEvent, } from '../../../widgets/data_table/types'; +import {memoize} from '../../../util/memoize'; @Component({ selector: 'runs-data-table', templateUrl: 'runs_data_table.ng.html', @@ -42,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; @@ -56,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; @@ -67,32 +66,30 @@ export class RunsDataTable { @Output() addFilter = new EventEmitter(); @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, + }, + ] ); } 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') ); }); 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({