Conversation
There was a problem hiding this comment.
Pull request overview
This pull request replaces the old protomaps-themes-base package (v1.3.1) with the newer @protomaps/basemaps package (v5.7.0), introducing support for vector-based Protomaps basemaps with multiple theme variants (light, dark, white, grayscale, black) across the application. The default basemap is changed from Carto to Protomaps grayscale throughout the codebase.
Key changes:
- Replaces
protomaps-themes-basedependency with@protomaps/basemapsv5.7.0 - Adds five new Protomaps basemap variants with vector tile support
- Updates all map components to default to
protomaps-grayscaleinstead ofcarto - Implements special handling for vector basemaps (Protomaps) vs raster basemaps (Carto, Nearmap)
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes old protomaps-themes-base@1.3.1 and adds @protomaps/basemaps@5.7.0 |
| package.json | Updates dependencies and exports new basemap utilities via "./lib/basemaps" |
| src/runtime/composables/useBasemapLayers.ts | Adds five Protomaps basemap configurations, exports Protomaps utilities, and defines constants for glyphs/sprite URLs |
| src/runtime/components/pathway-map.vue | Changes default basemap from 'carto' to 'protomaps-grayscale' |
| src/runtime/apps/transfers/station-select-map.vue | Switches to Protomaps grayscale basemap with proper vector layer initialization |
| src/runtime/apps/transfers/station-bbox-select-map.vue | Switches to Protomaps grayscale basemap with proper vector layer initialization |
| src/runtime/apps/transfers/platform-pathway-map.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/station-editor.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/pathway-map.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/pages/station-stops.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/pages/station-pathways.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/level-map.vue | Adds comprehensive vector basemap support with visibility toggling and switches default to Protomaps |
| src/runtime/apps/stations/level-editor.vue | Changes default basemap to 'protomaps-grayscale' |
| src/runtime/apps/stations/basemap-control.vue | Changes default basemap prop to 'protomaps-grayscale' |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| export function useBasemapLayers () { | ||
| const config = useRuntimeConfig() | ||
| const protomapsApikey = config.public.tlv2?.protomapsApikey |
There was a problem hiding this comment.
The protomapsApikey is being interpolated directly into tile URLs without validation. If the API key is undefined or null, this will result in malformed URLs like "...?key=undefined" for all Protomaps basemap variants. Consider adding validation or a fallback to handle missing API key configuration gracefully, or at minimum document that the API key is required.
| export const PROTOMAPS_GLYPHS_URL = 'https://protomaps.github.io/basemaps-assets/fonts/{fontstack}/{range}.pbf' | ||
|
|
||
| /** | ||
| * Protomaps sprite URL base for MapLibre styles (append flavor name, e.g. /light) |
There was a problem hiding this comment.
The documentation comment states to "append flavor name, e.g. /light" but this is misleading. The actual URL should append the specific theme identifier (e.g., "/light" or "/dark"). Consider clarifying the comment to specify the exact format expected, such as "append theme identifier (e.g., '/light', '/dark')" to be more precise.
| * Protomaps sprite URL base for MapLibre styles (append flavor name, e.g. /light) | |
| * Protomaps sprite URL base for MapLibre styles (append theme identifier, e.g. '/light', '/dark') |
| type: 'raster', | ||
| tiles: basemaps.carto.source.tiles, | ||
| }, | ||
| 'protomaps-base': grayscaleBasemap.source as any, |
There was a problem hiding this comment.
Using 'as any' type assertion bypasses TypeScript's type safety. The source configuration should match MapLibre's expected source type. Consider properly typing the grayscaleBasemap.source or using a more specific type assertion to maintain type safety.
| version: 8, | ||
| sources: { | ||
| carto: basemaps.carto.source | ||
| 'protomaps-base': grayscaleBasemap.source as any |
There was a problem hiding this comment.
Using 'as any' type assertion bypasses TypeScript's type safety. The source configuration should match MapLibre's expected source type. Consider properly typing the grayscaleBasemap.source or using a more specific type assertion to maintain type safety.
| 'protomaps-light': { | ||
| label: 'Protomaps (light)', | ||
| source: { | ||
| type: 'vector', | ||
| tiles: [`https://api.protomaps.com/tiles/v4/{z}/{x}/{y}.pbf?key=${protomapsApikey}`], | ||
| maxzoom: 14, | ||
| attribution: '<a href="https://www.transit.land/terms">Transitland</a> | <a href="https://protomaps.com">Protomaps</a> | © <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>' | ||
| }, | ||
| layer: { | ||
| // For vector sources, we need to add multiple layers, not just one | ||
| // This will be handled specially in the map initialization | ||
| isVector: true, | ||
| flavor: LIGHT | ||
| } | ||
| }, | ||
| 'protomaps-dark': { | ||
| label: 'Protomaps (dark)', | ||
| source: { | ||
| type: 'vector', | ||
| tiles: [`https://api.protomaps.com/tiles/v4/{z}/{x}/{y}.pbf?key=${protomapsApikey}`], | ||
| maxzoom: 14, | ||
| attribution: '<a href="https://www.transit.land/terms">Transitland</a> | <a href="https://protomaps.com">Protomaps</a> | © <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>' | ||
| }, | ||
| layer: { | ||
| isVector: true, | ||
| flavor: DARK | ||
| } | ||
| }, | ||
| 'protomaps-white': { | ||
| label: 'Protomaps (white)', | ||
| source: { | ||
| type: 'vector', | ||
| tiles: [`https://api.protomaps.com/tiles/v4/{z}/{x}/{y}.pbf?key=${protomapsApikey}`], | ||
| maxzoom: 14, | ||
| attribution: '<a href="https://www.transit.land/terms">Transitland</a> | <a href="https://protomaps.com">Protomaps</a> | © <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>' | ||
| }, | ||
| layer: { | ||
| isVector: true, | ||
| flavor: WHITE | ||
| } | ||
| }, | ||
| 'protomaps-grayscale': { | ||
| label: 'Protomaps (grayscale)', | ||
| source: { | ||
| type: 'vector', | ||
| tiles: [`https://api.protomaps.com/tiles/v4/{z}/{x}/{y}.pbf?key=${protomapsApikey}`], | ||
| maxzoom: 14, | ||
| attribution: '<a href="https://www.transit.land/terms">Transitland</a> | <a href="https://protomaps.com">Protomaps</a> | © <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>' | ||
| }, | ||
| layer: { | ||
| isVector: true, | ||
| flavor: GRAYSCALE | ||
| } | ||
| }, | ||
| 'protomaps-black': { | ||
| label: 'Protomaps (black)', | ||
| source: { | ||
| type: 'vector', | ||
| tiles: [`https://api.protomaps.com/tiles/v4/{z}/{x}/{y}.pbf?key=${protomapsApikey}`], | ||
| maxzoom: 14, | ||
| attribution: '<a href="https://www.transit.land/terms">Transitland</a> | <a href="https://protomaps.com">Protomaps</a> | © <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>' | ||
| }, | ||
| layer: { | ||
| isVector: true, | ||
| flavor: BLACK | ||
| } | ||
| }, |
There was a problem hiding this comment.
The source configuration for all five Protomaps basemap variants contains identical code with the same tiles URL, maxzoom, and attribution. This repetition violates DRY principles and makes maintenance harder. Consider extracting the common configuration into a shared constant or helper function to reduce duplication.
No description provided.