Implement Add Favorites Button #119
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an “Add Favorites” entry point in the Favorites tab and updates the add-favorites flow to use a ModalBottomSheet, keeping the sheet open while confirming additions via toast.
Changes:
- Introduces an
AddFavoriteButton(with new plus icon) and wires it into the Favorites list UI. - Refactors the Add Favorites sheet from
BottomSheetScaffoldtoModalBottomSheet, with ViewModel-backed visibility state. - Updates add-item behavior to show a toast confirmation instead of dismissing the sheet per item.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/drawable/ic_addition.xml | Adds a plus icon used by the new Add Favorites button. |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt | Adds state + setter for controlling Add Favorites sheet visibility. |
| app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt | Switches to ModalBottomSheet for Add Favorites and adjusts add-item behavior/toasts. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt | Inserts the new Add Favorites button into the Favorites filter list and plumbs callback. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/BottomSheetFilterItem.kt | Updates label color to use theme tokens for active/inactive states. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesSearchSheet.kt | Minor modifier ordering change for layout. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoriteButton.kt | New composable for the “Add Favorites” button with icon + styling. |
| app/build.gradle.kts | Updates build type signing configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesButton.kt
Show resolved
Hide resolved
app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoriteButton.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoriteButton.kt
Outdated
Show resolved
Hide resolved
| onAddFavoriteClick: () -> Unit, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| Button( |
There was a problem hiding this comment.
Nit: In the future, would be good to have a general button component eg. NaviButton that you can reuse for any type of button component to reduce redundancy
|
Would be good to link to figma designs in the PR description for reviewers to reference |
| ), | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .padding(horizontal = 12.dp, vertical = 20.dp) |
There was a problem hiding this comment.
horizontal padding here doesn't seem necessary if you are giving it fillmaxwidth. Should probably give it any horizontal padding outside of component if necessary
There was a problem hiding this comment.
Same thing with vertical padding. If you need padding for this component for the composables inside the button itself, then you should probably use the contentPadding parameter of button
app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesButton.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
Outdated
Show resolved
Hide resolved
| ExperimentalFoundationApi::class, | ||
| ) | ||
| @Composable | ||
| fun HomeScreen( |
There was a problem hiding this comment.
Nit: eventually, we should refactor so that there is something like HomeScreenContent and HomeScreen to separate stuff like viewmodels, so we can make use of previews since using emulator is inefficient
| ExperimentalFoundationApi::class, | ||
| ) | ||
| @Composable | ||
| fun HomeScreen( |
There was a problem hiding this comment.
Nit: Eventually, we should refactor this composable so that it makes use of some composables because it is way too big 😭
app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| var editState by remember { |
There was a problem hiding this comment.
Unsure about the context, but this and the editText seem like things we should refactor to have stored in some sort of view model
app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
Outdated
Show resolved
Hide resolved
|
Should probably also add screen recording to pr description since some of the functionality involves opening up a modal and showing toasts |
|
Also, make sure to resolve the conflicts before I can give approval for merging |
|
Would also be good to point out that the reviewer should set the ecosystem feature flag for the debug release to true when they are testing in order to see the changes |
|
Overall, nice work! Just some minor changes, and a lot of my comments are just to bring up some potential refactors for the future to keep in mind. |
2 participants
Overview
Changes Made
Test Coverage
Next Steps
Screenshots
Add Favorites Button