Skip to content

Commit

Permalink
water-fountains#491 use map.center for loc as jumpTo requires it
Browse files Browse the repository at this point in the history 
moreover:
- also use the first load strategy for ?loc= (move into inner
  actOnFirstLoad function)
- don't navigate in router.component.ts if neither the city nor the
  query params have changed
  • Loading branch information
@robstoll
robstoll committed Dec 17, 2021
1 parent b3315db commit b75569c
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 72 deletions.
30 changes: 8 additions & 22 deletions src/app/city/map.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ export interface MapState {
location: LngLat;
zoom: number | 'auto';
}
export type MapStateOmitCalculatedFields = Omit<MapState, 'location'>;

@Injectable()
export class MapService {
Expand Down Expand Up @@ -126,26 +125,21 @@ export class MapService {

this.updateState({
bounds: bounds,
location: sharedLocation.location,
zoom: sharedLocation.zoom,
city: this.parseCity(cityIdOrAlias),
});
}

updateState(mapState: MapStateOmitCalculatedFields): void {
const calculcatedMapState: MapState = this.calculateFields(mapState);

if (!_.isEqual(calculcatedMapState, this.stateSubject.value)) {
updateState(newMapState: MapState): void {
if (!_.isEqual(newMapState, this.stateSubject.value)) {
if (!environment.production) {
console.log('updating map state to', calculcatedMapState);
console.log('updating map state to', newMapState, 'loc', newMapState.location);
}
this.stateSubject.next(calculcatedMapState);
this.stateSubject.next(newMapState);
}
}

calculateFields(mapState: MapStateOmitCalculatedFields): MapState {
return { ...mapState, location: getCentre(mapState.bounds) };
}

get currentCity(): City | undefined {
return this.stateSubject.value?.city;
}
Expand All @@ -158,15 +152,6 @@ export class MapService {
);
}

get sharedLocation(): Observable<SharedLocation> {
return (
this.state
.map(s => this.mapStateToSharedLocation(s))
// we don't want to propagate a change if e.g. the bounds change (due to resizing of the browser for instance)
.pipe(distinctUntilChanged((x, y) => _.isEqual(x, y)))
);
}

private mapStateToSharedLocation(state: MapState): SharedLocation {
return { location: state.location, zoom: state.zoom };
}
Expand All @@ -176,10 +161,11 @@ export class MapService {
}

setCity(city: City) {
const bounds = getCityBounds(city);
this.updateState({
city: city,
bounds: getCityBounds(city),
// if we have not yet zoomed, then we want to fit to bounds
bounds: bounds,
location: getCentre(bounds),
zoom: 'auto',
});
}
Expand Down
75 changes: 42 additions & 33 deletions src/app/map/map.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { DirectionsService } from '../directions/directions.service';
import { FountainService } from '../fountain/fountain.service';
import { City } from '../locations';
import { Bounds, Fountain, FountainCollection, LngLat } from '../types';
import { MapService, MapState, MapStateOmitCalculatedFields } from '../city/map.service';
import { MapService, MapState } from '../city/map.service';
import { MapConfig } from './map.config';
import { UserLocationService } from './user-location.service';
import _ from 'lodash';
Expand Down Expand Up @@ -87,8 +87,8 @@ export class MapComponent implements OnInit {

this.mapLocationChangeSubject
.pipe(
// don't trigger multiple change events in case of an animation
debounceTime(50)
// don't trigger multiple change events shortly after each other (e.g. when resizing window)
debounceTime(100)
)
.subscribe(_ => this.handleMapLocationChange()),

Expand Down Expand Up @@ -186,53 +186,62 @@ export class MapComponent implements OnInit {
private firstStateChange = true;

private adjustMapToStateChange(newState: MapState) {
const currentState = this.mapService.calculateFields(this.toMapStateOmitCaluclatedFields(newState.city));
const self = this;

const currentState = this.toMapState(newState.city);

if (this.firstStateChange || !_.isEqual(currentState, newState)) {
if (this.firstStateChange) {
// we need to resize the map once as it is slightly displaced otherwise
this.map.resize();
}
if (newState.zoom === 'auto') {
try {
// only refit city bounds if not zoomed into a fountain and still same city
if (this._mode === 'map' && this.mapService.currentCity === newState.city) {
const bounds = newState.bounds;
const options = {
maxDuration: 500,
pitch: 0,
bearing: 0,
offset: [0, 0] as [number, number],
};
this.map.fitBounds([bounds.min, bounds.max], options);
if (this.firstStateChange) {
const afterFirstStyleLoad = () => {
// on first load, it happens that fitBounds does not trigger `moveend` all the time
// so we trigger one manually
this.mapLocationChangeSubject.next();
this.map.off('styledata', afterFirstStyleLoad);
// only refit city bounds or jump to location if not zommed into fountain
if (this._mode === 'map') {
if (newState.zoom === 'auto') {
try {
// only refit city bounds if still same city
if (this.mapService.currentCity === newState.city) {
const options = {
maxDuration: 500,
pitch: 0,
bearing: 0,
};
this.map.on('styledata', afterFirstStyleLoad);
this.map.fitBounds([newState.bounds.min, newState.bounds.max], options);
actOnFirstLoad();
}
} catch (e: unknown) {
console.error(e);
}
} catch (e: unknown) {
console.error(e);
} else {
this.map.jumpTo({
center: newState.location,
zoom: newState.zoom,
around: newState.location,
});
actOnFirstLoad();
}
} else {
this.map.jumpTo({
center: [newState.location.lng, newState.location.lat],
zoom: newState.zoom,
});
}
this.firstStateChange = false;
}

function actOnFirstLoad() {
if (self.firstStateChange) {
const afterFirstStyleLoad = () => {
// on first load, it happens that fitBounds and jumpTo do not trigger `moveend` (at least not always)
// so we trigger one manually
self.mapLocationChangeSubject.next();
self.map.off('styledata', afterFirstStyleLoad);
};
self.map.on('styledata', afterFirstStyleLoad);
}
}
}

private toMapStateOmitCaluclatedFields(city: City | undefined): MapStateOmitCalculatedFields {
private toMapState(city: City | undefined): MapState {
const mapboxBounds = this.map.getBounds();
const sw = mapboxBounds.getSouthWest();
const ne = mapboxBounds.getNorthEast();
return { bounds: Bounds(sw, ne), city: city, zoom: this.map.getZoom() };
return { bounds: Bounds(sw, ne), location: this.map.getCenter(), city: city, zoom: this.map.getZoom() };
}

private zoomOut(): void {
Expand Down Expand Up @@ -530,7 +539,7 @@ export class MapComponent implements OnInit {
}

private handleMapLocationChange() {
const state = this.toMapStateOmitCaluclatedFields(this.mapService.currentCity);
const state = this.toMapState(this.mapService.currentCity);
this.mapService.updateState(state);
}

Expand Down
38 changes: 22 additions & 16 deletions src/app/router/router.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import { ActivatedRoute, Router } from '@angular/router';
import { combineLatest } from 'rxjs/index';
import { SubscriptionService } from '../core/subscription.service';
import { MapService, MapState } from '../city/map.service';
import { first, switchMap } from 'rxjs/operators';
import { distinctUntilChanged, first, map, switchMap } 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 { environment } from '../../environments/environment';
import _ from 'lodash';
import { City } from '../locations';

@Component({
selector: 'app-router',
Expand All @@ -37,35 +40,38 @@ export class RouterComponent implements OnInit {
ngOnInit(): void {
this.subscriptionService.registerSubscriptions(
combineLatest([this.route.paramMap, this.route.queryParamMap])
.pipe(
// we only want to know about the first url (i.e. the url the user entered into the addressbar)
// and not programmatic changes to the url
first(),
// need to use pipe(switchMap(...)) as WEBPACK somehow messes up everything and cannot find the extension method
switchMap(([routeParam, queryParam]) => this.routeValidator.handleUrlChange(routeParam, queryParam))
)
// 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)))
.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).
// first it changes to https://old.water-fountains.org/ch-zh?l=de and then it changes to https://old.water-fountains.org/ch-zh?l=de&i=Q27229673
// this is because city and fountainSelector are independent states (subscribed independently)
// Yet, they cannot really change independently and should be consolidated in one state
combineLatest([this.mapService.state, this.fountainService.fountainSelector]).subscribe(([state, selector]) => {
this.navigateToNewRoute(state, selector);
})
combineLatest([this.mapService.state, this.fountainService.fountainSelector])
.pipe(
map(
([state, selector]) => [state.city, this.toQueryParams(state, selector)] as [City | undefined, QueryParams]
),
distinctUntilChanged((x, y) => _.isEqual(x, y))
)
.subscribe(([city, queryParams]) => {
console.log('navigate to', city, queryParams);
this.router.navigate([`/${city ? city : ''}`], {
queryParams: queryParams,
});
})
);
}

private navigateToNewRoute(state: MapState, fountainSelector: FountainSelector | undefined): void {
private toQueryParams(state: MapState, fountainSelector: FountainSelector | undefined): QueryParams {
const q: Omit<QueryParams, 'loc'> = fountainSelector ? this.getQueryParamsForSelector(fountainSelector) : {};
const queryParams: QueryParams = {
return {
loc: `${state.location.lat},${state.location.lng},` + (state.zoom === 'auto' ? 'auto' : `${state.zoom}z`),
...q,
l: this.languageService.currentLang,
};
this.router.navigate([`/${state.city ? state.city : ''}`], {
queryParams: queryParams,
});
}

private getQueryParamsForSelector(fountainSelector: FountainSelector): Omit<QueryParams, 'loc'> {
Expand Down
2 changes: 1 addition & 1 deletion src/app/services/routing.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ export class RoutingService {
const { lngLat, database, updateId } = data;
const city = this.getCityByLngLat(lngLat);
if (city !== undefined) {
this.updateFromId(database, updateId);
//TODO @ralf.hauser this function currently depends on that a fountain is in a city
this.layoutService.flyToCity(city);
this.updateFromId(database, updateId);
}
return true;
} else {
Expand Down

0 comments on commit b75569c

Please sign in to comment.