From dd20f1319c190c47654702d89de69d0bf1fcf6cc Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Tue, 8 Feb 2022 11:56:42 -0400 Subject: [PATCH 01/47] refactor: time range styles --- .../assets/styles/_interaction.scss | 8 +++++ .../src/time-range/time-range.component.scss | 36 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/projects/assets-library/assets/styles/_interaction.scss b/projects/assets-library/assets/styles/_interaction.scss index e96018a25..13b911cc7 100644 --- a/projects/assets-library/assets/styles/_interaction.scss +++ b/projects/assets-library/assets/styles/_interaction.scss @@ -99,3 +99,11 @@ cursor: not-allowed; } } + +@mixin time-range-button-background { + background-color: white; + + &:hover { + background-color: $gray-1; + } +} diff --git a/projects/components/src/time-range/time-range.component.scss b/projects/components/src/time-range/time-range.component.scss index 0e5f8c486..fbecf22b8 100644 --- a/projects/components/src/time-range/time-range.component.scss +++ b/projects/components/src/time-range/time-range.component.scss @@ -1,14 +1,17 @@ @import 'mixins'; +@mixin time-range-buttons() { + @include time-range-button-background; + @include body-1-medium($gray-6); + border: 1px solid $gray-3; +} .time-range { display: flex; align-items: center; } .time-range-selector { - @include top-bar-dark-button-background; - @include top-bar-dark-button-foreground; - min-width: 200px; + @include time-range-buttons; border-radius: 6px; } @@ -42,12 +45,35 @@ .refresh { margin-left: 8px; + &.emphasized { + ::ng-deep { + button.button.solid { + background-color: $gray-2; + border: 1px solid $gray-2; + &:hover { + background-color: $gray-1; + } + .label { + color: $gray-9; + } + } + + .icon.leading { + color: $gray-6; + } + } + } + &:not(.emphasized) { ::ng-deep { button.button.solid { - @include top-bar-dark-button-background; + @include time-range-button-background; + border: 1px solid $gray-3; .label { - @include top-bar-dark-button-foreground; + @include body-1-medium($gray-6); + } + .icon.leading { + color: $gray-6; } } } From 4bfa6d664159ae95e5ea23a362ad9128838e19c0 Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Fri, 11 Feb 2022 10:20:41 -0400 Subject: [PATCH 02/47] feat: initial browser stored page time range --- projects/common/src/public-api.ts | 1 + .../src/time/page-time-range.service.ts | 64 +++++++++++++++++++ .../common/src/time/time-range.service.ts | 6 +- .../navigation/nav-item/nav-item.component.ts | 48 ++++++++++++-- .../src/navigation/navigation.config.ts | 8 ++- .../src/time-range/time-range.component.ts | 6 +- 6 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 projects/common/src/time/page-time-range.service.ts diff --git a/projects/common/src/public-api.ts b/projects/common/src/public-api.ts index e935fbe1d..ed23c28ec 100644 --- a/projects/common/src/public-api.ts +++ b/projects/common/src/public-api.ts @@ -120,6 +120,7 @@ export * from './time/time-range.service'; export * from './time/time-range.type'; export * from './time/time-unit.type'; export * from './time/time'; +export * from './time/page-time-range.service'; // Validators export * from './utilities/validators'; diff --git a/projects/common/src/time/page-time-range.service.ts b/projects/common/src/time/page-time-range.service.ts new file mode 100644 index 000000000..1c3353e06 --- /dev/null +++ b/projects/common/src/time/page-time-range.service.ts @@ -0,0 +1,64 @@ +import { Injectable, OnDestroy } from '@angular/core'; +import { isNil } from 'lodash-es'; +import { BehaviorSubject, Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; +import { PreferenceService, StorageType } from '../preference/preference.service'; +// Import {TimeRange} from "./time-range"; +import { RelativeTimeRange } from './relative-time-range'; +import { TimeRangeService } from './time-range.service'; +// Import {TimeDuration} from "./time-duration"; + +@Injectable({ providedIn: 'root' }) +export class PageTimeRangeService implements OnDestroy { + // TODO change to local + private static readonly STORAGE_TYPE: StorageType = StorageType.Session; + private static readonly TIME_RANGE_PREFERENCE_KEY: string = 'page-time-range'; + private readonly pageTimeRanges$: BehaviorSubject = new BehaviorSubject({}); + + private readonly destroyed$: Subject = new Subject(); + + public constructor( + private readonly preferenceService: PreferenceService, + private readonly timeRangeService: TimeRangeService + ) { + this.preferenceService + .get(PageTimeRangeService.TIME_RANGE_PREFERENCE_KEY, {}, PageTimeRangeService.STORAGE_TYPE) + .pipe(takeUntil(this.destroyed$)) + .subscribe(values => { + // tslint:disable-next-line: no-console + console.log('stored map emission ', values); + this.pageTimeRanges$.next(values); + }); + } + + public getPageTimeRange(path: string): RelativeTimeRange | undefined { + const pageTimeMap = this.pageTimeRanges$.getValue(); + + if (isNil(pageTimeMap[path])) { + return undefined; + } + + return this.timeRangeService.timeRangeFromUrlString(pageTimeMap[path]) as RelativeTimeRange; + } + + public setPageTimeRange(path: string, value: RelativeTimeRange): void { + const pageTimeMap: PageTimeRangeMap = this.pageTimeRanges$.getValue(); + + const newMap: PageTimeRangeMap = { ...pageTimeMap, [path]: value.toUrlString() }; + + this.preferenceService.set( + PageTimeRangeService.TIME_RANGE_PREFERENCE_KEY, + newMap, + PageTimeRangeService.STORAGE_TYPE + ); + } + + public ngOnDestroy(): void { + this.destroyed$.next(); + this.destroyed$.complete(); + } +} + +interface PageTimeRangeMap { + [path: string]: string; +} diff --git a/projects/common/src/time/time-range.service.ts b/projects/common/src/time/time-range.service.ts index 583773d78..77fd947f1 100644 --- a/projects/common/src/time/time-range.service.ts +++ b/projects/common/src/time/time-range.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { isEmpty } from 'lodash-es'; import { EMPTY, ReplaySubject } from 'rxjs'; -import { catchError, defaultIfEmpty, filter, map, switchMap, take } from 'rxjs/operators'; +import { catchError, defaultIfEmpty, filter, map, switchMap, take, tap } from 'rxjs/operators'; import { NavigationService, QueryParamObject } from '../navigation/navigation.service'; import { ReplayObservable } from '../utilities/rxjs/rxjs-utils'; import { FixedTimeRange } from './fixed-time-range'; @@ -67,6 +67,8 @@ export class TimeRangeService { this.navigationService.navigation$ .pipe( take(1), // Wait for first navigation + // tslint:disable-next-line: no-console + tap(activated => console.log('## activated route: ', activated)), switchMap(activatedRoute => activatedRoute.queryParamMap), // Get the params from it take(1), // Only the first set of params map(paramMap => paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM)), // Extract the time range value from it @@ -80,7 +82,7 @@ export class TimeRangeService { }); } - private timeRangeFromUrlString(timeRangeFromUrl: string): TimeRange { + public timeRangeFromUrlString(timeRangeFromUrl: string): TimeRange { const duration = this.timeDurationService.durationFromString(timeRangeFromUrl); if (duration) { return new RelativeTimeRange(duration); diff --git a/projects/components/src/navigation/nav-item/nav-item.component.ts b/projects/components/src/navigation/nav-item/nav-item.component.ts index 47c9c2664..96c4baed4 100644 --- a/projects/components/src/navigation/nav-item/nav-item.component.ts +++ b/projects/components/src/navigation/nav-item/nav-item.component.ts @@ -1,6 +1,16 @@ -import { ChangeDetectionStrategy, Component, Input } from '@angular/core'; +import { ChangeDetectionStrategy, Component, HostListener, Input, OnInit } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import { FeatureState, NavigationParams, NavigationParamsType } from '@hypertrace/common'; +import { + FeatureState, + NavigationParams, + NavigationParamsType, + PageTimeRangeService, + RelativeTimeRange, + TimeDuration, + TimeRangeService, + TimeUnit +} from '@hypertrace/common'; +import { isNil } from 'lodash-es'; import { IconSize } from '../../icon/icon-size'; import { NavItemLinkConfig } from '../navigation.config'; @@ -44,7 +54,7 @@ import { NavItemLinkConfig } from '../navigation.config'; ` }) -export class NavItemComponent { +export class NavItemComponent implements OnInit { @Input() public config!: NavItemLinkConfig; @@ -54,6 +64,23 @@ export class NavItemComponent { @Input() public collapsed: boolean = true; + @HostListener('click') + public onClick(): void { + this.setTimeRangeForPage(); + } + + public constructor( + private readonly activatedRoute: ActivatedRoute, + private readonly timeRangeService: TimeRangeService, + private readonly pageTimeRangeService: PageTimeRangeService + ) {} + + public ngOnInit(): void { + if (isNil(this.pageTimeRangeService.getPageTimeRange(this.config.matchPaths[0]))) { + this.pageTimeRangeService.setPageTimeRange(this.config.matchPaths[0], this.buildDefaultPageTimeRange()); + } + } + public buildNavigationParam = (item: NavItemLinkConfig): NavigationParams => ({ navType: NavigationParamsType.InApp, path: item.matchPaths[0], @@ -61,5 +88,18 @@ export class NavItemComponent { replaceCurrentHistory: item.replaceCurrentHistory }); - public constructor(private readonly activatedRoute: ActivatedRoute) {} + public buildDefaultPageTimeRange(): RelativeTimeRange { + if (isNil(this.config.defaultPageTimeRange?.value) || isNil(this.config.defaultPageTimeRange?.unit)) { + throw Error('Time range not provided for navigation route'); + } + const value: number = this.config.defaultPageTimeRange?.value ?? 1; + const unit: TimeUnit = this.config.defaultPageTimeRange?.unit ?? TimeUnit.Hour; + + return new RelativeTimeRange(new TimeDuration(value, unit)); + } + + public setTimeRangeForPage(): void { + const timeRange: RelativeTimeRange = this.pageTimeRangeService.getPageTimeRange(this.config.matchPaths[0])!; + this.timeRangeService.setRelativeRange(timeRange.duration.value, timeRange.duration.unit); + } } diff --git a/projects/components/src/navigation/navigation.config.ts b/projects/components/src/navigation/navigation.config.ts index 18cbd7e0c..fbaeefb03 100644 --- a/projects/components/src/navigation/navigation.config.ts +++ b/projects/components/src/navigation/navigation.config.ts @@ -1,4 +1,4 @@ -import { Color, FeatureState } from '@hypertrace/common'; +import { Color, FeatureState, TimeUnit } from '@hypertrace/common'; import { Observable } from 'rxjs'; import { IconSize } from '../icon/icon-size'; @@ -17,6 +17,7 @@ export interface NavItemLinkConfig { trailingIconTooltip?: string; trailingIconColor?: Color; featureState$?: Observable; + defaultPageTimeRange?: NavItemTimeRange; } export type FooterItemConfig = FooterItemLinkConfig; @@ -45,3 +46,8 @@ export const enum NavItemType { Divider = 'divider', Footer = 'footer' } + +export interface NavItemTimeRange { + value: number; + unit: TimeUnit; +} diff --git a/projects/components/src/time-range/time-range.component.ts b/projects/components/src/time-range/time-range.component.ts index 85cb7141b..17a84ffee 100644 --- a/projects/components/src/time-range/time-range.component.ts +++ b/projects/components/src/time-range/time-range.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectionStrategy, Component, NgZone } from '@angular/core'; +import { ChangeDetectionStrategy, Component, EventEmitter, NgZone, Output } from '@angular/core'; import { IconType } from '@hypertrace/assets-library'; import { FixedTimeRange, @@ -75,6 +75,9 @@ export class TimeRangeComponent { public showCustom: boolean = false; + @Output() + public readonly timeRangeSelected: EventEmitter = new EventEmitter(); + public constructor(private readonly timeRangeService: TimeRangeService, private readonly ngZone: NgZone) {} public onPopoverOpen(popoverRef: PopoverRef): void { @@ -88,6 +91,7 @@ export class TimeRangeComponent { public setToRelativeTimeRange(timeRange: RelativeTimeRange): void { this.timeRangeService.setRelativeRange(timeRange.duration.value, timeRange.duration.unit); + this.timeRangeSelected.emit(timeRange); this.popoverRef!.close(); } From 63a665e7f3468018c6292fa3364b2be6f3d78194 Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Fri, 11 Feb 2022 10:43:11 -0400 Subject: [PATCH 03/47] fix: removed defaultTimeRange null check for throwing error --- .../src/navigation/nav-item/nav-item.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/components/src/navigation/nav-item/nav-item.component.ts b/projects/components/src/navigation/nav-item/nav-item.component.ts index 96c4baed4..eb08ff5cd 100644 --- a/projects/components/src/navigation/nav-item/nav-item.component.ts +++ b/projects/components/src/navigation/nav-item/nav-item.component.ts @@ -89,9 +89,9 @@ export class NavItemComponent implements OnInit { }); public buildDefaultPageTimeRange(): RelativeTimeRange { - if (isNil(this.config.defaultPageTimeRange?.value) || isNil(this.config.defaultPageTimeRange?.unit)) { - throw Error('Time range not provided for navigation route'); - } + // TODO if (isNil(this.config.defaultPageTimeRange?.value) || isNil(this.config.defaultPageTimeRange?.unit)) { + // TODO throw Error('Time range not provided for navigation route'); + // } const value: number = this.config.defaultPageTimeRange?.value ?? 1; const unit: TimeUnit = this.config.defaultPageTimeRange?.unit ?? TimeUnit.Hour; From b81ae1a34a7e76ab31b9e482f2c87e8c9c745571 Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Mon, 14 Feb 2022 15:03:45 -0400 Subject: [PATCH 04/47] refactor: time range logic to be query param centric --- .../common/src/navigation/ht-route-data.ts | 2 + .../src/time/page-time-range.service.ts | 44 ++++++----- .../common/src/time/time-range.service.ts | 29 +++++--- .../navigation/nav-item/nav-item.component.ts | 73 ++++++++++++------- .../src/navigation/navigation.config.ts | 8 +- .../src/time-range/time-range.component.ts | 3 +- 6 files changed, 92 insertions(+), 67 deletions(-) diff --git a/projects/common/src/navigation/ht-route-data.ts b/projects/common/src/navigation/ht-route-data.ts index 4d0d53d89..72af980d3 100644 --- a/projects/common/src/navigation/ht-route-data.ts +++ b/projects/common/src/navigation/ht-route-data.ts @@ -1,8 +1,10 @@ import { Observable } from 'rxjs'; +import { TimeRange } from '../time/time-range'; import { Breadcrumb } from './breadcrumb'; export interface HtRouteData { breadcrumb?: Breadcrumb | Observable; features?: string[]; title?: string; + defaultTimeRange?: TimeRange; } diff --git a/projects/common/src/time/page-time-range.service.ts b/projects/common/src/time/page-time-range.service.ts index 1c3353e06..029144201 100644 --- a/projects/common/src/time/page-time-range.service.ts +++ b/projects/common/src/time/page-time-range.service.ts @@ -1,47 +1,45 @@ import { Injectable, OnDestroy } from '@angular/core'; import { isNil } from 'lodash-es'; -import { BehaviorSubject, Subject } from 'rxjs'; -import { takeUntil } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subject } from 'rxjs'; +import { distinctUntilChanged, map, share, takeUntil } from 'rxjs/operators'; import { PreferenceService, StorageType } from '../preference/preference.service'; +import { TimeRange } from './time-range'; // Import {TimeRange} from "./time-range"; -import { RelativeTimeRange } from './relative-time-range'; import { TimeRangeService } from './time-range.service'; // Import {TimeDuration} from "./time-duration"; @Injectable({ providedIn: 'root' }) export class PageTimeRangeService implements OnDestroy { - // TODO change to local + // TODO change to local ?? private static readonly STORAGE_TYPE: StorageType = StorageType.Session; private static readonly TIME_RANGE_PREFERENCE_KEY: string = 'page-time-range'; - private readonly pageTimeRanges$: BehaviorSubject = new BehaviorSubject({}); - private readonly destroyed$: Subject = new Subject(); + private readonly pageTimeRanges$: BehaviorSubject = new BehaviorSubject({}); + private readonly storedTimeRanges$: Observable = this.preferenceService + .get(PageTimeRangeService.TIME_RANGE_PREFERENCE_KEY, {}, PageTimeRangeService.STORAGE_TYPE) + .pipe(takeUntil(this.destroyed$)); + public constructor( private readonly preferenceService: PreferenceService, private readonly timeRangeService: TimeRangeService ) { - this.preferenceService - .get(PageTimeRangeService.TIME_RANGE_PREFERENCE_KEY, {}, PageTimeRangeService.STORAGE_TYPE) - .pipe(takeUntil(this.destroyed$)) - .subscribe(values => { - // tslint:disable-next-line: no-console - console.log('stored map emission ', values); - this.pageTimeRanges$.next(values); - }); + this.storedTimeRanges$.subscribe(values => { + this.pageTimeRanges$.next(values); + }); } - public getPageTimeRange(path: string): RelativeTimeRange | undefined { - const pageTimeMap = this.pageTimeRanges$.getValue(); - - if (isNil(pageTimeMap[path])) { - return undefined; - } - - return this.timeRangeService.timeRangeFromUrlString(pageTimeMap[path]) as RelativeTimeRange; + public getPageTimeRange(path: string): Observable { + return this.storedTimeRanges$.pipe( + distinctUntilChanged((prev, curr) => prev[path] === curr[path]), + map(timeRanges => + !isNil(timeRanges[path]) ? this.timeRangeService.timeRangeFromUrlString(timeRanges[path]) : undefined + ), + share() + ); } - public setPageTimeRange(path: string, value: RelativeTimeRange): void { + public setPageTimeRange(path: string, value: TimeRange): void { const pageTimeMap: PageTimeRangeMap = this.pageTimeRanges$.getValue(); const newMap: PageTimeRangeMap = { ...pageTimeMap, [path]: value.toUrlString() }; diff --git a/projects/common/src/time/time-range.service.ts b/projects/common/src/time/time-range.service.ts index 77fd947f1..f895ab50c 100644 --- a/projects/common/src/time/time-range.service.ts +++ b/projects/common/src/time/time-range.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { isEmpty } from 'lodash-es'; -import { EMPTY, ReplaySubject } from 'rxjs'; -import { catchError, defaultIfEmpty, filter, map, switchMap, take, tap } from 'rxjs/operators'; +import { combineLatest, EMPTY, ReplaySubject } from 'rxjs'; +import { catchError, defaultIfEmpty, map, switchMap } from 'rxjs/operators'; import { NavigationService, QueryParamObject } from '../navigation/navigation.service'; import { ReplayObservable } from '../utilities/rxjs/rxjs-utils'; import { FixedTimeRange } from './fixed-time-range'; @@ -64,16 +64,23 @@ export class TimeRangeService { } private initializeTimeRange(): void { - this.navigationService.navigation$ + combineLatest([ + this.navigationService.navigation$, + this.navigationService.navigation$.pipe(switchMap(activatedRoute => activatedRoute.queryParamMap)) + ]) .pipe( - take(1), // Wait for first navigation - // tslint:disable-next-line: no-console - tap(activated => console.log('## activated route: ', activated)), - switchMap(activatedRoute => activatedRoute.queryParamMap), // Get the params from it - take(1), // Only the first set of params - map(paramMap => paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM)), // Extract the time range value from it - filter((timeRangeString): timeRangeString is string => !isEmpty(timeRangeString)), // Only valid time ranges - map(timeRangeString => this.timeRangeFromUrlString(timeRangeString)), + map(([activeRoute, paramMap]) => { + const defaultPageTimeRange = activeRoute.snapshot.data?.defaultTimeRange; + const timeRangeString = paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM); + + if (typeof timeRangeString === 'string' && !isEmpty(timeRangeString)) { + return this.timeRangeFromUrlString(timeRangeString); + } + if (defaultPageTimeRange) { + return defaultPageTimeRange; + } + throw Error(); + }), catchError(() => EMPTY), defaultIfEmpty(this.defaultTimeRange) ) diff --git a/projects/components/src/navigation/nav-item/nav-item.component.ts b/projects/components/src/navigation/nav-item/nav-item.component.ts index eb08ff5cd..66f7da66d 100644 --- a/projects/components/src/navigation/nav-item/nav-item.component.ts +++ b/projects/components/src/navigation/nav-item/nav-item.component.ts @@ -1,16 +1,19 @@ -import { ChangeDetectionStrategy, Component, HostListener, Input, OnInit } from '@angular/core'; +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnChanges, OnDestroy } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { FeatureState, NavigationParams, NavigationParamsType, + NavigationService, PageTimeRangeService, - RelativeTimeRange, - TimeDuration, + QueryParamObject, + TimeRange, TimeRangeService, - TimeUnit + TypedSimpleChanges } from '@hypertrace/common'; import { isNil } from 'lodash-es'; +import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; import { IconSize } from '../../icon/icon-size'; import { NavItemLinkConfig } from '../navigation.config'; @@ -19,7 +22,7 @@ import { NavItemLinkConfig } from '../navigation.config'; styleUrls: ['./nav-item.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, template: ` - + - - + + - + @@ -63,6 +68,8 @@ import { HeaderContentSecondaryDirective } from '../header-content/header-conten ` }) export class PageHeaderComponent implements OnInit { + public readonly showPageTimeRange: Observable; + @Input() public persistenceId?: string; @@ -72,11 +79,11 @@ export class PageHeaderComponent implements OnInit { @Input() public isBeta: boolean = false; - @ContentChild(HeaderContentPrimaryDirective) - public readonly contentPrimary?: HeaderContentPrimaryDirective; + @ContentChild(HeaderPrimaryRowContentDirective) + public readonly primaryRowContent?: HeaderPrimaryRowContentDirective; - @ContentChild(HeaderContentSecondaryDirective) - public readonly contentSecondary?: HeaderContentSecondaryDirective; + @ContentChild(HeaderSecondaryRowContentDirective) + public readonly secondaryRowContent?: HeaderSecondaryRowContentDirective; public breadcrumbs$: Observable = this.breadcrumbsService.breadcrumbs$.pipe( map(breadcrumbs => (breadcrumbs.length > 0 ? breadcrumbs : undefined)) @@ -90,8 +97,13 @@ export class PageHeaderComponent implements OnInit { protected readonly navigationService: NavigationService, protected readonly preferenceService: PreferenceService, protected readonly subscriptionLifecycle: SubscriptionLifecycle, - protected readonly breadcrumbsService: BreadcrumbsService - ) {} + protected readonly breadcrumbsService: BreadcrumbsService, + protected readonly featureResolver: FeatureStateResolver + ) { + this.showPageTimeRange = this.featureResolver + .getFeatureState(PageTimeRangeFeature.PageTimeRange) + .pipe(map(featureState => featureState === FeatureState.Enabled)); + } public ngOnInit(): void { this.subscriptionLifecycle.add( diff --git a/projects/components/src/header/page/page-header.module.ts b/projects/components/src/header/page/page-header.module.ts index 3c25ac278..60f99535a 100644 --- a/projects/components/src/header/page/page-header.module.ts +++ b/projects/components/src/header/page/page-header.module.ts @@ -1,14 +1,13 @@ +import { TimeRangeForPageModule } from '../../time-range/time-range-for-page/time-range-for-page.module'; import { HeaderContentModule } from '../header-content/header-content.module'; import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { BetaTagModule } from '../../beta-tag/beta-tag.module'; import { BreadcrumbsModule } from '../../breadcrumbs/breadcrumbs.module'; -import { FeatureConfigCheckModule } from '../../feature-check/feature-config-check.module'; import { IconModule } from '../../icon/icon.module'; import { LabelModule } from '../../label/label.module'; import { NavigableTabModule } from '../../tabs/navigable/navigable-tab.module'; -import { PageTimeRangeModule } from '../../time-range/page-time-range/page-time-range.module'; import { TimeRangeModule } from '../../time-range/time-range.module'; import { PageHeaderComponent } from './page-header.component'; @@ -23,9 +22,8 @@ import { PageHeaderComponent } from './page-header.component'; LabelModule, NavigableTabModule, BetaTagModule, - PageTimeRangeModule, - HeaderContentModule, - FeatureConfigCheckModule + TimeRangeForPageModule, + HeaderContentModule ] }) export class PageHeaderModule {} diff --git a/projects/components/src/navigation/nav-item/nav-item.component.test.ts b/projects/components/src/navigation/nav-item/nav-item.component.test.ts index 964f73f88..189075883 100644 --- a/projects/components/src/navigation/nav-item/nav-item.component.test.ts +++ b/projects/components/src/navigation/nav-item/nav-item.component.test.ts @@ -7,10 +7,10 @@ import { MemoizeModule, NavigationParamsType, NavigationService, - PageTimeRangeService, RelativeTimeRange, TimeDuration, TimeRange, + TimeRangeForPageService, TimeUnit } from '@hypertrace/common'; import { BetaTagComponent, IconComponent, LinkComponent } from '@hypertrace/components'; @@ -41,8 +41,8 @@ describe('Navigation Item Component', () => { mockProvider(FeatureStateResolver, { getCombinedFeatureState: () => of(FeatureState.Enabled) }), - mockProvider(PageTimeRangeService, { - getPageTimeRange: jest + mockProvider(TimeRangeForPageService, { + getTimeRangeForCurrentPage: jest .fn() .mockReturnValue(of(new FixedTimeRange(new Date('Feb 15 2022 16:00:00'), new Date('Feb 17 2022 17:32:01')))) }), @@ -138,8 +138,8 @@ describe('Navigation Item Component', () => { spectator = createHost(``, { hostProps: { navItem: navItem }, providers: [ - mockProvider(PageTimeRangeService, { - getPageTimeRange: jest.fn().mockReturnValue(of(timeRange)) + mockProvider(TimeRangeForPageService, { + getTimeRangeForCurrentPage: jest.fn().mockReturnValue(of(timeRange)) }) ] }); diff --git a/projects/components/src/navigation/nav-item/nav-item.component.ts b/projects/components/src/navigation/nav-item/nav-item.component.ts index 3aed07ae1..68151f39e 100644 --- a/projects/components/src/navigation/nav-item/nav-item.component.ts +++ b/projects/components/src/navigation/nav-item/nav-item.component.ts @@ -1,7 +1,14 @@ -import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core'; +import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import { FeatureState, NavigationParams, NavigationParamsType, PageTimeRangeService } from '@hypertrace/common'; -import { Observable, of } from 'rxjs'; +import { + FeatureState, + NavigationParams, + NavigationParamsType, + TimeRangeForPageService, + TypedSimpleChanges +} from '@hypertrace/common'; +import { isNil } from 'lodash-es'; +import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { IconSize } from '../../icon/icon-size'; import { NavItemLinkConfig } from '../navigation.config'; @@ -46,12 +53,12 @@ import { NavItemLinkConfig } from '../navigation.config'; ` }) -export class NavItemComponent implements OnInit { +export class NavItemComponent implements OnChanges { private static readonly TIME_RANGE_QUERY_PARAM: string = 'time'; - public navItemParams$: Observable = of(undefined); + public navItemParams$: Observable | undefined; @Input() - public config!: NavItemLinkConfig; + public config?: NavItemLinkConfig; @Input() public active: boolean = true; @@ -61,19 +68,25 @@ export class NavItemComponent implements OnInit { public constructor( private readonly activatedRoute: ActivatedRoute, - private readonly pageTimeRangeService: PageTimeRangeService + private readonly timeRangeForPageService: TimeRangeForPageService ) {} - public ngOnInit(): void { - this.navItemParams$ = this.pageTimeRangeService.getPageTimeRange(this.config.matchPaths[0]).pipe( + public ngOnChanges(changes: TypedSimpleChanges): void { + if (changes.config && !isNil(this.config)) { + this.navItemParams$ = this.getNavParamsForPath(this.config.matchPaths[0]); + } + } + + private getNavParamsForPath(path: string): Observable { + return this.timeRangeForPageService.getTimeRangeForCurrentPage(path).pipe( map(timeRange => ({ navType: NavigationParamsType.InApp, - path: this.config.matchPaths[0], + path: path, relativeTo: this.activatedRoute, queryParams: { [NavItemComponent.TIME_RANGE_QUERY_PARAM]: timeRange.toUrlString() }, - replaceCurrentHistory: this.config.replaceCurrentHistory + replaceCurrentHistory: this.config!.replaceCurrentHistory })) ); } diff --git a/projects/components/src/public-api.ts b/projects/components/src/public-api.ts index 3ada29bf0..81b05f024 100644 --- a/projects/components/src/public-api.ts +++ b/projects/components/src/public-api.ts @@ -126,8 +126,8 @@ export * from './header/application/application-header.module'; export * from './header/page/page-header.component'; export * from './header/page/page-header.module'; export * from './header/header-content/header-content.module'; -export * from './header/header-content/header-content-primary.directive'; -export * from './header/header-content/header-content-secondary.directive'; +export * from './header/header-content/header-primary-row-content.directive'; +export * from './header/header-content/header-secondary-row-content.directive'; // Icon export * from './icon/icon-size'; @@ -333,8 +333,8 @@ export * from './time-range/time-range.component'; export * from './time-range/time-range.module'; // Page Time Range -export * from './time-range/page-time-range/page-time-range.component'; -export * from './time-range/page-time-range/page-time-range.module'; +export * from './time-range/time-range-for-page/time-range-for-page.component'; +export * from './time-range/time-range-for-page/time-range-for-page.module'; // Titled Content export { diff --git a/projects/components/src/time-range/page-time-range/page-time-range.module.ts b/projects/components/src/time-range/page-time-range/page-time-range.module.ts deleted file mode 100644 index 996ec4f57..000000000 --- a/projects/components/src/time-range/page-time-range/page-time-range.module.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { NgModule } from '@angular/core'; - -import { CommonModule } from '@angular/common'; -import { FeatureConfigCheckModule } from '../../feature-check/feature-config-check.module'; -import { TimeRangeModule } from '../time-range.module'; -import { PageTimeRangeComponent } from './page-time-range.component'; - -@NgModule({ - declarations: [PageTimeRangeComponent], - exports: [PageTimeRangeComponent], - imports: [CommonModule, TimeRangeModule, FeatureConfigCheckModule], - providers: [] -}) -export class PageTimeRangeModule {} diff --git a/projects/components/src/time-range/page-time-range/page-time-range.component.test.ts b/projects/components/src/time-range/time-range-for-page/time-range-for-page.component.test.ts similarity index 85% rename from projects/components/src/time-range/page-time-range/page-time-range.component.test.ts rename to projects/components/src/time-range/time-range-for-page/time-range-for-page.component.test.ts index 221012f11..bd67efd0f 100644 --- a/projects/components/src/time-range/page-time-range/page-time-range.component.test.ts +++ b/projects/components/src/time-range/time-range-for-page/time-range-for-page.component.test.ts @@ -3,18 +3,19 @@ import { FeatureState, FeatureStateResolver, NavigationService, - PageTimeRangeService, RelativeTimeRange, TimeDuration, + TimeRangeForPageService, TimeUnit } from '@hypertrace/common'; -import { FeatureConfigCheckModule, PageTimeRangeComponent, TimeRangeComponent } from '@hypertrace/components'; +import { FeatureConfigCheckModule, TimeRangeComponent } from '@hypertrace/components'; import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; import { of } from 'rxjs'; +import { TimeRangeForPageComponent } from './time-range-for-page.component'; describe('Page time range component', () => { - let spectator: SpectatorHost; + let spectator: SpectatorHost; const route = { snapshot: { data: { @@ -26,14 +27,14 @@ describe('Page time range component', () => { const createHost = createHostFactory({ shallow: true, - component: PageTimeRangeComponent, + component: TimeRangeForPageComponent, declarations: [MockComponent(TimeRangeComponent)], imports: [FeatureConfigCheckModule], providers: [ mockProvider(NavigationService, { getCurrentActivatedRoute: jest.fn().mockReturnValue(route) }), - mockProvider(PageTimeRangeService), + mockProvider(TimeRangeForPageService), mockProvider(FeatureStateResolver, { getCombinedFeatureState: () => of(FeatureState.Enabled) }) @@ -41,7 +42,7 @@ describe('Page time range component', () => { }); test('should not attempt to save time range when route does not have default range', () => { - spectator = createHost(``); + spectator = createHost(``); const timeRange = new RelativeTimeRange(new TimeDuration(2, TimeUnit.Hour)); const timeRangeComponent = spectator.query(TimeRangeComponent); @@ -53,7 +54,7 @@ describe('Page time range component', () => { }); test('should not attempt to save time range when route is not a first level route', () => { - spectator = createHost(``, { + spectator = createHost(``, { providers: [ mockProvider(NavigationService, { getCurrentActivatedRoute: jest.fn().mockReturnValue({ @@ -78,7 +79,7 @@ describe('Page time range component', () => { }); test('should save time range when route is first level, and the defaultTimeRange property is present', () => { - spectator = createHost(``, { + spectator = createHost(``, { providers: [ mockProvider(NavigationService, { getCurrentActivatedRoute: jest.fn().mockReturnValue({ diff --git a/projects/components/src/time-range/page-time-range/page-time-range.component.ts b/projects/components/src/time-range/time-range-for-page/time-range-for-page.component.ts similarity index 64% rename from projects/components/src/time-range/page-time-range/page-time-range.component.ts rename to projects/components/src/time-range/time-range-for-page/time-range-for-page.component.ts index 4bc45df4e..3672bb58d 100644 --- a/projects/components/src/time-range/page-time-range/page-time-range.component.ts +++ b/projects/components/src/time-range/time-range-for-page/time-range-for-page.component.ts @@ -1,25 +1,16 @@ import { ChangeDetectionStrategy, Component } from '@angular/core'; import { ActivatedRoute, UrlSegment } from '@angular/router'; -import { NavigationService, PageTimeRangeService, TimeRange } from '@hypertrace/common'; +import { NavigationService, TimeRange, TimeRangeForPageService } from '@hypertrace/common'; import { isNil } from 'lodash-es'; -export const enum PageTimeRangeFeature { - PageTimeRange = 'ui.page-time-range' -} - @Component({ - selector: 'ht-page-time-range', - template: ` - - `, + selector: 'ht-time-range-for-page', + template: ` `, changeDetection: ChangeDetectionStrategy.OnPush }) -export class PageTimeRangeComponent { +export class TimeRangeForPageComponent { public constructor( - private readonly pageTimeRangeService: PageTimeRangeService, + private readonly timeRangeForPageService: TimeRangeForPageService, private readonly navigationService: NavigationService ) {} @@ -38,7 +29,7 @@ export class PageTimeRangeComponent { public savePageTimeRange(selectedTimeRange: TimeRange, segment: UrlSegment): void { if (!isNil(segment.path)) { - this.pageTimeRangeService.setPageTimeRange(segment.path, selectedTimeRange); + this.timeRangeForPageService.setTimeRangeForCurrentPage(segment.path, selectedTimeRange); } else { throw Error('No segment provided to set page time range'); } diff --git a/projects/components/src/time-range/time-range-for-page/time-range-for-page.module.ts b/projects/components/src/time-range/time-range-for-page/time-range-for-page.module.ts new file mode 100644 index 000000000..768a47f7a --- /dev/null +++ b/projects/components/src/time-range/time-range-for-page/time-range-for-page.module.ts @@ -0,0 +1,13 @@ +import { NgModule } from '@angular/core'; + +import { CommonModule } from '@angular/common'; +import { TimeRangeModule } from '../time-range.module'; +import { TimeRangeForPageComponent } from './time-range-for-page.component'; + +@NgModule({ + declarations: [TimeRangeForPageComponent], + exports: [TimeRangeForPageComponent], + imports: [CommonModule, TimeRangeModule], + providers: [] +}) +export class TimeRangeForPageModule {} diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts index 03c94343a..d18a0d1e1 100644 --- a/projects/observability/src/pages/explorer/explorer.component.test.ts +++ b/projects/observability/src/pages/explorer/explorer.component.test.ts @@ -6,6 +6,7 @@ import { RouterTestingModule } from '@angular/router/testing'; import { IconLibraryTestingModule } from '@hypertrace/assets-library'; import { DEFAULT_COLOR_PALETTE, + FeatureStateResolver, LayoutChangeService, NavigationService, PreferenceService, @@ -85,6 +86,9 @@ describe('Explorer component', () => { mockProvider(GraphQlRequestService, { query: jest.fn().mockReturnValueOnce(of(mockAttributes)).mockReturnValue(EMPTY) }), + mockProvider(FeatureStateResolver, { + getFeatureState: jest.fn().mockReturnValue(of(true)) + }), mockProvider(TimeRangeService, { getCurrentTimeRange: () => testTimeRange, getTimeRangeAndChanges: () => NEVER.pipe(startWith(testTimeRange)) From 6e78e54e87113cdae2dabe754f15cee7d304f74c Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Tue, 1 Mar 2022 17:22:06 -0400 Subject: [PATCH 20/47] fix: export feature enum --- projects/components/src/header/page/page-header.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/components/src/header/page/page-header.component.ts b/projects/components/src/header/page/page-header.component.ts index 5dc25a73a..610678aeb 100644 --- a/projects/components/src/header/page/page-header.component.ts +++ b/projects/components/src/header/page/page-header.component.ts @@ -16,7 +16,7 @@ import { NavigableTab } from '../../tabs/navigable/navigable-tab'; import { HeaderPrimaryRowContentDirective } from '../header-content/header-primary-row-content.directive'; import { HeaderSecondaryRowContentDirective } from '../header-content/header-secondary-row-content.directive'; -const enum PageTimeRangeFeature { +export const enum PageTimeRangeFeature { PageTimeRange = 'ui.page-time-range' } @Component({ From c225cf9ad98fac1abfaf64ff8119c73d06bb2449 Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Wed, 2 Mar 2022 15:21:56 -0400 Subject: [PATCH 21/47] refactor: replaced time range icon with calendar --- projects/assets-library/assets/icons/calendar-dates.svg | 8 ++++++++ projects/assets-library/src/icons/icon-library.module.ts | 1 + projects/assets-library/src/icons/icon-type.ts | 1 + .../components/src/time-range/time-range.component.ts | 2 +- 4 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 projects/assets-library/assets/icons/calendar-dates.svg diff --git a/projects/assets-library/assets/icons/calendar-dates.svg b/projects/assets-library/assets/icons/calendar-dates.svg new file mode 100644 index 000000000..3c8c8f497 --- /dev/null +++ b/projects/assets-library/assets/icons/calendar-dates.svg @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/projects/assets-library/src/icons/icon-library.module.ts b/projects/assets-library/src/icons/icon-library.module.ts index b00809752..c5abfa164 100644 --- a/projects/assets-library/src/icons/icon-library.module.ts +++ b/projects/assets-library/src/icons/icon-library.module.ts @@ -25,6 +25,7 @@ const iconsRootPath = 'assets/icons'; { key: IconType.ArrowUpLeft, url: `${iconsRootPath}/arrow-up-left.svg` }, { key: IconType.ArrowUpRight, url: `${iconsRootPath}/arrow-up-right.svg` }, { key: IconType.Calls, url: `${iconsRootPath}/calls.svg` }, + { key: IconType.Calendar, url: `${iconsRootPath}/calendar-dates.svg` }, { key: IconType.CheckCircle, url: `${iconsRootPath}/check-circle.svg` }, { key: IconType.CheckCircleFill, url: `${iconsRootPath}/check-circle-fill.svg` }, { key: IconType.ChevronDown, url: `${iconsRootPath}/chevron-down.svg` }, diff --git a/projects/assets-library/src/icons/icon-type.ts b/projects/assets-library/src/icons/icon-type.ts index bf52f732f..946529f72 100644 --- a/projects/assets-library/src/icons/icon-type.ts +++ b/projects/assets-library/src/icons/icon-type.ts @@ -15,6 +15,7 @@ export const enum IconType { ArrowUpLeft = 'svg:arrow-up-left', ArrowUpRight = 'svg:arrow-up-right', Calls = 'svg:calls', + Calendar = 'svg:calendar-dates', Cancel = 'cancel', CheckCircle = 'svg:check-circle', CheckCircleFill = 'svg:check-circle-fill', diff --git a/projects/components/src/time-range/time-range.component.ts b/projects/components/src/time-range/time-range.component.ts index 01f19fa6c..2bf3ed768 100644 --- a/projects/components/src/time-range/time-range.component.ts +++ b/projects/components/src/time-range/time-range.component.ts @@ -24,7 +24,7 @@ import { PopoverRef } from '../popover/popover-ref';
- +
From 7bc71c7a68c18ba099b9c646795693369663b80d Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Thu, 3 Mar 2022 19:21:20 -0400 Subject: [PATCH 22/47] fix: requested changes --- projects/common/src/time/time-range.service.test.ts | 10 ++++------ projects/common/src/time/time-range.service.ts | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/projects/common/src/time/time-range.service.test.ts b/projects/common/src/time/time-range.service.test.ts index 548e17a77..42f0d5ed1 100644 --- a/projects/common/src/time/time-range.service.test.ts +++ b/projects/common/src/time/time-range.service.test.ts @@ -123,12 +123,10 @@ describe('Time range service', () => { const spectator = buildService({ providers: [ mockProvider(NavigationService, { - get navigation$(): Observable { - return cold('-a-b', { - a: buildMockRoute(firstMockRoute), - b: buildMockRoute(secondMockRoute) - }); - } + navigation$: cold('-a-b', { + a: buildMockRoute(firstMockRoute), + b: buildMockRoute(secondMockRoute) + }) }) ] }); diff --git a/projects/common/src/time/time-range.service.ts b/projects/common/src/time/time-range.service.ts index 75d8a15db..90a5bb29f 100644 --- a/projects/common/src/time/time-range.service.ts +++ b/projects/common/src/time/time-range.service.ts @@ -80,9 +80,9 @@ export class TimeRangeService { return this.timeRangeFromUrlString(timeRangeString); } - const defaultPageTimeRange = activeRoute.snapshot.data?.defaultTimeRange; - if (defaultPageTimeRange) { - return defaultPageTimeRange; + const activeRouteTimeRange = activeRoute.snapshot.data?.defaultTimeRange; + if (activeRouteTimeRange) { + return activeRouteTimeRange; } throw Error(); }), From b4b4722b7d3ea52e31bd6dc0e3747e69a6898b8d Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Fri, 4 Mar 2022 11:26:26 -0400 Subject: [PATCH 23/47] fix: backwards compatability and depency issue changes --- .../header/page/page-header.component.scss | 51 ++++++++++--------- .../src/header/page/page-header.component.ts | 38 +++++++++----- .../src/header/page/page-header.module.ts | 4 +- 3 files changed, 55 insertions(+), 38 deletions(-) diff --git a/projects/components/src/header/page/page-header.component.scss b/projects/components/src/header/page/page-header.component.scss index ecdc73510..6bb1863b8 100644 --- a/projects/components/src/header/page/page-header.component.scss +++ b/projects/components/src/header/page/page-header.component.scss @@ -5,42 +5,47 @@ margin: 24px 24px 0; display: flex; flex-direction: column; - .column-alignment { display: flex; flex-direction: column; + } - .primary-content { - display: flex; - justify-content: space-between; + .row-alignment { + display: flex; + justify-content: space-between; + } - .breadcrumb-container { - align-items: center; - margin-right: auto; + .primary-content { + display: flex; + justify-content: flex-end; + width: 100%; - .breadcrumb-separator { - padding: 0 20px; - } + .breadcrumb-container { + margin-right: auto; + align-items: center; - .title { - @include font-subtitle-medium($gray-7); - display: flex; - align-items: center; + .breadcrumb-separator { + padding: 0 20px; + } - .icon { - padding-right: 12px; - } + .title { + @include font-subtitle-medium($gray-7); + display: flex; + align-items: center; - .beta { - margin-left: 12px; - } + .icon { + padding-right: 12px; } - } - .time-range { - margin-left: 10px; + .beta { + margin-left: 12px; + } } } + + .time-range { + margin-left: 10px; + } } .row-alignment { diff --git a/projects/components/src/header/page/page-header.component.ts b/projects/components/src/header/page/page-header.component.ts index 610678aeb..5f8189dec 100644 --- a/projects/components/src/header/page/page-header.component.ts +++ b/projects/components/src/header/page/page-header.component.ts @@ -1,8 +1,6 @@ import { ChangeDetectionStrategy, Component, ContentChild, Input, OnInit } from '@angular/core'; import { Breadcrumb, - FeatureState, - FeatureStateResolver, isNonEmptyString, NavigationService, PreferenceService, @@ -30,7 +28,9 @@ export const enum PageTimeRangeFeature { class="page-header" [class.bottom-border]="!this.tabs?.length" > -
+ +
- +
- +
@@ -68,8 +74,6 @@ export const enum PageTimeRangeFeature { ` }) export class PageHeaderComponent implements OnInit { - public readonly showPageTimeRange: Observable; - @Input() public persistenceId?: string; @@ -79,6 +83,12 @@ export class PageHeaderComponent implements OnInit { @Input() public isBeta: boolean = false; + /** + * Alignment must be set to column (default) for secondaryRowContent projection, + * */ + @Input() + public contentAlignment: PageHeaderContentAlignment = PageHeaderContentAlignment.Column; + @ContentChild(HeaderPrimaryRowContentDirective) public readonly primaryRowContent?: HeaderPrimaryRowContentDirective; @@ -97,13 +107,8 @@ export class PageHeaderComponent implements OnInit { protected readonly navigationService: NavigationService, protected readonly preferenceService: PreferenceService, protected readonly subscriptionLifecycle: SubscriptionLifecycle, - protected readonly breadcrumbsService: BreadcrumbsService, - protected readonly featureResolver: FeatureStateResolver - ) { - this.showPageTimeRange = this.featureResolver - .getFeatureState(PageTimeRangeFeature.PageTimeRange) - .pipe(map(featureState => featureState === FeatureState.Enabled)); - } + protected readonly breadcrumbsService: BreadcrumbsService + ) {} public ngOnInit(): void { this.subscriptionLifecycle.add( @@ -142,3 +147,8 @@ export class PageHeaderComponent implements OnInit { interface PageHeaderPreferences { selectedTabPath?: string; } + +export const enum PageHeaderContentAlignment { + Column = 'column-alignment', + Row = 'row-alignment' +} diff --git a/projects/components/src/header/page/page-header.module.ts b/projects/components/src/header/page/page-header.module.ts index 60f99535a..1b8cc9cbc 100644 --- a/projects/components/src/header/page/page-header.module.ts +++ b/projects/components/src/header/page/page-header.module.ts @@ -1,3 +1,4 @@ +import { FeatureConfigCheckModule } from '../../feature-check/feature-config-check.module'; import { TimeRangeForPageModule } from '../../time-range/time-range-for-page/time-range-for-page.module'; import { HeaderContentModule } from '../header-content/header-content.module'; @@ -23,7 +24,8 @@ import { PageHeaderComponent } from './page-header.component'; NavigableTabModule, BetaTagModule, TimeRangeForPageModule, - HeaderContentModule + HeaderContentModule, + FeatureConfigCheckModule ] }) export class PageHeaderModule {} From a528987cf4ae597f41ef89e5170e767f20069c23 Mon Sep 17 00:00:00 2001 From: Christian Quinn Date: Mon, 7 Mar 2022 19:13:42 -0400 Subject: [PATCH 24/47] fix: requested changes --- projects/common/src/public-api.ts | 2 +- .../common/src/time/time-range.service.ts | 39 +++++-------- ...user-specified-time-range-service.test.ts} | 20 ++++--- ...s => user-specified-time-range.service.ts} | 28 +++++----- .../header-primary-row-content.directive.ts | 2 +- .../header-secondary-row-content.directive.ts | 2 +- .../src/header/page/page-header.component.ts | 44 +++++++++++---- .../src/header/page/page-header.module.ts | 6 +- .../navigation/nav-item/nav-item.component.ts | 56 +++++++------------ .../navigation-list-component.service.ts | 27 ++++++++- .../navigation/navigation-list.component.ts | 39 +++++++++++-- .../src/navigation/navigation.config.ts | 3 +- projects/components/src/public-api.ts | 4 -- .../time-range-for-page.module.ts | 13 ----- projects/observability/src/public-api.ts | 4 ++ ...pecified-time-range-selector.component.ts} | 10 ++-- ...er-specified-time-range-selector.module.ts | 13 +++++ ...ser-specified-time-range-selector.test.ts} | 6 +- .../shared/navigation/navigation.component.ts | 16 +++++- 19 files changed, 199 insertions(+), 135 deletions(-) rename projects/common/src/time/{time-range-for-page-service.test.ts => user-specified-time-range-service.test.ts} (71%) rename projects/common/src/time/{time-range-for-page.service.ts => user-specified-time-range.service.ts} (70%) delete mode 100644 projects/components/src/time-range/time-range-for-page/time-range-for-page.module.ts rename projects/{components/src/time-range/time-range-for-page/time-range-for-page.component.ts => observability/src/shared/components/user-specified-time-range-selector/user-specified-time-range-selector.component.ts} (75%) create mode 100644 projects/observability/src/shared/components/user-specified-time-range-selector/user-specified-time-range-selector.module.ts rename projects/{components/src/time-range/time-range-for-page/time-range-for-page.component.test.ts => observability/src/shared/components/user-specified-time-range-selector/user-specified-time-range-selector.test.ts} (94%) diff --git a/projects/common/src/public-api.ts b/projects/common/src/public-api.ts index c75388546..ecc6aaa9d 100644 --- a/projects/common/src/public-api.ts +++ b/projects/common/src/public-api.ts @@ -120,7 +120,7 @@ export * from './time/time-range.service'; export * from './time/time-range.type'; export * from './time/time-unit.type'; export * from './time/time'; -export * from './time/time-range-for-page.service'; +export * from './time/user-specified-time-range.service'; // Validators export * from './utilities/validators'; diff --git a/projects/common/src/time/time-range.service.ts b/projects/common/src/time/time-range.service.ts index 90a5bb29f..f3fdd1a52 100644 --- a/projects/common/src/time/time-range.service.ts +++ b/projects/common/src/time/time-range.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { isEmpty } from 'lodash-es'; -import { combineLatest, EMPTY, ReplaySubject } from 'rxjs'; -import { catchError, defaultIfEmpty, map, switchMap } from 'rxjs/operators'; +import { EMPTY, ReplaySubject } from 'rxjs'; +import { catchError, defaultIfEmpty, filter, map, switchMap, take } from 'rxjs/operators'; import { NavigationService, QueryParamObject } from '../navigation/navigation.service'; import { ReplayObservable } from '../utilities/rxjs/rxjs-utils'; import { FixedTimeRange } from './fixed-time-range'; @@ -17,7 +17,7 @@ import { TimeUnit } from './time-unit.type'; export class TimeRangeService { private static readonly TIME_RANGE_QUERY_PARAM: string = 'time'; - private readonly defaultTimeRange: TimeRange = new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour)); + private readonly defaultTimeRange: TimeRange = new RelativeTimeRange(new TimeDuration(1, TimeUnit.Day)); private readonly timeRangeSubject$: ReplaySubject = new ReplaySubject(1); private currentTimeRange?: TimeRange; @@ -64,28 +64,14 @@ export class TimeRangeService { } private initializeTimeRange(): void { - // This is to capture the time range changes when the user navigates, specifically when they are navigating via left nav - // A time range is chosen with this order of precedence: - // 1. Url query param - // 2. 'static' time range for current page. Defined in root routes in data property. Retrieved from ActivatedRoute - // 3. Global default time range, set in this service. - combineLatest([ - this.navigationService.navigation$, - this.navigationService.navigation$.pipe(switchMap(activatedRoute => activatedRoute.queryParamMap)) - ]) + this.navigationService.navigation$ .pipe( - map(([activeRoute, paramMap]) => { - const timeRangeString = paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM); - if (typeof timeRangeString === 'string' && !isEmpty(timeRangeString)) { - return this.timeRangeFromUrlString(timeRangeString); - } - - const activeRouteTimeRange = activeRoute.snapshot.data?.defaultTimeRange; - if (activeRouteTimeRange) { - return activeRouteTimeRange; - } - throw Error(); - }), + take(1), // Wait for first navigation + switchMap(activatedRoute => activatedRoute.queryParamMap), // Get the params from it + take(1), // Only the first set of params + map(paramMap => paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM)), // Extract the time range value from it + filter((timeRangeString): timeRangeString is string => !isEmpty(timeRangeString)), // Only valid time ranges + map(timeRangeString => this.timeRangeFromUrlString(timeRangeString)), catchError(() => EMPTY), defaultIfEmpty(this.defaultTimeRange) ) @@ -133,3 +119,8 @@ export class TimeRangeService { }; } } + +// TODO consolidate these +export const enum PageTimeRangeFeature { + PageTimeRange = 'ui.page-time-range' +} diff --git a/projects/common/src/time/time-range-for-page-service.test.ts b/projects/common/src/time/user-specified-time-range-service.test.ts similarity index 71% rename from projects/common/src/time/time-range-for-page-service.test.ts rename to projects/common/src/time/user-specified-time-range-service.test.ts index 1e6e5ac19..e9c89b6f6 100644 --- a/projects/common/src/time/time-range-for-page-service.test.ts +++ b/projects/common/src/time/user-specified-time-range-service.test.ts @@ -4,18 +4,22 @@ import { RelativeTimeRange, TimeDuration, TimeRange, - TimeRangeForPageService, TimeRangeService, TimeUnit } from '@hypertrace/common'; import { runFakeRxjs } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider } from '@ngneat/spectator/jest'; +import { UserSpecifiedTimeRangeService } from './user-specified-time-range.service'; describe('Page time range service', () => { const defaultPageTimeRange = new RelativeTimeRange(new TimeDuration(2, TimeUnit.Hour)); const serviceFactory = createServiceFactory({ - service: TimeRangeForPageService, - providers: [mockProvider(NavigationService)] + service: UserSpecifiedTimeRangeService, + providers: [ + mockProvider(NavigationService, { + getRouteConfig: jest.fn().mockReturnValue({ data: { defaultTimeRange: defaultPageTimeRange } }) + }) + ] }); test('Setting fixed time range emits corresponding time range from preferences', () => { @@ -28,13 +32,12 @@ describe('Page time range service', () => { }) ] }); - jest.spyOn(spectator.service, 'getDefaultPageTimeRange').mockImplementation(() => defaultPageTimeRange); cold('-a|', { - a: () => spectator.service.setTimeRangeForCurrentPage('foo', timeRange) + a: () => spectator.service.setUserSpecifiedTimeRangeForPage('foo', timeRange) }).subscribe(update => update()); - expectObservable(spectator.service.getTimeRangeForCurrentPage('foo')).toBe('da', { + expectObservable(spectator.service.getUserSpecifiedTimeRangeForPage('foo')).toBe('da', { d: defaultPageTimeRange, a: timeRange }); @@ -51,13 +54,12 @@ describe('Page time range service', () => { }) ] }); - jest.spyOn(spectator.service, 'getDefaultPageTimeRange').mockImplementation(() => defaultPageTimeRange); cold('-b|', { - b: () => spectator.service.setTimeRangeForCurrentPage('bar', timeRange) + b: () => spectator.service.setUserSpecifiedTimeRangeForPage('bar', timeRange) }).subscribe(update => update()); - expectObservable(spectator.service.getTimeRangeForCurrentPage('bar')).toBe('db', { + expectObservable(spectator.service.getUserSpecifiedTimeRangeForPage('bar')).toBe('db', { d: defaultPageTimeRange, b: timeRange }); diff --git a/projects/common/src/time/time-range-for-page.service.ts b/projects/common/src/time/user-specified-time-range.service.ts similarity index 70% rename from projects/common/src/time/time-range-for-page.service.ts rename to projects/common/src/time/user-specified-time-range.service.ts index ed20f8866..bcc970f4f 100644 --- a/projects/common/src/time/time-range-for-page.service.ts +++ b/projects/common/src/time/user-specified-time-range.service.ts @@ -8,16 +8,17 @@ import { TimeRange } from './time-range'; import { TimeRangeService } from './time-range.service'; @Injectable({ providedIn: 'root' }) -export class TimeRangeForPageService { - // TODO change to local ?? +export class UserSpecifiedTimeRangeService { private static readonly STORAGE_TYPE: StorageType = StorageType.Session; private static readonly TIME_RANGE_PREFERENCE_KEY: string = 'page-time-range'; - private readonly pageTimeRanges$: BehaviorSubject = new BehaviorSubject({}); + private readonly pageTimeRangesSubject$: BehaviorSubject = new BehaviorSubject( + {} + ); private readonly storedTimeRanges$: Observable = this.preferenceService.get( - TimeRangeForPageService.TIME_RANGE_PREFERENCE_KEY, + UserSpecifiedTimeRangeService.TIME_RANGE_PREFERENCE_KEY, {}, - TimeRangeForPageService.STORAGE_TYPE + UserSpecifiedTimeRangeService.STORAGE_TYPE ); public constructor( @@ -26,10 +27,10 @@ export class TimeRangeForPageService { private readonly navigationService: NavigationService ) { this.storedTimeRanges$.subscribe(values => { - this.pageTimeRanges$.next(values); + this.pageTimeRangesSubject$.next(values); }); } - public getDefaultPageTimeRange(path: string): TimeRange { + private getDefaultPageTimeRange(path: string): TimeRange { const defaultTimeRange = this.navigationService.getRouteConfig([path], this.navigationService.rootRoute())?.data ?.defaultTimeRange; @@ -41,13 +42,13 @@ export class TimeRangeForPageService { return defaultTimeRange; } - public getTimeRangeForCurrentPage(path: string): Observable { + public getUserSpecifiedTimeRangeForPage(path: string): Observable { return this.storedTimeRanges$.pipe( distinctUntilChanged((prev, curr) => prev[path] === curr[path]), map(timeRanges => { if (isNil(timeRanges[path])) { const timeRangeForPath = this.getDefaultPageTimeRange(path); - this.setTimeRangeForCurrentPage(path, timeRangeForPath); + this.setUserSpecifiedTimeRangeForPage(path, timeRangeForPath); return timeRangeForPath; } @@ -58,15 +59,14 @@ export class TimeRangeForPageService { ); } - public setTimeRangeForCurrentPage(path: string, value: TimeRange): void { - const pageTimeMap: PageTimeRangeMap = this.pageTimeRanges$.getValue(); + public setUserSpecifiedTimeRangeForPage(path: string, value: TimeRange): void { + const pageTimeMap: PageTimeRangeMap = this.pageTimeRangesSubject$.getValue(); const newMap: PageTimeRangeMap = { ...pageTimeMap, [path]: value.toUrlString() }; - this.preferenceService.set( - TimeRangeForPageService.TIME_RANGE_PREFERENCE_KEY, + UserSpecifiedTimeRangeService.TIME_RANGE_PREFERENCE_KEY, newMap, - TimeRangeForPageService.STORAGE_TYPE + UserSpecifiedTimeRangeService.STORAGE_TYPE ); } } diff --git a/projects/components/src/header/header-content/header-primary-row-content.directive.ts b/projects/components/src/header/header-content/header-primary-row-content.directive.ts index f04dbaa03..b16f87d05 100644 --- a/projects/components/src/header/header-content/header-primary-row-content.directive.ts +++ b/projects/components/src/header/header-content/header-primary-row-content.directive.ts @@ -4,5 +4,5 @@ import { Directive, TemplateRef } from '@angular/core'; selector: '[htHeaderPrimaryRowContent]' }) export class HeaderPrimaryRowContentDirective { - public constructor(public templateRef: TemplateRef) {} + public constructor(public readonly templateRef: TemplateRef) {} } diff --git a/projects/components/src/header/header-content/header-secondary-row-content.directive.ts b/projects/components/src/header/header-content/header-secondary-row-content.directive.ts index 8f1317442..97a917f71 100644 --- a/projects/components/src/header/header-content/header-secondary-row-content.directive.ts +++ b/projects/components/src/header/header-content/header-secondary-row-content.directive.ts @@ -4,5 +4,5 @@ import { Directive, TemplateRef } from '@angular/core'; selector: '[htHeaderSecondaryRowContent]' }) export class HeaderSecondaryRowContentDirective { - public constructor(public templateRef: TemplateRef) {} + public constructor(public readonly templateRef: TemplateRef) {} } diff --git a/projects/components/src/header/page/page-header.component.ts b/projects/components/src/header/page/page-header.component.ts index 5f8189dec..6cdc786f6 100644 --- a/projects/components/src/header/page/page-header.component.ts +++ b/projects/components/src/header/page/page-header.component.ts @@ -14,9 +14,6 @@ import { NavigableTab } from '../../tabs/navigable/navigable-tab'; import { HeaderPrimaryRowContentDirective } from '../header-content/header-primary-row-content.directive'; import { HeaderSecondaryRowContentDirective } from '../header-content/header-secondary-row-content.directive'; -export const enum PageTimeRangeFeature { - PageTimeRange = 'ui.page-time-range' -} @Component({ selector: 'ht-page-header', styleUrls: ['./page-header.component.scss'], @@ -28,9 +25,12 @@ export const enum PageTimeRangeFeature { class="page-header" [class.bottom-border]="!this.tabs?.length" > - -
+ +
- +
+ +
+ + + +
+
+ +