Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion e2e-tests/e2e_test_data_initial.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
"is_free": "true",
"speed_limit": "10",
"amenities": [
"benches"
"toilets"
],
"uuid": "5986e755-1eaa-4121-a01c-4fef1d5d1da1"
},
Expand Down Expand Up @@ -272,6 +272,19 @@
"cars"
]
},
"marker_styles": {
"icon_field": "type_of_place",
"color_field": "speed_limit",
"icons": {
"big bridge": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg",
"small bridge": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/footprints-fill.svg"
},
"colors": {
"10": "#2e7d32",
"30": "#ef6c00",
"50": "#c62828"
}
},
"visible_data": [
"remark",
"accessible_by",
Expand Down
106 changes: 106 additions & 0 deletions e2e-tests/tests/basic/test_marker_styles.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
"""
Marker Styles Tests

Tests that the map picks pin icon/color per marker_styles (icon_field:
type_of_place, color_field: speed_limit - see e2e_test_data_initial.json), and
that a location with both a remark and a marker_styles match keeps its
type/color styling with an asterisk badge overlay, rather than losing it to a
plain, unstyled asterisk badge (see getTypedMarkerIcon.jsx/MarkerPopup.jsx).
"""

from playwright.sync_api import Page, expect

from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, open_test_popup

# "big bridge" and "small bridge" each get their own Phosphor Icons (MIT) type
# icon - see e2e_test_data_initial.json's marker_styles.icons and
# getTypedMarkerIcon.jsx (icon URLs are CSS mask-image'd onto the pin, tinted
# by the matched color, rather than embedded as inline SVG path data).
BIG_BRIDGE_TYPE_ICON_URL = (
"https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg"
)
SMALL_BRIDGE_TYPE_ICON_URL = (
"https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/footprints-fill.svg"
)


class TestMarkerStyles:
"""Test suite for marker_styles-driven pin icons/colors"""

def test_fast_bridge_marker_uses_type_icon_and_red_speed_color(self, page: Page):
"""Pokoju (big bridge, speed_limit=50, no remark) is the only seeded bridge
with all three of lighting+benches+toilets (amenities is an "and" category -
see test_and_filter_within_category_narrows_results in test_map.py), so
checking all three isolates its marker without relying on clustering
distance/zoom assumptions."""
page.goto(BASE_URL, wait_until="domcontentloaded")

# "cars" is checked by default (Pokoju is cars-accessible); narrow further.
for amenity in ("lighting", "benches", "toilets"):
page.get_by_role("checkbox", name=amenity, exact=False).click()

marker = page.locator(".custom-typed-marker-icon")
expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT)

# The pin shape itself (a masked div, not an inline <path>), filled with
# speed_limit=50's color.
pin = marker.locator(".custom-typed-marker-pin")
expect(pin).to_have_css("background-color", "rgb(198, 40, 40)") # #c62828
# The type_of_place icon, configured for "big bridge" - masked onto a div
# via CSS rather than embedded as an inline <path>.
type_icon = marker.locator(".custom-typed-marker-type-icon")
expect(type_icon).to_have_count(1)
expect(type_icon).to_have_css("mask-image", f'url("{BIG_BRIDGE_TYPE_ICON_URL}")')
# No remark on Pokoju, so no asterisk badge.
expect(marker.locator("span")).to_have_count(0)

def test_slow_bridge_marker_uses_type_icon_and_green_speed_color(self, page: Page):
"""Piaskowy (small bridge, speed_limit=10, no remark, toilets) is the
only seeded speed<=10 bridge with toilets - the other two speed=10
bridges (Zwierzyniecka, Tumski) have lighting/benches but neither has
toilets, so combining the speed_limit=10 radio with the toilets
checkbox isolates it without relying on clustering distance/zoom
assumptions. "cars" is unchecked first since Piaskowy is
pedestrians-only."""
page.goto(BASE_URL, wait_until="domcontentloaded")

