Skip to content

feat: ski runs layer in map-external-layer sample#116

Open
lpernelle-woosmap wants to merge 2 commits intomasterfrom
feat/map-external-layer-ski-sample
Open

feat: ski runs layer in map-external-layer sample#116
lpernelle-woosmap wants to merge 2 commits intomasterfrom
feat/map-external-layer-ski-sample

Conversation

@lpernelle-woosmap
Copy link
Copy Markdown
Contributor

Summary

  • Replace ArcGIS satellite tiles with OpenSnowMap ski runs as the default third-party layer in the map-external-layer sample
  • Default style set to Light Grey, initial view centered on Oz-en-Oisans (zoom 15.64)
  • OpenWeather layers (temperature, precipitation) now fly to a US view when selected
  • Layer change updates the map bounds via flyTo; opacity change no longer recreates the map

Code quality

While in there, fixed several issues in the existing sample code:

  • Bug — overlay stacking: changeLayer used insertAt(0, …) without clearing previous overlays. After N changes, N raster layers were stacked with cumulative opacity. Now applyLayer calls overlayMapTypes.clear() first.
  • Bug — opacity validation: parseFloat(opacityInput.value) was called twice and NaN (empty/non-numeric input) silently passed validation. Centralized in readOpacity() with a proper isNaN check.
  • Bug — opacity change reset bounds: the original single change handler called changeStyle on every event, recreating the map even on opacity changes. Split into onStyleChange / onLayerChange / onOpacityChange.
  • Refactor: replaced if/else chains for style and layer selection with Record<string, …> lookups (STYLE_PRESETS, LAYER_CONFIGS, LAYER_VIEWS). Reduces cognitive complexity and makes adding a new layer/style a one-line change.
  • TypeScript: split combined let styleSelect, layerSelect: HTMLSelectElement (where only layerSelect was actually typed), Stringstring, =====, consistent indentation, semicolons.

Follow-up (out of scope)

  • The OpenWeather API key is still hardcoded in samples/map-external-layer/index.ts:1 (pre-existing on master). Should be rotated and replaced with a placeholder.
  • dist/samples/map-external-layer/** is intentionally not included in this PR — CI regenerates it via the chore: update dist folder [skip ci] job.

Test plan

  • Load the sample — map renders centered on Oz-en-Oisans with Light Grey style and OpenSnowMap ski runs overlay
  • Switch layer to OpenWeather temperature → map flies to US, overlay updates with no flicker / no stacking
  • Switch back to ski runs → flies back to Oz, overlay swaps
  • Change opacity slider — overlay opacity updates without recreating the map / changing center
  • Switch style (Default / Light Grey / Night / Retro) — base map style updates and current layer is reapplied

🤖 Generated with Claude Code

…nal-layer sample

- Swap default third-party layer from ArcGIS satellite to OpenSnowMap ski runs
- Default style set to Light Grey, view centered on Oz-en-Oisans
- OpenWeather layers (temperature, precipitation) fly to a US view
- Refactor: replace if/else chains with Record-based STYLE_PRESETS and LAYER_CONFIGS
- Fix overlay stacking bug (clear before insertAt)
- Fix opacity validation (handle NaN, dedupe parseFloat)
- Split layer/opacity handlers so opacity changes no longer reset bounds
- TypeScript cleanup: split combined declarations, String -> string, ===

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

- Add panel title and stack labels above selects (was cramped at 100px width)
- Replace opacity number input with range slider + live percentage readout
- Style selects and slider with consistent borders, focus state, accent color
- Wrap controls in <label> so clicking the label activates the input
- Apply mapTypeControl: true to the initial map (was only set after style change)
- Drop opacity alert — the slider HTML attributes already enforce the 0..1 bounds

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@shwetadeshmukh shwetadeshmukh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants