Skip to content

feat(trail): add trail recording with pause/resume and GPX 1.1 export#85

Merged
jasoneplumb merged 2 commits intomainlinefrom
feature/trail-recording
Apr 11, 2026
Merged

feat(trail): add trail recording with pause/resume and GPX 1.1 export#85
jasoneplumb merged 2 commits intomainlinefrom
feature/trail-recording

Conversation

@jasoneplumb
Copy link
Copy Markdown
Owner

Closes #77

Summary

  • Trail recording state machine (idle → recording → paused → idle) with real-time polyline rendered on the map
  • Start, pause, resume, stop controls in the bottom-right recording panel; record button disabled until location is active
  • GPS accuracy filter (>30m discard) and distance threshold (<5m skip) on every fix
  • GPX 1.1 auto-download on stop: multi-segment support preserves pause gaps as separate <trkseg> elements; <ele> tag now populated from device altitude when available
  • Stats bar shows elapsed time, distance, and speed during recording

Test plan

  • Enable location, tap Record — polyline appears and stats bar shows
  • Tap Pause — dot turns amber, stats freeze; tap Resume — new segment begins (no straight-line gap artifact)
  • Tap Stop → confirm dialog → GPX file downloads as track-YYYY-MM-DD-HHmm.gpx
  • Import GPX into Strava or AllTrails — track renders correctly
  • On a device with barometric altitude, verify <ele> values appear in GPX

🤖 Generated with Claude Code

Implements issue #77: trail recording with real-time polyline, pause/resume
state machine, and GPX 1.1 auto-download on stop. Adds elevation (<ele>)
capture from device GPS to satisfy GPX 1.1 schema and Strava/Garmin import.
Applies accuracy >30m discard and <5m distance jitter filters on every fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jasoneplumb jasoneplumb added enhancement New feature or request review-requested Triggers Claude code review and removed review-requested Triggers Claude code review labels Apr 11, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 11, 2026

Code Review

This is a clean, well-scoped PR. The altitude feature is added with minimal footprint across exactly the right three files. A few observations:


What looks good

Type propagation is consistent. altM?: number is added to both trailPoints and trailSegments in types.ts, and the optional is threaded cleanly through appendTrailPoint and formatPoint. Nothing is left half-wired.

GPX element ordering is spec-compliant. GPX 1.1 schema requires <ele> before <time> inside a <trkpt>, and ${eleTag}${timeTag}${speedTag} respects that order.

Null → undefined conversion is the right pattern. e.altitude !== null ? e.altitude : undefined correctly maps the browser's null-when-unavailable into TypeScript's optional semantics.


One concern worth verifying

e.altitude is typed as number in Leaflet's LocationEvent, not number | null. With strict mode + noUncheckedIndexedAccess, TypeScript may flag e.altitude !== null as always-true, or the compiler may silently accept it since number !== null is always true at the type level (meaning the undefined branch is dead code from TypeScript's perspective).

Run npm run type-check to confirm it passes cleanly. If the Leaflet types have been patched locally to reflect the real-world nullable behavior this is already handled; if not, you may want a type assertion (e.altitude as number | null) to make the intent explicit and keep the runtime guard without confusing future readers:

const altM = (e.altitude as number | null) !== null ? e.altitude : undefined;

Minor nit

altM is appended as the last parameter to appendTrailPoint, after the already-established state and map parameters. That's the right call for an additive optional — no callers need to change and the signature stays readable. No action needed, just confirming this was intentional.


Summary

The implementation is correct and the scope is tight. The only thing worth a quick double-check is whether tsc is happy with the e.altitude !== null guard given Leaflet's non-nullable type declaration. Everything else looks solid.

…time behavior

Leaflet types altitude as number but it is null at runtime when the
device does not report elevation. The cast makes the null-guard
intentional and explicit rather than a no-op from TypeScript's view.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jasoneplumb jasoneplumb added review-requested Triggers Claude code review and removed review-requested Triggers Claude code review labels Apr 11, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 11, 2026

Code Review

The previous review flagged the Leaflet type mismatch on e.altitude (typed number, null at runtime). This iteration already addresses that with the explicit cast — good follow-through.


What looks good

Previous concern resolved. (e.altitude as number | null) !== null ? e.altitude : undefined makes the intent explicit and keeps the runtime guard legible. TypeScript won't complain about a redundant null-check and future readers understand why the cast is there.

GPX spec compliance confirmed. GPX 1.1 requires <ele> before <time> inside <trkpt>${eleTag}${timeTag}${speedTag} is correct.

altM?: number threads cleanly. Both trailPoints and trailSegments in types.ts are updated in lockstep, appendTrailPoint takes it as a trailing optional (no existing callers affected), and formatPoint destructures it correctly.


One informational note (no action required)

e.altitude from the Geolocation API is WGS84 ellipsoidal height — altitude above the mathematical ellipsoid, not above sea level (MSL). Consumer GPS devices often report ellipsoidal height, which can differ from MSL by 10–100 m depending on location. Strava and AllTrails both accept WGS84 altitude in GPX files without issue, so this won't break import; just worth documenting if users ever question elevation discrepancies vs. topo maps.


Summary

Clean, minimal PR. The scope is tight (10 additions, 6 deletions across exactly the right three files), the previous review concern is addressed, and the GPX output is spec-compliant. No blockers.

@jasoneplumb jasoneplumb merged commit d782f35 into mainline Apr 11, 2026
2 of 3 checks passed
@jasoneplumb jasoneplumb deleted the feature/trail-recording branch April 11, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request review-requested Triggers Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(recording): trail recording with pause/resume and GPX 1.1 export

1 participant