[APP] Dish detail page#359
Conversation
Reviewer's GuideImplements a new dish details modal flow and supporting UI components/styles, refines typical dishes interactions and layout, and extends domain/navigation schemas with vegan flags and dish detail navigation. Sequence diagram for the new dish details modal flowsequenceDiagram
actor User
participant FoodCard
participant NavigationService
participant TypicalDishesModalPage
participant TypicalDishesList
participant DishItem
participant DishDetailsModalPage
participant useDishDetailsModalPageLogic
participant useGetTripById
participant useGetWikimediaDishImage
User ->> FoodCard: Pressable onPress handleOpenModal
FoodCard ->> NavigationService: toTypicalDishesModal tripId
NavigationService ->> TypicalDishesModalPage: push Modals.TypicalDishes with tripId
User ->> DishItem: Pressable onPress
DishItem ->> TypicalDishesList: onPress searchTerm
TypicalDishesList ->> TypicalDishesModalPage: onDishPress searchTerm
TypicalDishesModalPage ->> NavigationService: toDishDetailsModal { tripId, searchTerm }
NavigationService ->> DishDetailsModalPage: push Modals.DishDetails with params
DishDetailsModalPage ->> useDishDetailsModalPageLogic: useDishDetailsModalPageLogic
useDishDetailsModalPageLogic ->> useGetTripById: useGetTripById tripId
useDishDetailsModalPageLogic ->> useGetWikimediaDishImage: useGetWikimediaDishImage searchTerm
useGetTripById -->> useDishDetailsModalPageLogic: trip
useGetWikimediaDishImage -->> useDishDetailsModalPageLogic: { url, isLoading }
useDishDetailsModalPageLogic -->> DishDetailsModalPage: dishName, dishDescription, dishIngredients, isGlutenFree, isVegetarian, isVegan, image, imageIsLoading
User ->> DishDetailsModalPage: Tap close
DishDetailsModalPage ->> useDishDetailsModalPageLogic: handleClose
useDishDetailsModalPageLogic ->> NavigationService: back
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 42 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a new dish details modal feature to the create-trip flow. Users can now press individual dishes in the typical dishes modal to view dietary attributes (vegan, vegetarian, gluten-free status), ingredients, description, and a fetched image. The modal is accessible from the typical dishes list via a new navigation route wired through the navigation service and router. ChangesDish Details Modal Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Architecture violations foundThe following dependency-cruiser rules were violated. Fix them before merging. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Dish Details' modal page to display typical dish information, including its description, ingredients, and dietary badges (Gluten-Free, Vegan, Vegetarian), along with supporting navigation, schemas, and translations. Feedback on the changes suggests avoiding dynamic StyleSheet.create calls in the Badge component to prevent performance issues, exposing and handling the trip query loading state with skeleton screens in the modal page to avoid visual flashes, and localizing the hardcoded 'Ingredients' string.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export const styles = (active: boolean, backgroundColor: string) => | ||
| StyleSheet.create({ | ||
| container: { | ||
| alignItems: 'center', | ||
| gap: spacing.Single, | ||
| }, | ||
| circleWrapper: { | ||
| width: components.badgeCircleSize, | ||
| height: components.badgeCircleSize, | ||
| }, | ||
| circle: { | ||
| width: components.badgeCircleSize, | ||
| height: components.badgeCircleSize, | ||
| borderRadius: components.badgeCircleSize / 2, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| backgroundColor: active ? backgroundColor : colors.primaryRed, | ||
| }, | ||
| checkBadge: { | ||
| position: 'absolute', | ||
| bottom: 0, | ||
| right: 0, | ||
| width: components.badgeCheckSize, | ||
| height: components.badgeCheckSize, | ||
| borderRadius: components.badgeCheckSize / 2, | ||
| backgroundColor: colors.primaryWhite, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| }, | ||
| badgeImage: { | ||
| width: components.badgeCircleSize * 0.6, | ||
| height: components.badgeCircleSize * 0.6, | ||
| }, | ||
| label: { | ||
| fontSize: fontSize.XS, | ||
| fontFamily: fontFamily.interBold, | ||
| color: colors.primaryBlack, | ||
| textAlign: 'center', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Creating a StyleSheet dynamically inside a function or render path is a performance anti-pattern in React Native. StyleSheet.create should be called once at the module level so that styles are sent to the native bridge only once. Dynamic values (like backgroundColor based on active) should be passed as inline styles or merged with static styles.
export const styles = StyleSheet.create({
container: {
alignItems: 'center',
gap: spacing.Single,
},
circleWrapper: {
width: components.badgeCircleSize,
height: components.badgeCircleSize,
},
circle: {
width: components.badgeCircleSize,
height: components.badgeCircleSize,
borderRadius: components.badgeCircleSize / 2,
alignItems: 'center',
justifyContent: 'center',
},
checkBadge: {
position: 'absolute',
bottom: 0,
right: 0,
width: components.badgeCheckSize,
height: components.badgeCheckSize,
borderRadius: components.badgeCheckSize / 2,
backgroundColor: colors.primaryWhite,
alignItems: 'center',
justifyContent: 'center',
},
badgeImage: {
width: components.badgeCircleSize * 0.6,
height: components.badgeCircleSize * 0.6,
},
label: {
fontSize: fontSize.XS,
fontFamily: fontFamily.interBold,
color: colors.primaryBlack,
textAlign: 'center',
},
});| import { CustomIcon } from '@/features/core/ui/components/basic/CustomIcon/CustomIcon'; | ||
| import { CustomImage } from '@/features/core/ui/components/basic/CustomImage/CustomImage'; | ||
| import { CustomText } from '@/features/core/ui/components/basic/CustomText/CustomText'; | ||
| import { styles as badgeStyle } from '@/features/core/ui/components/composite/Badge/Badge.style'; | ||
| import { colors } from '@/features/core/ui/style/colors'; | ||
| import { spacing } from '@/features/core/ui/style/dimensions/spacing'; | ||
| import { icons } from '@/features/core/ui/style/icons'; | ||
| import type { FC } from 'react'; | ||
| import type { ImageSourcePropType } from 'react-native'; | ||
| import { View } from 'react-native'; | ||
|
|
||
| type BadgeProps = { | ||
| label: string; | ||
| image: ImageSourcePropType; | ||
| backgroundColor: string; | ||
| active: boolean; | ||
| }; | ||
|
|
||
| export const Badge: FC<BadgeProps> = ({ label, image, backgroundColor, active }) => { | ||
| const styles = badgeStyle(active, backgroundColor); | ||
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <View style={styles.circleWrapper}> | ||
| <View style={styles.circle}> | ||
| <CustomImage source={image} style={styles.badgeImage} /> | ||
| </View> | ||
| <View style={styles.checkBadge}> | ||
| <CustomIcon | ||
| name={active ? icons.success : icons.closeCircle} | ||
| size={spacing.TripleAndHalf} | ||
| color={active ? colors.tertiaryGreen : colors.primaryRed} | ||
| /> | ||
| </View> | ||
| </View> | ||
| <CustomText text={label.toLocaleUpperCase()} style={styles.label} /> | ||
| </View> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Update the Badge component to use the static stylesheet and apply the dynamic background color inline.
import { CustomIcon } from '@/features/core/ui/components/basic/CustomIcon/CustomIcon';
import { CustomImage } from '@/features/core/ui/components/basic/CustomImage/CustomImage';
import { CustomText } from '@/features/core/ui/components/basic/CustomText/CustomText';
import { styles } from '@/features/core/ui/components/composite/Badge/Badge.style';
import { colors } from '@/features/core/ui/style/colors';
import { spacing } from '@/features/core/ui/style/dimensions/spacing';
import { icons } from '@/features/core/ui/style/icons';
import type { FC } from 'react';
import type { ImageSourcePropType } from 'react-native';
import { View } from 'react-native';
type BadgeProps = {
label: string;
image: ImageSourcePropType;
backgroundColor: string;
active: boolean;
};
export const Badge: FC<BadgeProps> = ({ label, image, backgroundColor, active }) => {
return (
<View style={styles.container}>
<View style={styles.circleWrapper}>
<View style={[styles.circle, { backgroundColor: active ? backgroundColor : colors.primaryRed }]}>
<CustomImage source={image} style={styles.badgeImage} />
</View>
<View style={styles.checkBadge}>
<CustomIcon
name={active ? icons.success : icons.closeCircle}
size={spacing.TripleAndHalf}
color={active ? colors.tertiaryGreen : colors.primaryRed}
/>
</View>
</View>
<CustomText text={label.toLocaleUpperCase()} style={styles.label} />
</View>
);
};
| export const useDishDetailsModalPageLogic = () => { | ||
| const { tripId, searchTerm } = useLocalSearchParams<{ tripId: string; searchTerm: string }>(); | ||
| const { data, isLoading } = useGetWikimediaDishImage(searchTerm); | ||
| const { trip } = useGetTripById(tripId); | ||
|
|
||
| const dish = trip?.tripAiResp?.food?.typicalDishes.find(d => d.searchTerm === searchTerm); | ||
|
|
||
| const handleClose = () => navigationService.back(); | ||
|
|
||
| return { | ||
| dishName: dish?.name ?? '', | ||
| dishDescription: dish?.description ?? '', | ||
| dishIngredients: dish?.ingredients ?? [], | ||
| handleClose, | ||
| image: data?.url, | ||
| imageIsLoading: isLoading, | ||
| isVegetarian: dish?.isVegetarian ?? false, | ||
| isGlutenFree: dish?.isGlutenFree ?? false, | ||
| isVegan: dish?.isVegan ?? false, | ||
| glutenFreeImage, | ||
| veganImage, | ||
| vegetarianImage, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
To prevent an unwanted visual flash of empty content before the trip data loads, avoid prematurely applying fallback values (like ?? '' or ?? []) in the hook. Instead, expose the loading state of the trip query so that the consumer component (DishDetailsModalPage) can render appropriate skeleton screens for the title, description, and ingredients list.
export const useDishDetailsModalPageLogic = () => {
const { tripId, searchTerm } = useLocalSearchParams<{ tripId: string; searchTerm: string }>();
const { data, isLoading: imageIsLoading } = useGetWikimediaDishImage(searchTerm);
const { trip, isLoading: tripIsLoading } = useGetTripById(tripId);
const dish = trip?.tripAiResp?.food?.typicalDishes.find(d => d.searchTerm === searchTerm);
const handleClose = () => navigationService.back();
return {
dish,
isLoading: tripIsLoading,
handleClose,
image: data?.url,
imageIsLoading,
glutenFreeImage,
veganImage,
vegetarianImage,
};
};References
- Do not prematurely apply fallback values to query data in custom hooks if the consumer component handles the loading state (e.g., by showing a skeleton), as this can cause an unwanted visual flash instead of a smooth loading transition.
| export const DishDetailsModalPage = () => { | ||
| const { | ||
| dishName, | ||
| dishDescription, | ||
| dishIngredients, | ||
| handleClose, | ||
| image, | ||
| imageIsLoading, | ||
| isVegetarian, | ||
| isGlutenFree, | ||
| isVegan, | ||
| glutenFreeImage, | ||
| veganImage, | ||
| vegetarianImage, | ||
| } = useDishDetailsModalPageLogic(); | ||
|
|
||
| return ( | ||
| <ScrollView contentContainerStyle={styles.contentContainer} style={styles.container}> | ||
| <BottomSheetHeader title={dishName} onClose={handleClose} /> | ||
| <View style={styles.bodyContainer}> | ||
| {imageIsLoading ? ( | ||
| <BaseSkeleton style={styles.image} /> | ||
| ) : ( | ||
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | ||
| )} | ||
| <IngredientsList title="Ingredients" ingredients={dishIngredients} /> | ||
| </View> | ||
| <CustomText text={dishDescription} style={styles.description} /> | ||
| <View style={styles.badgesContainer}> | ||
| <Badge | ||
| label="MY_TRIP.GLUTEN_FREE" | ||
| image={glutenFreeImage} | ||
| backgroundColor={colors.tertiaryGreen} | ||
| active={isGlutenFree} | ||
| /> | ||
| <Badge label="MY_TRIP.VEGAN" image={veganImage} backgroundColor={colors.tertiaryGreen} active={isVegan} /> | ||
| <Badge | ||
| label="MY_TRIP.VEGETARIAN" | ||
| image={vegetarianImage} | ||
| backgroundColor={colors.tertiaryGreen} | ||
| active={isVegetarian} | ||
| /> | ||
| </View> | ||
| </ScrollView> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
To prevent an unwanted visual flash of empty content before the trip data loads, avoid prematurely applying fallback values in the hook. Instead, handle the loading state of the trip query by rendering appropriate skeleton screens for the title, description, and ingredients list. Additionally, the string 'Ingredients' is hardcoded; it should be defined in the translation files and referenced using an i18n key (e.g., MY_TRIP.INGREDIENTS) to support internationalization.
export const DishDetailsModalPage = () => {
const {
dish,
isLoading,
handleClose,
image,
imageIsLoading,
glutenFreeImage,
veganImage,
vegetarianImage,
} = useDishDetailsModalPageLogic();
if (isLoading || !dish) {
return (
<ScrollView contentContainerStyle={styles.contentContainer} style={styles.container}>
<BottomSheetHeader title="" onClose={handleClose} />
<View style={styles.bodyContainer}>
<BaseSkeleton style={styles.image} />
<View style={{ flex: 1, gap: 10 }}>
<BaseSkeleton style={{ height: 20, width: '60%' }} />
<BaseSkeleton style={{ height: 40, width: '100%' }} />
</View>
</View>
<BaseSkeleton style={{ height: 100, width: '100%', marginTop: 20 }} />
</ScrollView>
);
}
return (
<ScrollView contentContainerStyle={styles.contentContainer} style={styles.container}>
<BottomSheetHeader title={dish.name} onClose={handleClose} />
<View style={styles.bodyContainer}>
{imageIsLoading ? (
<BaseSkeleton style={styles.image} />
) : (
<CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} />
)}
<IngredientsList title="MY_TRIP.INGREDIENTS" ingredients={dish.ingredients} />
</View>
<CustomText text={dish.description} style={styles.description} />
<View style={styles.badgesContainer}>
<Badge
label="MY_TRIP.GLUTEN_FREE"
image={glutenFreeImage}
backgroundColor={colors.tertiaryGreen}
active={dish.isGlutenFree}
/>
<Badge label="MY_TRIP.VEGAN" image={veganImage} backgroundColor={colors.tertiaryGreen} active={dish.isVegan} />
<Badge
label="MY_TRIP.VEGETARIAN"
image={vegetarianImage}
backgroundColor={colors.tertiaryGreen}
active={dish.isVegetarian}
/>
</View>
</ScrollView>
);
};
References
- Do not prematurely apply fallback values to query data in custom hooks if the consumer component handles the loading state (e.g., by showing a skeleton), as this can cause an unwanted visual flash instead of a smooth loading transition.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
Badge.tsxyou referenceicons.success, but the icons map only declarescheckmarkandcloseCircle, so this will resolve toundefinedat runtime—either addsuccessto the icons map or reuse an existing icon key. - The
Badgecomponent uppercases translation keys vialabel.toLocaleUpperCase()before passing them toCustomText, which likely expects the raw i18n key; this will break lookups—consider leaving the key unchanged and handling casing via styles or by uppercasing the translated text instead. - The new
IngredientsListandDishDetailsModalPagecomponents mix plain strings (e.g."Ingredients") and translation keys; for consistency with the rest of the app, consider standardizing on translation keys and lettingCustomTexthandle localization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Badge.tsx` you reference `icons.success`, but the icons map only declares `checkmark` and `closeCircle`, so this will resolve to `undefined` at runtime—either add `success` to the icons map or reuse an existing icon key.
- The `Badge` component uppercases translation keys via `label.toLocaleUpperCase()` before passing them to `CustomText`, which likely expects the raw i18n key; this will break lookups—consider leaving the key unchanged and handling casing via styles or by uppercasing the translated text instead.
- The new `IngredientsList` and `DishDetailsModalPage` components mix plain strings (e.g. `"Ingredients"`) and translation keys; for consistency with the rest of the app, consider standardizing on translation keys and letting `CustomText` handle localization.
## Individual Comments
### Comment 1
<location path="features/core/ui/components/composite/Badge/Badge.tsx" line_range="29-30" />
<code_context>
+ <CustomImage source={image} style={styles.badgeImage} />
+ </View>
+ <View style={styles.checkBadge}>
+ <CustomIcon
+ name={active ? icons.success : icons.closeCircle}
+ size={spacing.TripleAndHalf}
+ color={active ? colors.tertiaryGreen : colors.primaryRed}
</code_context>
<issue_to_address>
**issue (bug_risk):** The `icons.success` key is not defined in the icons map and will likely cause a runtime error.
`icons.checkmark` is the defined success glyph in the icons map; `icons.success` does not exist. As written, this will pass `undefined` as the icon name, leading to a missing or broken icon at runtime. Please switch to `icons.checkmark` (or another existing key) so the success state renders properly.
</issue_to_address>
### Comment 2
<location path="features/trips/ui/components/IngredientsList/IngredientsList.tsx" line_range="14-24" />
<code_context>
+ <View style={styles.container}>
+ <CustomText text={title.toLocaleUpperCase()} style={styles.title} />
+ <View style={styles.chipsRow}>
+ {ingredients.map(ingredient => (
+ <Cheap
+ key={ingredient}
+ title={ingredient}
+ color={colors.secondaryGrey}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using the ingredient value as the React key can lead to duplicate keys if the list contains repeated ingredients.
If an ingredient appears more than once, React will get duplicate keys, which can cause unstable rendering. Use a guaranteed-unique key instead, such as the map index or a combination of value and index (e.g. `key={`${ingredient}-${index}`}`).
```suggestion
<View style={styles.chipsRow}>
{ingredients.map((ingredient, index) => (
<Cheap
key={`${ingredient}-${index}`}
title={ingredient}
color={colors.secondaryGrey}
icon={icons.checkmark}
uppercase={false}
/>
))}
</View>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <CustomIcon | ||
| name={active ? icons.success : icons.closeCircle} |
There was a problem hiding this comment.
issue (bug_risk): The icons.success key is not defined in the icons map and will likely cause a runtime error.
icons.checkmark is the defined success glyph in the icons map; icons.success does not exist. As written, this will pass undefined as the icon name, leading to a missing or broken icon at runtime. Please switch to icons.checkmark (or another existing key) so the success state renders properly.
| <View style={styles.chipsRow}> | ||
| {ingredients.map(ingredient => ( | ||
| <Cheap | ||
| key={ingredient} | ||
| title={ingredient} | ||
| color={colors.secondaryGrey} | ||
| icon={icons.checkmark} | ||
| uppercase={false} | ||
| /> | ||
| ))} | ||
| </View> |
There was a problem hiding this comment.
suggestion (bug_risk): Using the ingredient value as the React key can lead to duplicate keys if the list contains repeated ingredients.
If an ingredient appears more than once, React will get duplicate keys, which can cause unstable rendering. Use a guaranteed-unique key instead, such as the map index or a combination of value and index (e.g. key={${ingredient}-${index}}).
| <View style={styles.chipsRow}> | |
| {ingredients.map(ingredient => ( | |
| <Cheap | |
| key={ingredient} | |
| title={ingredient} | |
| color={colors.secondaryGrey} | |
| icon={icons.checkmark} | |
| uppercase={false} | |
| /> | |
| ))} | |
| </View> | |
| <View style={styles.chipsRow}> | |
| {ingredients.map((ingredient, index) => ( | |
| <Cheap | |
| key={`${ingredient}-${index}`} | |
| title={ingredient} | |
| color={colors.secondaryGrey} | |
| icon={icons.checkmark} | |
| uppercase={false} | |
| /> | |
| ))} | |
| </View> |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/trips/ui/components/FoodCard/components/DishItem/DishItem.tsx`:
- Line 14: The Pressable in the DishItem component needs accessibility
semantics: add accessibilityRole="button" to the Pressable and supply an
accessibilityLabel derived from the dish name prop (e.g., `${name}, button` or
similar) so screen readers announce it properly; also forward accessibilityState
(e.g., disabled if onPress is undefined) and keep the existing onPress and style
props (look for the Pressable using styles.container and the onPress handler in
DishItem) so behavior and visual styling are unchanged.
In `@features/trips/ui/components/IngredientsList/IngredientsList.tsx`:
- Around line 15-18: The current IngredientsList map uses key={ingredient} which
can collide for duplicate ingredients; update the map in the IngredientsList
component to use a guaranteed-unique key (e.g., include the map index or a
unique id) by changing the map callback to accept the index and pass a composite
key such as `${ingredient}-${index}` (or use an ingredient unique identifier if
one exists) to the Cheap component's key prop to avoid reconciliation issues.
In `@features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx`:
- Around line 27-31: When imageIsLoading is false but image is missing,
CustomImage may receive an undefined source; update the rendering logic in
DishDetailsModalPage so it checks for a loaded-but-no-image case before
rendering CustomImage. Specifically, inside the JSX that currently switches on
imageIsLoading, add an explicit branch for when imageIsLoading === false &&
!image to render a fallback (e.g., BaseSkeleton or a placeholder view) using
styles.image, otherwise render CustomImage with the existing source logic;
adjust the conditional that references imageIsLoading, image, CustomImage, and
BaseSkeleton accordingly.
- Line 32: Replace the hardcoded "Ingredients" string passed to IngredientsList
in DishDetailsModalPage with a localized string lookup (e.g., use your i18n
translate function such as t('dishDetails.ingredients') or a locale prop) so the
title comes from translations; update the IngredientsList invocation to:
title={t('...')} (or equivalent) and add the corresponding translation key to
your locale files (refer to IngredientsList, dishIngredients, and
DishDetailsModalPage to locate the change).
🪄 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: CHILL
Plan: Pro
Run ID: 6abe06b9-c0a8-4400-a4b5-a51e851ab70c
⛔ Files ignored due to path filters (3)
features/core/ui/assets/images/gluten_free.pngis excluded by!**/*.pngfeatures/core/ui/assets/images/vegan.pngis excluded by!**/*.pngfeatures/core/ui/assets/images/vegetarian.pngis excluded by!**/*.png
📒 Files selected for processing (35)
app/(main)/(authenticated)/create-trip/_layout.tsxapp/(main)/(authenticated)/create-trip/dish-details-modal.tsxconvex/validators/Trips.tsfeatures/core/navigation/data/services/NavigationService.tsfeatures/core/navigation/domain/entities/Modals.tsfeatures/core/navigation/domain/entities/services/INavigationService.tsfeatures/core/translations/libraries/locales/en.jsonfeatures/core/translations/libraries/locales/it.jsonfeatures/core/ui/components/basic/Cheap/Cheap.tsxfeatures/core/ui/components/composite/Badge/Badge.style.tsfeatures/core/ui/components/composite/Badge/Badge.tsxfeatures/core/ui/components/composite/BottomSheetHeader/BottomSheetHeader.style.tsfeatures/core/ui/components/composite/BottomSheetHeader/BottomSheetHeader.tsxfeatures/core/ui/index.tsfeatures/core/ui/style/colors.tsfeatures/core/ui/style/dimensions/components.tsfeatures/core/ui/style/dimensions/images.tsfeatures/core/ui/style/icons.tsfeatures/trip-generation/domain/schemas/GenerateTripSchema.tsfeatures/trips/domain/entities/TypicalDish.tsfeatures/trips/pages.tsfeatures/trips/ui/components/FoodCard/FoodCard.style.tsfeatures/trips/ui/components/FoodCard/FoodCard.tsxfeatures/trips/ui/components/FoodCard/components/DishItem/DishItem.style.tsfeatures/trips/ui/components/FoodCard/components/DishItem/DishItem.tsxfeatures/trips/ui/components/IngredientsList/IngredientsList.style.tsfeatures/trips/ui/components/IngredientsList/IngredientsList.tsxfeatures/trips/ui/components/TypicalDishesList/TypicalDishesList.style.tsfeatures/trips/ui/components/TypicalDishesList/TypicalDishesList.tsxfeatures/trips/ui/components/TypicalDishesModalHeader/TypicalDishesModalHeader.style.tsfeatures/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.logic.tsfeatures/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.style.tsfeatures/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsxfeatures/trips/ui/pages/TypicalDishesModalPage/TypicalDishesModalPage.logic.tsfeatures/trips/ui/pages/TypicalDishesModalPage/TypicalDishesModalPage.tsx
💤 Files with no reviewable changes (1)
- features/trips/ui/components/TypicalDishesModalHeader/TypicalDishesModalHeader.style.ts
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <Pressable style={({ pressed }) => [styles.container, pressed && styles.pressed]} onPress={onPress}> |
There was a problem hiding this comment.
Add accessibility semantics to the new tappable row.
On Line 14, the new Pressable should expose button semantics for screen readers (accessibilityRole, and ideally a label based on the dish name) so assistive-tech users can reliably identify and activate it.
Suggested patch
- <Pressable style={({ pressed }) => [styles.container, pressed && styles.pressed]} onPress={onPress}>
+ <Pressable
+ style={({ pressed }) => [styles.container, pressed && styles.pressed]}
+ onPress={onPress}
+ accessibilityRole="button"
+ accessibilityLabel={dish.name}
+ >📝 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.
| <Pressable style={({ pressed }) => [styles.container, pressed && styles.pressed]} onPress={onPress}> | |
| <Pressable | |
| style={({ pressed }) => [styles.container, pressed && styles.pressed]} | |
| onPress={onPress} | |
| accessibilityRole="button" | |
| accessibilityLabel={dish.name} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/trips/ui/components/FoodCard/components/DishItem/DishItem.tsx` at
line 14, The Pressable in the DishItem component needs accessibility semantics:
add accessibilityRole="button" to the Pressable and supply an accessibilityLabel
derived from the dish name prop (e.g., `${name}, button` or similar) so screen
readers announce it properly; also forward accessibilityState (e.g., disabled if
onPress is undefined) and keep the existing onPress and style props (look for
the Pressable using styles.container and the onPress handler in DishItem) so
behavior and visual styling are unchanged.
| {ingredients.map(ingredient => ( | ||
| <Cheap | ||
| key={ingredient} | ||
| title={ingredient} |
There was a problem hiding this comment.
Use a guaranteed-unique key for ingredient chips.
On Line 17, key={ingredient} will collide when the same ingredient appears more than once, which can cause incorrect list reconciliation.
Suggested fix
- {ingredients.map(ingredient => (
+ {ingredients.map((ingredient, index) => (
<Cheap
- key={ingredient}
+ key={`${ingredient}-${index}`}
title={ingredient}
color={colors.secondaryGrey}
icon={icons.checkmark}
uppercase={false}
/>
))}📝 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.
| {ingredients.map(ingredient => ( | |
| <Cheap | |
| key={ingredient} | |
| title={ingredient} | |
| {ingredients.map((ingredient, index) => ( | |
| <Cheap | |
| key={`${ingredient}-${index}`} | |
| title={ingredient} | |
| color={colors.secondaryGrey} | |
| icon={icons.checkmark} | |
| uppercase={false} | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/trips/ui/components/IngredientsList/IngredientsList.tsx` around
lines 15 - 18, The current IngredientsList map uses key={ingredient} which can
collide for duplicate ingredients; update the map in the IngredientsList
component to use a guaranteed-unique key (e.g., include the map index or a
unique id) by changing the map callback to accept the index and pass a composite
key such as `${ingredient}-${index}` (or use an ingredient unique identifier if
one exists) to the Cheap component's key prop to avoid reconciliation issues.
| {imageIsLoading ? ( | ||
| <BaseSkeleton style={styles.image} /> | ||
| ) : ( | ||
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | ||
| )} |
There was a problem hiding this comment.
Handle the resolved-without-image case before rendering CustomImage.
On Line 30, when imageIsLoading is false and image is missing, CustomImage gets an undefined source. Add an explicit fallback branch for “loaded but no image”.
Suggested fix
- {imageIsLoading ? (
+ {imageIsLoading ? (
<BaseSkeleton style={styles.image} />
+ ) : !image ? (
+ <View style={styles.image} />
) : (
<CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} />
)}📝 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.
| {imageIsLoading ? ( | |
| <BaseSkeleton style={styles.image} /> | |
| ) : ( | |
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | |
| )} | |
| {imageIsLoading ? ( | |
| <BaseSkeleton style={styles.image} /> | |
| ) : !image ? ( | |
| <View style={styles.image} /> | |
| ) : ( | |
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx` around
lines 27 - 31, When imageIsLoading is false but image is missing, CustomImage
may receive an undefined source; update the rendering logic in
DishDetailsModalPage so it checks for a loaded-but-no-image case before
rendering CustomImage. Specifically, inside the JSX that currently switches on
imageIsLoading, add an explicit branch for when imageIsLoading === false &&
!image to render a fallback (e.g., BaseSkeleton or a placeholder view) using
styles.image, otherwise render CustomImage with the existing source logic;
adjust the conditional that references imageIsLoading, image, CustomImage, and
BaseSkeleton accordingly.
| ) : ( | ||
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | ||
| )} | ||
| <IngredientsList title="Ingredients" ingredients={dishIngredients} /> |
There was a problem hiding this comment.
Avoid hardcoded English copy in the modal body.
Line 32 uses a literal "Ingredients", which bypasses localization.
Suggested fix
- <IngredientsList title="Ingredients" ingredients={dishIngredients} />
+ <IngredientsList title="MY_TRIP.INGREDIENTS" ingredients={dishIngredients} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx` at
line 32, Replace the hardcoded "Ingredients" string passed to IngredientsList in
DishDetailsModalPage with a localized string lookup (e.g., use your i18n
translate function such as t('dishDetails.ingredients') or a locale prop) so the
title comes from translations; update the IngredientsList invocation to:
title={t('...')} (or equivalent) and add the corresponding translation key to
your locale files (refer to IngredientsList, dishIngredients, and
DishDetailsModalPage to locate the change).
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="features/trips/ui/components/FoodCard/components/DishItem/DishItem.logic.ts">
<violation number="1" location="features/trips/ui/components/FoodCard/components/DishItem/DishItem.logic.ts:4">
P2: Duplicate `require()` calls for the same dietary badge images across two logic files. The same three images (`gluten_free.png`, `vegan.png`, `vegetarian.png`) are required independently in `DishItem.logic.ts` and `DishDetailsModalPage.logic.ts`, creating a maintenance burden when adding/removing badges.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export const useDishItemLogic = (searchTerm: string) => { | ||
| const { data, isLoading } = useGetWikimediaDishImage(searchTerm); | ||
| return { image: data?.url, isLoading }; | ||
| const glutenFreeImage = require('@/features/core/ui/assets/images/gluten_free.png'); |
There was a problem hiding this comment.
P2: Duplicate require() calls for the same dietary badge images across two logic files. The same three images (gluten_free.png, vegan.png, vegetarian.png) are required independently in DishItem.logic.ts and DishDetailsModalPage.logic.ts, creating a maintenance burden when adding/removing badges.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At features/trips/ui/components/FoodCard/components/DishItem/DishItem.logic.ts, line 4:
<comment>Duplicate `require()` calls for the same dietary badge images across two logic files. The same three images (`gluten_free.png`, `vegan.png`, `vegetarian.png`) are required independently in `DishItem.logic.ts` and `DishDetailsModalPage.logic.ts`, creating a maintenance burden when adding/removing badges.</comment>
<file context>
@@ -1,6 +1,23 @@
-export const useDishItemLogic = (searchTerm: string) => {
- const { data, isLoading } = useGetWikimediaDishImage(searchTerm);
- return { image: data?.url, isLoading };
+const glutenFreeImage = require('@/features/core/ui/assets/images/gluten_free.png');
+const veganImage = require('@/features/core/ui/assets/images/vegan.png');
+const vegetarianImage = require('@/features/core/ui/assets/images/vegetarian.png');
</file context>
There was a problem hiding this comment.
3 issues found across 39 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="features/trips/ui/components/FoodCard/components/DishItem/DishItem.tsx">
<violation number="1" location="features/trips/ui/components/FoodCard/components/DishItem/DishItem.tsx:42">
P2: Badge images missing accessibilityLabel — dietary indicators have no accessible description for screen readers.</violation>
</file>
<file name="features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx">
<violation number="1" location="features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx:32">
P2: The ingredients section title is hardcoded in English, so it won’t localize with the rest of the modal.</violation>
</file>
<file name="convex/validators/Trips.ts">
<violation number="1" location="convex/validators/Trips.ts:57">
P2: `isVegan` is introduced as a required persisted field without a compatibility path, which can break mutations on existing trip documents missing that field.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -3,15 +3,25 @@ import type { TypicalDish } from '@/features/trips/domain/entities/TypicalDish'; | |||
| import { useDishItemLogic } from '@/features/trips/ui/components/FoodCard/components/DishItem/DishItem.logic'; | |||
There was a problem hiding this comment.
P2: Badge images missing accessibilityLabel — dietary indicators have no accessible description for screen readers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At features/trips/ui/components/FoodCard/components/DishItem/DishItem.tsx, line 42:
<comment>Badge images missing accessibilityLabel — dietary indicators have no accessible description for screen readers.</comment>
<file context>
@@ -21,8 +31,20 @@ export const DishItem: FC<DishItemProps> = ({ dish }) => {
+ />
+ {hasBadge && (
+ <View style={styles.badgeContainer}>
+ {isGlutenFree && <CustomImage source={glutenFreeImage} style={styles.badge} />}
+ {isVegan && <CustomImage source={veganImage} style={styles.badge} />}
+ {isVegetarian && <CustomImage source={vegetarianImage} style={styles.badge} />}
</file context>
| ) : ( | ||
| <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} /> | ||
| )} | ||
| <IngredientsList title="Ingredients" ingredients={dishIngredients} /> |
There was a problem hiding this comment.
P2: The ingredients section title is hardcoded in English, so it won’t localize with the rest of the modal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At features/trips/ui/pages/DishDetailsModalPage/DishDetailsModalPage.tsx, line 32:
<comment>The ingredients section title is hardcoded in English, so it won’t localize with the rest of the modal.</comment>
<file context>
@@ -0,0 +1,52 @@
+ ) : (
+ <CustomImage source={typeof image === 'string' ? { uri: image } : image} style={styles.image} />
+ )}
+ <IngredientsList title="Ingredients" ingredients={dishIngredients} />
+ </View>
+ <CustomText text={dishDescription} style={styles.description} />
</file context>
| ingredients: v.array(v.string()), | ||
| isGlutenFree: v.boolean(), | ||
| isVegetarian: v.boolean(), | ||
| isVegan: v.boolean(), |
There was a problem hiding this comment.
P2: isVegan is introduced as a required persisted field without a compatibility path, which can break mutations on existing trip documents missing that field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At convex/validators/Trips.ts, line 57:
<comment>`isVegan` is introduced as a required persisted field without a compatibility path, which can break mutations on existing trip documents missing that field.</comment>
<file context>
@@ -54,6 +54,7 @@ export const TypicalDish = v.object({
ingredients: v.array(v.string()),
isGlutenFree: v.boolean(),
isVegetarian: v.boolean(),
+ isVegan: v.boolean(),
});
</file context>
| isVegan: v.boolean(), | |
| isVegan: v.optional(v.boolean()), |
Fixes #356
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Localization