-
Notifications
You must be signed in to change notification settings - Fork 12
feat: added markers styling #393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 7 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
95bf915
feat: added markers styling
raven-wing c50fa7d
fixes
raven-wing 242cb5f
fix popup
raven-wing c76d8ef
some icon fixes
raven-wing b99e8e5
fix lint
raven-wing e15aa03
fix linting
raven-wing 30088a2
some fixes
raven-wing 6b2bc2c
fixes
raven-wing 049554d
some trims
raven-wing d063439
some fixes
raven-wing 3e19819
added missing marker
raven-wing e289645
cleanup
raven-wing 4ff6c8a
cleanup
raven-wing 0c3ee6a
cleanup comment
raven-wing 3597a66
more human readable icons
raven-wing 3457d33
some cleanup
raven-wing d2096e4
cleanup
raven-wing 121cd64
cleanup comments
raven-wing 56d1b13
cleanup
raven-wing d62f19d
fixes
raven-wing e2a608a
fixes
raven-wing 34a1f84
fixes
raven-wing 41cfcac
fix
raven-wing 309a79d
added missing files
raven-wing 0063c97
fix
raven-wing fe0fb27
refactor
raven-wing 50fad55
refactor
raven-wing 3c0b60d
lot of files changes
raven-wing b676b67
fixes
raven-wing ff4dcd8
a lot of refactor
raven-wing bbbbd22
removed compress
raven-wing bda07f7
fix sonar errors
raven-wing 746c996
fixes after review
raven-wing d316152
lint fixes
raven-wing 77bf36e
fix for ci problems
raven-wing d45271c
fixes
raven-wing f51abbe
update
raven-wing c4c8821
missing file
raven-wing c31459e
less comments
raven-wing 08a870b
less comments
raven-wing 954c924
less comments
raven-wing fe9925f
little simplification
raven-wing db52516
simplify
raven-wing 092ace9
less code
raven-wing b606ebf
fif
raven-wing File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| """ | ||
| 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 | ||
| the plain asterisk icon (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) glyph - | ||
| # 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_GLYPH_URL = ( | ||
| "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg" | ||
| ) | ||
| SMALL_BRIDGE_GLYPH_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_glyph_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 glyph, configured for "big bridge" - masked onto a div | ||
| # via CSS rather than embedded as an inline <path>. | ||
| glyph = marker.locator(".custom-typed-marker-glyph") | ||
| expect(glyph).to_have_count(1) | ||
| expect(glyph).to_have_css("mask-image", f'url("{BIG_BRIDGE_GLYPH_URL}")') | ||
| # No remark on Pokoju, so no asterisk badge. | ||
| expect(marker.locator("span")).to_have_count(0) | ||
|
|
||
| # Note: a second real-browser color case (e.g. speed_limit=10 -> green) isn't | ||
| # covered here. The only speed=10 bridge without a remark (Piaskowy) can't be | ||
| # isolated to a standalone marker via the left panel's filters - its amenities | ||
| # ([benches]) are a subset of a remarked neighbor's (Tumski, [lighting, | ||
| # benches]) barely 230m away, so any filter combo that includes Piaskowy also | ||
| # includes Tumski, and Leaflet.markercluster groups them into one cluster | ||
| # bubble at the map's default zoom, hiding both individual markers. The | ||
| # color-lookup logic itself (arbitrary field values, including a "10" -> | ||
| # green case) is covered generically at the unit level in | ||
| # frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx. | ||
|
|
||
| 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 the plain asterisk icon | ||
| (every type_of_place/speed_limit value happens to be covered by | ||
| marker_styles in this seeded dataset, so that plain-icon fallback path | ||
| isn't exercised here - it's covered at the unit level instead, see | ||
| getTypedMarkerIcon.test.jsx's "falls back to the plain asterisk icon" | ||
| case).""" | ||
| 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) | ||
| glyph = marker.locator(".custom-typed-marker-glyph") | ||
| expect(glyph).to_have_count(1) | ||
| expect(glyph).to_have_css("mask-image", f'url("{SMALL_BRIDGE_GLYPH_URL}")') | ||
| expect(marker.locator("span")).to_have_text("*") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
147 changes: 147 additions & 0 deletions
147
frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import { DivIcon } from 'leaflet'; | ||
| import ReactDOMServer from 'react-dom/server'; | ||
|
|
||
| const PIN_WIDTH = 72; | ||
| const PIN_HEIGHT = 80; | ||
| // Matches the accent color used elsewhere on the page (buttons, left panel). | ||
| const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || '#2a81cb'; | ||
|
|
||
| // Phosphor Icons "map-pin-simple" glyph (MIT, phosphoricons.com), masked as | ||
| // the pin body - see PinIcon below. PIN_WIDTH is wider than the icon's own | ||
| // aspect ratio so its ball has room for the glyph. | ||
| const PIN_SHAPE_URL = | ||
| 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; | ||
|
|
||
| // The icon's artwork leaves blank margin below the stem tip, which becomes | ||
| // real empty space once stretched to fill the box - so the anchor has to | ||
| // target the actual rendered tip, not the box edge, or the marker floats | ||
| // above its true location. Measured empirically from a screenshot rather | ||
| // than the raw path coordinates, since drop-shadow/antialiasing shift the | ||
| // rendered edge slightly. | ||
| const STEM_TIP_FRACTION = 0.8875; | ||
|
|
||
| const GLYPH_SIZE = 24; | ||
| const GLYPH_OFFSET_TOP = 10; | ||
| const GLYPH_OFFSET_LEFT = 24; | ||
|
|
||
| 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 glyph (`glyphUrl`) inside | ||
| * its head, and an asterisk badge when `hasRemark` is set - so a remarked | ||
| * location keeps its type/color styling instead of losing it to a plain | ||
| * asterisk marker. | ||
| */ | ||
| const PinIcon = ({ color, glyphUrl, 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), | ||
| }} | ||
| /> | ||
| {glyphUrl !== '' && ( | ||
| <div | ||
| className="custom-typed-marker-glyph" | ||
| style={{ | ||
| position: 'absolute', | ||
| top: GLYPH_OFFSET_TOP, | ||
| left: GLYPH_OFFSET_LEFT, | ||
| width: GLYPH_SIZE, | ||
| height: GLYPH_SIZE, | ||
| ...maskStyle(glyphUrl, '#ffffff'), | ||
| }} | ||
| /> | ||
| )} | ||
| {hasRemark && ( | ||
| <span | ||
| style={{ | ||
| position: 'absolute', | ||
| top: -3, | ||
| left: 38, | ||
| fontSize: 36, | ||
| 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, | ||
| glyphUrl: PropTypes.string.isRequired, | ||
| hasRemark: PropTypes.bool.isRequired, | ||
| }; | ||
|
|
||
| /** | ||
| * Builds a Leaflet icon for `place` from the deployment's marker styling | ||
| * lookup table (window.MARKER_STYLES, see goodmap's db.get_marker_styles), or | ||
| * `null` when neither `icon_field` nor `color_field` matched - callers should | ||
| * omit the `icon` prop then and fall back to Leaflet's default marker (or the | ||
| * plain asterisk icon for a remarked location). | ||
| * | ||
| * Expected shape of window.MARKER_STYLES: | ||
| * { | ||
| * icon_field: 'type_of_place', | ||
| * color_field: 'status', | ||
| * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, | ||
| * colors: { open: '#2e7d32' }, | ||
| * default_color: '#2a81cb', | ||
| * } | ||
| * | ||
| * @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, | ||
| default_color: defaultColor, | ||
| } = markerStyles; | ||
|
|
||
| const glyphUrl = icons?.[place[iconField]] || ''; | ||
| const matchedColor = colors?.[place[colorField]] || ''; | ||
|
|
||
| if (!glyphUrl && !matchedColor) { | ||
| return null; | ||
| } | ||
|
|
||
| return new DivIcon({ | ||
| html: ReactDOMServer.renderToString( | ||
| <PinIcon | ||
| color={matchedColor || defaultColor || FALLBACK_COLOR} | ||
| glyphUrl={glyphUrl} | ||
| hasRemark={Boolean(place.has_remark)} | ||
| />, | ||
| ), | ||
| className: 'custom-typed-marker-icon', | ||
| iconSize: [PIN_WIDTH, PIN_HEIGHT], | ||
| iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT * STEM_TIP_FRACTION], | ||
| popupAnchor: [0, -PIN_HEIGHT * STEM_TIP_FRACTION], | ||
| }); | ||
| }; | ||
|
|
||
| export default getTypedMarkerIcon; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.