Skip to content

Commit

Permalink
Makes data table add column sticky (#6839)
Browse files Browse the repository at this point in the history
## Motivation for features / changes
The add column button in the runs table will be hidden when the table is
wide, which will often be the case. This makes the "add" feature very
difficult to discover.

context: b/332788091

## Technical description of changes
- Moves the add button column outside of and adjacent to the data table
(previously it was a column of the table) and makes it sticky
- Adds additional configuration options to data table to allow runs and
scalar tables to have slightly different designs

## Screenshots of UI changes (or N/A)
Notice the add column is fixed to the right:

![sticky](https://github.com/tensorflow/tensorboard/assets/736199/0f7aca5c-0903-438a-bafd-ddc033e262f7)

Scalar table add column is slightly narrower:

![image](https://github.com/tensorflow/tensorboard/assets/736199/e8c37505-4c3f-41bd-98f3-3b31f99e206e)

## Alternate designs / implementations considered (or N/A)
- Tried adding the add columns to the consumer components of the data
table (runs data table and scalar card data table) instead. But this led
to too much code duplication in the components and templates.
- Tried making the add column a regular table column, instead of adding
it outside in a separate div. But making the column sticky requires
styling the table rows, which data table doesn't provide access to (rows
are configured solely in the consumer components). Also, the chosen
implementation is arguably more consistent with the current design as
the add "column" doesn't actually function as a table column at all
  • Loading branch information
hoonji committed Apr 29, 2024
1 parent e9d0009 commit ad1da0c
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
FilterAddedEvent,
ReorderColumnEvent,
AddColumnEvent,
AddColumnSize,
} from '../../../widgets/data_table/types';
import {isDatumVisible} from './utils';
import {memoize} from '../../../util/memoize';
Expand Down Expand Up @@ -73,7 +74,8 @@ export class ScalarCardDataTable {
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
@Output() loadAllColumns = new EventEmitter<null>();

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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
12 changes: 0 additions & 12 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ $_circle-size: 20px;

:host {
min-width: 100%;

& ::ng-deep tb-data-table .data-table {
@include tb-theme-foreground-prop(border-top, border, 1px solid);
}
}

.color-container {
Expand Down Expand Up @@ -55,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,
Expand Down
6 changes: 4 additions & 2 deletions tensorboard/webapp/theme/_tb_theme.template.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 16 additions & 7 deletions tensorboard/webapp/widgets/data_table/data_table_component.ng.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,33 @@
></tb-data-table-filter>
</ng-template>

<div class="data-table">
<div class="header">
<ng-content select="[header]"></ng-content>
<div class="data-table-wrapper" [class.should-add-borders]="shouldAddBorders">
<div class="left-section">
<div class="data-table">
<div class="header">
<ng-content select="[header]"></ng-content>
</div>
<ng-content select="[content]"></ng-content>
</div>
</div>
<div
class="right-section"
*ngIf="selectableColumns && selectableColumns.length"
>
<div
class="add-button-cell"
*ngIf="selectableColumns && selectableColumns.length"
class="add-button-column"
[class.small-add-button]="addColumnSize === AddColumnSize.SMALL"
>
<button
mat-icon-button
class="add-column-btn"
class="add-button"
(click)="openColumnSelector({event: $event})"
title="Add Column"
>
<mat-icon svgIcon="add_24px"></mat-icon>
</button>
</div>
</div>
<ng-content select="[content]"></ng-content>
</div>
<div *ngIf="loading" class="loading">
<mat-spinner mode="indeterminate" diameter="28"></mat-spinner>
Expand Down
61 changes: 52 additions & 9 deletions tensorboard/webapp/widgets/data_table/data_table_component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -55,7 +67,38 @@ $_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 {
--tb-icon-button-width: 20px;
--tb-icon-button-height: 20px;
}
}
4 changes: 4 additions & 0 deletions tensorboard/webapp/widgets/data_table/data_table_component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
ReorderColumnEvent,
Side,
AddColumnEvent,
AddColumnSize,
} from './types';
import {HeaderCellComponent} from './header_cell_component';
import {Subscription} from 'rxjs';
Expand Down Expand Up @@ -67,6 +68,8 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
@Input() hasMoreColumnsToLoad!: boolean;
@Input() columnFilters!: Map<string, DiscreteFilter | IntervalFilter>;
@Input() loading: boolean = false;
@Input() shouldAddBorders: boolean = false;
@Input() addColumnSize: AddColumnSize = AddColumnSize.DEFAULT;

@ContentChildren(HeaderCellComponent)
headerCells!: QueryList<HeaderCellComponent>;
Expand Down Expand Up @@ -102,6 +105,7 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {

readonly SortingOrder = SortingOrder;
readonly Side = Side;
readonly AddColumnSize = AddColumnSize;

constructor(
private readonly customModal: CustomModal,
Expand Down
6 changes: 4 additions & 2 deletions tensorboard/webapp/widgets/data_table/data_table_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import {ColumnSelectorModule} from './column_selector_module';
import {FilterDialog} from './filter_dialog_component';
import {CustomModal} from '../custom_modal/custom_modal';

const ADD_BUTTON_PREDICATE = By.css('.add-button');

@Component({
selector: 'testable-comp',
template: `
Expand Down Expand Up @@ -595,7 +597,7 @@ describe('data table', () => {

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', () => {
Expand All @@ -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))
Expand Down
5 changes: 5 additions & 0 deletions tensorboard/webapp/widgets/data_table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,8 @@ export enum ColumnGroup {
HPARAM = 'HPARAM',
OTHER = 'OTHER',
}

export enum AddColumnSize {
DEFAULT = 'DEFAULT',
SMALL = 'SMALL',
}

0 comments on commit ad1da0c

Please sign in to comment.