Skip to content

Commit

Permalink
- Hide left side bar after navigation update to provide more space fo…
Browse files Browse the repository at this point in the history
…r visualizations

- Make app module loading message more specific and all loading message self explanatory for different stages

PiperOrigin-RevId: 719017600
  • Loading branch information
zzzaries authored and copybara-github committed Jan 23, 2025
1 parent 8411e8a commit a504320
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 47 deletions.
2 changes: 1 addition & 1 deletion frontend/app/app.ng.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<ng-template #progress_bar>
<div class="progress-container" *ngIf="loading">
<div class="loading-message">Loading data</div>
<div class="loading-message">Initializing runs and tools...</div>
<!-- TODO(go/progressbar-aria): Replace the aria-label with a better description, e.g. "Fetching user data" -->
<mat-progress-bar color="primary" mode="indeterminate" aria-label="Loading"></mat-progress-bar>
</div>
Expand Down
2 changes: 2 additions & 0 deletions frontend/app/common/interfaces/navigation_event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ export declare interface NavigationEvent {
host?: string;
// Graph Viewer crosslink params
opName?: string;
// Navigation controlling params
firstLoad?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ export class InferenceProfile implements OnDestroy {
i++;
}
}
console.log('allRequestTables', this.allRequestTables);
console.log('allRequestProperties', this.allRequestProperties);
console.log('allBatchTables', this.allBatchTables);
console.log('allBatchProperties', this.allBatchProperties);
console.log('allTensorPatternTables', this.allTensorPatternTables);
console.log('allTensorPatternProperties', this.allTensorPatternProperties);

return true;
}
Expand Down
18 changes: 15 additions & 3 deletions frontend/app/components/main_page/main_page.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {Component, OnDestroy} from '@angular/core';
import {Store} from '@ngrx/store';
import {NavigationEvent} from 'org_xprof/frontend/app/common/interfaces/navigation_event';
import {CommunicationService} from 'org_xprof/frontend/app/services/communication_service/communication_service';
import {getLoadingState} from 'org_xprof/frontend/app/store/selectors';
import {LoadingState} from 'org_xprof/frontend/app/store/state';
Expand Down Expand Up @@ -32,9 +33,20 @@ export class MainPage implements OnDestroy {
this.loading = loadingState.loading;
this.loadingMessage = loadingState.message;
});
this.communicationService.navigationReady.subscribe(() => {
this.navigationReady = true;
});
this.communicationService.navigationReady.subscribe(
(navigationEvent: NavigationEvent) => {
this.navigationReady = true;
// TODO(fe-unification): Remove this constraint once the sidepanel
// content of the 3 tools are moved out from sidenav with consolidated
// templates.
const toolsWithSideNav =
['op_profile', 'memory_viewer', 'pod_viewer'];
this.isSideNavOpen =
(navigationEvent.firstLoad ||
toolsWithSideNav
.filter(tool => navigationEvent?.tag?.startsWith(tool))
.length > 0);
});
}

ngOnDestroy() {
Expand Down
48 changes: 19 additions & 29 deletions frontend/app/components/sidenav/sidenav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class SideNav implements OnInit, OnDestroy {
selectedRunInternal = '';
selectedTagInternal = '';
selectedHostInternal = '';
navigationParams: {[key: string]: string} = {};
navigationParams: {[key: string]: string|boolean} = {};

constructor(
private readonly router: Router,
Expand All @@ -53,10 +53,6 @@ export class SideNav implements OnInit, OnDestroy {
this.selectedRunInternal = run;
}
});
this.communicationService.navigationChange.subscribe(
(navigationEvent: NavigationEvent) => {
this.onUpdateRoute(navigationEvent);
});
this.communicationService.toolQueryParamsChange.subscribe(
(queryParams: ToolQueryParams) => {
this.navigationParams = {
Expand Down Expand Up @@ -97,17 +93,25 @@ export class SideNav implements OnInit, OnDestroy {
this.hosts[0] || '';
}

// Initial page load with url params
navigateWithUrl() {
const params = new URLSearchParams(window.parent.location.search);
const navigationEvent: NavigationEvent = {
run: params.get('run') || '',
tag: params.get('tool') || '',
host: params.get('host') || '',
};
if (params.has('opName')) {
navigationEvent.opName = params.get('opName') || '';
const run = params.get('run') || '';
const tag = params.get('tool') || '';
const host = params.get('host') || '';
const opName = params.get('opName') || '';
this.navigationParams['firstLoad'] = true;
if (opName) {
this.navigationParams['opName'] = opName;
}
this.onUpdateRoute(navigationEvent);
if (this.selectedRunInternal === run && this.selectedTagInternal === tag &&
this.selectedHostInternal === host) {
return;
}
this.selectedRunInternal = run;
this.selectedTagInternal = tag;
this.selectedHostInternal = host;
this.update();
}

ngOnInit() {
Expand Down Expand Up @@ -222,30 +226,16 @@ export class SideNav implements OnInit, OnDestroy {
}

navigateTools() {
this.communicationService.onNavigateReady();
const navigationEvent = this.getNavigationEvent();
this.communicationService.onNavigateReady(navigationEvent);
this.router.navigate([
this.selectedTag || 'empty',
navigationEvent,
]);
delete this.navigationParams['firstLoad'];
this.updateUrlHistory();
}

onUpdateRoute(event: NavigationEvent) {
const {run = '', tag = '', host = ''} = event;
this.selectedRunInternal = run;
this.selectedTagInternal = tag;
this.selectedHostInternal = host;
this.navigationParams = {
...this.navigationParams,
...event,
};
delete this.navigationParams['run'];
delete this.navigationParams['tag'];
delete this.navigationParams['host'];
this.update();
}

update() {
this.afterUpdateRun();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@ export type ToolQueryParams = NavigationEvent;
*/
@Injectable({providedIn: 'root'})
export class CommunicationService {
@Output() readonly navigationChange = new EventEmitter();
@Output() readonly navigationReady = new EventEmitter();
@Output() readonly toolQueryParamsChange = new EventEmitter();

// Trigger navigation in sidenav component
onNavigate(navigationEvent: NavigationEvent) {
this.navigationChange.emit(navigationEvent);
}

onNavigateReady() {
this.navigationReady.emit({});
// Show a navigating status when populating navigation chains
// eg. tools for selected run
onNavigateReady(navigationEvent: NavigationEvent) {
this.navigationReady.emit(navigationEvent);
}

onToolQueryParamsChange(queryParams: ToolQueryParams) {
Expand Down

0 comments on commit a504320

Please sign in to comment.