Skip to content

Commit

Permalink
water-fountains#490 check for programmatic routing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
robstoll committed Dec 17, 2021
1 parent 7722b64 commit 0faf238
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
32 changes: 25 additions & 7 deletions src/app/router/router.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@

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';
import { RoutingService } from '../services/routing.service';
import _ from 'lodash';
import { City } from '../locations';

const programmaticRouting = 'programmaticRouting';

@Component({
selector: 'app-router',
templateUrl: './router.component.html',
Expand All @@ -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).
Expand All @@ -58,6 +75,7 @@ export class RouterComponent implements OnInit {
.subscribe(([city, queryParams]) => {
this.router.navigate([`/${city ? city : ''}`], {
queryParams: queryParams,
state: { programmaticRouting: true },
});
})
);
Expand Down
2 changes: 2 additions & 0 deletions src/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 0faf238

Please sign in to comment.