From a0389498c8e5fcacf47116a5cb261499edcc3b04 Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Tue, 26 Jan 2021 12:30:07 +0200 Subject: [PATCH 1/8] fet(settings-nav): option to skip location change --- .../src/navigation/navigation.service.test.ts | 31 ++++++++++++++++++- .../src/navigation/navigation.service.ts | 10 ++++-- .../navigation/navigation-list.component.ts | 3 +- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index 6bffabc5c..bc3e78243 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -3,7 +3,7 @@ import { Router, UrlSegment } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { patchRouterNavigateForTest } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; -import { NavigationService } from './navigation.service'; +import { NavigationService, NavigationParamsType } from './navigation.service'; describe('Navigation Service', () => { const firstChildRouteConfig = { @@ -205,4 +205,33 @@ describe('Navigation Service', () => { }) }); }); + + test('test build navigation options with skip location (shadow option)', () => { + expect( + spectator.service.buildNavigationParams({ + navType: NavigationParamsType.InApp, + path: 'child', + shadow: true + }) + ).toEqual({ + path: 'child', + extras: expect.objectContaining({ + // tslint:disable-next-line: no-null-keyword + queryParams: { time: null }, + relativeTo: undefined, + skipLocationChange: true + }) + }); + }); + + test('can run navigation with skip location (shadow option)', () => { + router.navigate = jest.fn().mockResolvedValue(true); + spectator.service.navigateWithinApp(['root', 'child'], undefined, undefined, true); + expect(router.navigate).toHaveBeenCalledWith( + ['root', 'child'], + expect.objectContaining({ + skipLocationChange: true + }) + ); + }); }); diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index dac33fe6b..8897ffcd5 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -93,7 +93,8 @@ export class NavigationService { queryParams: params?.queryParams ?? this.buildQueryParam(), queryParamsHandling: params?.queryParamsHandling, replaceUrl: params?.replaceCurrentHistory, - relativeTo: params?.relativeTo + relativeTo: params?.relativeTo, + skipLocationChange: params?.shadow } }; } @@ -105,13 +106,15 @@ export class NavigationService { public navigateWithinApp( path: NavigationPath, relativeTo?: ActivatedRoute, - preserveParameters?: string[] + preserveParameters?: string[], + shadow?: boolean ): Observable { return this.navigate({ navType: NavigationParamsType.InApp, path: path, queryParams: this.buildQueryParam(preserveParameters ?? []), - relativeTo: relativeTo + relativeTo: relativeTo, + shadow: shadow }); } @@ -304,6 +307,7 @@ export interface InAppNavigationParams { queryParams?: QueryParamObject; queryParamsHandling?: 'merge' | 'preserve'; relativeTo?: ActivatedRoute; + shadow?: boolean; } export interface ExternalNavigationParamsNew { diff --git a/projects/components/src/navigation/navigation-list.component.ts b/projects/components/src/navigation/navigation-list.component.ts index 22b580855..3a5e2f974 100644 --- a/projects/components/src/navigation/navigation-list.component.ts +++ b/projects/components/src/navigation/navigation-list.component.ts @@ -83,7 +83,7 @@ export class NavigationListComponent { } public navigate(item: NavItemLinkConfig): void { - this.navigationService.navigateWithinApp(item.matchPaths[0], this.activatedRoute); + this.navigationService.navigateWithinApp(item.matchPaths[0], this.activatedRoute, undefined, item.shadow); } public toggleView(): void { @@ -116,6 +116,7 @@ export interface NavItemLinkConfig { label: string; matchPaths: string[]; // For now, default path is index 0 features?: string[]; + shadow?: boolean; } export type FooterItemConfig = FooterItemLinkConfig; From e7575fbf03a97a952170ac87f47a0e0ada0aa96f Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Tue, 26 Jan 2021 12:48:30 +0200 Subject: [PATCH 2/8] fix(settings-nav): linting and testing --- .../src/navigation/navigation.service.test.ts | 2 +- .../navigation-list.component.test.ts | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index bc3e78243..ddc92a776 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -3,7 +3,7 @@ import { Router, UrlSegment } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { patchRouterNavigateForTest } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; -import { NavigationService, NavigationParamsType } from './navigation.service'; +import { NavigationParamsType, NavigationService } from './navigation.service'; describe('Navigation Service', () => { const firstChildRouteConfig = { diff --git a/projects/components/src/navigation/navigation-list.component.test.ts b/projects/components/src/navigation/navigation-list.component.test.ts index 84beff130..925089168 100644 --- a/projects/components/src/navigation/navigation-list.component.test.ts +++ b/projects/components/src/navigation/navigation-list.component.test.ts @@ -95,7 +95,32 @@ describe('Navigation List Component', () => { spectator.click(spectator.query(NavItemComponent, { read: ElementRef })!); expect(spectator.inject(NavigationService).navigateWithinApp).toHaveBeenCalledWith( 'foo', - spectator.inject(ActivatedRoute) + spectator.inject(ActivatedRoute), + undefined, + undefined + ); + }); + + test('should navigate to first match on click, relative to activated route with skip location change option', () => { + const navItems: NavItemConfig[] = [ + { + type: NavItemType.Link, + icon: 'icon', + label: 'Foo Label', + matchPaths: ['foo', 'bar'], + shadow: true + } + ]; + spectator = createHost(``, { + hostProps: { navItems: navItems } + }); + + spectator.click(spectator.query(NavItemComponent, { read: ElementRef })!); + expect(spectator.inject(NavigationService).navigateWithinApp).toHaveBeenCalledWith( + 'foo', + spectator.inject(ActivatedRoute), + undefined, + true ); }); }); From 2f2a24ff415efe2e25a731b48f464048821c6519 Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Tue, 26 Jan 2021 20:58:55 +0200 Subject: [PATCH 3/8] fix(settings-nav): change variable naming --- .../common/src/navigation/navigation.service.test.ts | 6 +++--- projects/common/src/navigation/navigation.service.ts | 8 ++++---- .../src/navigation/navigation-list.component.ts | 9 +++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index bc3e78243..45480278c 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -206,12 +206,12 @@ describe('Navigation Service', () => { }); }); - test('test build navigation options with skip location (shadow option)', () => { + test('test build navigation options with skip location', () => { expect( spectator.service.buildNavigationParams({ navType: NavigationParamsType.InApp, path: 'child', - shadow: true + skipLocationChange: true }) ).toEqual({ path: 'child', @@ -224,7 +224,7 @@ describe('Navigation Service', () => { }); }); - test('can run navigation with skip location (shadow option)', () => { + test('can run navigation with skip location', () => { router.navigate = jest.fn().mockResolvedValue(true); spectator.service.navigateWithinApp(['root', 'child'], undefined, undefined, true); expect(router.navigate).toHaveBeenCalledWith( diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index 8897ffcd5..673b16896 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -94,7 +94,7 @@ export class NavigationService { queryParamsHandling: params?.queryParamsHandling, replaceUrl: params?.replaceCurrentHistory, relativeTo: params?.relativeTo, - skipLocationChange: params?.shadow + skipLocationChange: params?.skipLocationChange } }; } @@ -107,14 +107,14 @@ export class NavigationService { path: NavigationPath, relativeTo?: ActivatedRoute, preserveParameters?: string[], - shadow?: boolean + skipLocationChange?: boolean ): Observable { return this.navigate({ navType: NavigationParamsType.InApp, path: path, queryParams: this.buildQueryParam(preserveParameters ?? []), relativeTo: relativeTo, - shadow: shadow + skipLocationChange: skipLocationChange }); } @@ -307,7 +307,7 @@ export interface InAppNavigationParams { queryParams?: QueryParamObject; queryParamsHandling?: 'merge' | 'preserve'; relativeTo?: ActivatedRoute; - shadow?: boolean; + skipLocationChange?: boolean; } export interface ExternalNavigationParamsNew { diff --git a/projects/components/src/navigation/navigation-list.component.ts b/projects/components/src/navigation/navigation-list.component.ts index 3a5e2f974..4f2a6f7b5 100644 --- a/projects/components/src/navigation/navigation-list.component.ts +++ b/projects/components/src/navigation/navigation-list.component.ts @@ -83,7 +83,12 @@ export class NavigationListComponent { } public navigate(item: NavItemLinkConfig): void { - this.navigationService.navigateWithinApp(item.matchPaths[0], this.activatedRoute, undefined, item.shadow); + this.navigationService.navigateWithinApp( + item.matchPaths[0], + this.activatedRoute, + undefined, + item.skipLocationChange + ); } public toggleView(): void { @@ -116,7 +121,7 @@ export interface NavItemLinkConfig { label: string; matchPaths: string[]; // For now, default path is index 0 features?: string[]; - shadow?: boolean; + skipLocationChange?: boolean; } export type FooterItemConfig = FooterItemLinkConfig; From c8c77e0899241291ec2e68664aa1d99c34541889 Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Tue, 26 Jan 2021 21:06:49 +0200 Subject: [PATCH 4/8] fix(settings-nav): rename variable in test --- .../components/src/navigation/navigation-list.component.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/components/src/navigation/navigation-list.component.test.ts b/projects/components/src/navigation/navigation-list.component.test.ts index 925089168..c34d2e4ca 100644 --- a/projects/components/src/navigation/navigation-list.component.test.ts +++ b/projects/components/src/navigation/navigation-list.component.test.ts @@ -108,7 +108,7 @@ describe('Navigation List Component', () => { icon: 'icon', label: 'Foo Label', matchPaths: ['foo', 'bar'], - shadow: true + skipLocationChange: true } ]; spectator = createHost(``, { From af214466ae5ac75fd49eb41a9a1478fd4d1ad8dc Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Wed, 27 Jan 2021 00:10:59 +0200 Subject: [PATCH 5/8] fix(settings-nav): refactoring --- .../src/navigation/navigation.service.test.ts | 26 ++++------------- .../src/navigation/navigation.service.ts | 10 ++----- .../navigation-list.component.test.ts | 28 +++++++++---------- .../navigation/navigation-list.component.ts | 14 +++++----- 4 files changed, 30 insertions(+), 48 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index 7fa4ddcd9..986fb7b98 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -206,27 +206,13 @@ describe('Navigation Service', () => { }); }); - test('test build navigation options with skip location', () => { - expect( - spectator.service.buildNavigationParams({ - navType: NavigationParamsType.InApp, - path: 'child', - skipLocationChange: true - }) - ).toEqual({ - path: 'child', - extras: expect.objectContaining({ - // tslint:disable-next-line: no-null-keyword - queryParams: { time: null }, - relativeTo: undefined, - skipLocationChange: true - }) - }); - }); - - test('can run navigation with skip location', () => { + test('can run navigation with location replace', () => { router.navigate = jest.fn().mockResolvedValue(true); - spectator.service.navigateWithinApp(['root', 'child'], undefined, undefined, true); + spectator.service.navigate({ + navType: NavigationParamsType.InApp, + path: ['root', 'child'], + replaceCurrentHistory: true + }); expect(router.navigate).toHaveBeenCalledWith( ['root', 'child'], expect.objectContaining({ diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index 673b16896..dac33fe6b 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -93,8 +93,7 @@ export class NavigationService { queryParams: params?.queryParams ?? this.buildQueryParam(), queryParamsHandling: params?.queryParamsHandling, replaceUrl: params?.replaceCurrentHistory, - relativeTo: params?.relativeTo, - skipLocationChange: params?.skipLocationChange + relativeTo: params?.relativeTo } }; } @@ -106,15 +105,13 @@ export class NavigationService { public navigateWithinApp( path: NavigationPath, relativeTo?: ActivatedRoute, - preserveParameters?: string[], - skipLocationChange?: boolean + preserveParameters?: string[] ): Observable { return this.navigate({ navType: NavigationParamsType.InApp, path: path, queryParams: this.buildQueryParam(preserveParameters ?? []), - relativeTo: relativeTo, - skipLocationChange: skipLocationChange + relativeTo: relativeTo }); } @@ -307,7 +304,6 @@ export interface InAppNavigationParams { queryParams?: QueryParamObject; queryParamsHandling?: 'merge' | 'preserve'; relativeTo?: ActivatedRoute; - skipLocationChange?: boolean; } export interface ExternalNavigationParamsNew { diff --git a/projects/components/src/navigation/navigation-list.component.test.ts b/projects/components/src/navigation/navigation-list.component.test.ts index 925089168..bc72b8a3f 100644 --- a/projects/components/src/navigation/navigation-list.component.test.ts +++ b/projects/components/src/navigation/navigation-list.component.test.ts @@ -1,7 +1,7 @@ import { ElementRef } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService } from '@hypertrace/common'; +import { NavigationService, NavigationParamsType, NavigationParams } from '@hypertrace/common'; import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; import { EMPTY, of } from 'rxjs'; @@ -93,12 +93,12 @@ describe('Navigation List Component', () => { }); spectator.click(spectator.query(NavItemComponent, { read: ElementRef })!); - expect(spectator.inject(NavigationService).navigateWithinApp).toHaveBeenCalledWith( - 'foo', - spectator.inject(ActivatedRoute), - undefined, - undefined - ); + expect(spectator.inject(NavigationService).navigate).toHaveBeenCalledWith({ + navType: NavigationParamsType.InApp, + path: 'foo', + relativeTo: spectator.inject(ActivatedRoute), + replaceCurrentHistory: undefined + }); }); test('should navigate to first match on click, relative to activated route with skip location change option', () => { @@ -108,7 +108,7 @@ describe('Navigation List Component', () => { icon: 'icon', label: 'Foo Label', matchPaths: ['foo', 'bar'], - shadow: true + skipLocationChange: true } ]; spectator = createHost(``, { @@ -116,11 +116,11 @@ describe('Navigation List Component', () => { }); spectator.click(spectator.query(NavItemComponent, { read: ElementRef })!); - expect(spectator.inject(NavigationService).navigateWithinApp).toHaveBeenCalledWith( - 'foo', - spectator.inject(ActivatedRoute), - undefined, - true - ); + expect(spectator.inject(NavigationService).navigate).toHaveBeenCalledWith({ + navType: NavigationParamsType.InApp, + path: 'foo', + relativeTo: spectator.inject(ActivatedRoute), + replaceCurrentHistory: true + }); }); }); diff --git a/projects/components/src/navigation/navigation-list.component.ts b/projects/components/src/navigation/navigation-list.component.ts index 4f2a6f7b5..c5918a540 100644 --- a/projects/components/src/navigation/navigation-list.component.ts +++ b/projects/components/src/navigation/navigation-list.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService } from '@hypertrace/common'; +import { NavigationService, NavigationParamsType } from '@hypertrace/common'; import { Observable } from 'rxjs'; import { map, startWith } from 'rxjs/operators'; import { IconSize } from '../icon/icon-size'; @@ -83,12 +83,12 @@ export class NavigationListComponent { } public navigate(item: NavItemLinkConfig): void { - this.navigationService.navigateWithinApp( - item.matchPaths[0], - this.activatedRoute, - undefined, - item.skipLocationChange - ); + this.navigationService.navigate({ + navType: NavigationParamsType.InApp, + path: item.matchPaths[0], + relativeTo: this.activatedRoute, + replaceCurrentHistory: item.skipLocationChange + }); } public toggleView(): void { From a610cb21bde8cd3a52cf59cc4c57e7bc74f6b412 Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Wed, 27 Jan 2021 17:39:27 +0200 Subject: [PATCH 6/8] fix(settings-nav): testing --- projects/common/src/navigation/navigation.service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index 986fb7b98..e781ff12a 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -216,7 +216,7 @@ describe('Navigation Service', () => { expect(router.navigate).toHaveBeenCalledWith( ['root', 'child'], expect.objectContaining({ - skipLocationChange: true + replaceUrl: true }) ); }); From 235dcea08b4b522040302d6cbe06ec5b3d6333c1 Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Wed, 27 Jan 2021 18:57:19 +0200 Subject: [PATCH 7/8] fix(settings-nav): linting --- .../components/src/navigation/navigation-list.component.test.ts | 2 +- projects/components/src/navigation/navigation-list.component.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/components/src/navigation/navigation-list.component.test.ts b/projects/components/src/navigation/navigation-list.component.test.ts index bc72b8a3f..3403a15a5 100644 --- a/projects/components/src/navigation/navigation-list.component.test.ts +++ b/projects/components/src/navigation/navigation-list.component.test.ts @@ -1,7 +1,7 @@ import { ElementRef } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService, NavigationParamsType, NavigationParams } from '@hypertrace/common'; +import { NavigationParams, NavigationParamsType, NavigationService } from '@hypertrace/common'; import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; import { EMPTY, of } from 'rxjs'; diff --git a/projects/components/src/navigation/navigation-list.component.ts b/projects/components/src/navigation/navigation-list.component.ts index c5918a540..e4cb88497 100644 --- a/projects/components/src/navigation/navigation-list.component.ts +++ b/projects/components/src/navigation/navigation-list.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService, NavigationParamsType } from '@hypertrace/common'; +import { NavigationParamsType, NavigationService } from '@hypertrace/common'; import { Observable } from 'rxjs'; import { map, startWith } from 'rxjs/operators'; import { IconSize } from '../icon/icon-size'; From 1e8fd7fe0cfbe3023f434e58902d13e0cbc57f4b Mon Sep 17 00:00:00 2001 From: Dmitriy Potychkin Date: Wed, 27 Jan 2021 21:19:09 +0200 Subject: [PATCH 8/8] fix(settings-nav): change variable naming --- .../src/navigation/navigation-list.component.test.ts | 2 +- .../components/src/navigation/navigation-list.component.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/components/src/navigation/navigation-list.component.test.ts b/projects/components/src/navigation/navigation-list.component.test.ts index 3403a15a5..e3acfeb58 100644 --- a/projects/components/src/navigation/navigation-list.component.test.ts +++ b/projects/components/src/navigation/navigation-list.component.test.ts @@ -108,7 +108,7 @@ describe('Navigation List Component', () => { icon: 'icon', label: 'Foo Label', matchPaths: ['foo', 'bar'], - skipLocationChange: true + replaceCurrentHistory: true } ]; spectator = createHost(``, { diff --git a/projects/components/src/navigation/navigation-list.component.ts b/projects/components/src/navigation/navigation-list.component.ts index e4cb88497..c4e4c6860 100644 --- a/projects/components/src/navigation/navigation-list.component.ts +++ b/projects/components/src/navigation/navigation-list.component.ts @@ -87,7 +87,7 @@ export class NavigationListComponent { navType: NavigationParamsType.InApp, path: item.matchPaths[0], relativeTo: this.activatedRoute, - replaceCurrentHistory: item.skipLocationChange + replaceCurrentHistory: item.replaceCurrentHistory }); } @@ -121,7 +121,7 @@ export interface NavItemLinkConfig { label: string; matchPaths: string[]; // For now, default path is index 0 features?: string[]; - skipLocationChange?: boolean; + replaceCurrentHistory?: boolean; } export type FooterItemConfig = FooterItemLinkConfig;