Skip to content

HParams: Add button to make the runs table full screen #6451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rileyajones merged 3 commits into from
Jun 26, 2023
Merged

HParams: Add button to make the runs table full screen #6451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e025ce8
make changes to runs table
rileyajones Jun 24, 2023
2b7ddde
changes to layout files
rileyajones Jun 24, 2023
24038fd
add tests
rileyajones Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions tensorboard/webapp/core/views/layout_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ import {
} from '@angular/core';
import {Store} from '@ngrx/store';
import {fromEvent, Observable, Subject} from 'rxjs';
import {filter, takeUntil} from 'rxjs/operators';
import {combineLatestWith, filter, map, takeUntil} from 'rxjs/operators';
import {MouseEventButtons} from '../../util/dom';
import {sideBarWidthChanged} from '../actions';
import {State} from '../state';
import {getSideBarWidthInPercent} from '../store/core_selectors';
import {
getRunsTableFullScreen,
getSideBarWidthInPercent,
} from '../store/core_selectors';

@Component({
selector: 'tb-dashboard-layout',
Expand All @@ -41,6 +44,7 @@ import {getSideBarWidthInPercent} from '../store/core_selectors';
class="sidebar"
[style.width.%]="width$ | async"
[style.minWidth.px]="MINIMUM_SIDEBAR_WIDTH_IN_PX"
[style.maxWidth.%]="(runsTableFullScreen$ | async) ? 100 : ''"
>
<ng-content select="[sidebar]"></ng-content>
</nav>
Expand All @@ -55,9 +59,15 @@ import {getSideBarWidthInPercent} from '../store/core_selectors';
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class LayoutContainer implements OnDestroy {
readonly width$: Observable<number> = this.store.select(
getSideBarWidthInPercent
);
readonly runsTableFullScreen$ = this.store.select(getRunsTableFullScreen);
readonly width$: Observable<number> = this.store
.select(getSideBarWidthInPercent)
.pipe(
combineLatestWith(this.runsTableFullScreen$),
Copy link
Contributor

@JamesHollyer JamesHollyer Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly this works. However, I thought combineLatestWith would cause this pipe to not trigger an update when runsTableFullScreen changes.

Copy link
Contributor Author

@rileyajones rileyajones Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you are mixing up combineLatestWith and withLatestFrom (I know, it's a hard mistake to make /s). Combine latest with does trigger this to re-emit.

If it did not then I would not be able to toggle full screen after resizing it.
9c6394e3-d258-47ee-8954-9964f1f57f35

map(([percentageWidth, fullScreen]) => {
return fullScreen ? 100 : percentageWidth;
})
);
private readonly ngUnsubscribe = new Subject<void>();
private resizing: boolean = false;

Expand Down
14 changes: 13 additions & 1 deletion tensorboard/webapp/core/views/layout_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ import {provideMockTbStore} from '../../testing/utils';
import {MouseEventButtons} from '../../util/dom';
import {sideBarWidthChanged} from '../actions';
import {State} from '../state';
import {getSideBarWidthInPercent} from '../store/core_selectors';
import {
getRunsTableFullScreen,
getSideBarWidthInPercent,
} from '../store/core_selectors';
import {LayoutContainer} from './layout_container';

@Component({
Expand Down Expand Up @@ -137,6 +140,15 @@ describe('layout test', () => {
expect(navEl.styles['width']).toBe('70%');
});

it('overrides max width when the runs table full screen is true', () => {
store.overrideSelector(getRunsTableFullScreen, true);
const fixture = TestBed.createComponent(TestableComponent);
fixture.detectChanges();

const navEl = fixture.debugElement.query(byCss.SIDEBAR_CONTAINER);
expect(navEl.styles['width']).toBe('100%');
});

describe('interactions', () => {
function triggerMouseMove(
fixture: ComponentFixture<TestableComponent>,
Expand Down
3 changes: 3 additions & 0 deletions tensorboard/webapp/metrics/views/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ tf_ng_module(
"//tensorboard/webapp:selectors",
"//tensorboard/webapp/angular:expect_angular_material_icon",
"//tensorboard/webapp/core",
"//tensorboard/webapp/core/store",
"//tensorboard/webapp/customization",
"//tensorboard/webapp/feature_flag/store",
"//tensorboard/webapp/feature_flag/store:types",
Expand Down Expand Up @@ -80,9 +81,11 @@ tf_ts_library(
"//tensorboard/webapp/angular:expect_angular_core_testing",
"//tensorboard/webapp/angular:expect_angular_platform_browser_animations",
"//tensorboard/webapp/angular:expect_ngrx_store_testing",
"//tensorboard/webapp/core/store",
"//tensorboard/webapp/feature_flag/store",
"//tensorboard/webapp/metrics/actions",
"//tensorboard/webapp/metrics/data_source",
"//tensorboard/webapp/metrics/views/main_view",
"//tensorboard/webapp/runs/views/runs_selector",
"//tensorboard/webapp/testing:utils",
"@npm//@angular/core",
Expand Down
9 changes: 7 additions & 2 deletions tensorboard/webapp/metrics/views/metrics_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ limitations under the License.
import {ChangeDetectionStrategy, Component} from '@angular/core';
import {Store} from '@ngrx/store';
import {getEnableHparamsInTimeSeries} from '../../feature_flag/store/feature_flag_selectors';
import {State} from '../../feature_flag/store/feature_flag_types';
import {State} from '../../app_state';
import {getRunsTableFullScreen} from '../../core/store/core_selectors';

@Component({
selector: 'metrics-dashboard',
Expand All @@ -25,14 +26,18 @@ import {State} from '../../feature_flag/store/feature_flag_types';
[showHparamsAndMetrics]="showHparamsAndMetrics$ | async"
sidebar
></runs-selector>
<metrics-main-view main></metrics-main-view>
<metrics-main-view
main
*ngIf="!(runsTableFullScreen$ | async)"
></metrics-main-view>
</tb-dashboard-layout>
`,
styleUrls: ['metrics_container.css'],
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class MetricsDashboardContainer {
showHparamsAndMetrics$ = this.store.select(getEnableHparamsInTimeSeries);
runsTableFullScreen$ = this.store.select(getRunsTableFullScreen);

constructor(readonly store: Store<State>) {}
}
12 changes: 12 additions & 0 deletions tensorboard/webapp/metrics/views/metrics_container_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {getEnableHparamsInTimeSeries} from '../../feature_flag/store/feature_fla
import {RunsSelectorContainer} from '../../runs/views/runs_selector/runs_selector_container';
import {provideMockTbStore} from '../../testing/utils';
import {MetricsDashboardContainer} from './metrics_container';
import {getRunsTableFullScreen} from '../../core/store/core_selectors';
import {MainViewContainer} from './main_view/main_view_container';

describe('metrics view', () => {
let store: MockStore<State>;
Expand Down Expand Up @@ -64,4 +66,14 @@ describe('metrics view', () => {
.showHparamsAndMetrics
).toBeFalse();
});

it('hides main view when the runs table is full screen is true', () => {
store.overrideSelector(getRunsTableFullScreen, true);
const fixture = TestBed.createComponent(MetricsDashboardContainer);
fixture.detectChanges();

expect(
fixture.debugElement.query(By.directive(MainViewContainer))
).toBeNull();
});
});
1 change: 1 addition & 0 deletions tensorboard/webapp/runs/views/runs_table/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ tf_ng_module(
"//tensorboard/webapp/angular:expect_angular_material_table",
"//tensorboard/webapp/app_routing",
"//tensorboard/webapp/app_routing:types",
"//tensorboard/webapp/core/actions",
"//tensorboard/webapp/experiments:types",
"//tensorboard/webapp/feature_flag/store",
"//tensorboard/webapp/hparams",
Expand Down
13 changes: 13 additions & 0 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,16 @@
</ng-container>
</ng-container>
</tb-data-table>
<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>
50 changes: 46 additions & 4 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ limitations under the License.
@use '@angular/material' as mat;
@import 'tensorboard/webapp/theme/tb_theme';
$_circle-size: 20px;
$_arrow_size: 16px;

:host {
width: 100%;
}

.color-container {
display: flex;
Expand All @@ -30,10 +35,6 @@ $_circle-size: 20px;
outline: none;
}

:host {
width: 100%;
}

tb-data-table-content-row,
tb-data-table-header-cell {
height: 44px;
Expand All @@ -52,3 +53,44 @@ tb-data-table-header-cell {
padding-right: 16px;
}
}

.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;
}
}
}
2 changes: 2 additions & 0 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export class RunsDataTable {
@Input() sortingInfo!: SortingInfo;
@Input() experimentIds!: string[];
@Input() regexFilter!: string;
@Input() isFullScreen!: boolean;

ColumnHeaderType = ColumnHeaderType;

Expand All @@ -46,6 +47,7 @@ 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;
Expand Down
13 changes: 13 additions & 0 deletions tensorboard/webapp/runs/views/runs_table/runs_table_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
getRunSelectorRegexFilter,
getRunSelectorSort,
getRunsLoadState,
getRunsTableFullScreen,
getRunsTableHeaders,
getRunsTableSortingInfo,
} from '../../../selectors';
Expand Down Expand Up @@ -90,6 +91,7 @@ import {
import {RunsTableColumn, RunTableItem} from './types';
import {getFilteredRenderableRunsFromRoute} from '../../../metrics/views/main_view/common_selectors';
import {RunToHParamValues} from '../../data_source/runs_data_source_types';
import {runsTableFullScreenToggled} from '../../../core/actions';

const getRunsLoading = createSelector<
State,
Expand Down Expand Up @@ -239,19 +241,25 @@ function matchFilter(
[sortingInfo]="sortingInfo$ | async"
[experimentIds]="experimentIds"
[regexFilter]="regexFilter$ | async"
[isFullScreen]="runsTableFullScreen$ | async"
(sortDataBy)="sortDataBy($event)"
(orderColumns)="orderColumns($event)"
(onSelectionToggle)="onRunSelectionToggle($event)"
(onAllSelectionToggle)="onAllSelectionToggle($event)"
(onRunColorChange)="onRunColorChange($event)"
(onRegexFilterChange)="onRegexFilterChange($event)"
(toggleFullScreen)="toggleFullScreen()"
></runs-data-table>
`,
host: {
'[class.flex-layout]': 'useFlexibleLayout',
},
styles: [
`
:host {
position: relative;
}

:host.flex-layout {
display: flex;
}
Expand Down Expand Up @@ -310,6 +318,7 @@ export class RunsTableContainer implements OnInit, OnDestroy {
regexFilter$ = this.store.select(getRunSelectorRegexFilter);
HParamsEnabled = new BehaviorSubject<boolean>(false);
runsColumns$ = this.store.select(getRunsTableHeaders);
runsTableFullScreen$ = this.store.select(getRunsTableFullScreen);

runToHParamValues$ = this.store
.select(getFilteredRenderableRunsFromRoute)
Expand Down Expand Up @@ -777,6 +786,10 @@ export class RunsTableContainer implements OnInit, OnDestroy {
})
);
}

toggleFullScreen() {
this.store.dispatch(runsTableFullScreenToggled());
}
}

export const TEST_ONLY = {
Expand Down