Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ActivityCard UI and sheet-driven activity creation flow (entry autofill + manual creation), a categories selector and form-row helper, refactors Activities screen to use a FlatList and imperative sheets, and makes small search-location typing/cleanup tweaks. Changes
Sequence DiagramsequenceDiagram
participant User
participant Activities as ActivitiesScreen
participant Entry as AddActivityEntrySheet
participant Manual as AddActivityManualSheet
participant Categories as CategoriesSheet
participant Search as SearchLocation
participant API
User->>Activities: Open ActivitiesScreen
Activities->>Activities: Render FlatList of ActivityCard
User->>Activities: Tap "Add an activity"
Activities->>Entry: open()
User->>Entry: Tap "Autofill from link"
Entry->>API: parseActivityLink(tripID, url)
API-->>Entry: parsedData
Entry->>Activities: onAutofilled(parsedData)
Activities->>Manual: open(prefill data)
alt choose categories
User->>Manual: Tap category row
Manual->>Categories: open()
User->>Categories: Select categories -> Done
Categories-->>Manual: selected categories
end
alt pick location
User->>Manual: Tap location row
Manual->>Search: navigate(returnTo)
Search->>Search: User selects location
Search->>Activities: set pending location & navigate back
Activities->>Manual: setLocation(prefill)
end
User->>Manual: Tap Save
Manual->>API: createActivity(payload)
API-->>Manual: createdActivity
Manual->>Activities: onSaved(createdActivity)
Activities->>Activities: Prepend activity to list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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)/index.tsx:
- Around line 37-38: The logout call from useUserStore((s) => s.logout) must be
routed through a logout helper that both signs the user out and
clears/invalidate react-query caches; create or use an existing helper (e.g.
logoutAndClearCache or handleLogout) that calls logout (from useUserStore), then
invokes the app's queryClient.clear(), queryClient.invalidateQueries(['trips']),
and queryClient.invalidateQueries(['activities']) (or queryClient.clear() if
appropriate), and replace direct calls to logout in the components (including
the occurrences around lines 103–111 and the existing const logout =
useUserStore((s) => s.logout)) to call this helper so cached trip/activity data
is removed on sign-out.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-entry-sheet.tsx:
- Around line 51-55: The sheet can be dismissed while parseActivityLink is still
running, allowing its late resolution to call onAutofilled and reopen the manual
flow; update the flow around parseActivityLink/handleClose to prevent this by
adding a request token or cancel flag (e.g., a local incrementing requestId or
AbortController) that is set when starting parseActivityLink and checked before
calling onAutofilled, and ensure handleClose sets the cancel flag (or increments
a token) and clears state via setVisible, setUrl, onClose so any in-flight
response is ignored; alternatively disable backdrop/close while an autofill
promise is pending by tracking an isAutofilling boolean and preventing dismissal
in handleClose and the backdrop handler until it is false (apply same guard
where parseActivityLink resolves and where onAutofilled is invoked).
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx:
- Around line 85-92: The discard check currently only looks at name, description
and thumbnailUri; expand it to consider all editable state used in this
component—categories, locationName, locationLat, locationLng, price,
dateRange.start/dateRange.end, link (in addition to name, description,
thumbnailUri) —create a single hasData/hasUnsavedChanges predicate that inspects
categories.length, non-null locationName/Lat/Lng, non-empty price, non-null
dateRange.start or end, non-empty link, or non-empty
name/description/thumbnailUri, and use that predicate wherever the current
discard/close confirmation is performed (replace the existing
name/description/thumbnailUri-only check in the close handler and any related
functions) so the sheet prompts before losing any edited fields.
- Around line 252-321: The form blocks use raw TextInput rows and ad-hoc Box
layouts (see name/description TextInput, price/link inline TextInput, and the
category chips) which duplicates styling/behavior; refactor these to use the
app's shared field primitives (e.g., replace the plain TextInput usages in
add-activity-manual-sheet.tsx with the centralized FormField/FormRow or
ControlledInput components, wire their value/onChange to the existing state
setters like setName, setDescription, setPrice, setLink, and reuse the shared
spacing/colors instead of
styles.nameInput/styles.descriptionInput/styles.inlineInput), ensure the
location and dates keep using FormRow (keep handleLocationPress and
setIsDatePickerVisible) and remove duplicated inline Box styling by composing
primitives so validation/theming/interaction stay consistent across the app.
- Around line 60-67: formatDateRange and the payload creation are inconsistent
and use toISOString which shifts local calendar dates to UTC; ensure when
building the payload you default end to start for single-day selections (so a
DateRange with only start is persisted) and replace any toISOString() usage with
a local-date-only serialization that extracts year/month/day from the Date
(e.g., build "YYYY-MM-DD" from date.getFullYear(), date.getMonth()+1,
date.getDate()) so timezone offsets don't change the calendar day; update the
code paths that construct the API payload (the payload creation logic near where
start/end are read) and keep formatDateRange unchanged for display.
In `@frontend/app/`(app)/trips/[id]/activities/components/categories-sheet.tsx:
- Around line 88-102: The "Create amenity" chip is rendered as a Pressable with
no onPress handler (Pressable > Box ... Text "Create amenity"), producing a dead
tappable control; either remove the entire Pressable/Box block or wire it to the
real category-creation flow by adding an onPress that calls the existing
create/open handler (e.g., openCreateCategoryModal or createCategory function in
this component or parent) and keep styling via styles.chip, Plus and Text as-is
so the UI remains consistent; if the feature isn't ready, prefer removing or
rendering it disabled/non-interactive instead of leaving an empty Pressable.
- Around line 20-21: The categories list derived from
useGetCategoriesByTripID(tripID) may include items with undefined
ModelsCategoryAPIResponse.name which are currently used as React keys and passed
into toggle(), causing unstable keys and invalid selection entries; before
rendering (where categories is used and passed to toggle()), filter or map the
fetched data to only include categories with a defined non-empty name (or
normalize missing names to a stable placeholder id), e.g., produce a
sanitizedCategories variable from data?.categories that excludes entries lacking
name (or assigns a deterministic fallback like `category-<id>`), then use
sanitizedCategories everywhere React keys and toggle() are called so keys and
selection values remain stable.
🪄 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: 28342679-3ad7-4cf5-8313-33f595851460
⛔ Files ignored due to path filters (1)
frontend/assets/images/binoculars.pngis excluded by!**/*.png
📒 Files selected for processing (8)
frontend/app/(app)/index.tsxfrontend/app/(app)/trips/[id]/activities/components/activity-card.tsxfrontend/app/(app)/trips/[id]/activities/components/add-activity-entry-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/categories-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/form-row.tsxfrontend/app/(app)/trips/[id]/activities/index.tsxfrontend/app/(app)/trips/[id]/search-location.tsx
Comment on lines
+51
to
+55
| const handleClose = () => { | ||
| setVisible(false); | ||
| setUrl(""); | ||
| onClose(); | ||
| }; |
Contributor
There was a problem hiding this comment.
Do not apply autofill results after the sheet is dismissed.
Backdrop close stays active while parseActivityLink is in flight. If the user dismisses the sheet, the pending request can still resolve and call onAutofilled, reopening the manual flow after the user canceled. Disable dismissal during autofill or ignore late responses with a request token/cancel flag.
Also applies to: 63-75, 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-entry-sheet.tsx
around lines 51 - 55, The sheet can be dismissed while parseActivityLink is
still running, allowing its late resolution to call onAutofilled and reopen the
manual flow; update the flow around parseActivityLink/handleClose to prevent
this by adding a request token or cancel flag (e.g., a local incrementing
requestId or AbortController) that is set when starting parseActivityLink and
checked before calling onAutofilled, and ensure handleClose sets the cancel flag
(or increments a token) and clears state via setVisible, setUrl, onClose so any
in-flight response is ignored; alternatively disable backdrop/close while an
autofill promise is pending by tracking an isAutofilling boolean and preventing
dismissal in handleClose and the backdrop handler until it is false (apply same
guard where parseActivityLink resolves and where onAutofilled is invoked).
Comment on lines
+60
to
+67
| function formatDateRange(range: DateRange): string | null { | ||
| if (!range.start) return null; | ||
| const fmt = (d: Date) => | ||
| d.toLocaleDateString("en-US", { month: "short", day: "numeric" }); | ||
| if (!range.end || range.start.getTime() === range.end.getTime()) | ||
| return fmt(range.start); | ||
| return `${fmt(range.start)} – ${fmt(range.end)}`; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the target file
fd -t f "add-activity-manual-sheet.tsx"Repository: GenerateNU/toggo
Length of output: 141
🏁 Script executed:
# Read the file with line numbers, focusing on the critical sections
cat -n frontend/app/\(app\)/trips/\[id\]/activities/components/add-activity-manual-sheet.tsx | head -170 | tail -120Repository: GenerateNU/toggo
Length of output: 5169
🏁 Script executed:
# Also check the type definitions for DateRange
rg "type DateRange|interface DateRange" -A 5Repository: GenerateNU/toggo
Length of output: 1219
🏁 Script executed:
# Check the date picker implementation
cat -n frontend/design-system/primitives/date-picker.tsx | head -100Repository: GenerateNU/toggo
Length of output: 3460
🏁 Script executed:
# Look for any timezone handling utilities in the codebase
rg "timezone|getTime|UTC" --type ts --type tsx -B 2 -A 2Repository: GenerateNU/toggo
Length of output: 87
🏁 Script executed:
# Check if there are any other date serialization patterns in the codebase
rg "toISOString" --type ts --type tsx -B 3 -A 3Repository: GenerateNU/toggo
Length of output: 87
🏁 Script executed:
# Search for toISOString usage in the codebase
rg "toISOString" -B 3 -A 3Repository: GenerateNU/toggo
Length of output: 2799
🏁 Script executed:
# Check the backend date expectations more clearly
cat -n backend/internal/models/activity.go | grep -A 10 "DateRange"Repository: GenerateNU/toggo
Length of output: 5335
🏁 Script executed:
# Look for how dates are created/used elsewhere in the frontend
rg "new Date" frontend/design-system/primitives/date-picker.tsx -B 2 -A 2Repository: GenerateNU/toggo
Length of output: 1144
Fix timezone-dependent date loss and single-day selection handling.
The code has two related issues:
-
Single-day selections won't persist:
formatDateRangeaccepts a DateRange with onlystart(line 65), but the payload creation requires bothstartandend(line 152). Users who select a single date will see it displayed but it won't be saved to the API. -
toISOString()corrupts calendar dates in non-UTC timezones: This method converts to UTC before extracting the date string. In timezones east of UTC, midnight local time becomes the previous day in UTC. For example, Jan 1 00:00:00 SGT (UTC+8) converts to Dec 31 16:00:00 UTC, losing the intended calendar day.
Use local date component extraction instead of ISO string conversion, and default end to start for single-day selections:
Suggested payload normalization
+ const formatLocalDate = (d: Date) =>
+ `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, "0")}-${String(d.getDate()).padStart(2, "0")}`;
+
const dates =
- dateRange.start && dateRange.end
+ dateRange.start
? [
{
- start: dateRange.start.toISOString().split("T")[0]!,
- end: dateRange.end.toISOString().split("T")[0]!,
+ start: formatLocalDate(dateRange.start),
+ end: formatLocalDate(dateRange.end ?? dateRange.start),
},
]
: undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
around lines 60 - 67, formatDateRange and the payload creation are inconsistent
and use toISOString which shifts local calendar dates to UTC; ensure when
building the payload you default end to start for single-day selections (so a
DateRange with only start is persisted) and replace any toISOString() usage with
a local-date-only serialization that extracts year/month/day from the Date
(e.g., build "YYYY-MM-DD" from date.getFullYear(), date.getMonth()+1,
date.getDate()) so timezone offsets don't change the calendar day; update the
code paths that construct the API payload (the payload creation logic near where
start/end are read) and keep formatDateRange unchanged for display.
Comment on lines
+85
to
+92
| const [categories, setCategories] = useState<string[]>([]); | ||
| const [locationName, setLocationName] = useState<string | null>(null); | ||
| const [locationLat, setLocationLat] = useState<number | null>(null); | ||
| const [locationLng, setLocationLng] = useState<number | null>(null); | ||
| const [price, setPrice] = useState(""); | ||
| const [dateRange, setDateRange] = useState<DateRange>({ start: null, end: null }); | ||
| const [link, setLink] = useState(""); | ||
| const [isDatePickerVisible, setIsDatePickerVisible] = useState(false); |
Contributor
There was a problem hiding this comment.
Track all edited fields before discarding the draft.
Lines 191-193 only treat name, description, and thumbnailUri as unsaved input. If the user changes categories, location, price, dates, or link, the sheet closes without confirmation and drops that work.
Suggested `hasData` check
- const hasData = name.trim() || description.trim() || thumbnailUri;
+ const hasData =
+ name.trim() ||
+ description.trim() ||
+ thumbnailUri ||
+ categories.length > 0 ||
+ locationName ||
+ locationLat != null ||
+ locationLng != null ||
+ price.trim() ||
+ dateRange.start ||
+ dateRange.end ||
+ link.trim();Also applies to: 191-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
around lines 85 - 92, The discard check currently only looks at name,
description and thumbnailUri; expand it to consider all editable state used in
this component—categories, locationName, locationLat, locationLng, price,
dateRange.start/dateRange.end, link (in addition to name, description,
thumbnailUri) —create a single hasData/hasUnsavedChanges predicate that inspects
categories.length, non-null locationName/Lat/Lng, non-empty price, non-null
dateRange.start or end, non-empty link, or non-empty
name/description/thumbnailUri, and use that predicate wherever the current
discard/close confirmation is performed (replace the existing
name/description/thumbnailUri-only check in the close handler and any related
functions) so the sheet prompts before losing any edited fields.
Comment on lines
+252
to
+321
| <Box paddingHorizontal="sm" gap="xxs"> | ||
| <TextInput | ||
| value={name} | ||
| onChangeText={setName} | ||
| placeholder="New Activity" | ||
| placeholderTextColor={ColorPalette.gray400} | ||
| style={styles.nameInput} | ||
| /> | ||
| <TextInput | ||
| value={description} | ||
| onChangeText={setDescription} | ||
| placeholder="Add a description" | ||
| placeholderTextColor={ColorPalette.gray400} | ||
| multiline | ||
| style={styles.descriptionInput} | ||
| /> | ||
| </Box> | ||
|
|
||
| <Box paddingHorizontal="sm"> | ||
| <Box flexDirection="row" flexWrap="wrap" gap="xs" alignItems="center"> | ||
| {categories.map((cat) => ( | ||
| <Box key={cat} paddingHorizontal="sm" paddingVertical="xxs" style={styles.categoryChip}> | ||
| <Text variant="bodyXsMedium" color="gray700">{cat}</Text> | ||
| </Box> | ||
| ))} | ||
| <Pressable onPress={() => categoriesSheetRef.current?.snapToIndex(0)}> | ||
| <Box flexDirection="row" alignItems="center" gap="xxs" paddingHorizontal="sm" paddingVertical="xxs" style={styles.addCategoryChip}> | ||
| <Plus size={12} color={ColorPalette.gray500} /> | ||
| <Text variant="bodyXsMedium" color="gray500">Add category</Text> | ||
| </Box> | ||
| </Pressable> | ||
| </Box> | ||
| </Box> | ||
|
|
||
| <Box paddingHorizontal="sm" gap="xxs" borderTopWidth={1} borderTopColor="gray100" paddingTop="sm"> | ||
| <FormRow | ||
| icon={MapPin} | ||
| value={locationName ?? undefined} | ||
| placeholder="Location" | ||
| onPress={handleLocationPress} | ||
| /> | ||
| <Box flexDirection="row" alignItems="center" gap="sm" paddingVertical="xs"> | ||
| <DollarSign size={16} color={price ? ColorPalette.gray700 : ColorPalette.gray400} /> | ||
| <TextInput | ||
| value={price} | ||
| onChangeText={setPrice} | ||
| placeholder="Price" | ||
| placeholderTextColor={ColorPalette.gray400} | ||
| keyboardType="decimal-pad" | ||
| style={styles.inlineInput} | ||
| /> | ||
| </Box> | ||
| <FormRow | ||
| icon={Calendar} | ||
| value={formatDateRange(dateRange) ?? undefined} | ||
| placeholder="Dates" | ||
| onPress={() => setIsDatePickerVisible(true)} | ||
| /> | ||
| <Box flexDirection="row" alignItems="center" gap="sm" paddingVertical="xs"> | ||
| <Link size={16} color={link ? ColorPalette.gray700 : ColorPalette.gray400} /> | ||
| <TextInput | ||
| value={link} | ||
| onChangeText={setLink} | ||
| placeholder="Link" | ||
| placeholderTextColor={ColorPalette.gray400} | ||
| autoCapitalize="none" | ||
| keyboardType="url" | ||
| style={styles.inlineInput} | ||
| /> | ||
| </Box> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Build this form from shared field primitives instead of custom TextInput rows.
This new core flow duplicates field styling with raw TextInputs, inline layout objects, and custom row implementations instead of the shared form components. That will drift from shared validation, theming, and interaction behavior in a high-traffic screen.
As per coding guidelines "React/Expo UI must use a centralized design system; avoid hard-coded color values, magic spacing numbers, and duplicated style definitions" and "React/Expo components must be reusable, typed, composable, and modular; prefer primitive components (Box, Text, Stack) and build complex components from primitives (Button, Avatar, Card)".
Also applies to: 379-418
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
around lines 252 - 321, The form blocks use raw TextInput rows and ad-hoc Box
layouts (see name/description TextInput, price/link inline TextInput, and the
category chips) which duplicates styling/behavior; refactor these to use the
app's shared field primitives (e.g., replace the plain TextInput usages in
add-activity-manual-sheet.tsx with the centralized FormField/FormRow or
ControlledInput components, wire their value/onChange to the existing state
setters like setName, setDescription, setPrice, setLink, and reuse the shared
spacing/colors instead of
styles.nameInput/styles.descriptionInput/styles.inlineInput), ensure the
location and dates keep using FormRow (keep handleLocationPress and
setIsDatePickerVisible) and remove duplicated inline Box styling by composing
primitives so validation/theming/interaction stay consistent across the app.
Comment on lines
+20
to
+21
| const { data } = useGetCategoriesByTripID(tripID); | ||
| const categories = data?.categories ?? []; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "categories-sheet.tsx" | head -20Repository: GenerateNU/toggo
Length of output: 134
🏁 Script executed:
cd frontend && git ls-files | grep -E "(categories-sheet|ModelsCategoryAPIResponse|types)" | head -20Repository: GenerateNU/toggo
Length of output: 207
🏁 Script executed:
# Get the overall structure of the frontend directory
ls -la frontend/Repository: GenerateNU/toggo
Length of output: 2008
🏁 Script executed:
cat -n frontend/app/\(app\)/trips/\[id\]/activities/components/categories-sheet.tsxRepository: GenerateNU/toggo
Length of output: 4636
🏁 Script executed:
# Search for the ModelsCategoryAPIResponse type definition
rg "ModelsCategoryAPIResponse" frontend/types/ -A 10Repository: GenerateNU/toggo
Length of output: 2952
🏁 Script executed:
rg "export type ModelsCategoryAPIResponse" frontend/types/types.gen.ts -A 30Repository: GenerateNU/toggo
Length of output: 577
🏁 Script executed:
rg "useGetCategoriesByTripID" frontend/api/ -A 20Repository: GenerateNU/toggo
Length of output: 12389
🏁 Script executed:
# Also check the schema for the response type
rg "ModelsCategoryListResponse" frontend/types/types.gen.ts -B 2 -A 10Repository: GenerateNU/toggo
Length of output: 569
🏁 Script executed:
# Verify the actual query response type for GetCategoriesByTripID
cat -n frontend/api/categories/useGetCategoriesByTripID.ts | head -50Repository: GenerateNU/toggo
Length of output: 1865
🏁 Script executed:
rg "GetCategoriesByTripIDQueryResponse" frontend/types/types.gen.ts -B 2 -A 2Repository: GenerateNU/toggo
Length of output: 232
Ensure categories have a stable name before rendering.
ModelsCategoryAPIResponse.name is optional, but lines 64-67 use it as the React key and pass it to toggle(). A category without a name creates an unstable key and pushes undefined into the selection state, breaking toggle behavior. Filter out or normalize categories to guarantee each has a name before rendering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/components/categories-sheet.tsx
around lines 20 - 21, The categories list derived from
useGetCategoriesByTripID(tripID) may include items with undefined
ModelsCategoryAPIResponse.name which are currently used as React keys and passed
into toggle(), causing unstable keys and invalid selection entries; before
rendering (where categories is used and passed to toggle()), filter or map the
fetched data to only include categories with a defined non-empty name (or
normalize missing names to a stable placeholder id), e.g., produce a
sanitizedCategories variable from data?.categories that excludes entries lacking
name (or assigns a deterministic fallback like `category-<id>`), then use
sanitizedCategories everywhere React keys and toggle() are called so keys and
selection values remain stable.
Comment on lines
+88
to
+102
| <Pressable> | ||
| <Box | ||
| flexDirection="row" | ||
| alignItems="center" | ||
| gap="xxs" | ||
| paddingHorizontal="sm" | ||
| paddingVertical="xxs" | ||
| style={styles.chip} | ||
| > | ||
| <Plus size={12} color={ColorPalette.gray500} /> | ||
| <Text variant="bodyXsMedium" color="gray500"> | ||
| Create amenity | ||
| </Text> | ||
| </Box> | ||
| </Pressable> |
Contributor
There was a problem hiding this comment.
Remove or wire the “Create amenity” chip before shipping.
This renders as a tappable control with no handler, so the new activity flow exposes a dead-end action. Hide it, disable it, or connect it to the actual category-creation flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/components/categories-sheet.tsx
around lines 88 - 102, The "Create amenity" chip is rendered as a Pressable with
no onPress handler (Pressable > Box ... Text "Create amenity"), producing a dead
tappable control; either remove the entire Pressable/Box block or wire it to the
real category-creation flow by adding an onPress that calls the existing
create/open handler (e.g., openCreateCategoryModal or createCategory function in
this component or parent) and keep styling via styles.chip, Plus and Text as-is
so the UI remains consistent; if the feature isn't ready, prefer removing or
rendering it disabled/non-interactive instead of leaving an empty Pressable.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/(app)/trips/[id]/search-location.tsx (1)
86-114:⚠️ Potential issue | 🟠 MajorPlace-details failures leave the user on a blank screen.
This flow clears the predictions before requesting details. If that request fails, the code only logs the raw error and then falls through to the empty gray box at Line 149, so the user gets no message and no retry path. Restore the prior list or render a user-friendly error state here.
As per coding guidelines, "React/Expo UI error handling must not expose raw API errors; use UI states (LoadingState, ErrorState, EmptyState); errors shown to users must be clear and user-friendly."
Also applies to: 149-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/search-location.tsx around lines 86 - 114, The handler handleSelectPrediction clears predictions then fetches details; when getPlaceDetailsCustom fails the code only logs the raw error and leaves the UI blank. Modify handleSelectPrediction to (1) preserve the previous predictions in a local variable before clearing, (2) on any fetch error call setIsSelectingPlace(false) and setIsLoadingDetails(false), restore the saved predictions via setPredictions(savedPredictions), and set a user-friendly error state (e.g., set an ErrorState / setShowPlaceError(true) or set an error message string) instead of logging the raw error, and (3) ensure navigation (router.navigate/router.back) only runs on success; update any UI to render that ErrorState with a retry action that re-invokes handleSelectPrediction or re-queries getPlaceDetailsCustom.
🤖 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 52-55: The component currently reads tripID from
useLocalSearchParams and uses it to build return destinations, which can
mismatch the route param; change it to read the route param instead (useParams
from next/navigation) and treat that as the single source of truth for the trip
id (e.g., const { id: tripID } = useParams()). Replace usages that build return
paths from the query tripID (and any logic that falls back to router.back()) to
always construct return destinations using this route-derived tripID and
preserve returnTo only for path suffixes; update the same pattern in the other
occurrence around the block that currently reads useLocalSearchParams (lines
referenced as 96-108) so all return path construction, navigation pushes, and
conditionals reference the route param variable rather than the query param.
---
Outside diff comments:
In `@frontend/app/`(app)/trips/[id]/search-location.tsx:
- Around line 86-114: The handler handleSelectPrediction clears predictions then
fetches details; when getPlaceDetailsCustom fails the code only logs the raw
error and leaves the UI blank. Modify handleSelectPrediction to (1) preserve the
previous predictions in a local variable before clearing, (2) on any fetch error
call setIsSelectingPlace(false) and setIsLoadingDetails(false), restore the
saved predictions via setPredictions(savedPredictions), and set a user-friendly
error state (e.g., set an ErrorState / setShowPlaceError(true) or set an error
message string) instead of logging the raw error, and (3) ensure navigation
(router.navigate/router.back) only runs on success; update any UI to render that
ErrorState with a retry action that re-invokes handleSelectPrediction or
re-queries getPlaceDetailsCustom.
🪄 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: 4c608b54-9261-48cf-8a7a-a85bf39bd418
📒 Files selected for processing (1)
frontend/app/(app)/trips/[id]/search-location.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
frontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx (2)
200-208:⚠️ Potential issue | 🟠 Major
hasDatacheck misses most form fields.The discard confirmation only considers
name,description, andthumbnailUri. If the user sets categories, location, price, dates, or link and then cancels, those changes are silently discarded without confirmation.Proposed fix
const handleCancel = () => { - const hasData = name.trim() || description.trim() || thumbnailUri; + const hasData = + name.trim() || + description.trim() || + thumbnailUri || + categories.length > 0 || + locationName || + price.trim() || + dateRange.start || + link.trim(); if (hasData) { setShowCancelConfirm(true); } else { resetForm(); onClose(); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx around lines 200 - 208, The discard-confirmation logic in handleCancel only checks name, description, and thumbnailUri (hasData) and ignores other form fields; update the hasData calculation inside handleCancel to also consider all other form state variables used in this component (e.g., categories, location, price, startDate/endDate or dateRange, link, and any other form flags or arrays) so any non-empty or modified value triggers setShowCancelConfirm(true); if none are set, continue to call resetForm() and onClose() as before. Ensure you reference the exact state variables used by the form (the ones declared alongside name, description, thumbnailUri) when updating hasData so no field is missed.
159-167:⚠️ Potential issue | 🟠 MajorDate serialization loses local calendar day in some timezones.
toISOString()converts to UTC before extracting the date string. In timezones east of UTC, local midnight becomes the previous day in UTC. For example, selecting Jan 1 at midnight in UTC+8 produces "Dec 31" in the payload.Additionally, single-day selections won't persist:
formatDateRangedisplays a single date correctly (line 63-64), but the payload is only created when bothstartandendare non-null.Proposed fix
+ const formatLocalDate = (d: Date) => + `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, "0")}-${String(d.getDate()).padStart(2, "0")}`; + const dates = - dateRange.start && dateRange.end + dateRange.start ? [ { - start: dateRange.start.toISOString().split("T")[0]!, - end: dateRange.end.toISOString().split("T")[0]!, + start: formatLocalDate(dateRange.start), + end: formatLocalDate(dateRange.end ?? dateRange.start), }, ] : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx around lines 159 - 167, The payload date serialization uses toISOString() which converts to UTC and can shift the day in timezones east of UTC, and it also only builds dates when both dateRange.start and dateRange.end are present so single-day selections are dropped; update the dates construction around the dateRange variable so it formats dates in local calendar day (e.g., use a local YYYY-MM-DD formatter such as toLocaleDateString('en-CA') or the existing formatDate utility) and ensure a single selected day is preserved by treating missing end as equal to start (i.e., if dateRange.start exists create an object with start = localFormatted(dateRange.start) and end = localFormatted(dateRange.end ?? dateRange.start)).
🤖 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]/activities/components/activity-card.tsx:
- Around line 37-44: The "New • 1m" badge and "1 new comment" text are hardcoded
placeholders; update the ActivityCard to derive these from the activity model
instead of static strings: use isNew together with activity.createdAt to render
a relative timestamp (e.g., formatDistanceToNow(activity.createdAt) or your
app's date util) for the badge text, and replace "1 new comment" with the actual
count from activity.commentCount or activity.comments?.length (render nothing or
hide the element if the value is undefined/zero). If these UI bits are
intentionally not implemented yet, remove the placeholder strings and the
conditional render blocks around them (isNew badge and comment-count span) until
real data is available. Ensure you update the JSX locations referencing isNew,
activity.createdAt, activity.commentCount, and activity.comments accordingly.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx:
- Around line 109-119: The focus-effect currently calls getPendingLocation()
(which clears module-level state) inside useFocusEffect, risking lost/misapplied
location; change the flow so the component does not consume fragile global state
on focus: instead read the pending location from a safe source passed via
navigation params or a stable API (e.g., route.params.pendingLocation or a new
peekPendingLocation() that does not clear), then call an explicit
clearPendingLocation(id) only after applying it; update the useFocusEffect
callback that sets setLocationName, setLocationLat, setLocationLng to use the
new safe source and explicit clear function, and/or adjust getPendingLocation to
provide a non-destructive peek + separate consume function so
search-location.tsx and add-activity-manual-sheet.tsx no longer rely on a single
destructive getter.
In `@frontend/app/`(app)/trips/[id]/activities/index.tsx:
- Around line 54-62: The handlers handleAutofilled and handleManual use a
hard-coded 300ms setTimeout after calling entrySheetRef.current?.close() before
opening manualSheetRef, which couples behavior to an implicit animation
duration; replace the magic number by either using the sheet component's
close-completion callback/promise (listen for a close/animationend event or
await a returned Promise from entrySheetRef.close()) to then call
manualSheetRef.current?.open(...) OR extract the 300 into a named constant like
ANIMATION_CLOSE_DELAY_MS and document it so the delay is configurable and
obvious; update both handleAutofilled and handleManual to use the chosen
approach.
- Around line 144-157: The sheets are being rendered with tripID fallback to an
empty string which can trigger backend errors; update the rendering logic around
AddActivityEntrySheet and AddActivityManualSheet so they only mount when a valid
tripID exists (e.g., guard with if/tripID check and render an error/placeholder
otherwise) and pass the actual tripID (not "") into the components; locate the
JSX referencing AddActivityEntrySheet, AddActivityManualSheet, entrySheetRef,
manualSheetRef, and the handlers (handleManual, handleAutofilled, handleSaved)
and wrap those components in a conditional that prevents mounting when tripID is
undefined or render an explicit missing-param error state instead.
- Around line 39-46: The destructured search params include unused fields
(locationName, locationLat, locationLng) from the useLocalSearchParams call;
either remove those unused properties from the generic type so the hook is just
useLocalSearchParams<{ id: string }>() or, if they are intended to be used,
actually read and use the variables (locationName, locationLat, locationLng)
elsewhere in this component (e.g., pass them to state, props, or helper
functions); update the destructuring at the useLocalSearchParams invocation and
any related code paths (references to tripID, useLocalSearchParams) accordingly
to eliminate the unused variables or make use of them.
In `@frontend/app/`(app)/trips/[id]/search-location.tsx:
- Around line 50-56: The module-level variable _pendingLocation and its
read-and-clear accessor getPendingLocation create fragile cross-screen coupling;
replace them with an explicit state manager—e.g., create a small Zustand store
(usePendingLocationStore) that exposes setPendingLocation(location),
getPendingLocation(), and clearPendingLocation() (or use navigation params to
pass the location back to the return route) and update all references in
search-location.tsx to use the store/API instead of the module variable; remove
_pendingLocation and getPendingLocation once all callers use the new store or
navigation param flow.
- Around line 134-142: The function handleConfirm has inconsistent indentation
(its body is not indented relative to the declaration); fix by reformatting the
handleConfirm block so the body lines (the if check, _pendingLocation assignment
and router.back() call) are indented one level inside the function, ensuring
consistent spacing around the function declaration and braces and matching
surrounding file style where handleConfirm is defined.
---
Duplicate comments:
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx:
- Around line 200-208: The discard-confirmation logic in handleCancel only
checks name, description, and thumbnailUri (hasData) and ignores other form
fields; update the hasData calculation inside handleCancel to also consider all
other form state variables used in this component (e.g., categories, location,
price, startDate/endDate or dateRange, link, and any other form flags or arrays)
so any non-empty or modified value triggers setShowCancelConfirm(true); if none
are set, continue to call resetForm() and onClose() as before. Ensure you
reference the exact state variables used by the form (the ones declared
alongside name, description, thumbnailUri) when updating hasData so no field is
missed.
- Around line 159-167: The payload date serialization uses toISOString() which
converts to UTC and can shift the day in timezones east of UTC, and it also only
builds dates when both dateRange.start and dateRange.end are present so
single-day selections are dropped; update the dates construction around the
dateRange variable so it formats dates in local calendar day (e.g., use a local
YYYY-MM-DD formatter such as toLocaleDateString('en-CA') or the existing
formatDate utility) and ensure a single selected day is preserved by treating
missing end as equal to start (i.e., if dateRange.start exists create an object
with start = localFormatted(dateRange.start) and end =
localFormatted(dateRange.end ?? dateRange.start)).
🪄 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: facd3a0b-0a11-4b48-8b78-ba8302e7bc28
📒 Files selected for processing (4)
frontend/app/(app)/trips/[id]/activities/components/activity-card.tsxfrontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsxfrontend/app/(app)/trips/[id]/activities/index.tsxfrontend/app/(app)/trips/[id]/search-location.tsx
Comment on lines
+37
to
+44
| {isNew && ( | ||
| <Box style={styles.newBadge}> | ||
| <Text variant="bodyXxsMedium" color="white"> | ||
| New • 1m | ||
| </Text> | ||
| </Box> | ||
| )} | ||
| </Box> |
Contributor
There was a problem hiding this comment.
Hardcoded placeholder text when isNew is true.
Lines 40 and 101 display static strings ("New • 1m", "1 new comment") that don't reflect actual activity data. If isNew is meant to indicate genuinely new activities, the badge timestamp and comment count should come from the activity model. If these are intentional placeholders, consider removing them until the feature is implemented.
Also applies to: 98-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/components/activity-card.tsx around
lines 37 - 44, The "New • 1m" badge and "1 new comment" text are hardcoded
placeholders; update the ActivityCard to derive these from the activity model
instead of static strings: use isNew together with activity.createdAt to render
a relative timestamp (e.g., formatDistanceToNow(activity.createdAt) or your
app's date util) for the badge text, and replace "1 new comment" with the actual
count from activity.commentCount or activity.comments?.length (render nothing or
hide the element if the value is undefined/zero). If these UI bits are
intentionally not implemented yet, remove the placeholder strings and the
conditional render blocks around them (isNew badge and comment-count span) until
real data is available. Ensure you update the JSX locations referencing isNew,
activity.createdAt, activity.commentCount, and activity.comments accordingly.
frontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+50
to
+56
| let _pendingLocation: { name: string; lat: number; lng: number } | null = null; | ||
|
|
||
| export function getPendingLocation() { | ||
| const loc = _pendingLocation; | ||
| _pendingLocation = null; | ||
| return loc; | ||
| } |
Contributor
There was a problem hiding this comment.
Module-level mutable state creates fragile cross-screen coupling.
_pendingLocation is shared global state that persists across component lifecycles. The read-and-clear pattern in getPendingLocation() is fragile: if the consumer doesn't call it (e.g., navigation is interrupted or focus callback doesn't fire), the stale value persists for the next unrelated navigation. If called twice, the second call gets null.
Consider using a proper state management approach such as:
- Zustand store with explicit clear semantics
- Navigation params passed directly to the return route
- React context scoped to the navigation stack
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/search-location.tsx around lines 50 - 56, The
module-level variable _pendingLocation and its read-and-clear accessor
getPendingLocation create fragile cross-screen coupling; replace them with an
explicit state manager—e.g., create a small Zustand store
(usePendingLocationStore) that exposes setPendingLocation(location),
getPendingLocation(), and clearPendingLocation() (or use navigation params to
pass the location back to the return route) and update all references in
search-location.tsx to use the store/API instead of the module variable; remove
_pendingLocation and getPendingLocation once all callers use the new store or
navigation param flow.
Comment on lines
+134
to
+142
| const handleConfirm = () => { | ||
| if (!selectedLocation) return; | ||
| _pendingLocation = { | ||
| name: selectedLocation.name || selectedLocation.formatted_address, | ||
| lat: selectedLocation.geometry.location.lat, | ||
| lng: selectedLocation.geometry.location.lng, | ||
| }; | ||
| router.back(); | ||
| }; |
Contributor
There was a problem hiding this comment.
handleConfirm indentation is inconsistent.
The function body is not indented relative to the function declaration, which breaks code formatting conventions.
Proposed fix
const handleConfirm = () => {
- if (!selectedLocation) return;
- _pendingLocation = {
- name: selectedLocation.name || selectedLocation.formatted_address,
- lat: selectedLocation.geometry.location.lat,
- lng: selectedLocation.geometry.location.lng,
- };
- router.back();
-};
+ if (!selectedLocation) return;
+ _pendingLocation = {
+ name: selectedLocation.name || selectedLocation.formatted_address,
+ lat: selectedLocation.geometry.location.lat,
+ lng: selectedLocation.geometry.location.lng,
+ };
+ router.back();
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/search-location.tsx around lines 134 - 142,
The function handleConfirm has inconsistent indentation (its body is not
indented relative to the declaration); fix by reformatting the handleConfirm
block so the body lines (the if check, _pendingLocation assignment and
router.back() call) are indented one level inside the function, ensuring
consistent spacing around the function declaration and braces and matching
surrounding file style where handleConfirm is defined.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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]/activities/components/add-activity-manual-sheet.tsx:
- Around line 147-162: The image-upload block currently swallows errors and
still shows the "Activity added" success state; update the catch for
uploadImage.mutateAsync / getImageURL in add-activity-manual-sheet.tsx to
surface the failure to the user by invoking the same toast mechanism used for
success (or appending a note to the success message) — e.g., call showToast or
toast.warn with a clear message like "Thumbnail upload failed; activity saved
without cover image" and ensure thumbnailURL remains undefined so UI/state
reflects missing image; keep logging the original uploadErr for diagnostics.
🪄 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: 1b95ef0e-a466-43aa-b9d7-8c64c12cee1d
📒 Files selected for processing (1)
frontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
Comment on lines
+147
to
+162
| if (thumbnailUri?.startsWith("http")) { | ||
| // Remote URL from autofill — use directly | ||
| thumbnailURL = thumbnailUri; | ||
| } else if (thumbnailUri) { | ||
| // Local file — upload and get presigned URL | ||
| try { | ||
| const res = await uploadImage.mutateAsync({ | ||
| uri: thumbnailUri, | ||
| sizes: ["medium"], | ||
| }); | ||
| const urlRes = await getImageURL(res.imageId, "medium"); | ||
| thumbnailURL = urlRes.url; | ||
| } catch (uploadErr) { | ||
| console.warn("Image upload failed, skipping thumbnail:", uploadErr); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Silent image upload failure may confuse users.
When image upload fails (lines 159-161), the activity still saves successfully and shows the "Activity added" toast. The user has no indication their cover image was not saved. Consider showing a warning toast or appending a note to the success message when the thumbnail upload fails.
Suggested approach
+ let thumbnailSkipped = false;
if (thumbnailUri?.startsWith("http")) {
thumbnailURL = thumbnailUri;
} else if (thumbnailUri) {
try {
const res = await uploadImage.mutateAsync({
uri: thumbnailUri,
sizes: ["medium"],
});
const urlRes = await getImageURL(res.imageId, "medium");
thumbnailURL = urlRes.url;
} catch (uploadErr) {
console.warn("Image upload failed, skipping thumbnail:", uploadErr);
+ thumbnailSkipped = true;
}
}
// ...after save...
toast.show({
- message: "Activity added",
+ message: thumbnailSkipped ? "Activity added (image upload failed)" : "Activity added",
action: { label: "View", onPress: () => {} },
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/app/`(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
around lines 147 - 162, The image-upload block currently swallows errors and
still shows the "Activity added" success state; update the catch for
uploadImage.mutateAsync / getImageURL in add-activity-manual-sheet.tsx to
surface the failure to the user by invoking the same toast mechanism used for
success (or appending a note to the success message) — e.g., call showToast or
toast.warn with a clear message like "Thumbnail upload failed; activity saved
without cover image" and ensure thumbnailURL remains undefined so UI/state
reflects missing image; keep logging the original uploadErr for diagnostics.
aahiltn
force-pushed
the
create-activity-flow
branch
from
79723ee to
6b020bd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
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/index.tsx (2)
82-89:⚠️ Potential issue | 🔴 CriticalSyntax error: malformed code block.
The pipeline reports a syntax error at line 88. This appears to be incomplete refactoring where
useCreateActivitysetup is broken. The dependencies array syntax is malformed ([prependActivity],on line 88 appears to be a leftover fragment).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/index.tsx around lines 82 - 89, The code block calling useCreateActivity is malformed due to a stray dependencies array fragment; fix the call to useCreateActivity by removing the trailing "[prependActivity]," and ensure the options object correctly includes mutation: { onSuccess: (data) => { if (data) prependActivity(data); } } (and if the hook requires a separate deps argument, pass prependActivity as that second parameter or memoize prependActivity with useCallback before passing). Locate useCreateActivity, createActivity, isCreating, and prependActivity to correct the call so the object literal is closed properly and no stray array tokens remain.
163-222:⚠️ Potential issue | 🔴 CriticalLeftover code from previous implementation.
Lines 163-222 contain remnants of the old implementation (
CommentSection,Screenwrapper, old button styles) that conflict with the new sheet-based approach. This file needs cleanup to remove the dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(app)/trips/[id]/activities/index.tsx around lines 163 - 222, Remove the leftover old UI block and state tied to the pre-sheet implementation: delete the Pressable "View on map" block (including usage of encodeMapViewActivitiesParam and activitiesWithLocation), the legacy CommentSection instance and its props, and the surrounding Screen wrapper closing tags and Box that belong to the old layout; also remove related styles (mapButton, mapButtonDisabled, mapButtonPressed) and any state/vars no longer referenced such as activeActivityId if the new sheet-based CommentSection replaces them, then clean up imports that become unused after removing these symbols so the file only contains the new sheet-based comment UI.
🤖 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]/activities/components/add-activity-manual-sheet.tsx:
- Around line 143-169: Remove the git conflict markers and keep the updated
implementation that distinguishes remote vs local thumbnails: delete the
<<<<<<<, =======, and >>>>>>> blocks and retain the branch that checks
thumbnailUri?.startsWith("http") to set thumbnailURL directly for remote URLs,
and otherwise calls uploadImage.mutateAsync(...) and getImageURL(... ) inside a
try/catch to assign thumbnailURL and handle uploadErr; ensure references to
thumbnailUri, uploadImage.mutateAsync, getImageURL, and thumbnailURL remain
intact and the console warning for upload failures is preserved.
In `@frontend/app/`(app)/trips/[id]/activities/components/form-row.tsx:
- Around line 18-32: The Pressable currently always wraps the row, creating an
inert tappable area when onPress is undefined; update the component in
form-row.tsx to render Pressable only when the onPress prop is provided and
otherwise render a non-interactive Box (or View) with the same children (Icon,
Text using hasValue, value, placeholder) so layout and styles remain identical
but no inactive touch area exists; locate the Pressable usage and replace it
with a conditional that checks onPress and returns either <Pressable
onPress={onPress}>...</Pressable> or the plain Box fallback.
In `@frontend/app/`(app)/trips/[id]/activities/creation/index.tsx:
- Around line 54-62: handleAutofilled and handleManual use a hardcoded
setTimeout(…, 300) to wait for entrySheetRef.current?.close() before opening
manualSheetRef, coupling behavior to an implicit animation duration; replace the
magic 300ms by either using a named constant (e.g. SHEET_CLOSE_DURATION_MS)
or—preferably—use the sheet API's completion callback/promise (e.g.
entrySheetRef.current.close().then(...) or an onClose handler) to open
manualSheetRef only after the close completes so timing is explicit and robust;
update references in handleAutofilled and handleManual to use that approach
(entrySheetRef, manualSheetRef).
- Around line 144-157: The sheets are being rendered with an empty-string
fallback (tripID ?? "") which can trigger backend validation/404s; update the
render logic to avoid mounting AddActivityEntrySheet and AddActivityManualSheet
until a valid tripID exists (e.g., only render these components when tripID is a
non-empty string), keeping the same refs (entrySheetRef, manualSheetRef) and
callbacks (handleManual, handleAutofilled, handleSaved, onClose) so behavior is
unchanged once tripID is available.
---
Outside diff comments:
In `@frontend/app/`(app)/trips/[id]/activities/index.tsx:
- Around line 82-89: The code block calling useCreateActivity is malformed due
to a stray dependencies array fragment; fix the call to useCreateActivity by
removing the trailing "[prependActivity]," and ensure the options object
correctly includes mutation: { onSuccess: (data) => { if (data)
prependActivity(data); } } (and if the hook requires a separate deps argument,
pass prependActivity as that second parameter or memoize prependActivity with
useCallback before passing). Locate useCreateActivity, createActivity,
isCreating, and prependActivity to correct the call so the object literal is
closed properly and no stray array tokens remain.
- Around line 163-222: Remove the leftover old UI block and state tied to the
pre-sheet implementation: delete the Pressable "View on map" block (including
usage of encodeMapViewActivitiesParam and activitiesWithLocation), the legacy
CommentSection instance and its props, and the surrounding Screen wrapper
closing tags and Box that belong to the old layout; also remove related styles
(mapButton, mapButtonDisabled, mapButtonPressed) and any state/vars no longer
referenced such as activeActivityId if the new sheet-based CommentSection
replaces them, then clean up imports that become unused after removing these
symbols so the file only contains the new sheet-based comment UI.
🪄 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: 53e94baa-7364-4a0a-b384-27cb0f86e4a4
⛔ Files ignored due to path filters (1)
frontend/assets/images/binoculars.pngis excluded by!**/*.png
📒 Files selected for processing (8)
frontend/app/(app)/trips/[id]/activities/components/activity-card.tsxfrontend/app/(app)/trips/[id]/activities/components/add-activity-entry-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/categories-sheet.tsxfrontend/app/(app)/trips/[id]/activities/components/form-row.tsxfrontend/app/(app)/trips/[id]/activities/creation/index.tsxfrontend/app/(app)/trips/[id]/activities/index.tsxfrontend/app/(app)/trips/[id]/search-location.tsx
frontend/app/(app)/trips/[id]/activities/components/add-activity-manual-sheet.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+18
to
+32
| <Pressable onPress={onPress}> | ||
| <Box flexDirection="row" alignItems="center" gap="sm" paddingVertical="xs"> | ||
| <Icon | ||
| size={16} | ||
| color={hasValue ? ColorPalette.gray700 : ColorPalette.gray400} | ||
| /> | ||
| <Text | ||
| variant="bodySmDefault" | ||
| color={hasValue ? "gray900" : "gray400"} | ||
| style={{ flex: 1 }} | ||
| > | ||
| {value ?? placeholder} | ||
| </Text> | ||
| </Box> | ||
| </Pressable> |
Contributor
There was a problem hiding this comment.
Pressable wraps content even when onPress is undefined.
The Pressable is always rendered regardless of whether onPress is provided. This creates a tappable area that does nothing when onPress is undefined, which can confuse users expecting feedback.
Proposed fix
export function FormRow({ icon: Icon, value, placeholder, onPress }: FormRowProps) {
const hasValue = !!value;
+ const content = (
+ <Box flexDirection="row" alignItems="center" gap="sm" paddingVertical="xs">
+ <Icon
+ size={16}
+ color={hasValue ? ColorPalette.gray700 : ColorPalette.gray400}
+ />
+ <Text
+ variant="bodySmDefault"
+ color={hasValue ? "gray900" : "gray400"}
+ style={{ flex: 1 }}
+ >
+ {value ?? placeholder}
+ </Text>
+ </Box>
+ );
+
+ if (!onPress) return content;
+
return (
- <Pressable onPress={onPress}>
- <Box flexDirection="row" alignItems="center" gap="sm" paddingVertical="xs">
- <Icon
- size={16}
- color={hasValue ? ColorPalette.gray700 : ColorPalette.gray400}
- />
- <Text
- variant="bodySmDefault"
- color={hasValue ? "gray900" : "gray400"}
- style={{ flex: 1 }}
- >
- {value ?? placeholder}
- </Text>
- </Box>
- </Pressable>
+ <Pressable onPress={onPress}>{content}</Pressable>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/components/form-row.tsx around
lines 18 - 32, The Pressable currently always wraps the row, creating an inert
tappable area when onPress is undefined; update the component in form-row.tsx to
render Pressable only when the onPress prop is provided and otherwise render a
non-interactive Box (or View) with the same children (Icon, Text using hasValue,
value, placeholder) so layout and styles remain identical but no inactive touch
area exists; locate the Pressable usage and replace it with a conditional that
checks onPress and returns either <Pressable onPress={onPress}>...</Pressable>
or the plain Box fallback.
Comment on lines
+54
to
+62
| const handleAutofilled = useCallback((data: ModelsParsedActivityData) => { | ||
| entrySheetRef.current?.close(); | ||
| setTimeout(() => manualSheetRef.current?.open(data), 300); | ||
| }, []); | ||
|
|
||
| const result = await ImagePicker.launchImageLibraryAsync({ | ||
| mediaTypes: ["images"], | ||
| quality: 1, | ||
| }); | ||
| const handleManual = useCallback(() => { | ||
| entrySheetRef.current?.close(); | ||
| setTimeout(() => manualSheetRef.current?.open(), 300); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Magic 300ms delay couples behavior to implicit animation duration.
The setTimeout(..., 300) assumes sheet closing animation takes exactly 300ms. Consider using a named constant or the sheet's close completion callback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/creation/index.tsx around lines 54
- 62, handleAutofilled and handleManual use a hardcoded setTimeout(…, 300) to
wait for entrySheetRef.current?.close() before opening manualSheetRef, coupling
behavior to an implicit animation duration; replace the magic 300ms by either
using a named constant (e.g. SHEET_CLOSE_DURATION_MS) or—preferably—use the
sheet API's completion callback/promise (e.g.
entrySheetRef.current.close().then(...) or an onClose handler) to open
manualSheetRef only after the close completes so timing is explicit and robust;
update references in handleAutofilled and handleManual to use that approach
(entrySheetRef, manualSheetRef).
Comment on lines
+144
to
+157
| <AddActivityEntrySheet | ||
| ref={entrySheetRef} | ||
| tripID={tripID ?? ""} | ||
| onManual={handleManual} | ||
| onAutofilled={handleAutofilled} | ||
| onClose={() => entrySheetRef.current?.close()} | ||
| /> | ||
|
|
||
| <AddActivityManualSheet | ||
| ref={manualSheetRef} | ||
| tripID={tripID ?? ""} | ||
| onSaved={handleSaved} | ||
| onClose={() => manualSheetRef.current?.close()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Empty string fallback for tripID may cause API errors.
Passing tripID ?? "" when tripID is undefined will likely result in 404 or validation errors from the backend. Consider guarding against rendering the sheets when tripID is undefined.
Proposed fix
+ {tripID && (
+ <>
<AddActivityEntrySheet
ref={entrySheetRef}
- tripID={tripID ?? ""}
+ tripID={tripID}
onManual={handleManual}
onAutofilled={handleAutofilled}
onClose={() => entrySheetRef.current?.close()}
/>
<AddActivityManualSheet
ref={manualSheetRef}
- tripID={tripID ?? ""}
+ tripID={tripID}
onSaved={handleSaved}
onClose={() => manualSheetRef.current?.close()}
/>
+ </>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <AddActivityEntrySheet | |
| ref={entrySheetRef} | |
| tripID={tripID ?? ""} | |
| onManual={handleManual} | |
| onAutofilled={handleAutofilled} | |
| onClose={() => entrySheetRef.current?.close()} | |
| /> | |
| <AddActivityManualSheet | |
| ref={manualSheetRef} | |
| tripID={tripID ?? ""} | |
| onSaved={handleSaved} | |
| onClose={() => manualSheetRef.current?.close()} | |
| /> | |
| {tripID && ( | |
| <> | |
| <AddActivityEntrySheet | |
| ref={entrySheetRef} | |
| tripID={tripID} | |
| onManual={handleManual} | |
| onAutofilled={handleAutofilled} | |
| onClose={() => entrySheetRef.current?.close()} | |
| /> | |
| <AddActivityManualSheet | |
| ref={manualSheetRef} | |
| tripID={tripID} | |
| onSaved={handleSaved} | |
| onClose={() => manualSheetRef.current?.close()} | |
| /> | |
| </> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/activities/creation/index.tsx around lines 144
- 157, The sheets are being rendered with an empty-string fallback (tripID ??
"") which can trigger backend validation/404s; update the render logic to avoid
mounting AddActivityEntrySheet and AddActivityManualSheet until a valid tripID
exists (e.g., only render these components when tripID is a non-empty string),
keeping the same refs (entrySheetRef, manualSheetRef) and callbacks
(handleManual, handleAutofilled, handleSaved, onClose) so behavior is unchanged
once tripID is available.
afnantuffaha
force-pushed
the
create-activity-flow
branch
from
c7550d4 to
8a348e9
Compare
revert search location changes remove weird ahh modal chore: remove placeholder new activity
afnantuffaha
force-pushed
the
create-activity-flow
branch
from
2daaa19 to
9ca6bd0
Compare
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
1 participant
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
Features
Fixes
Improvements
Author contribution table (lines added/removed/net)
Sequence diagram (activity creation flow)
User -> Activities screen: tap "Add an activity"
Activities screen -> AddActivityEntrySheet: open
AddActivityEntrySheet -> User: choose "Autofill from link" or "Add manually"
Autofill path:
AddActivityEntrySheet -> Server: parseActivityLink(url)
Server -> AddActivityEntrySheet: parsed data
AddActivityEntrySheet -> AddActivityManualSheet: open(prefill=parsed data)
Manual path:
AddActivityEntrySheet -> AddActivityManualSheet: open()
AddActivityManualSheet -> Server: createActivity(data)
Server -> AddActivityManualSheet: created activity
AddActivityManualSheet -> Activities screen: onSaved(created activity) -> prepend to list