From 1683afca3e541dbde8e502aef122b590b9173a02 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Tue, 6 Oct 2026 14:25:11 +0300 Subject: [PATCH 1/3] Convert TileLayerContainer to a function component Create the Leaflet GridLayer directly instead of extending react-leaflet's GridLayer, and use useLeaflet/useConfigContext/useRouter instead of wrapper components. The layer is now removed on unmount, and the popup class is computed per render so 'vehicle-popup' no longer leaks to later popups. Extract sendSelectionAnalytics and rewrite the unit tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../map/tile-layer/TileLayerContainer.jsx | 822 +++++++++--------- .../tile-layer/TileLayerContainer.test.jsx | 165 ++-- 2 files changed, 521 insertions(+), 466 deletions(-) diff --git a/app/component/map/tile-layer/TileLayerContainer.jsx b/app/component/map/tile-layer/TileLayerContainer.jsx index d06ca567ad..86906cc7b8 100644 --- a/app/component/map/tile-layer/TileLayerContainer.jsx +++ b/app/component/map/tile-layer/TileLayerContainer.jsx @@ -1,20 +1,21 @@ import connectToStores from 'fluxible-addons-react/connectToStores'; import PropTypes from 'prop-types'; -import React from 'react'; +import React, { + useCallback, + useContext, + useEffect, + useRef, + useState, +} from 'react'; import { ReactRelayContext } from 'react-relay'; -import GridLayer from 'react-leaflet/es/GridLayer'; +import L from 'leaflet'; import SphericalMercator from '@mapbox/sphericalmercator'; import lodashFilter from 'lodash/filter'; import isEqual from 'lodash/isEqual'; import Popup from 'react-leaflet/es/Popup'; -import { withLeaflet } from 'react-leaflet/es/context'; +import { useLeaflet } from 'react-leaflet/es/context'; import { useRouter } from 'found'; -import { - routerShape, - relayShape, - configShape, - vehicleShape, -} from '../../../../utils/client/shapes'; +import { vehicleShape } from '../../../../utils/client/shapes'; import { mapLayerShape } from '../../../store/MapLayerStore'; import MarkerSelectPopup from './MarkerSelectPopup'; import LocationPopup from '../popups/LocationPopup'; @@ -34,446 +35,441 @@ import SelectVehicleContainer from './SelectVehicleContainer'; import { withCurrentTime } from '../../../hooks/TimeContext'; import { useConfigContext } from '../../../client/ConfigContext'; -const initialState = { - selectableTargets: undefined, - coords: undefined, - showSpinner: true, - zoom: undefined, -}; - -// TODO eslint doesn't know that TileLayerContainer is a react component, -// because it doesn't inherit it directly. This will force the detection -/** @extends React.Component */ -class TileLayerContainer extends GridLayer { - static propTypes = { - tileSize: PropTypes.number.isRequired, - zoomOffset: PropTypes.number.isRequired, - locationPopup: PropTypes.string, // all, none, reversegeocoding, origindestination - onSelectLocation: PropTypes.func, - mergeStops: PropTypes.bool, - mapLayers: mapLayerShape.isRequired, - // When true, clicks on map icons (stops, terminals, citybikes, etc.) - // are ignored instead of selecting/navigating to them. - disableIconClick: PropTypes.bool, - leaflet: PropTypes.shape({ - map: PropTypes.shape({ - addLayer: PropTypes.func.isRequired, - addEventParent: PropTypes.func.isRequired, - closePopup: PropTypes.func.isRequired, - removeEventParent: PropTypes.func.isRequired, - _popup: PropTypes.shape({ - isOpen: PropTypes.func, - }), - }).isRequired, - }).isRequired, - relayEnvironment: relayShape.isRequired, - highlightedStops: PropTypes.arrayOf(PropTypes.string), - stopsToShow: PropTypes.arrayOf(PropTypes.string), - objectsToHide: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.string)), - vehicles: PropTypes.objectOf(vehicleShape), - currentTime: PropTypes.number.isRequired, - config: configShape.isRequired, - router: routerShape.isRequired, - }; +const DEFAULT_OBJECTS_TO_HIDE = { vehicleRentalStations: [] }; - static defaultProps = { - objectsToHide: { vehicleRentalStations: [] }, - mergeStops: true, - }; - - PopupOptions = { - offset: [0, 0], - autoPanPaddingTopLeft: [5, 125], - className: 'popup', - ref: 'popup', - onClose: () => this.setState({ ...initialState }), - autoPan: false, - onOpen: () => this.sendAnalytics(), - relayEnvironment: relayShape.isRequired, - }; +const POPUP_OPTIONS = { + offset: [0, 0], + autoPanPaddingTopLeft: [5, 125], + autoPan: false, +}; - merc = new SphericalMercator({ - size: this.props.tileSize || 256, +/** + * Send an analytics event on opening popup + */ +export function sendSelectionAnalytics(selectableTargets, config) { + if (!selectableTargets || selectableTargets.length === 0) { + // event for clicking somewhere else on the map will be handled in LocationPopup + return; + } + let name = null; + let type = null; + if (selectableTargets.length === 1) { + const target = selectableTargets[0]; + const { properties } = target.feature; + name = target.layer; + if (name === 'stop') { + ({ type } = properties); + if (properties.stops) { + type += '_TERMINAL'; + } + } + } else { + name = 'multiple'; + } + const pathPrefixMatch = window.location.pathname.match(/^\/([a-z]{2,})\//); + const context = + pathPrefixMatch && pathPrefixMatch[1] !== config.indexPath + ? pathPrefixMatch[1] + : 'index'; + addAnalyticsEvent({ + action: 'SelectMapPoint', + category: 'Map', + name, + type, + source: context, }); +} - constructor(props, context) { - super(props, context); - this.state = { - ...initialState, - }; - this.leafletElement.createTile = this.createTile; +/** + * Handles a click on a tile. Navigates directly for single targets that have + * their own page, otherwise toggles the selection popup. Reads the latest + * props/state via ref, because tiles outlive the render that created them. + */ +function onSelectableTargetClicked( + latest, + setSelection, + tile, + selectableTargets, + coords, + forceOpen = false, +) { + const { + props: { mapLayers }, + leaflet: { map }, + router, + selection, + } = latest.current; + const prevCoords = selection?.coords; + const popup = map._popup; // eslint-disable-line no-underscore-dangle + // navigate to citybike stop page if single stop is clicked + if ( + selectableTargets.length === 1 && + selectableTargets[0].layer === 'citybike' + ) { + router.push( + `/${PREFIX_BIKESTATIONS}/${encodeURIComponent( + selectableTargets[0].feature.properties.id, + )}`, + ); + return; } - - componentDidMount() { - super.componentDidMount(); - this.props.leaflet.map.addEventParent(this.leafletElement); - this.leafletElement.on('click contextmenu', this.onClick); + if ( + (selectableTargets.length === 1 && + selectableTargets[0].layer === 'scooter') || + (selectableTargets.length > 1 && + selectableTargets.every(target => target.layer === 'scooter')) + // scooters are not shown in the selection popup as there can be too many. + // Instead, the user is directed to the scooter cluster view or the first one in a group of singles. + ) { + const cluster = selectableTargets.find( + target => target.feature.properties.cluster, + ); + const networks = cluster ? cluster.feature.properties.networks : ''; + const id = cluster + ? cluster.feature.properties.scooterId + : selectableTargets[0].feature.properties.id; + // adding networks directs to scooter cluster view + router.push( + `/${PREFIX_RENTALVEHICLES}/${encodeURIComponent(id)}/${[...networks]}`, + ); + return; + } + // ... Or to stop page + if (selectableTargets.length === 1 && selectableTargets[0].layer === 'stop') { + router.push( + stopPagePath( + selectableTargets[0].feature.properties.stops, + selectableTargets[0].feature.properties.gtfsId, + ), + ); + return; } - componentDidUpdate(prevProps) { - if (this.context.popupContainer != null) { - this.context.popupContainer.openPopup(); - } - if (!isEqual(prevProps.mapLayers, this.props.mapLayers)) { - this.leafletElement.redraw(); - } - if (!isEqual(prevProps.highlightedStops, this.props.highlightedStops)) { - this.leafletElement.redraw(); + if ( + selectableTargets.length === 1 && + (selectableTargets[0].layer === 'parkAndRide' || + selectableTargets[0].layer === 'parkAndRideForBikes') + ) { + const { layer } = selectableTargets[0]; + let parkingId; + // hubs have nested vehicleParking + if (selectableTargets[0].feature.properties?.vehicleParking) { + const parksInHub = + selectableTargets[0].feature.properties?.vehicleParking?.filter( + parking => + layer === 'parkAndRide' ? parking.carPlaces : parking.bicyclePlaces, + ); + if (parksInHub.length === 1) { + parkingId = parksInHub[0].id; + } + } else { + parkingId = selectableTargets[0].feature.properties?.id; } - if (prevProps.currentTime !== this.props.currentTime) { - this.onTimeChange(); + if (parkingId) { + router.push( + `/${ + layer === 'parkAndRide' ? PREFIX_CARPARK : PREFIX_BIKEPARK + }/${encodeURIComponent(parkingId)}`, + ); + return; } } - componentWillUnmount() { - this.leafletElement.off('click contextmenu', this.onClick); + if ( + popup && + popup.isOpen() && + (!forceOpen || (coords && coords.equals(prevCoords))) + ) { + map.closePopup(); + return; } - onTimeChange = () => { - /* eslint-disable no-underscore-dangle */ - const activeTiles = lodashFilter( - this.leafletElement._tiles, - tile => tile.active, - ); - /* eslint-enable no-underscore-dangle */ - activeTiles.forEach( - tile => - tile.el.layers && - tile.el.layers.forEach(layer => { - if (layer.onTimeChange) { - layer.onTimeChange(this.props.config.language); - } - }), - ); + setSelection({ + selectableTargets: selectableTargets.filter( + target => + target.layer === 'realTimeVehicle' || + isFeatureLayerEnabled(target.feature, target.layer, mapLayers), + ), + coords, + zoom: tile.coords.z, + }); +} + +function createTile(tileCoords, done, latest, setSelection) { + const { props, config, relayEnvironment } = latest.current; + const tile = new TileContainer( + tileCoords, + done, + props, + config, + props.mergeStops, + relayEnvironment, + props.highlightedStops, + props.vehicles, + props.stopsToShow, + props.objectsToHide, + config.language, + ); + tile.onSelectableTargetClicked = (...args) => + onSelectableTargetClicked(latest, setSelection, tile, ...args); + return tile.el; +} + +function getLayerOptions(props, leaflet) { + const { map } = leaflet; + const options = { + tileSize: props.tileSize, + zoomOffset: props.zoomOffset, }; + const pane = props.pane ?? leaflet.pane; + if (pane != null) { + options.pane = pane; + } + if (map?.options?.maxZoom != null) { + options.maxZoom = map.options.maxZoom; + } + if (map?.options?.minZoom != null) { + options.minZoom = map.options.minZoom; + } + return options; +} - onClick = e => { - /* eslint-disable no-underscore-dangle */ - Object.keys(this.leafletElement._tiles) - .filter(key => this.leafletElement._tiles[key].active) - .filter(key => this.leafletElement._keyToBounds(key).contains(e.latlng)) - .forEach(key => - this.leafletElement._tiles[key].el.onMapClick( - e, - this.merc.px( - [e.latlng.lng, e.latlng.lat], - Number(key.split(':')[2]) + this.props.zoomOffset, - ), - ), - ); - /* eslint-enable no-underscore-dangle */ +function TileLayerContainer(props) { + const { + locationPopup, + onSelectLocation, + mapLayers, + highlightedStops, + vehicles, + currentTime, + } = props; + const config = useConfigContext(); + const { router } = useRouter(); + const { environment: relayEnvironment } = useContext(ReactRelayContext); + const leaflet = useLeaflet(); + const [selection, setSelection] = useState(null); + + // Tile callbacks outlive renders, so they read current values through this ref. + const latest = useRef(); + latest.current = { + props: { + ...props, + mergeStops: props.mergeStops ?? true, + objectsToHide: props.objectsToHide ?? DEFAULT_OBJECTS_TO_HIDE, + }, + config, + router, + relayEnvironment, + leaflet, + selection, }; - createTile = (tileCoords, done) => { - const tile = new TileContainer( - tileCoords, - done, - this.props, - this.props.config, - this.props.mergeStops, - this.props.relayEnvironment, - this.props.highlightedStops, - this.props.vehicles, - this.props.stopsToShow, - this.props.objectsToHide, - this.props.config.language, - ); - tile.onSelectableTargetClicked = ( - selectableTargets, - coords, - forceOpen = false, - ) => { - const { - leaflet: { map }, - mapLayers, - } = this.props; - const { coords: prevCoords } = this.state; - const popup = map._popup; // eslint-disable-line no-underscore-dangle - // navigate to citybike stop page if single stop is clicked - if ( - selectableTargets.length === 1 && - selectableTargets[0].layer === 'citybike' - ) { - this.props.router.push( - `/${PREFIX_BIKESTATIONS}/${encodeURIComponent( - selectableTargets[0].feature.properties.id, - )}`, - ); - return; - } - if ( - (selectableTargets.length === 1 && - selectableTargets[0].layer === 'scooter') || - (selectableTargets.length > 1 && - selectableTargets.every(target => target.layer === 'scooter')) - // scooters are not shown in the selection popup as there can be too many. - // Instead, the user is directed to the scooter cluster view or the first one in a group of singles. - ) { - const cluster = selectableTargets.find( - target => target.feature.properties.cluster, - ); - const networks = cluster ? cluster.feature.properties.networks : ''; - const id = cluster - ? cluster.feature.properties.scooterId - : selectableTargets[0].feature.properties.id; - // adding networks directs to scooter cluster view - this.props.router.push( - `/${PREFIX_RENTALVEHICLES}/${encodeURIComponent(id)}/${[ - ...networks, - ]}`, - ); - return; - } - // ... Or to stop page - if ( - selectableTargets.length === 1 && - selectableTargets[0].layer === 'stop' - ) { - this.props.router.push( - stopPagePath( - selectableTargets[0].feature.properties.stops, - selectableTargets[0].feature.properties.gtfsId, + const instance = useRef(null); + if (instance.current === null) { + const layer = new L.GridLayer(getLayerOptions(props, leaflet)); + layer.createTile = (tileCoords, done) => + createTile(tileCoords, done, latest, setSelection); + instance.current = { + layer, + merc: new SphericalMercator({ size: props.tileSize || 256 }), + }; + } + const { layer, merc } = instance.current; + + useEffect(() => { + const { map, layerContainer } = leaflet; + const container = layerContainer || map; + /* eslint-disable no-underscore-dangle */ + const onClick = e => { + Object.keys(layer._tiles) + .filter(key => layer._tiles[key].active) + .filter(key => layer._keyToBounds(key).contains(e.latlng)) + .forEach(key => + layer._tiles[key].el.onMapClick( + e, + merc.px( + [e.latlng.lng, e.latlng.lat], + Number(key.split(':')[2]) + latest.current.props.zoomOffset, + ), ), ); - return; - } + }; + /* eslint-enable no-underscore-dangle */ + container.addLayer(layer); + map.addEventParent(layer); + layer.on('click contextmenu', onClick); + return () => { + layer.off('click contextmenu', onClick); + map.removeEventParent(layer); + container.removeLayer(layer); + }; + }, []); - if ( - selectableTargets.length === 1 && - (selectableTargets[0].layer === 'parkAndRide' || - selectableTargets[0].layer === 'parkAndRideForBikes') - ) { - const { layer } = selectableTargets[0]; - let parkingId; - // hubs have nested vehicleParking - if (selectableTargets[0].feature.properties?.vehicleParking) { - const parksInHub = - selectableTargets[0].feature.properties?.vehicleParking?.filter( - parking => - layer === 'parkAndRide' - ? parking.carPlaces - : parking.bicyclePlaces, - ); - if (parksInHub.length === 1) { - parkingId = parksInHub[0].id; - } - } else { - parkingId = selectableTargets[0].feature.properties?.id; - } - if (parkingId) { - this.props.router.push( - `/${ - layer === 'parkAndRide' ? PREFIX_CARPARK : PREFIX_BIKEPARK - }/${encodeURIComponent(parkingId)}`, - ); - return; - } - } + const previous = useRef({ mapLayers, highlightedStops, currentTime }); + useEffect(() => { + const prev = previous.current; + previous.current = { mapLayers, highlightedStops, currentTime }; + if ( + !isEqual(prev.mapLayers, mapLayers) || + !isEqual(prev.highlightedStops, highlightedStops) + ) { + layer.redraw(); + } + if (prev.currentTime !== currentTime) { + /* eslint-disable no-underscore-dangle */ + lodashFilter(layer._tiles, tile => tile.active).forEach( + tile => + tile.el.layers && + tile.el.layers.forEach(l => { + if (l.onTimeChange) { + l.onTimeChange(config.language); + } + }), + ); + /* eslint-enable no-underscore-dangle */ + } + }); - if ( - popup && - popup.isOpen() && - (!forceOpen || (coords && coords.equals(prevCoords))) - ) { - map.closePopup(); - return; - } + const closePopup = useCallback(() => setSelection(null), []); + const onPopupOpen = useCallback( + () => + sendSelectionAnalytics( + latest.current.selection?.selectableTargets, + latest.current.config, + ), + [], + ); + const selectRow = useCallback( + option => setSelection(prev => ({ ...prev, selectableTargets: [option] })), + [], + ); - this.setState({ - selectableTargets: selectableTargets.filter( - target => - target.layer === 'realTimeVehicle' || - isFeatureLayerEnabled(target.feature, target.layer, mapLayers), - ), - coords, - zoom: tile.coords.z, - }); - }; + if (!selection) { + return null; + } - return tile.el; + const { selectableTargets, coords, zoom } = selection; + const popupOptions = { + ...POPUP_OPTIONS, + onClose: closePopup, + onOpen: onPopupOpen, }; + const smallScreenPopupHidden = + !config.map.showStopMarkerPopupOnMobile && + getClientBreakpoint() === 'small'; - selectRow = option => this.setState({ selectableTargets: [option] }); - - /** - * Send an analytics event on opening popup - */ - sendAnalytics() { - let name = null; - let type = null; - if (this.state.selectableTargets.length === 0) { - return; - // event for clicking somewhere else on the map will be handled in LocationPopup - } - if (this.state.selectableTargets.length === 1) { - const target = this.state.selectableTargets[0]; - const { properties } = target.feature; - name = target.layer; - switch (name) { - case 'stop': - ({ type } = properties); - if (properties.stops) { - type += '_TERMINAL'; - } - break; - default: - break; + if (selectableTargets.length === 1) { + const target = selectableTargets[0]; + const nestedParking = target.feature.properties?.vehicleParking; + let id; + let contents; + let latlng = coords; + let isVehicle = false; + if ( + (target.layer === 'parkAndRide' && + nestedParking?.filter(parking => parking.carPlaces).length > 1) || + (target.layer === 'parkAndRideForBikes' && + nestedParking?.filter(parking => parking.bicyclePlaces).length > 1) + ) { + id = `parkAndRide_${nestedParking[0].id}`; + contents = ( + + ); + } else if (target.layer === 'realTimeVehicle') { + const { vehicle } = target.feature; + const realTimeInfoVehicle = vehicles[vehicle.id]; + if (realTimeInfoVehicle) { + latlng = { + lat: realTimeInfoVehicle.lat, + lng: realTimeInfoVehicle.long, + }; } - } else { - name = 'multiple'; + isVehicle = true; + contents = ; } - const pathPrefixMatch = window.location.pathname.match(/^\/([a-z]{2,})\//); - const context = - pathPrefixMatch && pathPrefixMatch[1] !== this.props.config.indexPath - ? pathPrefixMatch[1] - : 'index'; - addAnalyticsEvent({ - action: 'SelectMapPoint', - category: 'Map', - name, - type, - source: context, - }); + return ( + + {contents} + + ); } - render() { - let popup = null; - let latlng = this.state.coords; - let contents; - const breakpoint = getClientBreakpoint(); - let showPopup = true; - const { locationPopup } = this.props; - - if (typeof this.state.selectableTargets !== 'undefined') { - if (this.state.selectableTargets.length === 1) { - let id; - if ( - (this.state.selectableTargets[0].layer === 'parkAndRide' && - this.state.selectableTargets[0].feature.properties.vehicleParking?.filter( - parking => parking.carPlaces, - ).length > 1) || - (this.state.selectableTargets[0].layer === 'parkAndRideForBikes' && - this.state.selectableTargets[0].feature.properties.vehicleParking?.filter( - parking => parking.bicyclePlaces, - ).length > 1) - ) { - id = `parkAndRide_${this.state.selectableTargets[0].feature.properties.vehicleParking[0].id}`; - contents = ( - - ); - } else if ( - this.state.selectableTargets[0].layer === 'realTimeVehicle' - ) { - const { vehicle } = this.state.selectableTargets[0].feature; - const realTimeInfoVehicle = this.props.vehicles[vehicle.id]; - if (realTimeInfoVehicle) { - latlng = { - lat: realTimeInfoVehicle.lat, - lng: realTimeInfoVehicle.long, - }; - } - this.PopupOptions.className = 'vehicle-popup'; + if (smallScreenPopupHidden) { + return null; + } - contents = ; - } - popup = ( - - {contents} - - ); - } else if (this.state.selectableTargets.length > 1) { - if ( - !this.props.config.map.showStopMarkerPopupOnMobile && - breakpoint === 'small' - ) { - showPopup = false; - } - popup = ( - - - - ); - } else if (this.state.selectableTargets.length === 0) { - if ( - !this.props.config.map.showStopMarkerPopupOnMobile && - breakpoint === 'small' - ) { - showPopup = false; - } - popup = locationPopup !== 'none' && ( - - - - ); - } - } - return showPopup ? popup : null; + if (selectableTargets.length > 1) { + return ( + + + + ); } -} -// Wraps the class component and supplies config/router via hooks instead of -// legacy React context, since class components cannot use hooks directly. -function TileLayerContainerWithContext(props) { - const config = useConfigContext(); - const { router } = useRouter(); + if (locationPopup === 'none') { + return null; + } return ( - - {({ environment }) => ( - - )} - + + + ); } -const connectedComponent = withLeaflet( - connectToStores( - withCurrentTime(TileLayerContainerWithContext), - [RealTimeInformationStore], - context => ({ - vehicles: context.getStore(RealTimeInformationStore).vehicles, - }), - ), +TileLayerContainer.propTypes = { + tileSize: PropTypes.number.isRequired, + zoomOffset: PropTypes.number.isRequired, + locationPopup: PropTypes.string, // all, none, reversegeocoding, origindestination + onSelectLocation: PropTypes.func, + mergeStops: PropTypes.bool, + mapLayers: mapLayerShape.isRequired, + // When true, clicks on map icons (stops, terminals, citybikes, etc.) + // are ignored instead of selecting/navigating to them. + disableIconClick: PropTypes.bool, + highlightedStops: PropTypes.arrayOf(PropTypes.string), + stopsToShow: PropTypes.arrayOf(PropTypes.string), + objectsToHide: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.string)), + vehicles: PropTypes.objectOf(vehicleShape), + currentTime: PropTypes.number.isRequired, +}; + +const connectedComponent = connectToStores( + withCurrentTime(TileLayerContainer), + [RealTimeInformationStore], + context => ({ + vehicles: context.getStore(RealTimeInformationStore).vehicles, + }), ); export { connectedComponent as default, TileLayerContainer as Component }; diff --git a/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx b/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx index 675e321d09..4ef303d3d6 100644 --- a/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx +++ b/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx @@ -1,72 +1,131 @@ import React from 'react'; +import L from 'leaflet'; +import { LeafletProvider } from 'react-leaflet/es/context'; import { renderWithProviders } from '../../../helpers/mock-providers'; import { mockContext } from '../../../helpers/mock-context'; -import { Component } from '../../../../../app/component/map/tile-layer/TileLayerContainer'; +import { + Component, + sendSelectionAnalytics, +} from '../../../../../app/component/map/tile-layer/TileLayerContainer'; import * as analytics from '../../../../../utils/shared/analyticsUtils'; describe('', () => { - const props = { + const config = { ...mockContext.config, vehicleRental: {} }; + + const createMap = () => ({ + addLayer: vi.fn(), + removeLayer: vi.fn(), + addEventParent: vi.fn(), + removeEventParent: vi.fn(), + closePopup: vi.fn(), + options: { maxZoom: null, minZoom: null }, + }); + + const baseProps = { tileSize: 1, zoomOffset: 1, mapLayers: { stop: {}, terminal: {} }, - leaflet: { - map: { - addLayer: () => null, - addEventParent: () => null, - closePopup: () => null, - off: () => null, - removeEventParent: () => null, - options: { maxZoom: null, minZoom: null }, - }, - }, - lang: 'fi', currentTime: 123457890, - config: { ...mockContext.config, vehicleRental: {} }, - router: mockContext.router, }; - it('should send analytics for a terminal stop target', () => { - const spy = vi.spyOn(analytics, 'addAnalyticsEvent'); - const componentRef = React.createRef(); - renderWithProviders( - , - { - config: { ...mockContext.config, vehicleRental: {} }, - }, + const renderLayer = (map, props = {}) => { + const element = p => ( + + + ); - componentRef.current.state.selectableTargets = [ - { - layer: 'stop', - feature: { - properties: { - stops: 'HSL:1234', - type: 'BUS', + const result = renderWithProviders(element(props), { config }); + return { + ...result, + update: p => result.rerender(element({ ...props, ...p })), + }; + }; + + describe('sendSelectionAnalytics', () => { + afterEach(() => vi.restoreAllMocks()); + + it('should send analytics for a terminal stop target', () => { + const spy = vi.spyOn(analytics, 'addAnalyticsEvent'); + sendSelectionAnalytics( + [ + { + layer: 'stop', + feature: { properties: { stops: 'HSL:1234', type: 'BUS' } }, }, - }, - }, - ]; - componentRef.current.PopupOptions.onOpen(); - expect(spy.mock.calls.length).toBe(1); - expect(spy.mock.calls[0][0]).toEqual({ - action: 'SelectMapPoint', - category: 'Map', - name: 'stop', - type: 'BUS_TERMINAL', - source: 'index', + ], + config, + ); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0][0]).toEqual({ + action: 'SelectMapPoint', + category: 'Map', + name: 'stop', + type: 'BUS_TERMINAL', + source: 'index', + }); + }); + + it('should send analytics for multiple targets', () => { + const spy = vi.spyOn(analytics, 'addAnalyticsEvent'); + const target = { layer: 'stop', feature: { properties: {} } }; + sendSelectionAnalytics([target, target], config); + expect(spy.mock.calls[0][0]).toMatchObject({ name: 'multiple' }); + }); + + it('should not send analytics when there are no selected targets', () => { + const spy = vi.spyOn(analytics, 'addAnalyticsEvent'); + sendSelectionAnalytics([], config); + sendSelectionAnalytics(undefined, config); + expect(spy).not.toHaveBeenCalled(); }); }); - it('should not send analytics when there are no selected targets', () => { - const spy = vi.spyOn(analytics, 'addAnalyticsEvent'); - const componentRef = React.createRef(); - renderWithProviders( - , - { - config: { ...mockContext.config, vehicleRental: {} }, - }, - ); - componentRef.current.state.selectableTargets = []; - componentRef.current.PopupOptions.onOpen(); - expect(spy.mock.calls.length).toBe(0); + describe('leaflet layer lifecycle', () => { + afterEach(() => vi.restoreAllMocks()); + + it('should add the layer on mount and remove it on unmount', () => { + const map = createMap(); + const { unmount } = renderLayer(map); + expect(map.addLayer).toHaveBeenCalledTimes(1); + expect(map.addEventParent).toHaveBeenCalledTimes(1); + const layer = map.addLayer.mock.calls[0][0]; + expect(layer).toBeInstanceOf(L.GridLayer); + expect(map.addEventParent).toHaveBeenCalledWith(layer); + + unmount(); + expect(map.removeLayer).toHaveBeenCalledWith(layer); + expect(map.removeEventParent).toHaveBeenCalledWith(layer); + }); + + it('should redraw only when mapLayers or highlightedStops change', () => { + const redraw = vi.spyOn(L.GridLayer.prototype, 'redraw'); + const { update } = renderLayer(createMap()); + expect(redraw).not.toHaveBeenCalled(); + + update({ mapLayers: { stop: {}, terminal: {} } }); + expect(redraw).not.toHaveBeenCalled(); + + const changedLayers = { stop: {}, terminal: {}, citybike: true }; + update({ mapLayers: changedLayers }); + expect(redraw).toHaveBeenCalledTimes(1); + + update({ mapLayers: changedLayers, highlightedStops: ['HSL:1'] }); + expect(redraw).toHaveBeenCalledTimes(2); + }); + + it('should notify active tile layers when time changes', () => { + const map = createMap(); + const { update } = renderLayer(map); + const layer = map.addLayer.mock.calls[0][0]; + const onTimeChange = vi.fn(); + // eslint-disable-next-line no-underscore-dangle + layer._tiles = { + a: { active: true, el: { layers: [{ onTimeChange }] } }, + b: { active: false, el: { layers: [{ onTimeChange }] } }, + }; + update({ currentTime: baseProps.currentTime + 1 }); + expect(onTimeChange).toHaveBeenCalledTimes(1); + expect(onTimeChange).toHaveBeenCalledWith(config.language); + }); }); }); From 038a2e803f87717095a38af2fca69a87b5d78d9b Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Tue, 6 Oct 2026 14:45:44 +0300 Subject: [PATCH 2/3] Simplify TileLayerContainer and use useCurrentTime Use optional chaining, share the parking hub filtering between the click handler and popup rendering, derive the vehicle popup flag from the target layer, and read the current time with useCurrentTime instead of withCurrentTime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../map/tile-layer/TileLayerContainer.jsx | 96 ++++++++----------- .../tile-layer/TileLayerContainer.test.jsx | 24 +++-- 2 files changed, 52 insertions(+), 68 deletions(-) diff --git a/app/component/map/tile-layer/TileLayerContainer.jsx b/app/component/map/tile-layer/TileLayerContainer.jsx index 86906cc7b8..3cd455db44 100644 --- a/app/component/map/tile-layer/TileLayerContainer.jsx +++ b/app/component/map/tile-layer/TileLayerContainer.jsx @@ -32,11 +32,21 @@ import { PREFIX_RENTALVEHICLES, } from '../../../../utils/shared/path'; import SelectVehicleContainer from './SelectVehicleContainer'; -import { withCurrentTime } from '../../../hooks/TimeContext'; +import { useCurrentTime } from '../../../hooks/TimeContext'; import { useConfigContext } from '../../../client/ConfigContext'; const DEFAULT_OBJECTS_TO_HIDE = { vehicleRentalStations: [] }; +const PARKING_LAYERS = ['parkAndRide', 'parkAndRideForBikes']; + +// A hub is a single map feature with several nested parking lots. Returns the +// lots relevant for the target's layer, or undefined if the target is not a hub. +function getHubParking({ layer, feature }) { + return feature.properties?.vehicleParking?.filter(parking => + layer === 'parkAndRide' ? parking.carPlaces : parking.bicyclePlaces, + ); +} + const POPUP_OPTIONS = { offset: [0, 0], autoPanPaddingTopLeft: [5, 125], @@ -67,10 +77,8 @@ export function sendSelectionAnalytics(selectableTargets, config) { name = 'multiple'; } const pathPrefixMatch = window.location.pathname.match(/^\/([a-z]{2,})\//); - const context = - pathPrefixMatch && pathPrefixMatch[1] !== config.indexPath - ? pathPrefixMatch[1] - : 'index'; + const prefix = pathPrefixMatch?.[1]; + const context = prefix && prefix !== config.indexPath ? prefix : 'index'; addAnalyticsEvent({ action: 'SelectMapPoint', category: 'Map', @@ -101,21 +109,18 @@ function onSelectableTargetClicked( } = latest.current; const prevCoords = selection?.coords; const popup = map._popup; // eslint-disable-line no-underscore-dangle + const single = selectableTargets.length === 1 ? selectableTargets[0] : null; // navigate to citybike stop page if single stop is clicked - if ( - selectableTargets.length === 1 && - selectableTargets[0].layer === 'citybike' - ) { + if (single?.layer === 'citybike') { router.push( `/${PREFIX_BIKESTATIONS}/${encodeURIComponent( - selectableTargets[0].feature.properties.id, + single.feature.properties.id, )}`, ); return; } if ( - (selectableTargets.length === 1 && - selectableTargets[0].layer === 'scooter') || + single?.layer === 'scooter' || (selectableTargets.length > 1 && selectableTargets.every(target => target.layer === 'scooter')) // scooters are not shown in the selection popup as there can be too many. @@ -124,7 +129,7 @@ function onSelectableTargetClicked( const cluster = selectableTargets.find( target => target.feature.properties.cluster, ); - const networks = cluster ? cluster.feature.properties.networks : ''; + const networks = cluster?.feature.properties.networks ?? ''; const id = cluster ? cluster.feature.properties.scooterId : selectableTargets[0].feature.properties.id; @@ -135,51 +140,37 @@ function onSelectableTargetClicked( return; } // ... Or to stop page - if (selectableTargets.length === 1 && selectableTargets[0].layer === 'stop') { + if (single?.layer === 'stop') { router.push( stopPagePath( - selectableTargets[0].feature.properties.stops, - selectableTargets[0].feature.properties.gtfsId, + single.feature.properties.stops, + single.feature.properties.gtfsId, ), ); return; } - if ( - selectableTargets.length === 1 && - (selectableTargets[0].layer === 'parkAndRide' || - selectableTargets[0].layer === 'parkAndRideForBikes') - ) { - const { layer } = selectableTargets[0]; + if (PARKING_LAYERS.includes(single?.layer)) { + const hubParking = getHubParking(single); let parkingId; - // hubs have nested vehicleParking - if (selectableTargets[0].feature.properties?.vehicleParking) { - const parksInHub = - selectableTargets[0].feature.properties?.vehicleParking?.filter( - parking => - layer === 'parkAndRide' ? parking.carPlaces : parking.bicyclePlaces, - ); - if (parksInHub.length === 1) { - parkingId = parksInHub[0].id; + if (hubParking) { + if (hubParking.length === 1) { + parkingId = hubParking[0].id; } } else { - parkingId = selectableTargets[0].feature.properties?.id; + parkingId = single.feature.properties?.id; } if (parkingId) { router.push( `/${ - layer === 'parkAndRide' ? PREFIX_CARPARK : PREFIX_BIKEPARK + single.layer === 'parkAndRide' ? PREFIX_CARPARK : PREFIX_BIKEPARK }/${encodeURIComponent(parkingId)}`, ); return; } } - if ( - popup && - popup.isOpen() && - (!forceOpen || (coords && coords.equals(prevCoords))) - ) { + if (popup?.isOpen() && (!forceOpen || coords?.equals(prevCoords))) { map.closePopup(); return; } @@ -241,8 +232,8 @@ function TileLayerContainer(props) { mapLayers, highlightedStops, vehicles, - currentTime, } = props; + const currentTime = useCurrentTime(); const config = useConfigContext(); const { router } = useRouter(); const { environment: relayEnvironment } = useContext(ReactRelayContext); @@ -317,14 +308,8 @@ function TileLayerContainer(props) { } if (prev.currentTime !== currentTime) { /* eslint-disable no-underscore-dangle */ - lodashFilter(layer._tiles, tile => tile.active).forEach( - tile => - tile.el.layers && - tile.el.layers.forEach(l => { - if (l.onTimeChange) { - l.onTimeChange(config.language); - } - }), + lodashFilter(layer._tiles, tile => tile.active).forEach(tile => + tile.el.layers?.forEach(l => l.onTimeChange?.(config.language)), ); /* eslint-enable no-underscore-dangle */ } @@ -360,22 +345,19 @@ function TileLayerContainer(props) { if (selectableTargets.length === 1) { const target = selectableTargets[0]; - const nestedParking = target.feature.properties?.vehicleParking; let id; let contents; let latlng = coords; - let isVehicle = false; + const isVehicle = target.layer === 'realTimeVehicle'; if ( - (target.layer === 'parkAndRide' && - nestedParking?.filter(parking => parking.carPlaces).length > 1) || - (target.layer === 'parkAndRideForBikes' && - nestedParking?.filter(parking => parking.bicyclePlaces).length > 1) + PARKING_LAYERS.includes(target.layer) && + getHubParking(target)?.length > 1 ) { - id = `parkAndRide_${nestedParking[0].id}`; + id = `parkAndRide_${target.feature.properties.vehicleParking[0].id}`; contents = ( ); - } else if (target.layer === 'realTimeVehicle') { + } else if (isVehicle) { const { vehicle } = target.feature; const realTimeInfoVehicle = vehicles[vehicle.id]; if (realTimeInfoVehicle) { @@ -384,7 +366,6 @@ function TileLayerContainer(props) { lng: realTimeInfoVehicle.long, }; } - isVehicle = true; contents = ; } return ( @@ -461,11 +442,10 @@ TileLayerContainer.propTypes = { stopsToShow: PropTypes.arrayOf(PropTypes.string), objectsToHide: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.string)), vehicles: PropTypes.objectOf(vehicleShape), - currentTime: PropTypes.number.isRequired, }; const connectedComponent = connectToStores( - withCurrentTime(TileLayerContainer), + TileLayerContainer, [RealTimeInformationStore], context => ({ vehicles: context.getStore(RealTimeInformationStore).vehicles, diff --git a/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx b/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx index 4ef303d3d6..5601c67e31 100644 --- a/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx +++ b/test/unit/component/map/tile-layer/TileLayerContainer.test.jsx @@ -8,6 +8,7 @@ import { sendSelectionAnalytics, } from '../../../../../app/component/map/tile-layer/TileLayerContainer'; import * as analytics from '../../../../../utils/shared/analyticsUtils'; +import { TimeContext } from '../../../../../app/hooks/TimeContext'; describe('', () => { const config = { ...mockContext.config, vehicleRental: {} }; @@ -25,19 +26,22 @@ describe('', () => { tileSize: 1, zoomOffset: 1, mapLayers: { stop: {}, terminal: {} }, - currentTime: 123457890, }; - - const renderLayer = (map, props = {}) => { - const element = p => ( - - - + const initialTime = 123457890; + + const renderLayer = (map, props = {}, currentTime = initialTime) => { + const element = (p, time) => ( + + + + + ); - const result = renderWithProviders(element(props), { config }); + const result = renderWithProviders(element(props, currentTime), { config }); return { ...result, - update: p => result.rerender(element({ ...props, ...p })), + update: (p, time = currentTime) => + result.rerender(element({ ...props, ...p }, time)), }; }; @@ -123,7 +127,7 @@ describe('', () => { a: { active: true, el: { layers: [{ onTimeChange }] } }, b: { active: false, el: { layers: [{ onTimeChange }] } }, }; - update({ currentTime: baseProps.currentTime + 1 }); + update({}, initialTime + 1); expect(onTimeChange).toHaveBeenCalledTimes(1); expect(onTimeChange).toHaveBeenCalledWith(config.language); }); From 16926ff22cfd5d81bf5f0eba2880f1987cb0027b Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Tue, 6 Oct 2026 14:56:14 +0300 Subject: [PATCH 3/3] Keep popup content mounted while it fades out Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../map/tile-layer/TileLayerContainer.jsx | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/app/component/map/tile-layer/TileLayerContainer.jsx b/app/component/map/tile-layer/TileLayerContainer.jsx index 3cd455db44..f662faa47d 100644 --- a/app/component/map/tile-layer/TileLayerContainer.jsx +++ b/app/component/map/tile-layer/TileLayerContainer.jsx @@ -47,6 +47,11 @@ function getHubParking({ layer, feature }) { ); } +// Leaflet fades a closed popup out for 200 ms; its content must stay mounted meanwhile. +const POPUP_FADE_OUT_MS = 250; + +let selectionCounter = 0; + const POPUP_OPTIONS = { offset: [0, 0], autoPanPaddingTopLeft: [5, 125], @@ -175,7 +180,10 @@ function onSelectableTargetClicked( return; } + selectionCounter += 1; setSelection({ + // a new id remounts the popup, so one still fading out is never reused + id: selectionCounter, selectableTargets: selectableTargets.filter( target => target.layer === 'realTimeVehicle' || @@ -315,7 +323,16 @@ function TileLayerContainer(props) { } }); - const closePopup = useCallback(() => setSelection(null), []); + const closeTimer = useRef(); + useEffect(() => () => clearTimeout(closeTimer.current), []); + const closePopup = useCallback(() => { + const closing = latest.current.selection; + clearTimeout(closeTimer.current); + closeTimer.current = setTimeout( + () => setSelection(current => (current === closing ? null : current)), + POPUP_FADE_OUT_MS, + ); + }, []); const onPopupOpen = useCallback( () => sendSelectionAnalytics( @@ -371,7 +388,7 @@ function TileLayerContainer(props) { return ( 1) { return (