diff --git a/.specify/specs/032-measure-tape/plan.md b/.specify/specs/032-measure-tape/plan.md new file mode 100644 index 00000000..16296cf1 --- /dev/null +++ b/.specify/specs/032-measure-tape/plan.md @@ -0,0 +1,154 @@ +# Implementation Plan: Map Measuring Tape + +> **Spec ID:** 032-measure-tape +> **Status:** Planning +> **Last Updated:** 2026-05-31 +> **Estimated Effort:** S + +## Summary + +Add a `measureMode` boolean to `Map`, a ruler toggle button in the existing +`ZoomLocateControl` cluster, and a new `MeasureTape` `useMap()` child component that +manages two draggable Leaflet markers + a polyline + a distance tooltip while active. +Frontend-only, no backend or DB changes. + +--- + +## Architecture + +### Component Diagram + +``` +┌─────────────────────── Map.jsx ───────────────────────┐ +│ state: measureMode (bool) │ +│ │ +│ │ +│ ┌──────────────────────────────────────────────┐ │ +│ │ ZoomLocateControl │ │ +│ │ + / − / locate / satellite / [MEASURE 📏] ──┼──▶ toggles measureMode +│ └──────────────────────────────────────────────┘ │ +│ ┌──────────────────────────────────────────────┐ │ +│ │ MeasureTape (active only) │ │ +│ │ markerA ●╌╌╌polyline╌╌╌● markerB │ │ +│ │ tooltip: "1.24 mi (2.0 km)" │ │ +│ └──────────────────────────────────────────────┘ │ +│ │ +└────────────────────────────────────────────────────────┘ +``` + +### Data Flow + +1. User clicks the ruler button → `ZoomLocateControl` calls `onToggleMeasure()` → `Map` flips `measureMode`. +2. `measureMode` true → `` mounts; on mount it computes two default + endpoints straddling the center of the current viewport via + `map.containerPointToLatLng()` and adds markers + polyline + tooltip to the map. +3. Dragging an endpoint fires `drag` → update the polyline latlngs and recompute the + label with `map.distance(a, b)`. +4. `measureMode` false (or unmount) → remove markers, polyline, tooltip. + +--- + +## Technology Choices + +| Component | Technology | Rationale | +|-----------|------------|-----------| +| Endpoints | `L.marker({ draggable: true })` with a `divIcon` handle | `L.marker` supports native dragging; `CircleMarker` does not | +| Line | `L.polyline` | Lightweight, redraws on drag | +| Label | `L.tooltip` bound to the polyline midpoint (permanent) | Stays attached, no extra DOM plumbing | +| Distance | `map.distance(a, b)` (geodesic, meters) | Accurate at any latitude; matches existing river-gauge code style | +| Toggle | Extra `` in the existing `ZoomLocateControl` `L.Control` | Reuses the established control pattern and styling | + +--- + +## Implementation Steps + +### Phase 1: Toggle plumbing + +- [ ] Add `measureMode` state + `onToggleMeasure` to `Map`. +- [ ] Add a ruler `` to `ZoomLocateControl`, wired to `onToggleMeasure`; reflect active state with an `active` class. +- [ ] Pass `useMeasure`/`onToggleMeasure` props into `ZoomLocateControl` (mirrors `useSatellite`/`onSatelliteToggle`). + +### Phase 2: MeasureTape component + +- [ ] New `MeasureTape({ active })` `useMap()` child. +- [ ] On activate: compute default A/B straddling viewport center (e.g. container points at 40%×50% and 60%×50%), add two draggable divIcon markers, a polyline, and a permanent midpoint tooltip. +- [ ] `drag` handlers update polyline + tooltip position + label live. +- [ ] `formatDistance(meters)` → imperial primary (ft `< 0.1 mi`, else mi 2dp) + metric secondary (m `< 1 km`, else km 2dp). +- [ ] Cleanup on deactivate/unmount removes all layers; re-activate resets to default position. + +### Phase 3: Styling & polish + +- [ ] CSS for `.measure-button` (matches sibling control buttons) + active state. +- [ ] CSS for `.measure-handle` divIcon (≥24px, grabbable) and `.measure-tooltip` label. +- [ ] `L.DomEvent.disableClickPropagation` so dragging doesn't pan/select. + +--- + +## File Changes + +### New Files + +| File | Purpose | +|------|---------| +| (none — `MeasureTape` lives in `Map.jsx` alongside the other `useMap` children) | Keeps the map components co-located, as `ZoomLocateControl` already is | + +### Modified Files + +| File | Changes | +|------|---------| +| `frontend/src/components/Map.jsx` | Add `measureMode` state; add `MeasureTape` component; add ruler button + props to `ZoomLocateControl`; render `` inside `MapContainer` | +| `frontend/src/App.css` | `.measure-button`, `.measure-handle`, `.measure-tooltip` styles (near the existing `.zoom-locate-btn` rules) | + +--- + +## Database Migrations + +None. + +--- + +## API Implementation + +None. + +--- + +## Testing Strategy + +### Manual Testing + +1. Click the ruler button → tape appears in the bottom-right with a distance label. +2. Drag endpoint A onto one POI and B onto another → label updates live and reads a plausible distance. +3. Zoom in/out and pan → endpoints stay glued to their map locations; the distance number stays stable until an endpoint is moved. +4. Verify dragging an endpoint does NOT pan the map. +5. Toggle the button off → tape fully disappears; toggle on → resets to bottom-right. +6. Touch test (or narrow viewport) → handles are grabbable. + +### Automated + +- Existing Playwright smoke suite must still pass (`./run.sh test`, run by `/deploy`). No new e2e required for v1; the tool is additive and inactive by default. + +--- + +## Rollback Plan + +1. Frontend-only and inactive by default — revert the `Map.jsx`/CSS changes. +2. No data migration to unwind. + +--- + +## Risks and Mitigations + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Endpoint drag pans the map | Med | `disableClickPropagation` + marker `draggable` handles its own events | +| Tooltip/markers leak on toggle | Low | Explicit cleanup in `useEffect` return; keyed on `active` | +| Distance label overlaps controls | Low | Default endpoints centered, clear of the top-left controls | + +--- + +## Changelog + +| Date | Changes | +|------|---------| +| 2026-05-31 | Initial plan | diff --git a/.specify/specs/032-measure-tape/spec.md b/.specify/specs/032-measure-tape/spec.md new file mode 100644 index 00000000..b5228840 --- /dev/null +++ b/.specify/specs/032-measure-tape/spec.md @@ -0,0 +1,145 @@ +# Specification: Map Measuring Tape + +> **Spec ID:** 032-measure-tape +> **Status:** Draft +> **Version:** 0.1.0 +> **Author:** Scott McCarty +> **Date:** 2026-05-31 + +## Overview + +Visitors want to know how far apart two places are on the map — trailheads, a +parking lot and a waterfall, two POIs they're deciding between. Today there is no +way to do that. This feature adds a **two-point measuring tape**: a toggleable map +tool that drops two draggable endpoints (A and B), draws a line between them, and +reports the real-world distance live as you drag either end or zoom/pan the map. + +Resolves [#452](https://github.com/crunchtools/rotv/issues/452) ("Map Scale Key"). +The issue asked for a zoom-aware scale key so users could gauge how far apart POIs +are; a draggable measuring tape solves that problem statement more directly than a +fixed scale bar. + +--- + +## User Stories + +### Distance Measurement + +**US-001: Measure between two points** +> As a visitor, I want to drop two points on the map and read the distance between +> them so that I can tell how far apart trailheads, POIs, or features are. + +Acceptance Criteria: +- [ ] A measure toggle button is available in the map control cluster (top-left, with zoom/locate/satellite). +- [ ] Activating it shows two draggable endpoint handles (A and B) connected by a line. +- [ ] A label shows the geodesic distance between A and B, in imperial primary (ft / mi) with metric secondary (m / km). +- [ ] Endpoints first appear near the center of the current viewport so they're immediately visible and easy to grab. + +**US-002: Drag endpoints to measure anything** +> As a visitor, I want to drag each endpoint independently so that I can line them up +> on the two things I actually want to measure. + +Acceptance Criteria: +- [ ] Both endpoints are independently draggable. +- [ ] The connecting line and the distance label update live during the drag. +- [ ] Endpoints are large enough to grab on a touchscreen. + +**US-003: Stays accurate through zoom and pan** +> As a visitor, I want the distance to stay correct when I zoom or pan so that I trust +> the number. + +Acceptance Criteria: +- [ ] Endpoints are anchored to geographic coordinates (lat/lng), not screen pixels — they stay on their map locations through zoom/pan. +- [ ] The reported distance is geodesic (`map.distance`) and does not change on zoom unless an endpoint is moved. + +**US-004: Turn it off / get out of the way** +> As a visitor, I want to dismiss the tape when I'm done so that it stops cluttering +> the map. + +Acceptance Criteria: +- [ ] Toggling the button off removes both endpoints, the line, and the label. +- [ ] The toggle button shows an active state while the tape is on. +- [ ] Turning the tape off and on again resets endpoints to the default centered position. + +--- + +## Data Model + +No database changes. This is a client-only, ephemeral UI tool — measurements are not +persisted. + +--- + +## API Endpoints + +None. No backend changes. + +--- + +## UI/UX Requirements + +### New Components + +- `MeasureTape` — a `useMap()` child of `MapContainer` that, while active, manages two + draggable Leaflet markers, a connecting polyline, and a distance tooltip. Renders + nothing when inactive. + +### New Control + +- A ruler-icon toggle button appended to the existing `ZoomLocateControl` button + cluster (top-left), driven by a `measureMode` boolean lifted into `Map`. + +### Wireframe + +``` + map controls (top-left) measuring tape (starts centered) + ┌───┐ + │ + │ + │ − │ A ●╌╌╌╌╌╌╌╌╌● B + │ ◎ │ ← locate ┌─────────────┐ + │ ▦ │ ← satellite │ 1.24 mi │ + │ 📏│ ← measure (NEW) │ (2.0 km) │ + └───┘ appended to cluster └─────────────┘ +``` + +### Units + +- Imperial primary (US national-park audience): `< 0.1 mi` shown in feet, otherwise miles (2 decimals). +- Metric secondary in parentheses: `< 1 km` shown in meters, otherwise km (2 decimals). + +--- + +## Non-Functional Requirements + +**NFR-001: No regression to existing map interaction** +- The tape must not block map clicks, POI selection, or other controls when inactive. +- Dragging an endpoint must not pan the map. + +**NFR-002: Accessibility & touch** +- Toggle button has `role="button"`, `aria-label`, and an `aria-pressed`/active state. +- Endpoint handles are at least 24×24px hit targets. + +**NFR-003: Code quality** +- Passes the Gourmand gate (no `//` line comments except JSDoc; no single-use helpers). + +--- + +## Dependencies + +- Depends on: existing `MapContainer` / `ZoomLocateControl` infrastructure in `Map.jsx`. +- Blocks: none. + +--- + +## Open Questions + +1. Should the tape support more than two points (multi-segment path)? — Out of scope for v1; two points only. +2. Should measurements persist across reloads? — No; ephemeral by design. + +--- + +## Changelog + +| Version | Date | Changes | +|---------|------|---------| +| 0.1.0 | 2026-05-31 | Initial draft | diff --git a/backend/tests/ui.integration.test.js b/backend/tests/ui.integration.test.js index 8196be35..0da4ce13 100644 --- a/backend/tests/ui.integration.test.js +++ b/backend/tests/ui.integration.test.js @@ -233,14 +233,15 @@ describe('UI Integration Tests', () => { // Get all buttons in order const buttons = await page.locator('.zoom-locate-control .zoom-locate-btn').all(); - expect(buttons.length).toBe(4); + expect(buttons.length).toBe(5); - // Verify order: zoom in, zoom out, locate, satellite + // Verify order: zoom in, zoom out, locate, satellite, measure const classNames = await Promise.all(buttons.map(btn => btn.getAttribute('class'))); expect(classNames[0]).toContain('zoom-in-btn'); expect(classNames[1]).toContain('zoom-out-btn'); expect(classNames[2]).toContain('locate-button'); expect(classNames[3]).toContain('satellite-toggle-button'); + expect(classNames[4]).toContain('measure-button'); }, 30000); it('should position map controls below header (not off-screen)', async () => { diff --git a/frontend/src/App.css b/frontend/src/App.css index 943370c6..d75af6e9 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -527,6 +527,77 @@ body { height: 16px; } +/* Measure-distance toggle button */ +.measure-button:hover { + color: #2d5016 !important; +} + +.measure-button.active { + color: #2d5016 !important; + background: #eaf2e1 !important; +} + +.measure-button svg { + width: 16px; + height: 16px; +} + +/* Measuring-tape endpoint handles (A / B) */ +.measure-handle { + display: flex; + align-items: center; + justify-content: center; + cursor: grab; +} + +.measure-handle:active { + cursor: grabbing; +} + +.measure-handle-dot { + position: absolute; + width: 28px; + height: 28px; + border-radius: 50%; + background: rgba(45, 80, 22, 0.25); + border: 2px solid #2d5016; + box-sizing: border-box; +} + +.measure-handle-letter { + position: relative; + font-size: 12px; + font-weight: 700; + color: #2d5016; + line-height: 1; +} + +/* Distance label riding the midpoint of the tape */ +.measure-tooltip.leaflet-tooltip { + width: auto; + min-width: 0; + max-width: none; + background: #2d5016; + color: #fff; + border: none; + border-radius: 6px; + padding: 4px 8px; + font-size: 13px; + font-weight: 700; + white-space: nowrap; + text-align: center; + box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3); +} + +.measure-tooltip.leaflet-tooltip::before { + display: none; +} + +.measure-tooltip-secondary { + font-weight: 400; + opacity: 0.85; +} + /* User location marker pulse effect */ .user-location-pulse { animation: user-pulse 2s infinite; diff --git a/frontend/src/components/Map.jsx b/frontend/src/components/Map.jsx index a570bf11..1e181146 100644 --- a/frontend/src/components/Map.jsx +++ b/frontend/src/components/Map.jsx @@ -698,7 +698,7 @@ function MapBoundsTracker({ destinations, visibleTypes, getDestinationIconType, return null; } -function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onSatelliteToggle }) { +function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onSatelliteToggle, measureMode, onMeasureToggle }) { const map = useMap(); const [locating, setLocating] = useState(false); const userMarkerRef = useRef(null); @@ -808,17 +808,26 @@ function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onS const satelliteToggle = L.DomUtil.create('a', 'zoom-locate-btn satellite-toggle-button', container); satelliteToggle.href = '#'; - satelliteToggle.title = useSatellite ? 'Switch to map view' : 'Switch to satellite view'; + satelliteToggle.title = 'Switch to satellite view'; satelliteToggle.setAttribute('role', 'button'); - satelliteToggle.setAttribute('aria-label', useSatellite ? 'Switch to map view' : 'Switch to satellite view'); + satelliteToggle.setAttribute('aria-label', 'Switch to satellite view'); satelliteToggle.innerHTML = ` `; - if (useSatellite) { - satelliteToggle.classList.add('active'); - } + + const measure = L.DomUtil.create('a', 'zoom-locate-btn measure-button', container); + measure.href = '#'; + measure.title = 'Measure distance'; + measure.setAttribute('role', 'button'); + measure.setAttribute('aria-label', 'Measure distance'); + measure.setAttribute('aria-pressed', 'false'); + measure.innerHTML = ` + + + + `; L.DomEvent.disableClickPropagation(container); @@ -844,6 +853,13 @@ function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onS } }); + L.DomEvent.on(measure, 'click', function(e) { + L.DomEvent.preventDefault(e); + if (onMeasureToggle) { + onMeasureToggle(); + } + }); + return container; } }); @@ -860,7 +876,9 @@ function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onS userCircleRef.current.remove(); } }; - }, [map, handleLocate, useSatellite, onSatelliteToggle]); + // Fix: onAdd reads no toggle state, so the control is built once and the + // sync effects below own each button's active state — no churn (PR #458 review) + }, [map, handleLocate, onSatelliteToggle, onMeasureToggle]); useEffect(() => { const button = document.querySelector('.locate-button'); @@ -888,6 +906,72 @@ function ZoomLocateControl({ onLocationFound, onLocationError, useSatellite, onS } }, [useSatellite]); + useEffect(() => { + const button = document.querySelector('.measure-button'); + if (button) { + button.classList.toggle('active', measureMode); + button.setAttribute('aria-pressed', measureMode ? 'true' : 'false'); + } + }, [measureMode]); + + return null; +} + +function MeasureTape({ active }) { + const map = useMap(); + + useEffect(() => { + if (!active) return undefined; + + const makeHandle = (letter) => L.divIcon({ + className: 'measure-handle', + html: `${letter}`, + iconSize: [28, 28], + iconAnchor: [14, 14] + }); + + // Default the two endpoints straddling the viewport center so they are + // immediately visible and easy to grab. + const size = map.getSize(); + const startA = map.containerPointToLatLng(L.point(size.x * 0.4, size.y * 0.5)); + const startB = map.containerPointToLatLng(L.point(size.x * 0.6, size.y * 0.5)); + + const markerA = L.marker(startA, { draggable: true, icon: makeHandle('A'), zIndexOffset: 1200 }).addTo(map); + const markerB = L.marker(startB, { draggable: true, icon: makeHandle('B'), zIndexOffset: 1200 }).addTo(map); + const line = L.polyline([startA, startB], { color: '#2d5016', weight: 3, dashArray: '6 6', interactive: false }).addTo(map); + const label = L.tooltip({ permanent: true, direction: 'center', className: 'measure-tooltip', interactive: false }); + + const update = () => { + const a = markerA.getLatLng(); + const b = markerB.getLatLng(); + line.setLatLngs([a, b]); + const meters = map.distance(a, b); + const miles = meters / 1609.344; + const imperial = miles >= 0.1 + ? `${miles.toFixed(2)} mi` + : `${Math.round(meters * 3.28084).toLocaleString()} ft`; + const metric = meters >= 1000 + ? `${(meters / 1000).toFixed(2)} km` + : `${Math.round(meters).toLocaleString()} m`; + label.setLatLng(L.latLng((a.lat + b.lat) / 2, (a.lng + b.lng) / 2)); + label.setContent(`${imperial} (${metric})`); + }; + + label.setLatLng(L.latLng((startA.lat + startB.lat) / 2, (startA.lng + startB.lng) / 2)).addTo(map); + update(); + markerA.on('drag', update); + markerB.on('drag', update); + + return () => { + markerA.off(); + markerB.off(); + markerA.remove(); + markerB.remove(); + line.remove(); + label.remove(); + }; + }, [active, map]); + return null; } @@ -1107,6 +1191,11 @@ function Map({ destinations, selectedPoi, selectedIsLinear, onSelectPoi, isAdmin }, [isLegendExpanded, setIsLegendExpanded]); const [useSatellite, setUseSatellite] = useState(false); + const [measureMode, setMeasureMode] = useState(false); + // Fix: stable callbacks so ZoomLocateControl's effect doesn't tear down and + // rebuild the whole control on every render (PR #458 review) + const handleSatelliteToggle = useCallback(() => setUseSatellite(prev => !prev), []); + const handleMeasureToggle = useCallback(() => setMeasureMode(prev => !prev), []); const [selectedFileName, setSelectedFileName] = useState(null); // Just for UI display const [importType, setImportType] = useState('trail'); const [importingFile, setImportingFile] = useState(false); @@ -1764,9 +1853,13 @@ function Map({ destinations, selectedPoi, selectedIsLinear, onSelectPoi, isAdmin setUseSatellite(prev => !prev)} + onSatelliteToggle={handleSatelliteToggle} + measureMode={measureMode} + onMeasureToggle={handleMeasureToggle} /> + + {newPOI && previewCoords && (