Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
dd20f13
refactor: time range styles
Feb 8, 2022
a8523ab
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Feb 9, 2022
4bfa6d6
feat: initial browser stored page time range
Feb 11, 2022
63a665e
fix: removed defaultTimeRange null check for throwing error
Feb 11, 2022
b81ae1a
refactor: time range logic to be query param centric
Feb 14, 2022
6e84fbe
refactor: update effected tests
Feb 14, 2022
ab466fe
Merge branch 'main' into BreadcrumbsTimeRange
Feb 14, 2022
3043f45
fix: replace arrow function for compiler
Feb 14, 2022
56c574b
fix: time range isCustom correction
Feb 15, 2022
c1be553
refactor: moved time range logic to sub component
Feb 16, 2022
8eb3a52
fix: adjusted time range style to match other header buttons
Feb 16, 2022
ffe3d6b
fix: linting
Feb 16, 2022
3f98a99
refactor: cleaned up nav params observable
Feb 16, 2022
f0d9ffe
fix: linting
Feb 16, 2022
e1bea69
refactor: feature flag added
Feb 17, 2022
b359fcd
test: tests for new time range
Feb 18, 2022
ab36992
test: linter
Feb 18, 2022
cd870bc
Merge branch 'main' into BreadcrumbsTimeRange
Feb 18, 2022
7a92d50
test: added new test for time range service
Feb 22, 2022
b241e0e
fix: pr changes
Feb 23, 2022
7e279e4
fix: linting
Feb 23, 2022
10e8225
Merge branch 'main' into BreadcrumbsTimeRange
Feb 23, 2022
1738b10
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Feb 23, 2022
c44b03b
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Feb 28, 2022
bd1cbbd
Merge branch 'BreadcrumbsTimeRange' of github.com:hypertrace/hypertra…
Feb 28, 2022
3adc661
fix: requested changes
Mar 1, 2022
6e78e54
fix: export feature enum
Mar 1, 2022
c225cf9
refactor: replaced time range icon with calendar
Mar 2, 2022
c2d0287
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Mar 3, 2022
7bc71c7
fix: requested changes
Mar 3, 2022
b4b4722
fix: backwards compatability and depency issue changes
Mar 4, 2022
a528987
fix: requested changes
Mar 7, 2022
63c4bd0
fix: requested changes
Mar 8, 2022
8742a2b
fix: requested changes
Mar 9, 2022
b795d39
fix: linting
Mar 9, 2022
9ed2b22
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Mar 9, 2022
67c8bfe
fix: moved tr selector back to components
Mar 9, 2022
0854000
fix: update test
Mar 9, 2022
62d1041
refactor: requested changes
Mar 9, 2022
5037200
refactor: requested changes
Christian862 Mar 10, 2022
11b0991
refactor: requested changes
Christian862 Mar 10, 2022
50153f9
refactor: requested changes
Christian862 Mar 10, 2022
12ab84d
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 10, 2022
ea6357a
refactor: requested changes
Christian862 Mar 11, 2022
b558d41
fix: linting
Christian862 Mar 11, 2022
0989a6a
fix: requested changes - naming and TR service
Christian862 Mar 11, 2022
3845357
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 14, 2022
a37a477
fix: requested changes
Christian862 Mar 14, 2022
00f785a
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 14, 2022
82a9612
fix: requested changes - broken
Christian862 Mar 15, 2022
9714df8
fix: requested changes
Christian862 Mar 15, 2022
4b0b74e
test: testing support for new TR init
Christian862 Mar 15, 2022
44a67b6
refactor: requested changes
Christian862 Mar 15, 2022
bfcb9b8
refactor: requested changes, updated route data for new changes
Christian862 Mar 16, 2022
38c5e82
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 16, 2022
efd4d15
refactor: requested changes
Christian862 Mar 18, 2022
5e5dcf5
refactor: requested changes
Christian862 Mar 18, 2022
7172091
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 18, 2022
505d5d7
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 18, 2022
748743e
refactor: requested changes
Christian862 Mar 18, 2022
09cf639
refactor: requested changes
Christian862 Mar 18, 2022
5c0cfe6
refactor: fix FF string
Christian862 Mar 18, 2022
541555f
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into Bread…
Christian862 Mar 21, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,16 @@ export class PageTimeRangePreferenceService {
.pipe(shareReplay(1));
}