page.get_by_role("checkbox", name="cars", exact=False).click()
page.get_by_role("radio", name="10 km/h", exact=False).click()
page.get_by_role("checkbox", name="toilets", exact=False).click()

marker = page.locator(".custom-typed-marker-icon")
expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT)

pin = marker.locator(".custom-typed-marker-pin")
expect(pin).to_have_css("background-color", "rgb(46, 125, 50)") # #2e7d32 (speed_limit=10)
type_icon = marker.locator(".custom-typed-marker-type-icon")
expect(type_icon).to_have_count(1)
expect(type_icon).to_have_css("mask-image", f'url("{SMALL_BRIDGE_TYPE_ICON_URL}")')
# No remark on Piaskowy, so no asterisk badge.
expect(marker.locator("span")).to_have_count(0)

def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, page: Page):
"""Zwierzyniecka has both a remark and marker_styles-matching fields
(small bridge, speed_limit=10) - it should render its normal typed/colored
pin plus an asterisk badge, not fall back to our own pin in the plain
fallback color with no type icon (every type_of_place/speed_limit value
happens to be covered by marker_styles in this seeded dataset, so that
fallback-color path isn't exercised here - it's covered at the unit
level instead, see getTypedMarkerIcon.test.jsx's "returns our own pin in
the fallback color with just the badge" case). Also guards against ever
reintroducing the old PNG-based asterisk icon this replaced.
"""
page.goto(BASE_URL, wait_until="domcontentloaded")
open_test_popup(page)

expect(page.locator('img[alt="Marker-Asterisk"]')).to_have_count(0)

marker = page.locator(".custom-typed-marker-icon")
expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT)

pin = marker.locator(".custom-typed-marker-pin")
expect(pin).to_have_css("background-color", "rgb(46, 125, 50)") # #2e7d32 (speed_limit=10)
type_icon = marker.locator(".custom-typed-marker-type-icon")
expect(type_icon).to_have_count(1)
expect(type_icon).to_have_css("mask-image", f'url("{SMALL_BRIDGE_TYPE_ICON_URL}")')
expect(marker.locator("span")).to_have_text("*")
33 changes: 14 additions & 19 deletions frontend/src/components/MarkerPopup/MarkerPopup.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@
import PropTypes from 'prop-types';
import { Marker } from 'react-leaflet';
import { isMobile } from 'react-device-detect';
import { Icon } from 'leaflet';
import { useTranslation } from 'react-i18next';
import httpService from '../../services/http/httpService';
import useMapStore from '../Map/store/map.store';
import useMarkerStylesStore from '../Map/store/markerStyles.store';

Check failure on line 8 in frontend/src/components/MarkerPopup/MarkerPopup.jsx

View workflow job for this annotation

GitHub Actions / lint

Unable to resolve path to module '../Map/store/markerStyles.store'

import LocationDetailsBox from './LocationDetails';
import MobilePopup from './MobilePopup';
import DesktopPopup from './DesktopPopup';
import iconAsterisk from '../../res/img/marker-icon-asterisk.png';
import getTypedMarkerIcon from './getTypedMarkerIcon';
import requestMarkerStyle from './requestMarkerStyle';

Check failure on line 14 in frontend/src/components/MarkerPopup/MarkerPopup.jsx

View workflow job for this annotation

GitHub Actions / lint

Missing file extension for "./requestMarkerStyle"

Check failure on line 14 in frontend/src/components/MarkerPopup/MarkerPopup.jsx

View workflow job for this annotation

GitHub Actions / lint

Unable to resolve path to module './requestMarkerStyle'

