From 0faf238ec8743301b35b8910b7680fef2a6fab81 Mon Sep 17 00:00:00 2001 From: Robert Stoll Date: Sat, 18 Dec 2021 00:48:15 +0100 Subject: [PATCH] #490 check for programmatic routing I figured it would be enough to have a guard in MapService to not emit a new state in case it is the same as the current. Yet, I was proven to be wrong and it definitely better to not process a programmatic route change twice. --- src/app/router/router.component.ts | 32 +++++++++++++++++++++++------- src/app/types.ts | 2 ++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/app/router/router.component.ts b/src/app/router/router.component.ts index b03bdfb9..711e24ea 100644 --- a/src/app/router/router.component.ts +++ b/src/app/router/router.component.ts @@ -7,11 +7,11 @@ import '../shared/importAllExtensions'; import { Component, OnInit } from '@angular/core'; -import { ActivatedRoute, Router } from '@angular/router'; -import { combineLatest } from 'rxjs/index'; +import { ActivatedRoute, NavigationStart, ParamMap, Router } from '@angular/router'; +import { combineLatest, of } from 'rxjs/index'; import { SubscriptionService } from '../core/subscription.service'; import { MapService, MapState } from '../city/map.service'; -import { distinctUntilChanged, map, switchMap } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map } from 'rxjs/operators'; import { FountainService } from '../fountain/fountain.service'; import { FountainSelector } from '../types'; import { LanguageService } from '../core/language.service'; @@ -19,6 +19,8 @@ import { RoutingService } from '../services/routing.service'; import _ from 'lodash'; import { City } from '../locations'; +const programmaticRouting = 'programmaticRouting'; + @Component({ selector: 'app-router', templateUrl: './router.component.html', @@ -38,10 +40,25 @@ export class RouterComponent implements OnInit { ngOnInit(): void { this.subscriptionService.registerSubscriptions( - combineLatest([this.route.paramMap, this.route.queryParamMap]) - // TODO after webpack update: currently we need to use pipe(switchMap(...)) as WEBPACK somehow messes up - // everything and cannot find the extension method (tries to call another function) - .pipe(switchMap(([routeParam, queryParam]) => this.routeValidator.handleUrlChange(routeParam, queryParam))) + this.router.events + .pipe(filter((e): e is NavigationStart => e instanceof NavigationStart && e.navigationTrigger === 'popstate')) + .subscribe((_: NavigationStart) => { + const state = this.router.getCurrentNavigation()?.extras?.state; + if (state !== undefined) { + state[programmaticRouting] = false; + } + }), + this.route.paramMap + .switchMap(routeParams => + this.route.queryParamMap.switchMap(queryParams => { + const state = this.router.getCurrentNavigation()?.extras?.state; + if (state?.[programmaticRouting] !== true) { + return this.routeValidator.handleUrlChange(routeParams, queryParams); + } else { + return of(); + } + }) + ) .subscribe(_ => undefined /* nothing to do as side effect (navigating) occurs in handleUrlChange */), // TODO @ralf.hauser - navigating to a fountain currently happens in two route changes (which is not nice IMO). @@ -58,6 +75,7 @@ export class RouterComponent implements OnInit { .subscribe(([city, queryParams]) => { this.router.navigate([`/${city ? city : ''}`], { queryParams: queryParams, + state: { programmaticRouting: true }, }); }) ); diff --git a/src/app/types.ts b/src/app/types.ts index 1c9448c7..a156a7ca 100644 --- a/src/app/types.ts +++ b/src/app/types.ts @@ -288,6 +288,8 @@ export interface FountainProperty { source_name?: string; issues?: DataIssue[]; } +// TODO it would make more sense to move common types to an own library which is consumed by both, datablue and proximap +// if you change something here, then you need to change it in datablue as well export type Database = SourceType; export interface FountainSelector {