feat(search): destination-first search + mobile bottom sheet#68
Conversation
The single search box on open is now the DESTINATION ('Where to?'), matching
Google/Apple Maps. Picking a destination:
- routes straight from the user's shared location (origin auto-seeded), or
- reveals the origin box to fill in when location is unavailable.
- Invert the phase machine: search/destination/route -> dest/planning/route.
'dest' shows the single destination box; 'planning' slides the origin in
above it; 'route' unchanged.
- Auto-seed origin from userLocation (one-shot, yields to manual edits and
deep-link routes).
- Thread userLocation from HomeClient into SearchPanel.
- New i18n keys search.origin + search.whereTo across all 16 locales.
- Update + extend tests (destination-first single box, route-from-location,
origin-reveal fallback). 536 tests pass.
Verified: SSR renders the 'Where to?' box (es: '¿A dónde vas?') with the origin
box collapsed; tsc, lint, build all clean.
On phones (<=640px) the route alternatives and station list now ride in a draggable bottom sheet over a full-height map, instead of stacking in a tall column that pushed the first station off-screen (reported issue). - New BottomSheet: drag-to-snap between peek/half/full; tap the handle to cycle. Pointer events (touch + mouse), summary strip shows distance/duration + count. - New useMediaQuery hook (useSyncExternalStore, SSR-safe, desktop baseline). - SearchPanel: extract route/station JSX into shared routeContent; desktop keeps the side-card layout, mobile renders routeContent inside the sheet. Collapse toggle is desktop-only (the sheet handle replaces it on mobile). Selecting a station lowers the sheet to peek so the map shows. - Tests: sheet renders (role=dialog) on mobile viewport, never on desktop. 538 pass. Desktop layout is unchanged (isMobile=false in jsdom/SSR). Verified: SSR 200, no errors; tsc, lint, build all clean.
|
Warning Review limit reached
More reviews will be available in 26 minutes and 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough
ChangesDestination-first search flow with mobile bottom sheet
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/search/search-panel.tsx (1)
155-196:⚠️ Potential issue | 🟠 MajorRemove
calculateRoutefrom the eslint-disable and add it to the dependency list.The dependency array at line 185 omits
calculateRoutewith a comment claiming it's stable, but this creates a stale closure risk. The callback captures an outdatedcalculateRoutereference ifonRoute(its only dependency) changes. IncludingcalculateRoutein the deps ensures the callback reflects the latest routing logic.- }, [t, onFlyTo, destination, waypoints]); + }, [t, onFlyTo, destination, waypoints, calculateRoute]);Remove the eslint-disable comment on line 184 — the dependency omission is not justified.
🤖 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 `@src/components/search/search-panel.tsx` around lines 155 - 196, The handleLocationSelect useCallback has an eslint-disable comment on line 184 that justifies omitting calculateRoute from the dependency array, but this creates a stale closure risk. Remove the eslint-disable comment entirely and add calculateRoute to the dependency array of handleLocationSelect so the callback always references the current version of calculateRoute, which depends on onRoute changes. This ensures the routing logic is never stale when handleLocationSelect calls calculateRoute.Source: Linters/SAST tools
🤖 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 `@src/components/search/bottom-sheet.tsx`:
- Around line 127-129: The aria-label attribute in the bottom-sheet.tsx
component contains a hardcoded user-facing string "Route and stations" instead
of using localization. To fix this, import and use the useI18n hook to get the t
translator function, then replace the hardcoded string in the aria-label with a
call to t() passing an appropriate translation key for the route and stations
label. This ensures the accessibility label is properly localized for all
supported languages.
In `@src/components/search/search-panel.test.tsx`:
- Around line 96-110: The test assertion in "reveals the origin box (no route
yet) when location is unavailable" uses toBeInTheDocument() to verify the origin
input is revealed, but this only checks if the element exists in the DOM, not if
it's actually visible. The origin input may stay mounted but hidden via CSS,
allowing the test to false-pass. Replace the toBeInTheDocument() matcher with
toBeVisible() when asserting that screen.getByPlaceholderText("search.origin")
is present after destination selection to properly verify the UI reveal behavior
and prevent regressions in the destination-first fallback experience.
In `@src/components/search/search-panel.tsx`:
- Around line 178-181: The cleanup callback around lines 178-181 that clears
originText and origin on geolocation failure is leaving the active route state
inconsistent with the UI. Instead of only clearing setOriginText and
setOrigin(null), also clear or reset the active route state in the same callback
to maintain consistency. Either preserve the existing origin while clearing the
route state, or clear both the origin and route to reset the component to its
planning state.
---
Outside diff comments:
In `@src/components/search/search-panel.tsx`:
- Around line 155-196: The handleLocationSelect useCallback has an
eslint-disable comment on line 184 that justifies omitting calculateRoute from
the dependency array, but this creates a stale closure risk. Remove the
eslint-disable comment entirely and add calculateRoute to the dependency array
of handleLocationSelect so the callback always references the current version of
calculateRoute, which depends on onRoute changes. This ensures the routing logic
is never stale when handleLocationSelect calls calculateRoute.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4091c097-2755-4190-a25c-483cba2be861
📒 Files selected for processing (6)
src/components/home-client.tsxsrc/components/search/bottom-sheet.tsxsrc/components/search/search-panel.test.tsxsrc/components/search/search-panel.tsxsrc/lib/i18n.tsxsrc/lib/use-media-query.ts
Multi-reviewer panel (+CodeRabbit) findings: - CRITICAL: remove `touch-none` from the bottom-sheet container — it blocked touch-scrolling the station list (the bug this PR set out to fix). touch-none now lives only on the drag handle; body gets touch-pan-y. - HIGH: 'My location' geolocation failure now shows a transient geo.denied toast and focuses the origin box, instead of silently blanking origin / orphaning a route. Never wipes a manual origin or active route. - HIGH: bottom-sheet drag handle is now a real <button> — keyboard operable (↑/↓/Enter/Space), aria-expanded, focusable. role=dialog → role=region (map stays interactive; no false modal semantics). aria-label localized via new sheet.routeAndStations / sheet.resize keys across 16 locales. - MED: collapsed leak — routeCollapsed = collapsed && !isMobile, so collapsing on desktop then resizing to mobile no longer yields an empty sheet. - MED: drag uses a ref for live translate + snapshotted clamp basis (fixes stale-flick snap + mid-drag resize); vh===0 guard + lazy init (no dead drag / first-paint flash); pointercancel restores instead of cycling. - Simplify: inline peekHeight constant (was an unused prop), drop dead sheetRef. - Tests: new bottom-sheet.test.tsx (snap math, tap-cycle, drag, keyboard, pointercancel, aria); fixed false-pass origin-reveal assertion; added geo-failure + auto-seed-guard tests. 548 pass. tsc/lint/build clean; SSR smoke 200.
1 participant
Two UX improvements to the route planner, both requested from mobile use.
1. Destination-first single box
The single search box on open is now the destination ("Where to?" / "¿A dónde vas?"), matching Google/Apple Maps. Picking a destination:
Implementation:
search/destination/route→dest/planning/route.destshows the single destination box;planningslides the origin in above it;routeunchanged.userLocation(one-shot; yields to manual edits and deep-link routes).userLocationfromHomeClientintoSearchPanel.search.origin+search.whereToacross all 16 locales (correct accents).2. Mobile bottom sheet
On phones (≤640px) the route alternatives + station list now ride in a draggable bottom sheet over a full-height map — fixing the reported bug where the controls (sort chips, basis toggle, two sliders) pushed the first station off-screen.
BottomSheet: drag-to-snap between peek / half / full; tap the handle to cycle. Pointer events (touch + mouse). Summary strip shows distance · duration · station count. The station list scrolls inside the sheet.useMediaQueryhook (useSyncExternalStore, SSR-safe, defaults to desktop).SearchPanel: route/station JSX extracted into a sharedrouteContent; desktop keeps the side-card layout, mobile renders it in the sheet. Collapse toggle is desktop-only (the sheet handle replaces it). Selecting a station drops the sheet to peek so the map shows.Desktop layout is unchanged —
isMobileisfalseunder SSR and jsdom, so the desktop path renders byte-for-byte as before.Verification
tsc --noEmit— passnpm run lint— 0 errors (73 pre-existing warnings)npm test— 538/538 (new: destination-first single box, route-from-location, origin-reveal fallback, sheet-renders-on-mobile, no-sheet-on-desktop)npm run build— compiled, 31/31 static pages/en+/esreturn 200, no React errors, correct placeholders (Where to?/¿A dónde vas?), accents intactSummary by CodeRabbit
New Features
Tests
Chores