/**
* Wrapper component that fetches full location details and renders them in a popup.
Expand All @@ -37,7 +38,7 @@
}
} catch (error) {
if (isMounted) {
console.error('Failed to fetch location:', error);

Check warning on line 41 in frontend/src/components/MarkerPopup/MarkerPopup.jsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement
setPlace({ error: true });
}
}
Expand Down Expand Up @@ -69,30 +70,20 @@
}).isRequired,
};

/**
* Custom Leaflet icon for markers with remarks/special annotations.
* Displays an asterisk icon to visually distinguish remarked locations from standard markers.
*/
const asteriskIcon = new Icon({
iconUrl: iconAsterisk,
iconSize: [40, 48], // size of the icon
iconAnchor: [19, 46], // point of the icon which will correspond to marker's location
popupAnchor: [0, -40], // point from which the popup should open relative to the iconAnchor
});

/**
* Interactive map marker component that displays location details in a popup when clicked.
* Supports special visual indication for locations with remarks using an asterisk icon.
*
* @param {Object} props - Component props
* @param {Object} props.place - Location data object
* @param {number[]} props.place.position - Coordinates [latitude, longitude]
* @param {boolean} [props.place.has_remark] - Whether this location has a remark (uses asterisk icon if true)
* @param {boolean} [props.place.has_remark] - Whether this location has a remark (adds an asterisk badge if true)
* @returns {React.ReactElement} Leaflet Marker component with click-to-show-details functionality
*/
const MarkerPopup = ({ place }) => {
const selectedLocationId = useMapStore(state => state.selectedLocationId);
const setSelectedLocationId = useMapStore(state => state.setSelectedLocationId);
const lazyMarkerStyle = useMarkerStylesStore(state => state.stylesByUuid[place.uuid]);
const [isClicked, setIsClicked] = useState(false);

// TODO: this only opens the popup if `place`'s Marker is actually attached to
Expand All @@ -114,18 +105,22 @@
setIsClicked(true);
};

const handleMarkerVisible = () => {
requestMarkerStyle(place.uuid);
};

const markerProps = {
position: place.position,
eventHandlers: {
click: handleMarkerClick,
add: handleMarkerVisible,
},
alt: place.has_remark ? 'Marker-Asterisk' : 'Marker',
};

