[Runs table] Moves expand button to layout container and fixes sideba…

…r scroll (#6837) ## Motivation for features / changes - Moving the expand button out of the runs table is required to make it compatible with the upcoming sticky add column b/332788091 - When the table is both vertically long and horizontally wide, it's very difficult to scroll horizontally: users must scroll all the way to the bottom of the table in order to access the horizontal scroll bar. b/332720882 ## Technical description of changes - Moves the "expand button" from inside the runs data table to the LayoutContainer (in the side bar). - Moves the "overflow" css property from the runs table to RunsSelectorComponent (the runs table wrapper) - Makes the runs table header sticky. This was already set, but wasn't being applied due to a missing "position: relative" parent. ## Screenshots of UI changes (or N/A) sticky header: ![image](https://github.com/tensorflow/tensorboard/assets/736199/f2569648-6395-4411-9302-f626b6bd5e9f) expand table button: ![image](https://github.com/tensorflow/tensorboard/assets/736199/35b2e624-f015-41dc-9114-0cab63b70eef)
tensorflow · Apr 29, 2024 · e9d0009 · e9d0009
1 parent 4023658
commit e9d0009
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 93 deletions.
diff --git 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
@@ -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: `
     <button
       *ngIf="(width$ | async) === 0"
-      class="expand"
+      class="expand-collapsed-sidebar"
       (click)="expandSidebar()"
     >
       <mat-icon svgIcon="expand_more_24px"></mat-icon>
@@ -47,6 +47,26 @@ import {
       [style.maxWidth.%]="(runsTableFullScreen$ | async) ? 100 : ''"
     >
       <ng-content select="[sidebar]"></ng-content>
+      <div
+        class="full-screen-toggle"
+        [ngClass]="{'full-screen': (runsTableFullScreen$ | async)}"
+      >
+        <button
+          mat-button
+          class="full-screen-btn"
+          [ngClass]="(runsTableFullScreen$ | async) ? 'collapse' : 'expand'"
+          (click)="toggleFullScreen()"
+        >
+          <mat-icon
+            class="expand-collapse-icon"
+            [svgIcon]="
+              (runsTableFullScreen$ | async)
+                ? 'arrow_back_24px'
+                : 'arrow_forward_24px'
+            "
+          ></mat-icon>
+        </button>
+      </div>
     </nav>
     <div
       *ngIf="(width$ | async) > 0"
@@ -125,4 +145,8 @@ export class LayoutContainer implements OnDestroy {
       })
     );
   }
+
+  toggleFullScreen() {
+    this.store.dispatch(runsTableFullScreenToggled());
+  }
 }
diff --git a/tensorboard/webapp/core/views/layout_test.ts b/tensorboard/webapp/core/views/layout_test.ts
@@ -67,7 +67,7 @@ describe('layout test', () => {
   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
@@ -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
@@ -106,16 +106,3 @@
     </ng-container>
   </tb-data-table>
 </div>
-<div class="full-screen-toggle" [ngClass]="{'full-screen': isFullScreen}">
-  <button
-    mat-button
-    class="full-screen-btn"
-    [ngClass]="isFullScreen ? 'collapse' : 'expand'"
-    (click)="toggleFullScreen.emit()"
-  >
-    <mat-icon
-      class="expand-collapse-icon"
-      [svgIcon]="isFullScreen ? 'arrow_back_24px' : 'arrow_forward_24px'"
-    ></mat-icon>
-  </button>
-</div>
diff --git 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
@@ -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<string>();
   @Output() onAllSelectionToggle = new EventEmitter<string[]>();
   @Output() onRegexFilterChange = new EventEmitter<string>();
-  @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
@@ -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({