Conversation
📝 WalkthroughWalkthroughThe changes adjust UI layout constants, component sizing, and styling across multiple app pages. Modifications include updated header heights, sheet overlap values, animation durations, button color defaults, and padding values. Header opacity animations were simplified by removing interpolated values. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 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: 5
🤖 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)/recommended/[id].tsx:
- Line 43: Replace the hard-coded numeric FIXED_HEADER_HEIGHT (currently 54)
with a design-system spacing token: import and use the centralized token (e.g.,
HEADER_HEIGHT, spacing.header or tokens.spacing.header) instead; if that token
does not exist yet, add a descriptive token (HEADER_HEIGHT or spacing.header) to
the design system tokens and then consume it in the component where
FIXED_HEADER_HEIGHT is defined so layout spacing is consistent and no magic
number remains.
In `@frontend/app/`(app)/trips/[id]/index.tsx:
- Around line 69-76: DEFAULT_TABS includes "settings" and "housing" which have
no content renderer so when activeTab is set to them the content area goes
blank; update the DEFAULT_TABS array (the DEFAULT_TABS constant / TabKey[] in
frontend/app/(app)/trips/[id]/index.tsx) to only include tabs that have
renderers (or alternatively implement a fallback renderer for tabs without
content), and ensure any logic that reads activeTab falls back to a valid tab
when the route param points to a non-rendered key.
- Around line 625-626: Replace the magic number paddingHorizontal: 12 with the
design-system spacing token from Layout.spacing (e.g., paddingHorizontal:
Layout.spacing.sm) in the style object inside
frontend/app/(app)/trips/[id]/index.tsx; locate the style that currently sets
paddingHorizontal and swap the literal 12 for the appropriate Layout.spacing
token (or explicit left/right composition using Layout.spacing.* if needed) so
the component uses the centralized spacing system.
- Line 304: Replace the magic number 12 in titleScrollThreshold with a
safe-area-aware value: obtain the top inset via useSafeAreaInsets() (or your
app's centralized safe-area helper) and combine it with the design-system
spacing token (e.g. SPACING.small or tokens.spacing.sm) instead of hard-coding
12; then compute titleScrollThreshold = CONTENT_CARD_TOP - (insetTop +
designSpacing) so header/title transition respects device top insets and
centralized spacing.
In `@frontend/design-system/components/navigation/arrow.tsx`:
- Line 16: The default color for the BackButton was changed to "gray500" in
navigation/arrow.tsx (symbol: BackButton), causing an app-wide visual
regression; revert the default prop/value in the BackButton component back to
"black" (restore the original default assignment where color = "black") and then
update only the specific screens that need the gray appearance by adding
color="gray500" to those BackButton usages rather than changing the component
default.
🪄 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: a8091aa4-759b-45d1-8d00-bff5a806da0c
📒 Files selected for processing (4)
frontend/app/(app)/components/trip-reminder-location-sheet.tsxfrontend/app/(app)/recommended/[id].tsxfrontend/app/(app)/trips/[id]/index.tsxfrontend/design-system/components/navigation/arrow.tsx
| const MAP_HEIGHT = 220; | ||
| const CONTENT_CARD_TOP = HERO_IMAGE_HEIGHT - 24; | ||
| const FIXED_HEADER_HEIGHT = 48; | ||
| const FIXED_HEADER_HEIGHT = 54; |
There was a problem hiding this comment.
Use a design token instead of a hard-coded header height.
Line 43 sets FIXED_HEADER_HEIGHT to a raw numeric value (54). This should come from the centralized design-system tokens (or be added there first) to keep layout spacing consistent and maintainable.
As per coding guidelines **/*.{tsx,ts,jsx,js}: React/Expo UI must use a centralized design system; 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)/recommended/[id].tsx at line 43, Replace the hard-coded
numeric FIXED_HEADER_HEIGHT (currently 54) with a design-system spacing token:
import and use the centralized token (e.g., HEADER_HEIGHT, spacing.header or
tokens.spacing.header) instead; if that token does not exist yet, add a
descriptive token (HEADER_HEIGHT or spacing.header) to the design system tokens
and then consume it in the component where FIXED_HEADER_HEIGHT is defined so
layout spacing is consistent and no magic number remains.
| const DEFAULT_TABS = [ | ||
| "new", | ||
| "itinerary", | ||
| "polls", | ||
| "activities", | ||
| "settings", | ||
| "housing", | ||
| ] as TabKey[]; |
There was a problem hiding this comment.
DEFAULT_TABS currently suppresses fallback for tabs with no content renderer.
DEFAULT_TABS includes "settings" and "housing", but this screen does not render content for those tabs. If activeTab is set from route params to one of them, the content area can become blank because the empty state is bypassed.
Proposed fix
-const DEFAULT_TABS = [
- "new",
- "itinerary",
- "polls",
- "activities",
- "settings",
- "housing",
-] as TabKey[];
+const CONTENT_TABS: ReadonlySet<TabKey> = new Set([
+ "new",
+ "itinerary",
+ "polls",
+ "activities",
+]);
...
- {!DEFAULT_TABS.includes(activeTab) && (
+ {!CONTENT_TABS.has(activeTab) && (
<EmptyState
title="Nothing here yet"
description="Post notes, photos, videos, and links."
/>
)}Also applies to: 458-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/index.tsx around lines 69 - 76, DEFAULT_TABS
includes "settings" and "housing" which have no content renderer so when
activeTab is set to them the content area goes blank; update the DEFAULT_TABS
array (the DEFAULT_TABS constant / TabKey[] in
frontend/app/(app)/trips/[id]/index.tsx) to only include tabs that have
renderers (or alternatively implement a fallback renderer for tabs without
content), and ensure any logic that reads activeTab falls back to a valid tab
when the route param points to a non-rendered key.
| } | ||
|
|
||
| const titleScrollThreshold = CONTENT_CARD_TOP - insets.top; | ||
| const titleScrollThreshold = CONTENT_CARD_TOP - 12; |
There was a problem hiding this comment.
Use safe-area-based threshold for header transition.
Line 304 uses a fixed 12 offset. This can shift the header/title transition on devices with larger top insets and produce inconsistent behavior.
Proposed fix
- const titleScrollThreshold = CONTENT_CARD_TOP - 12;
+ const titleScrollThreshold = CONTENT_CARD_TOP - insets.top;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."
📝 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.
| const titleScrollThreshold = CONTENT_CARD_TOP - 12; | |
| const titleScrollThreshold = CONTENT_CARD_TOP - insets.top; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/index.tsx at line 304, Replace the magic
number 12 in titleScrollThreshold with a safe-area-aware value: obtain the top
inset via useSafeAreaInsets() (or your app's centralized safe-area helper) and
combine it with the design-system spacing token (e.g. SPACING.small or
tokens.spacing.sm) instead of hard-coding 12; then compute titleScrollThreshold
= CONTENT_CARD_TOP - (insetTop + designSpacing) so header/title transition
respects device top insets and centralized spacing.
| paddingHorizontal: 12, | ||
| paddingVertical: Layout.spacing.xs, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace raw horizontal padding with design-system spacing composition.
Line 625 hard-codes 12, which is outside the declared spacing token set.
Proposed fix
- paddingHorizontal: 12,
+ paddingHorizontal: Layout.spacing.xs + Layout.spacing.xxs,Based on learnings: "React/Expo UI must use a centralized design system; avoid hard-coded color values, magic spacing numbers, and duplicated style definitions."
📝 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.
| paddingHorizontal: 12, | |
| paddingVertical: Layout.spacing.xs, | |
| paddingHorizontal: Layout.spacing.xs + Layout.spacing.xxs, | |
| paddingVertical: Layout.spacing.xs, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/index.tsx around lines 625 - 626, Replace the
magic number paddingHorizontal: 12 with the design-system spacing token from
Layout.spacing (e.g., paddingHorizontal: Layout.spacing.sm) in the style object
inside frontend/app/(app)/trips/[id]/index.tsx; locate the style that currently
sets paddingHorizontal and swap the literal 12 for the appropriate
Layout.spacing token (or explicit left/right composition using Layout.spacing.*
if needed) so the component uses the centralized spacing system.
| onPress, | ||
| hasBackground = false, | ||
| color = "black", | ||
| color = "gray500", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all BackButton usages that rely on default color (no explicit color prop).
python - <<'PY'
import re
from pathlib import Path
pattern = re.compile(r'<BackButton\b([\s\S]*?)\/>')
for path in Path('.').rglob('*.tsx'):
text = path.read_text(encoding='utf-8', errors='ignore')
for m in pattern.finditer(text):
attrs = m.group(1)
if 'color=' not in attrs:
line = text.count('\n', 0, m.start()) + 1
snippet = re.sub(r'\s+', ' ', m.group(0)).strip()
print(f"{path}:{line}: {snippet}")
PYRepository: GenerateNU/toggo
Length of output: 2131
🏁 Script executed:
cat -n frontend/design-system/components/navigation/arrow.tsx | head -30Repository: GenerateNU/toggo
Length of output: 1104
🏁 Script executed:
git log --oneline -n 20 frontend/design-system/components/navigation/arrow.tsxRepository: GenerateNU/toggo
Length of output: 118
🏁 Script executed:
git show 7e353fc:frontend/design-system/components/navigation/arrow.tsx | head -30Repository: GenerateNU/toggo
Length of output: 894
🏁 Script executed:
git show 7e353fc^:frontend/design-system/components/navigation/arrow.tsx | head -30Repository: GenerateNU/toggo
Length of output: 98
🏁 Script executed:
git log --all --oneline -- frontend/design-system/components/navigation/arrow.tsxRepository: GenerateNU/toggo
Length of output: 388
🏁 Script executed:
git show 6f1ed67:frontend/design-system/components/navigation/arrow.tsx | head -30Repository: GenerateNU/toggo
Length of output: 859
Revert default BackButton color to avoid unintended visual changes across the app.
The default color changed from "black" to "gray500" in the latest commit. This affects 30+ BackButton usages across the app that don't explicitly specify a color prop, causing an unscoped visual regression. Change the default back to "black" and explicitly set color="gray500" only at screens that require it.
Suggested fix
export const BackButton: React.FC<BackButtonProps> = ({
onPress,
hasBackground = false,
- color = "gray500",
+ color = "black",
}) => {📝 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.
| color = "gray500", | |
| export const BackButton: React.FC<BackButtonProps> = ({ | |
| onPress, | |
| hasBackground = false, | |
| color = "black", | |
| }) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/design-system/components/navigation/arrow.tsx` at line 16, The
default color for the BackButton was changed to "gray500" in
navigation/arrow.tsx (symbol: BackButton), causing an app-wide visual
regression; revert the default prop/value in the BackButton component back to
"black" (restore the original default assignment where color = "black") and then
update only the specific screens that need the gray appearance by adding
color="gray500" to those BackButton usages rather than changing the component
default.
2 participants
Description
2026-04-16.11-31-54.mov
How has this been tested?
Checklist
User-visible changes
Changes by category
Fixes
Improvements
Contribution summary
frontend/design-system/components/navigation/arrow.tsxfrontend/app/(app)/trips/[id]/index.tsxfrontend/app/(app)/recommended/[id].tsxfrontend/app/(app)/components/trip-reminder-location-sheet.tsx