Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds MapLibre dependency and Expo plugin, a modal "search-location" route and button, a new SearchLocationScreen with debounced typeahead and MapLibre preview, and a dummy-activity fallback for activity detail loading. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Router
participant SearchLocationScreen
participant SearchPlacesAPI
participant PlaceDetailsAPI
participant MapLibre as MapLibre(Map)
User->>Router: Open /trips/:id/search-location (modal)
Router->>SearchLocationScreen: Render screen
User->>SearchLocationScreen: Type query
SearchLocationScreen->>SearchLocationScreen: Debounce (300ms)
SearchLocationScreen->>SearchPlacesAPI: searchPlacesTypeahead(query)
SearchPlacesAPI-->>SearchLocationScreen: predictions[]
SearchLocationScreen->>User: Show predictions list
User->>SearchLocationScreen: Select prediction
SearchLocationScreen->>SearchLocationScreen: set isSelecting, clear predictions
SearchLocationScreen->>PlaceDetailsAPI: getPlaceDetailsCustom(place_id)
PlaceDetailsAPI-->>SearchLocationScreen: place geometry & details
SearchLocationScreen->>MapLibre: Render map, flyTo(location), add marker
MapLibre-->>User: Display map preview with marker
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
anish-sahoo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/app/(app)/trips/[id]/activities/[id]/index.tsx (2)
59-73:⚠️ Potential issue | 🟡 MinorUse dedicated LoadingState and ErrorState components.
The inline loading and error handling with plain
Textcomponents does not follow the guideline requiringLoadingState,ErrorState, orEmptyStateUI components. Replacing these with proper state components ensures consistent UX across the app and centralizes styling.Suggested approach
- {isLoading && ( - <Box padding="lg"> - <Text variant="bodyDefault" color="textSubtle"> - Loading... - </Text> - </Box> - )} - - {isError && ( - <Box padding="lg"> - <Text variant="bodyDefault" color="textSubtle"> - Failed to load activity. - </Text> - </Box> - )} + {isLoading && <LoadingState />} + {isError && <ErrorState message="Failed to load activity." />}Import
LoadingStateandErrorStatefrom the design system if available.As per coding guidelines: "React/Expo UI error handling must not expose raw API errors; use UI states (LoadingState, ErrorState, EmptyState)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/[id]/index.tsx around lines 59 - 73, Replace the inline loading/error JSX that uses Box and Text guarded by isLoading/isError with the designated UI state components: return or render <LoadingState /> when isLoading is true and <ErrorState /> when isError is true (instead of the manual Text message). Add imports for LoadingState and ErrorState at the top of the file, remove the inline messages inside the isLoading/isError blocks, and ensure you do not surface raw API errors (keep ErrorState generic). Update any surrounding markup so the new components match existing layout where the previous Box wrapper was used.
110-114: 🧹 Nitpick | 🔵 TrivialAvoid hardcoded pixel values for image dimensions.
The image style uses magic numbers (
width: 120,height: 120). Extract these to the design system tokens or define named constants to maintain consistency and ease future adjustments.As per coding guidelines: "avoid hard-coded color values, magic spacing numbers, and duplicated style definitions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/[id]/index.tsx around lines 110 - 114, The inline style block uses hardcoded pixels (width: 120, height: 120); replace these magic numbers with a named constant or design token (e.g., const ACTIVITY_IMAGE_SIZE = 120 or import a token like tokens.size.activityImage) and use that constant in the style object (alongside CornerRadius.sm) so dimensions are centralized and reusable; update any other occurrences to use the same constant and export it if needed from the design tokens or the component module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`(app)/trips/[id]/search-location.tsx:
- Around line 71-79: The fetchPredictions function currently swallows errors
from searchPlacesTypeahead; update it to catch errors,
setIsLoadingPredictions(false) in finally (keep), and surface a user-friendly
error to the UI (e.g., call a toast/notification or set an error state) so users
know the search failed; specifically wrap the await searchPlacesTypeahead(q,
TYPEAHEAD_LIMIT) in try/catch inside fetchPredictions, call setPredictions([])
or leave previous predictions as appropriate on error, and invoke your app's
notification helper (or set a local error state used by the component) with a
clear message; keep the finally block that calls setIsLoadingPredictions(false)
unchanged.
- Around line 81-94: The handler handleSelectPrediction currently sets
isSelectingPlace true but never resets it if getPlaceDetailsCustom throws,
leaving the component stuck; modify handleSelectPrediction to ensure
setIsSelectingPlace(false) is called in the finally block alongside
setIsLoadingDetails(false) (or in both catch and finally) so isSelectingPlace is
always reset after the request completes or fails, and keep setting
setSelectedLocation(res.data) only on success; reference handleSelectPrediction,
getPlaceDetailsCustom, isSelectingPlace, setIsSelectingPlace,
setIsLoadingDetails, and setSelectedLocation when making this change.
---
Outside diff comments:
In `@frontend/app/`(app)/trips/[id]/activities/[id]/index.tsx:
- Around line 59-73: Replace the inline loading/error JSX that uses Box and Text
guarded by isLoading/isError with the designated UI state components: return or
render <LoadingState /> when isLoading is true and <ErrorState /> when isError
is true (instead of the manual Text message). Add imports for LoadingState and
ErrorState at the top of the file, remove the inline messages inside the
isLoading/isError blocks, and ensure you do not surface raw API errors (keep
ErrorState generic). Update any surrounding markup so the new components match
existing layout where the previous Box wrapper was used.
- Around line 110-114: The inline style block uses hardcoded pixels (width: 120,
height: 120); replace these magic numbers with a named constant or design token
(e.g., const ACTIVITY_IMAGE_SIZE = 120 or import a token like
tokens.size.activityImage) and use that constant in the style object (alongside
CornerRadius.sm) so dimensions are centralized and reusable; update any other
occurrences to use the same constant and export it if needed from the design
tokens or the component module.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b12aea7b-b1c9-4f08-851e-de2d67502559
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
backend/internal/controllers/comment_reactions.gobackend/internal/models/comment_reactions.gobackend/internal/repository/comment_reactions.gobackend/internal/services/comment_reactions.gobackend/internal/tests/activity_test.gobackend/internal/tests/comment_reactions_test.gobackend/internal/validators/time_of_day.gofrontend/app.jsonfrontend/app/(app)/trips/[id]/_layout.tsxfrontend/app/(app)/trips/[id]/activities/[id]/index.tsxfrontend/app/(app)/trips/[id]/index.tsxfrontend/app/(app)/trips/[id]/search-location.tsxfrontend/package.json
💤 Files with no reviewable changes (5)
- backend/internal/controllers/comment_reactions.go
- backend/internal/services/comment_reactions.go
- backend/internal/validators/time_of_day.go
- backend/internal/repository/comment_reactions.go
- backend/internal/tests/comment_reactions_test.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`(app)/trips/[id]/search-location.tsx:
- Around line 320-340: The pin, pinDot and predictionItem styles currently use
hard-coded magic numbers (width: 24, height: 24, borderRadius: 12, pinDot
width/height 6 borderRadius 3, and predictionItem minHeight: 48); replace these
literals with design-system tokens (e.g., Spacing.SM/MD, Radius.SM/MD, Size.SM,
etc.) and/or centralized constants so sizes are consistent and not duplicated,
updating the style objects named pin, pinDot and predictionItem to reference
those tokens (keep ColorPalette usage as-is) and remove duplicated numeric
values across the file.
- Around line 91-94: The catch block in the getPlaceDetailsCustom call currently
only logs the error and resets setIsSelectingPlace, leaving the user without any
feedback; update the error handling in the catch to surface a user-friendly
message (e.g., call the app's toast helper or set a local error state like
setPlaceError) with a clear, non-technical message such as "Unable to load place
details. Please try again." and ensure the UI for this component (where place
results are rendered) reads that state or the toast so the user sees the error;
reference the getPlaceDetailsCustom call, the existing catch block, and
setIsSelectingPlace when making the change.
- Around line 51-98: SearchLocationScreen currently lets users pick a place
(handleSelectPrediction sets selectedLocation) but never returns it to the
parent; add a confirmation action (e.g., a Confirm/Done button shown when
selectedLocation is set and map view is active) that calls router.setParams({
selectedLocation: <serializedLocation> }) or navigates back with the location in
route params, then clears isSelectingPlace and selectedLocation as needed;
update the UI render to show the button when selectedLocation !== null and wire
its onPress to serialize the LocationDetails and pass it via router.setParams or
router.push to the trip details route so the parent receives the selection.
- Around line 87-90: The code assigns an untyped API response from
getPlaceDetailsCustom to selectedLocation (typed LocationDetails) via
setSelectedLocation, which risks runtime failures in LocationMapView when fields
change; update the API layer to return a typed response (e.g.,
request<LocationDetails>) or perform runtime validation before calling
setSelectedLocation—use a schema validator (zod/io-ts) or explicit checks for
geometry.location.lng/lat and required fields, then map/normalize the validated
payload to the LocationDetails shape before setting state.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 840d68bc-3600-49c7-aa30-f8a46041cbea
📒 Files selected for processing (1)
frontend/app/(app)/trips/[id]/search-location.tsx
in-mai-space
approved these changes
Apr 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
How has this been tested?
Checklist
User-visible changes
Changes by category
Features
Infra
expo start --iostoexpo run:ios(requires CocoaPods and new Expo image)Improvements
Author contribution