// Only add icon prop if we have a custom icon (for remarks)
// This prevents passing undefined which can cause issues with MarkerClusterGroup
if (place.has_remark) {
markerProps.icon = asteriskIcon;
const styledPlace = lazyMarkerStyle ? { ...place, ...lazyMarkerStyle } : place;
const typedIcon = getTypedMarkerIcon(styledPlace);
if (typedIcon) {
markerProps.icon = typedIcon;
}

return (
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/MarkerPopup/ReportProblemForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ const ReportProblemForm = ({ placeId }) => {
if (schemaError) {
return (
<ErrorMessage>
{t('loadReportFormError')}
<span role="alert">{t('loadReportFormError')}</span>
<div>
<RetryButton type="button" onClick={refetchLocationSchema}>
{t('retry')}
Expand Down
122 changes: 122 additions & 0 deletions frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import React from 'react';
import PropTypes from 'prop-types';
import { DivIcon } from 'leaflet';
import ReactDOMServer from 'react-dom/server';
import PIN_SHAPE_URL from '../../res/svg/marker-pin.svg';
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const PIN_WIDTH = 45;
const PIN_HEIGHT = 50;
// The marker's default color (used whenever color_field doesn't match) is
// always the page's own secondary color, not a separately configurable value.
const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || 'black';

const TYPE_ICON_SIZE = 20;
const TYPE_ICON_OFFSET_TOP = 8;
const TYPE_ICON_OFFSET_LEFT = 12;

const maskStyle = (url, color) => ({
backgroundColor: color,
WebkitMaskImage: `url(${url})`,
maskImage: `url(${url})`,
WebkitMaskSize: '100% 100%',
maskSize: '100% 100%',
WebkitMaskRepeat: 'no-repeat',
maskRepeat: 'no-repeat',
});

/**
* Pin shape masked to `color`, optionally holding a type icon (`typeIconUrl`)
* inside its head, and an asterisk badge when `hasRemark` is set - so a
* remarked location keeps its type/color styling (or just its fallback color,
* if nothing else matched) instead of losing it to an unrelated asterisk icon.
*/
const PinIcon = ({ color, typeIconUrl, hasRemark }) => (
<div style={{ position: 'relative', width: PIN_WIDTH, height: PIN_HEIGHT }}>
<div
className="custom-typed-marker-pin"
style={{
position: 'absolute',
inset: 0,
filter: 'drop-shadow(0 0 1px #fff) drop-shadow(0 0 1px #fff)',
...maskStyle(PIN_SHAPE_URL, color),
}}
/>
{typeIconUrl !== '' && (
<div
className="custom-typed-marker-type-icon"
style={{
position: 'absolute',
top: TYPE_ICON_OFFSET_TOP,
left: TYPE_ICON_OFFSET_LEFT,
width: TYPE_ICON_SIZE,
height: TYPE_ICON_SIZE,
...maskStyle(typeIconUrl, '#ffffff'),
}}
/>
)}
{hasRemark && (
<span
style={{
position: 'absolute',
top: 1,
left: 24,
fontSize: 21,
fontWeight: 'bold',
lineHeight: 1,
color: '#ffffff',
textShadow: [-1, 1]
.flatMap(x => [-1, 1].map(y => `${x}px ${y}px 0 ${color}`))
.join(', '),
}}
>
*
</span>
)}
</div>
);

PinIcon.propTypes = {
color: PropTypes.string.isRequired,
typeIconUrl: PropTypes.string.isRequired,
hasRemark: PropTypes.bool.isRequired,
};

/**
* Builds a Leaflet icon for `place`: colored/typed from the deployment's
* marker styling lookup table (window.MARKER_STYLES, see goodmap's
* db.get_marker_styles) when it matches, our own pin in the fallback color
* with just the asterisk badge when `place.has_remark` is set but nothing
* matched, or `null` (falls back to Leaflet's default marker) when there's
* neither a match nor a remark to show.
*
* @param {Object} place - Location data, as returned by GET /api/locations
* @returns {import('leaflet').DivIcon|null}
*/
const getTypedMarkerIcon = place => {
const markerStyles = globalThis.MARKER_STYLES || {};
const { icon_field: iconField, color_field: colorField, icons, colors } = markerStyles;

const typeIconUrl = icons?.[place[iconField]] || '';
const matchedColor = colors?.[place[colorField]] || '';
const hasRemark = Boolean(place.has_remark);

if (!typeIconUrl && !matchedColor && !hasRemark) {
return null;
}

return new DivIcon({
html: ReactDOMServer.renderToString(
<PinIcon
color={matchedColor || FALLBACK_COLOR}
typeIconUrl={typeIconUrl}
hasRemark={hasRemark}
/>,
),
className: 'custom-typed-marker-icon',
iconSize: [PIN_WIDTH, PIN_HEIGHT],
iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT],
popupAnchor: [0, -PIN_HEIGHT],
});
};

export default getTypedMarkerIcon;
Binary file removed frontend/src/res/img/marker-icon-asterisk.png
Binary file not shown.
1 change: 1 addition & 0 deletions frontend/src/res/svg/marker-pin.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 8 additions & 0 deletions frontend/src/services/http/endpoints.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ export const LOCATIONS = '/api/locations';
*/
export const LOCATIONS_CLUSTERED = '/api/locations-clustered';

/**
* API endpoint for lazily fetching marker styling field values (whatever
* marker_styles.icon_field/color_field point at) for specific locations, by uuid.
* Used once a location's marker becomes individually visible, instead of upfront
* for every location - see lazy-load-marker-styling-plan.md.
*/
export const LOCATIONS_MARKER_STYLES = '/api/locations/marker-styles';

/**
* External API endpoint for address search (forward geocoding) using OpenStreetMap Nominatim.
* Converts addresses/place names to geographic coordinates.
Expand Down
Loading
Loading