public getDefaultTimeRangeForCurrentRoute = (): TimeRange => {
public getDefaultTimeRangeForCurrentRoute(): TimeRange {
const currentRoute: ActivatedRoute = this.navigationService.getCurrentActivatedRoute();
// Right side for when FF is enabled but 'defaultTimeRange' is not set on AR data

return currentRoute.snapshot.data?.defaultTimeRange ?? this.getGlobalDefaultTimeRange();
};
}

public getGlobalDefaultTimeRange = (): TimeRange => new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour));
public getGlobalDefaultTimeRange(): TimeRange {
return new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour));
}
}

interface PageTimeRangeStringDictionary {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output } from '@angular/core';
import { ActivatedRoute } from '@angular/router';
import { IconType } from '@hypertrace/assets-library';
import { NavigationService, TimeRangeService, TypedSimpleChanges } from '@hypertrace/common';
import { NavigationService, TypedSimpleChanges } from '@hypertrace/common';
import { Observable } from 'rxjs';
import { map, startWith } from 'rxjs/operators';
import { IconSize } from '../icon/icon-size';
Expand Down Expand Up @@ -76,28 +76,26 @@ export class NavigationListComponent implements OnChanges {
@Output()
public readonly navItemSelected: EventEmitter<NavItemLinkConfig> = new EventEmitter();

@Output()
public readonly activeItemChange: EventEmitter<NavItemLinkConfig> = new EventEmitter();

public activeItem$?: Observable<NavItemLinkConfig | undefined>;

public constructor(
private readonly navigationService: NavigationService,
private readonly activatedRoute: ActivatedRoute,
private readonly navListComponentService: NavigationListComponentService,
private readonly timeRangeService: TimeRangeService
private readonly navListComponentService: NavigationListComponentService
) {}

public ngOnChanges(changes: TypedSimpleChanges<this>): void {
if (changes.navItems) {
this.navItems = this.navListComponentService.resolveFeaturesAndUpdateVisibilityForNavItems(this.navItems);

// Initialize the time range service
// Depending on FF status, the TR will be either global or page level for the init
this.activeItem$ = this.navigationService.navigation$.pipe(
startWith(this.navigationService.getCurrentActivatedRoute()),
map(() => {
const activeItem = this.findActiveItem(this.navItems);
if (!this.timeRangeService.isInitialized() && activeItem?.timeRangeResolver) {
this.timeRangeService.setDefaultTimeRange(activeItem.timeRangeResolver());
}
this.activeItemChange.emit(activeItem);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok with it but would be remiss not to point out - we're using this observable for side effects. If we happened to not read activeItem$ in this component because we didn't need to use it for our own template, it would create a bug where we also stopped emitting.

The alternative however is also not great, since it means we need to subscribe in the component and handle unsubscribing on destroy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah fair concern, i've made a code comment about it for anyone that sees this in the future.


return activeItem;
})
Expand Down
14 changes: 11 additions & 3 deletions src/app/application-frame/application-frame.component.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
import { IconType } from '@hypertrace/assets-library';
import { LayoutChangeService } from '@hypertrace/common';
import { LayoutChangeService, TimeRange, TimeRangeService } from '@hypertrace/common';
import { IconSize } from '@hypertrace/components';
import { Observable } from 'rxjs';
import { UserTelemetryOrchestrationService } from '../shared/telemetry/user-telemetry-orchestration.service';
@Component({
selector: 'ht-application-frame',
Expand All @@ -16,14 +17,21 @@ import { UserTelemetryOrchestrationService } from '../shared/telemetry/user-tele
</ht-application-header>
<div class="app-body">
<ht-navigation class="left-nav"></ht-navigation>
<div class="app-content">
<div class="app-content" *ngIf="this.timeRangeHasInit$ | async">
<router-outlet></router-outlet>
</div>
</div>
`
})
export class ApplicationFrameComponent implements OnInit {
public constructor(private readonly userTelemetryOrchestrationService: UserTelemetryOrchestrationService) {}
public readonly timeRangeHasInit$: Observable<TimeRange>;

public constructor(
private readonly userTelemetryOrchestrationService: UserTelemetryOrchestrationService,
private readonly timeRangeService: TimeRangeService
) {
this.timeRangeHasInit$ = this.timeRangeService.getTimeRangeAndChanges();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could take(1) here, will let this clean up rather than hanging around

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't that defeat the purpose ? it will take the first emission which will be undefined, and never show the app content

Copy link
Contributor

Choose a reason for hiding this comment

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

getTimeRangeAndChanges should never output an undefined - it just won't emit until a time range is assigned. If we broke that, that means it's mistyped and we should go fix.

}

public ngOnInit(): void {
this.userTelemetryOrchestrationService.initialize();
Expand Down
11 changes: 10 additions & 1 deletion src/app/application-frame/application-frame.module.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CommonModule } from '@angular/common';
import { HttpClientModule } from '@angular/common/http';
import { NgModule } from '@angular/core';
import { RouterModule } from '@angular/router';
Expand All @@ -6,7 +7,15 @@ import { NavigationModule } from '../shared/navigation/navigation.module';
import { ApplicationFrameComponent } from './application-frame.component';

@NgModule({
imports: [ApplicationHeaderModule, NavigationModule, PageHeaderModule, HttpClientModule, RouterModule, IconModule],
imports: [
CommonModule,
ApplicationHeaderModule,
NavigationModule,
PageHeaderModule,
HttpClientModule,
RouterModule,
IconModule
],
declarations: [ApplicationFrameComponent],
exports: [ApplicationFrameComponent]
})
Expand Down
20 changes: 13 additions & 7 deletions src/app/routes/root-routing.module.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NgModule } from '@angular/core';
import { PreloadAllModules, RouterModule } from '@angular/router';
import { IconType } from '@hypertrace/assets-library';
import { ExternalUrlNavigator, HtRoute } from '@hypertrace/common';
import { ExternalUrlNavigator, HtRoute, RelativeTimeRange, TimeDuration, TimeUnit } from '@hypertrace/common';
import { NotFoundComponent, NotFoundModule } from '@hypertrace/components';
import { ObservabilityIconType } from '@hypertrace/observability';
import { ApplicationFrameComponent } from '../application-frame/application-frame.component';
Expand All @@ -25,7 +25,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: IconType.Dashboard,
label: 'Dashboard'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () => import('../home/home.module').then(module => module.HomeModule)
},
Expand All @@ -35,7 +36,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: ObservabilityIconType.ApplicationFlow,
label: 'Application Flow'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () =>
import('./application-flow/application-flow-routing.module').then(
Expand All @@ -48,7 +50,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: ObservabilityIconType.Backend,
label: 'Backends'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () =>
import('./backends/backends-routing.module').then(module => module.BackendsRoutingModule)
Expand All @@ -59,7 +62,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: ObservabilityIconType.Service,
label: 'Services'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () =>
import('./services/services-routing.module').then(module => module.ServicesRoutingModule)
Expand All @@ -70,7 +74,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: ObservabilityIconType.Api,
label: 'Endpoints'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () =>
import('./endpoints/endpoint-routing.module').then(module => module.EndpointRoutingModule)
Expand All @@ -89,7 +94,8 @@ const ROUTE_CONFIG: HtRoute[] = [
breadcrumb: {
icon: IconType.Search,
label: 'Explorer'
}
},
defaultTimeRange: new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour))
},
loadChildren: () =>
import('./explorer/explorer-routing.module').then(module => module.ExplorerRoutingModule)
Expand Down
49 changes: 43 additions & 6 deletions src/app/shared/navigation/navigation.component.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { ActivatedRoute } from '@angular/router';
import { IconType } from '@hypertrace/assets-library';
import { PreferenceService } from '@hypertrace/common';
import { NavigationListService, NavItemConfig, NavItemType } from '@hypertrace/components';
import { FixedTimeRange, PreferenceService, RelativeTimeRange, TimeRangeService } from '@hypertrace/common';
import {
NavigationListComponentService,
NavigationListService,
NavItemConfig,
NavItemLinkConfig,
NavItemType
} from '@hypertrace/components';
import { ObservabilityIconType } from '@hypertrace/observability';
import { isNil } from 'lodash-es';
import { Observable } from 'rxjs';
import { shareReplay } from 'rxjs/operators';

@Component({
selector: 'ht-navigation',
Expand All @@ -13,17 +21,21 @@ import { Observable } from 'rxjs';
template: `
<div class="navigation">
<ht-navigation-list
[navItems]="this.navItems"
[navItems]="this.navItems$ | async"
*htLetAsync="this.isCollapsed$ as isCollapsed"
[collapsed]="isCollapsed"
(collapsedChange)="this.onViewToggle($event)"
(navItemSelected)="this.setPageTimeRangeForSelectedNavItem($event)"
(activeItemChange)="this.updateDefaultTimeRangeIfUnset($event)"
></ht-navigation-list>
</div>
`
})
export class NavigationComponent {
private static readonly COLLAPSED_PREFERENCE: string = 'app-navigation.collapsed';
public readonly navItems: NavItemConfig[];

public navItems$?: Observable<NavItemConfig[]>;

public readonly isCollapsed$: Observable<boolean>;

private readonly navItemDefinitions: NavItemConfig[] = [
Expand Down Expand Up @@ -76,14 +88,39 @@ export class NavigationComponent {
public constructor(
private readonly navigationListService: NavigationListService,
private readonly preferenceService: PreferenceService,
private readonly activatedRoute: ActivatedRoute
private readonly navListComponentService: NavigationListComponentService,
private readonly activatedRoute: ActivatedRoute,
private readonly timeRangeService: TimeRangeService
) {
this.navItems = this.navItemDefinitions.map(definition =>
const navItems = this.navItemDefinitions.map(definition =>
this.navigationListService.decorateNavItem(definition, this.activatedRoute)
);
// Decorate the nav items with the corresponding time ranges, depending on the FF state.
// The time ranges in nav items are streams that get the most recent value from page time range preference service
this.navItems$ = this.navListComponentService.resolveNavItemConfigTimeRanges(navItems).pipe(shareReplay());

this.isCollapsed$ = this.preferenceService.get(NavigationComponent.COLLAPSED_PREFERENCE, false);
}

public setPageTimeRangeForSelectedNavItem(navItemLink: NavItemLinkConfig): void {
if (!isNil(navItemLink.timeRangeResolver) && navItemLink.pageLevelTimeRangeIsEnabled) {
const timeRange = navItemLink.timeRangeResolver();
if (timeRange instanceof FixedTimeRange) {
this.timeRangeService.setFixedRange(timeRange.startTime, timeRange.endTime);
} else if (timeRange instanceof RelativeTimeRange) {
this.timeRangeService.setRelativeRange(timeRange.duration.value, timeRange.duration.unit);
}
}
}

public updateDefaultTimeRangeIfUnset(activeItem: NavItemLinkConfig): void {
// Initialize the time range service
// Depending on FF status, the TR will be either global or page level for the init
if (!this.timeRangeService.isInitialized()) {
this.timeRangeService.setDefaultTimeRange(activeItem.timeRangeResolver!());
}
}

public onViewToggle(collapsed: boolean): void {
this.preferenceService.set(NavigationComponent.COLLAPSED_PREFERENCE, collapsed);
}
Expand Down