Conversation
|
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:
📝 WalkthroughWalkthroughExtracted FavoritesScreen UI into a private Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt (1)
157-160: Move the new error copy into string resources.
"Failed to obtain eatery data"should come fromstrings.xmlfor localization and consistency.💡 Suggested change
+import androidx.compose.ui.res.stringResource ... - EateriesEmptyState("Failed to obtain eatery data") + EateriesEmptyState(stringResource(R.string.favorites_load_error))And add
favorites_load_errorinapp/src/main/res/values/strings.xml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt` around lines 157 - 160, Replace the hardcoded error string used in the FavoritesScreenViewState.Error branch (where EateriesEmptyState("Failed to obtain eatery data") is called) with a localized string resource; add an entry favorites_load_error to app/src/main/res/values/strings.xml and use that resource (via stringResource or context.getString) when constructing the EateriesEmptyState so the message is localized and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt`:
- Around line 157-160: Replace the hardcoded error string used in the
FavoritesScreenViewState.Error branch (where EateriesEmptyState("Failed to
obtain eatery data") is called) with a localized string resource; add an entry
favorites_load_error to app/src/main/res/values/strings.xml and use that
resource (via stringResource or context.getString) when constructing the
EateriesEmptyState so the message is localized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1953d28-086a-4243-b62c-0b013b58994f
📒 Files selected for processing (1)
app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt (1)
80-105: Good extraction for preview support.The refactoring cleanly separates ViewModel wiring from UI rendering, enabling preview coverage.
Minor naming inconsistency: the parameter
removeFavoriteMenuItem(line 101) is assignedtoggleFavoriteMenuItem(line 88). Consider aligning the naming for clarity—either rename the parameter totoggleFavoriteMenuItemor rename the ViewModel method toremoveFavoriteMenuItemif it's a one-way removal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt` around lines 80 - 105, The parameter name removeFavoriteMenuItem in FavoritesScreenContent is mismatched with the passed favoriteViewModel::toggleFavoriteMenuItem; rename one for clarity—either (A) rename the FavoritesScreenContent parameter to toggleFavoriteMenuItem and update its type/signature to reflect a toggling action, or (B) change the caller to pass favoriteViewModel::removeFavoriteMenuItem (or implement removeFavoriteMenuItem on the ViewModel) if the operation is strictly a removal; update all references to the chosen symbol (removeFavoriteMenuItem or toggleFavoriteMenuItem) to maintain consistent naming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt`:
- Around line 80-105: The parameter name removeFavoriteMenuItem in
FavoritesScreenContent is mismatched with the passed
favoriteViewModel::toggleFavoriteMenuItem; rename one for clarity—either (A)
rename the FavoritesScreenContent parameter to toggleFavoriteMenuItem and update
its type/signature to reflect a toggling action, or (B) change the caller to
pass favoriteViewModel::removeFavoriteMenuItem (or implement
removeFavoriteMenuItem on the ViewModel) if the operation is strictly a removal;
update all references to the chosen symbol (removeFavoriteMenuItem or
toggleFavoriteMenuItem) to maintain consistent naming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11956563-3a25-4bd3-be72-9d2a9218fe4f
📒 Files selected for processing (2)
app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.ktapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/strings.xml
Extract strings into strings.xml
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/components/general/FilterRow.kt`:
- Line 88: Replace the hardcoded accessibility label in the FilterRow composable
— change contentDescription = "Expand filters" to use a localized string
resource (e.g., contentDescription = stringResource(R.string.expand_filters))
and add the corresponding <string name="expand_filters">Expand filters</string>
to strings.xml if missing so the label is localized.
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/ReportBottomSheet.kt`:
- Around line 189-190: Selected option labels are using raw English strings via
Issue.option which bypasses localization; replace usages to read the string
resource ID (Issue.optionRes) and call stringResource(...) when rendering. In
ReportBottomSheet update the text binding that currently uses
selectedIssue?.option to use stringResource(selectedIssue?.optionRes ?:
R.string.report_choose_option) (or equivalent null-coalescing to the
choose-option string), and in IssueBottomSheet set title =
stringResource(issue.optionRes) instead of using issue.option.
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MealBottomSheet.kt`:
- Around line 120-126: In MealBottomSheet.kt replace each Icon's
contentDescription = null inside the IconButton (the selected/unselected Icon
pairs shown) with a stringResource that reflects the meal and state (use e.g.
stringResource(R.string.a11y_meal_selected_breakfast) when the meal is currently
selected and stringResource(R.string.a11y_meal_select_breakfast) when
unselected); apply the same pattern for lunch, dinner, and late_night icons (use
unique resource names like a11y_meal_selected_lunch / a11y_meal_select_lunch,
etc.), and add those string entries to strings.xml so TalkBack users receive
meaningful labels.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2dc6524a-154f-486a-a8a2-a1536cfb8c85
📒 Files selected for processing (35)
app/src/main/java/com/cornellappdev/android/eatery/ui/components/details/CalendarButton.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/details/EateryHourBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/details/EateryMenusBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/AppStoreRatingPopup.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/FavoriteButton.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/FilterRow.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/NoEateryFound.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/PaymentMethodsAvailable.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/PaymentMethodsBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/PermissionRequestDialog.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/SearchBar.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/home/EateryHomeSection.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/notifications/FavoriteItemRow.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/AppIconBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/ReportBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MealBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MenuCard.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/AboutScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/CompareMenusScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/HomeScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/LegalScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/NearestScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/NotificationsHomeScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/NotificationsSettingsScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/OnboardingScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/PrivacyScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/SearchScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/SettingsScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/SupportScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/UpcomingMenuScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/viewmodels/FavoritesViewModel.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/viewmodels/state/NetworkErrorHandler.ktapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (11)
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/AppIconBottomSheet.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/screens/NearestScreen.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/general/NoEateryFound.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/screens/LegalScreen.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/details/EateryMenusBottomSheet.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MenuCard.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/general/SearchBar.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/details/CalendarButton.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/home/EateryHomeSection.kt
- app/src/main/res/values/strings.xml
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/notifications/FavoriteItemRow.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/cornellappdev/android/eatery/ui/screens/FavoritesScreen.kt
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)
app/src/main/java/com/cornellappdev/android/eatery/ui/components/login/AccountPage.kt (1)
350-367:⚠️ Potential issue | 🟡 MinorLocalize settings icon accessibility text.
Line 365 and Line 386 still use
Icons.Outlined.Settings.name, which is not localized and can be awkward for TalkBack. Use a string resource instead.Proposed fix
@@ IconButton( modifier = Modifier.align(Alignment.CenterEnd), onClick = onSettingsClicked ) { Icon( modifier = Modifier.size(28.dp), imageVector = Icons.Outlined.Settings, - contentDescription = Icons.Outlined.Settings.name, + contentDescription = stringResource(R.string.a11y_settings), tint = Color.White ) } @@ IconButton( modifier = Modifier .padding(end = 16.dp) .align(Alignment.End) .size(32.dp), onClick = { onSettingsClicked() }) { Icon( modifier = Modifier.size(28.dp), imageVector = Icons.Outlined.Settings, - contentDescription = Icons.Outlined.Settings.name, + contentDescription = stringResource(R.string.a11y_settings), tint = Color.White ) }<!-- app/src/main/res/values/strings.xml --> <string name="a11y_settings">Open settings</string>Also applies to: 396-397
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/android/eatery/ui/components/login/AccountPage.kt` around lines 350 - 367, Replace the non-localized contentDescription that uses Icons.Outlined.Settings.name with a string resource (e.g., R.string.a11y_settings) for all occurrences where the settings Icon is created (look for Icon usages inside IconButton and any other Icon with imageVector = Icons.Outlined.Settings); update the UI composables (IconButton/Icon) to call stringResource(R.string.a11y_settings) for contentDescription so TalkBack reads a localized descriptive string instead of the enum name.
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.kt (2)
548-551: Avoid duplicate TalkBack announcements on icon+text buttons.Since the button text already labels the action, these icon
contentDescriptions can benullto prevent redundant screen-reader output.♿ Suggested accessibility tweak
Icon( painter = painterResource(id = R.drawable.ic_android_phone), - contentDescription = stringResource(R.string.a11y_phone_order_online) + contentDescription = null ) @@ Icon( painter = painterResource(id = R.drawable.ic_walk), - contentDescription = stringResource(R.string.a11y_walk_get_directions) + contentDescription = null )Also applies to: 602-605
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.kt` around lines 548 - 551, The Icon composable within EateryDetailScreen is providing a contentDescription that duplicates the button text (painterResource R.drawable.ic_android_phone with contentDescription = stringResource(R.string.a11y_phone_order_online)); change the Icon's contentDescription to null so TalkBack won't announce the icon in addition to the button label; apply the same change to the other Icon instance noted (the block around the second occurrence at the referenced lines) so both icon+text buttons use null contentDescription and rely on the button text for accessibility.
464-464: Keep loading fallback localization consistent in sticky header.
eatery.name ?: stringResource(R.string.loading)was updated in one place, butEateryHeaderstill uses hardcoded"Loading...", which can create mixed-language UI.🌐 Suggested consistency fix
- text = eatery.name ?: "Loading...", + text = eatery.name ?: stringResource(R.string.loading),Also applies to: 934-940
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.kt` at line 464, Update EateryHeader to use the localized loading fallback instead of the hardcoded "Loading...": replace occurrences of the literal "Loading..." in the EateryHeader composable (and the similar instances around the EateryDetailScreen refs noted near the 934-940 region) with stringResource(R.string.loading) so the sticky header and other UI elements use the same localized fallback as the rest of the screen.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/ReportBottomSheet.kt`:
- Around line 93-94: The selectedIssueLabel (stringResource) is UI text and must
not be sent to sendReport because it localizes the issue identifier; change the
value passed into sendReport (and anywhere you build the report content like the
"$issue: $report" concatenation) to use selectedIssue?.name (the stable enum
identifier) while continuing to use selectedIssueLabel for display in the UI and
any localized placeholders; update calls to sendReport and the content
construction in ReportBottomSheet (references: selectedIssueLabel,
selectedIssue?.name, sendReport, content) so the backend receives the stable
identifier instead of localized text.
---
Outside diff comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/components/login/AccountPage.kt`:
- Around line 350-367: Replace the non-localized contentDescription that uses
Icons.Outlined.Settings.name with a string resource (e.g.,
R.string.a11y_settings) for all occurrences where the settings Icon is created
(look for Icon usages inside IconButton and any other Icon with imageVector =
Icons.Outlined.Settings); update the UI composables (IconButton/Icon) to call
stringResource(R.string.a11y_settings) for contentDescription so TalkBack reads
a localized descriptive string instead of the enum name.
---
Nitpick comments:
In
`@app/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.kt`:
- Around line 548-551: The Icon composable within EateryDetailScreen is
providing a contentDescription that duplicates the button text (painterResource
R.drawable.ic_android_phone with contentDescription =
stringResource(R.string.a11y_phone_order_online)); change the Icon's
contentDescription to null so TalkBack won't announce the icon in addition to
the button label; apply the same change to the other Icon instance noted (the
block around the second occurrence at the referenced lines) so both icon+text
buttons use null contentDescription and rely on the button text for
accessibility.
- Line 464: Update EateryHeader to use the localized loading fallback instead of
the hardcoded "Loading...": replace occurrences of the literal "Loading..." in
the EateryHeader composable (and the similar instances around the
EateryDetailScreen refs noted near the 934-940 region) with
stringResource(R.string.loading) so the sticky header and other UI elements use
the same localized fallback as the rest of the screen.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 559aafde-3c14-4168-b840-a73df781d87c
📒 Files selected for processing (11)
app/src/main/java/com/cornellappdev/android/eatery/ui/components/general/FilterRow.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/general/SearchBar.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/login/AccountPage.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/notifications/FavoriteItemRow.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/settings/ReportBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MealBottomSheet.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MenuCard.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/EateryDetailScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/HomeScreen.ktapp/src/main/java/com/cornellappdev/android/eatery/ui/screens/SearchScreen.ktapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (4)
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/notifications/FavoriteItemRow.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/screens/SearchScreen.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/screens/HomeScreen.kt
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MenuCard.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/general/FilterRow.kt
- app/src/main/java/com/cornellappdev/android/eatery/ui/components/upcoming/MealBottomSheet.kt
This was referenced Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Related PRs or Issues
Screenshots
Summary by CodeRabbit
Refactor
Style
Chores
Accessibility / Localization
Testing / Preview