feat(location): custom pin with Maps URL parsing + restore on trail stop#54
feat(location): custom pin with Maps URL parsing + restore on trail stop#54jiunshinn wants to merge 3 commits into
Conversation
Adds a pure parser used by the location pin UI. Accepts raw `lat,lng`, full Google/Apple Maps long-form URLs, and short links (maps.app.goo.gl, goo.gl, maps.apple.com) — short links return a `redirect` result so the caller can follow the redirect server-side via curl (browsers can't read cross-origin redirect targets). Also exports `buildRedirectResolveCommand` so the caller doesn't have to hand-craft the curl invocation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `CustomLocationPin` to LocationEmulationTool — a paste-and-go input
that accepts raw lat,lng or any Google/Apple Maps URL. Short links are
resolved server-side via the existing /exec channel.
Fixes a session-origin bug introduced when the pin feature is in play:
on `Play`, the old code captured the *trail's* first point as the
"return-here-on-stop" location. So pinning Tokyo then playing the Apple
Park trail and pressing Stop would land you in Cupertino, not back at
Tokyo. The simulator's only off-trail location source is the user's
manual pin, so that's what we capture and restore now.
Implementation:
- New `location-session.ts` with `pickSessionOrigin` and
`buildEndSessionCommand` pure helpers (tested in isolation).
- `LocationEmulationTool` owns a `lastPinnedRef`; `CustomLocationPin`
calls back via `onPinned` so the parent learns about pin events.
- On play, capture from `lastPinnedRef` (not the trail point). On a
new pin, clear the captured origin so next play re-captures.
- Stop handler and unmount cleanup share `buildEndSessionCommand`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a CustomLocationPin UI to LocationEmulationTool, a parser for raw coordinates and Maps URLs with redirect resolution, session-origin helpers to pick/restore simulator location, and integrates these to pause playback when pinning and restore/clear location on stop. ChangesCustom Location Pin Feature
Sequence DiagramsequenceDiagram
participant User
participant CustomLocationPin
participant LocationEmulationTool
participant parseLocationInput
participant buildRedirectResolveCommand
participant buildEndSessionCommand
participant Simulator
User->>CustomLocationPin: Enter lat,lng or Maps URL
CustomLocationPin->>parseLocationInput: Parse input
alt Direct coordinates
parseLocationInput-->>CustomLocationPin: {kind: "coords", lat, lng}
CustomLocationPin->>Simulator: xcrun simctl location set
else Redirect needed
parseLocationInput-->>CustomLocationPin: {kind: "redirect", url}
CustomLocationPin->>buildRedirectResolveCommand: Resolve short link
buildRedirectResolveCommand-->>CustomLocationPin: Final URL
CustomLocationPin->>parseLocationInput: Re-parse final URL
parseLocationInput-->>CustomLocationPin: {kind: "coords", lat, lng}
CustomLocationPin->>Simulator: xcrun simctl location set
end
CustomLocationPin->>LocationEmulationTool: onBeforePin (pause playback)
CustomLocationPin->>LocationEmulationTool: onPinned (save to lastPinnedRef)
User->>LocationEmulationTool: Stop playback
LocationEmulationTool->>buildEndSessionCommand: Generate restore command
buildEndSessionCommand-->>Simulator: xcrun simctl location set/clear
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/serve-sim/src/client/location-session.ts (1)
21-25: 💤 Low valueConsider whether this wrapper function adds sufficient value.
The
pickSessionOriginfunction is trivially simple—it just returns its input. While the comments explain this is for testability and to document the bug fix, a direct reference tolastPinnedRef.currentat call sites might be clearer. That said, if the abstraction helps with testing and documentation, this is defensible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/serve-sim/src/client/location-session.ts` around lines 21 - 25, pickSessionOrigin is a trivial pass-through wrapper around the lastPinned value and adds little value; remove the unused abstraction by deleting the pickSessionOrigin function and update all call sites to reference lastPinnedRef.current (or the actual SimLocation variable) directly, and adjust any tests or mocks that referenced pickSessionOrigin to use the direct value or mock lastPinnedRef.current instead so behavior and testability remain intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/serve-sim/src/client/location-emulation-tool.tsx`:
- Around line 447-454: The onPinned handler currently sets lastPinnedRef.current
but clears sessionOriginRef.current, causing buildEndSessionCommand to receive
null and clear the simulator location; change the handler in onPinned (the arrow
function that receives (lat, lng)) to update sessionOriginRef.current = { lat,
lng } instead of setting it to null (so Stop restores the last manual pin),
leaving lastPinnedRef.current assignment intact; alternatively, if you prefer to
end the trail immediately, set the status to 'idle' there (update the same
onPinned handler) so the session ends when a user pins.
---
Nitpick comments:
In `@packages/serve-sim/src/client/location-session.ts`:
- Around line 21-25: pickSessionOrigin is a trivial pass-through wrapper around
the lastPinned value and adds little value; remove the unused abstraction by
deleting the pickSessionOrigin function and update all call sites to reference
lastPinnedRef.current (or the actual SimLocation variable) directly, and adjust
any tests or mocks that referenced pickSessionOrigin to use the direct value or
mock lastPinnedRef.current instead so behavior and testability remain intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a653a4eb-7d92-488b-a078-b62f5a4e43d4
📒 Files selected for processing (5)
packages/serve-sim/src/__tests__/location-input-parser.test.tspackages/serve-sim/src/__tests__/location-session.test.tspackages/serve-sim/src/client/location-emulation-tool.tsxpackages/serve-sim/src/client/location-input-parser.tspackages/serve-sim/src/client/location-session.ts
If the user pinned a location while a trail was playing or paused, onPinned cleared the captured session origin. A subsequent Stop then ran `simctl location clear` and wiped the pin the user had just set. Pinning is itself the user setting a "where I am" — make that pin the session origin so Stop restores to it. Caught by CodeRabbit review on EvanBacon#54. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review @coderabbitai — pushed Actionable comment (fixed): you're right, the Nit on |
|
Tip For best results, initiate chat on the files or code changes.
The fix for the Totally on board with keeping ✏️ Learnings added
|
1 participant
Summary
Bug repro (against `main` before this PR)
The bug only manifests with a pin set; no-pin users get the same `clear` behaviour as before.
Implementation
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests