Skip to content

Commit

Permalink
HParams: Load spinner in data table and do not reload table (#6658)
Browse files Browse the repository at this point in the history
## Motivation for features / changes
Issue number 5 in #6651. It is also just a better experience to keep
previously loaded data intact while fetching new data.

## Technical description of changes
In order to put the spinner below the headers I added the spinner to the
data-table. It seems like a general feature that should be in that
widget anyway.

## Screenshots of UI changes (or N/A)
![2023-10-20_15-13-34
(1)](https://github.com/tensorflow/tensorboard/assets/8672809/23bfa0ed-077c-45fb-9df5-a0c3db1db67d)
  • Loading branch information
JamesHollyer committed Oct 23, 2023
1 parent dd87678 commit 7f7d140
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 14 deletions.
Expand Up @@ -23,17 +23,14 @@
placeholder="Filter runs (regex)"
></tb-filter-input>
</div>
<div *ngIf="loading" class="loading">
<mat-spinner mode="indeterminate" diameter="28"></mat-spinner>
</div>
<div class="table-container">
<tb-data-table
*ngIf="!loading"
[headers]="headers"
[sortingInfo]="sortingInfo"
[columnCustomizationEnabled]="true"
[selectableColumns]="selectableColumns"
[columnFilters]="columnFilters"
[loading]="loading"
(sortDataBy)="sortDataBy.emit($event)"
(orderColumns)="orderColumns.emit($event)"
(addColumn)="addColumn.emit($event)"
Expand Down
10 changes: 0 additions & 10 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.scss
Expand Up @@ -128,13 +128,3 @@ tb-data-table-header-cell:last-of-type {
.table-container {
overflow-x: auto;
}

.loading {
align-items: center;
border: 0;
@include tb-theme-foreground-prop(border-bottom, border, 1px solid);
display: flex;
height: 48px;
padding: 0 24px;
justify-content: center;
}
Expand Up @@ -113,3 +113,6 @@
</div>
<ng-content select="[content]"></ng-content>
</div>
<div *ngIf="loading" class="loading">
<mat-spinner mode="indeterminate" diameter="28"></mat-spinner>
</div>
10 changes: 10 additions & 0 deletions tensorboard/webapp/widgets/data_table/data_table_component.scss
Expand Up @@ -45,6 +45,16 @@ $_accent: map-get(mat.get-color-config($tb-theme), accent);
}
}

.loading {
align-items: center;
border: 0;
@include tb-theme-foreground-prop(border-bottom, border, 1px solid);
display: flex;
height: 48px;
padding: 0 24px;
justify-content: center;
}

.add-button-cell {
display: table-cell;
width: 40px;
Expand Down
Expand Up @@ -65,6 +65,7 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
@Input() columnCustomizationEnabled!: boolean;
@Input() selectableColumns?: ColumnHeader[];
@Input() columnFilters!: Map<string, DiscreteFilter | IntervalFilter>;
@Input() loading: boolean = false;

@ContentChildren(HeaderCellComponent)
headerCells!: QueryList<HeaderCellComponent>;
Expand Down
2 changes: 2 additions & 0 deletions tensorboard/webapp/widgets/data_table/data_table_module.ts
Expand Up @@ -17,6 +17,7 @@ import {CommonModule} from '@angular/common';
import {NgModule} from '@angular/core';
import {MatIconModule} from '@angular/material/icon';
import {MatButtonModule} from '@angular/material/button';
import {MatProgressSpinnerModule} from '@angular/material/progress-spinner';
import {DataTableComponent} from './data_table_component';
import {HeaderCellComponent} from './header_cell_component';
import {DataTableHeaderModule} from './data_table_header_module';
Expand All @@ -43,6 +44,7 @@ import {FilterDialogModule} from './filter_dialog_module';
CommonModule,
MatIconModule,
MatButtonModule,
MatProgressSpinnerModule,
DataTableHeaderModule,
CustomModalModule,
ColumnSelectorModule,
Expand Down
21 changes: 21 additions & 0 deletions tensorboard/webapp/widgets/data_table/data_table_test.ts
Expand Up @@ -46,6 +46,7 @@ import {FilterDialog} from './filter_dialog_component';
[sortingInfo]="sortingInfo"
[selectableColumns]="selectableColumns"
[columnFilters]="columnFilters"
[loading]="loading"
(sortDataBy)="sortDataBy($event)"
(orderColumns)="orderColumns($event)"
(addColumn)="addColumn.emit($event)"
Expand Down Expand Up @@ -88,6 +89,7 @@ class TestableComponent {
@Input() orderColumns!: (newOrder: ColumnHeaderType[]) => void;
@Input() selectableColumns!: ColumnHeader[];
@Input() columnFilters!: Map<string, DiscreteFilter | IntervalFilter>;
@Input() loading!: boolean;

@Output() addColumn = new EventEmitter<{
header: ColumnHeader;
Expand Down Expand Up @@ -123,6 +125,7 @@ describe('data table', () => {
data?: TableData[];
potentialColumns?: ColumnHeader[];
columnFilters?: Map<string, DiscreteFilter | IntervalFilter>;
loading?: boolean;
}): ComponentFixture<TestableComponent> {
const fixture = TestBed.createComponent(TestableComponent);

Expand All @@ -140,6 +143,10 @@ describe('data table', () => {
fixture.componentInstance.selectableColumns = input.potentialColumns;
}

if (input.loading !== undefined) {
fixture.componentInstance.loading = input.loading;
}

fixture.componentInstance.columnFilters = input.columnFilters || new Map();

sortDataBySpy = jasmine.createSpy();
Expand All @@ -159,6 +166,20 @@ describe('data table', () => {
expect(dataTable).toBeTruthy();
});

it('renders spinner when loading', () => {
const fixture = createComponent({loading: true});
fixture.detectChanges();
const spinner = fixture.debugElement.query(By.css('.loading'));
expect(spinner).toBeTruthy();
});

it('does not renders spinner when not loading', () => {
const fixture = createComponent({loading: false});
fixture.detectChanges();
const spinner = fixture.debugElement.query(By.css('.loading'));
expect(spinner).toBeFalsy();
});

it('emits sortDataBy event when header emits headerClicked event', () => {
const fixture = createComponent({
headers: [
Expand Down

0 comments on commit 7f7d140

Please sign in to comment.