Merge pull request #672 from mz8i/fix/leaflet-update

This fixes a bug caused by the recent react-leaflet update, where the URL prop of the TileLayer component is no longer mutable in v3, so map layers which depended on the layer reloading upon a URL change, needed to be adjusted.

The solution was to add appropriate key props to the layers, to force recreating the layers.

To simplify the management of layer reloading and to ensure some base layers don't re-appear above higher layers, the layer organisation was reworked using custom leaflet Panes. The file organisation was also changed to separate the logic for the different layers into their own components, rather than having everything in map.tsx

app/src/frontend/config/category-maps-config.ts

@@ -1,4 +1,5 @@
 import { Category } from './categories-config';
+import { BuildingMapTileset } from './tileserver-config';
 
 export type LegendElement = {
     color: string;
@@ -16,7 +17,7 @@ export interface LegendConfig {
 }
 
 export interface CategoryMapDefinition {
-    mapStyle: string;
+    mapStyle: BuildingMapTileset;
     legend: LegendConfig;
 }
 

app/src/frontend/config/map-config.ts

@@ -0,0 +1,7 @@
+export const defaultMapPosition = {
+    lat: 51.5245255,
+    lng: -0.1338422,
+    zoom: 16
+};
+
+export type MapTheme = 'light' | 'night';

app/src/frontend/config/tileserver-config.ts

@@ -0,0 +1,19 @@
+/**
+ * This file defines all the valid tileset names that can be obtained from the tilserver.
+ * Adjust the values here if modifying the list of styles in the tileserver.
+ */
+
+export type BuildingMapTileset = 'date_year' | 
+    'size_height' |
+    'construction_core_material' |
+    'location' |
+    'likes' |
+    'planning_combined' |
+    'sust_dec' |
+    'building_attachment_form' |
+    'landuse' |
+    'dynamics_demolished_count';
+
+export type SpecialMapTileset = 'base_light' | 'base_night' | 'highlight' | 'number_labels';
+
+export type MapTileset = BuildingMapTileset | SpecialMapTileset;

app/src/frontend/map/layers/building-base-layer.tsx

@@ -0,0 +1,19 @@
+import * as React from 'react';
+import { TileLayer } from 'react-leaflet';
+
+import { MapTheme } from '../../config/map-config';
+import { MapTileset } from '../../config/tileserver-config';
+
+import {getTileLayerUrl } from './get-tile-layer-url';
+
+export function BuildingBaseLayer({ theme }: {theme: MapTheme}) {
+    const tileset = `base_${theme}` as const;
+
+    return <TileLayer
+                key={theme} /* needed because TileLayer url is not mutable in react-leaflet v3 */
+                url={getTileLayerUrl(tileset)}
+                minZoom={14}
+                maxZoom={19}
+                detectRetina={true}
+            />;
+}

app/src/frontend/map/layers/building-data-layer.tsx

@@ -0,0 +1,16 @@
+import * as React from 'react';
+import { TileLayer } from 'react-leaflet';
+
+import { BuildingMapTileset } from '../../config/tileserver-config';
+
+import {getTileLayerUrl } from './get-tile-layer-url';
+
+export function BuildingDataLayer({tileset, revisionId} : { tileset: BuildingMapTileset, revisionId: string }) {
+    return <TileLayer
+                key={`${tileset}-${revisionId}`} /* needed because TileLayer url is not mutable in react-leaflet v3 */
+                url={getTileLayerUrl(tileset, {rev: revisionId})}
+                minZoom={9}
+                maxZoom={19}
+                detectRetina={true}
+            />;
+}

app/src/frontend/map/layers/building-highlight-layer.tsx

@@ -0,0 +1,16 @@
+import * as React from 'react';
+import { TileLayer } from 'react-leaflet';
+
+import { BuildingMapTileset } from '../../config/tileserver-config';
+
+import { getTileLayerUrl } from './get-tile-layer-url';
+
+export function BuildingHighlightLayer({selectedBuildingId, baseTileset}: {selectedBuildingId: number, baseTileset: BuildingMapTileset}) {
+    return <TileLayer
+                key={`${selectedBuildingId}-${baseTileset}`} /* needed because TileLayer url is not mutable in react-leaflet v3 */
+                url={getTileLayerUrl('highlight', {highlight: `${selectedBuildingId}`, base: baseTileset})}
+                minZoom={13}
+                maxZoom={19}
+                detectRetina={true}
+            />;
+}

app/src/frontend/map/layers/building-numbers-layer.tsx

@@ -0,0 +1,14 @@
+import * as React from 'react';
+import { TileLayer } from 'react-leaflet';
+
+import {getTileLayerUrl } from './get-tile-layer-url';
+
+export function BuildingNumbersLayer({revisionId}: {revisionId: string}) {
+    return <TileLayer
+                key={`numbers-${revisionId}`} /* needed because TileLayer url is not mutable in react-leaflet v3 */
+                url={getTileLayerUrl('number_labels', {rev: revisionId})}
+                minZoom={17}
+                maxZoom={19}
+                detectRetina={true}
+            />;
+}

app/src/frontend/map/layers/city-base-map-layer.tsx

@@ -0,0 +1,33 @@
+import * as React from 'react';
+import { TileLayer } from 'react-leaflet';
+
+import { MapTheme } from '../../config/map-config';
+
+const OS_API_KEY = 'NVUxtY5r8eA6eIfwrPTAGKrAAsoeI9E9';
+
+/**
+ * Base raster layer for the map.
+ * @param theme map theme
+ */
+export function CityBaseMapLayer({theme}: {theme: MapTheme}) {
+
+    /**
+     * Ordnance Survey maps - UK / London specific
+     * (replace with appropriate base map for other cities/countries)
+     */
+    const key = OS_API_KEY;
+    const tilematrixSet = 'EPSG:3857';
+    const layer = theme === 'light' ? 'Light 3857' : 'Night 3857';
+    const baseUrl = `https://api2.ordnancesurvey.co.uk/mapping_api/v1/service/zxy/${tilematrixSet}/${layer}/{z}/{x}/{y}.png?key=${key}`;
+    const attribution = 'Building attribute data is © Colouring London contributors. Maps contain OS data © Crown copyright: OS Maps baselayers and building outlines. <a href=/ordnance-survey-licence.html>OS licence</a>';
+    
+    return <TileLayer
+        key={theme} /* needed because TileLayer.key is not mutabe in react-leaflet v3 */
+        url={baseUrl}
+        attribution={attribution}
+        maxNativeZoom={18}
+        maxZoom={19}
+        detectRetina={true}
+    />;
+}
+

app/src/frontend/map/layers/city-boundary-layer.tsx

@@ -0,0 +1,17 @@
+import { GeoJsonObject } from 'geojson';
+import React, { useEffect, useState } from 'react';
+import { GeoJSON } from 'react-leaflet';
+
+import { apiGet } from '../../apiHelpers';
+
+export function CityBoundaryLayer() {
+    const [boundaryGeojson, setBoundaryGeojson] = useState<GeoJsonObject>(null);
+
+    useEffect(() => {
+        apiGet('/geometries/boundary-detailed.geojson')
+            .then(data => setBoundaryGeojson(data as GeoJsonObject));
+    }, []);
+
+    return boundaryGeojson &&
+        <GeoJSON data={boundaryGeojson} style={{color: '#bbb', fill: false}}/>;
+}

app/src/frontend/map/layers/get-tile-layer-url.ts

@@ -0,0 +1,14 @@
+import { MapTileset } from '../../config/tileserver-config';
+
+/**
+ * Formats a CL tileserver URL for a tileset and a set of parameters
+ * @param tileset the name of the tileset
+ * @param parameters (optional) dictionary of parameter values
+ * @returns A string with the formatted URL
+ */
+export function getTileLayerUrl<T extends MapTileset = MapTileset>(tileset: T, parameters?: Record<string, string>) {
+    let paramString = parameters && new URLSearchParams(parameters).toString();
+    paramString = paramString == undefined ? '' : `?${paramString}`;
+
+    return `/tiles/${tileset}/{z}/{x}/{y}{r}.png${paramString}`;
+}

app/src/frontend/map/map.tsx

@@ -1,21 +1,26 @@
-import { GeoJsonObject } from 'geojson';
 import React, { Component, Fragment } from 'react';
-import { AttributionControl, GeoJSON, MapContainer, TileLayer, ZoomControl, useMapEvent } from 'react-leaflet';
+import { AttributionControl, MapContainer, ZoomControl, useMapEvent, Pane } from 'react-leaflet';
 
 import 'leaflet/dist/leaflet.css';
 import './map.css';
 
 import { apiGet } from '../apiHelpers';
 import { HelpIcon } from '../components/icons';
+import { categoryMapsConfig } from '../config/category-maps-config';
+import { Category } from '../config/categories-config';
+import { defaultMapPosition, MapTheme } from '../config/map-config';
+import { Building } from '../models/building';
+
+import { CityBaseMapLayer } from './layers/city-base-map-layer';
+import { CityBoundaryLayer } from './layers/city-boundary-layer';
+import { BuildingBaseLayer } from './layers/building-base-layer';
+import { BuildingDataLayer } from './layers/building-data-layer';
+import { BuildingNumbersLayer } from './layers/building-numbers-layer';
+import { BuildingHighlightLayer } from './layers/building-highlight-layer';
 
 import Legend from './legend';
 import SearchBox from './search-box';
 import ThemeSwitcher from './theme-switcher';
-import { categoryMapsConfig } from '../config/category-maps-config';
-import { Category } from '../config/categories-config';
-import { Building } from '../models/building';
-
-const OS_API_KEY = 'NVUxtY5r8eA6eIfwrPTAGKrAAsoeI9E9';
 
 interface ColouringMapProps {
     selectedBuildingId: number;
@@ -26,12 +31,12 @@ interface ColouringMapProps {
 }
 
 interface ColouringMapState {
-    theme: 'light' | 'night';
+    theme: MapTheme;
     lat: number;
     lng: number;
     zoom: number;
-    boundary: GeoJsonObject;
 }
+
 /**
  * Map area
  */
@@ -40,10 +45,7 @@ class ColouringMap extends Component<ColouringMapProps, ColouringMapState> {
         super(props);
         this.state = {
             theme: 'night',
-            lat: 51.5245255,
+            ...defaultMapPosition
-            lng: -0.1338422,
-            zoom: 16,
-            boundary: undefined,
         };
         this.handleClick = this.handleClick.bind(this);
         this.handleLocate = this.handleLocate.bind(this);
@@ -73,72 +75,12 @@ class ColouringMap extends Component<ColouringMapProps, ColouringMapState> {
         this.setState({theme: newTheme});
     }
 
-    async getBoundary() {
-        const data = await apiGet('/geometries/boundary-detailed.geojson') as GeoJsonObject;
-
-        this.setState({
-            boundary: data
-        });
-    }
-
-    componentDidMount() {
-        this.getBoundary();
-    }
-
     render() {
         const categoryMapDefinition = categoryMapsConfig[this.props.category];
 
         const position: [number, number] = [this.state.lat, this.state.lng];
 
-        // baselayer
-        const key = OS_API_KEY;
-        const tilematrixSet = 'EPSG:3857';
-        const layer = (this.state.theme === 'light')? 'Light 3857' : 'Night 3857';
-        const baseUrl = `https://api2.ordnancesurvey.co.uk/mapping_api/v1/service/zxy/${tilematrixSet}/${layer}/{z}/{x}/{y}.png?key=${key}`;
-        const attribution = 'Building attribute data is © Colouring London contributors. Maps contain OS data © Crown copyright: OS Maps baselayers and building outlines. <a href=/ordnance-survey-licence.html>OS licence</a>';
-        const baseLayer = <TileLayer
-            url={baseUrl}
-            attribution={attribution}
-            maxNativeZoom={18}
-            maxZoom={19}
-            detectRetina={true}
-        />;
-
-        const buildingsBaseUrl = `/tiles/base_${this.state.theme}/{z}/{x}/{y}{r}.png`;
-        const buildingBaseLayer = <TileLayer url={buildingsBaseUrl} minZoom={14} maxZoom={19}/>;
-
-        const boundaryLayer = this.state.boundary &&
-                <GeoJSON data={this.state.boundary} style={{color: '#bbb', fill: false}}/>;
-
         const tileset = categoryMapDefinition.mapStyle;
-        const dataLayer = tileset != undefined &&
-            <TileLayer
-                key={tileset}
-                url={`/tiles/${tileset}/{z}/{x}/{y}{r}.png?rev=${this.props.revisionId}`}
-                minZoom={9}
-                maxZoom={19}
-                detectRetina={true}
-            />;
-
-        // highlight
-        const highlightLayer = this.props.selectedBuildingId != undefined &&
-            <TileLayer
-                key={this.props.selectedBuildingId + tileset}
-                url={`/tiles/highlight/{z}/{x}/{y}{r}.png?highlight=${this.props.selectedBuildingId}&base=${tileset}`}
-                minZoom={13}
-                maxZoom={19}
-                zIndex={100}
-                detectRetina={true}
-            />;
-
-        const numbersLayer = <TileLayer
-            key={this.state.theme}
-            url={`/tiles/number_labels/{z}/{x}/{y}{r}.png?rev=${this.props.revisionId}`}
-            zIndex={200}
-            minZoom={17}
-            maxZoom={19}
-            detectRetina={true}
-        />
 
         const hasSelection = this.props.selectedBuildingId != undefined;
         const isEdit = ['edit', 'multi-edit'].includes(this.props.mode);
@@ -155,12 +97,39 @@ class ColouringMap extends Component<ColouringMapProps, ColouringMapState> {
                     attributionControl={false}
                 >
                     <ClickHandler onClick={this.handleClick} />
-                    { baseLayer }
+
-                    { buildingBaseLayer }
+                    <Pane
-                    { boundaryLayer }
+                        key={this.state.theme}
-                    { dataLayer }
+                        name={'cc-base-pane'}
-                    { highlightLayer }
+                        style={{zIndex: 50}}
-                    { numbersLayer }
+                    >
+                        <CityBaseMapLayer theme={this.state.theme} />
+                        <BuildingBaseLayer theme={this.state.theme} />
+                    </Pane>
+
+                    {
+                        tileset &&
+                            <BuildingDataLayer
+                                tileset={tileset}
+                                revisionId={this.props.revisionId}
+                            />
+                    }
+
+                    <Pane
+                        name='cc-overlay-pane'
+                        style={{zIndex: 300}}
+                    >
+                        <CityBoundaryLayer />
+                        <BuildingNumbersLayer revisionId={this.props.revisionId} />
+                        {
+                            this.props.selectedBuildingId &&
+                                <BuildingHighlightLayer
+                                    selectedBuildingId={this.props.selectedBuildingId}
+                                    baseTileset={tileset} 
+                                />
+                        }
+                    </Pane>
+
                     <ZoomControl position="topright" />
                     <AttributionControl prefix=""/>
                 </MapContainer>