Conversation
…ed basic filter button component
…t for readability
… of their components
…icking experience
…is provided between cards
There was a problem hiding this comment.
Pull request overview
This pull request implements the UI components for a filtering feature in the favorites section. The PR adds interactive filter buttons and bottom sheets that allow users to select location type filters (Gyms, Eateries, Libraries, Printers, Other), with selected filters displayed as removable tags. The actual filtering functionality is intentionally deferred to a future PR, as stated in the "Next Steps" section.
Changes:
- Added UI components for filter button, filter row with removable tags, and modal bottom sheet for filter selection
- Introduced filter state management in HomeViewModel to track selected and applied filters
- Created new drawable resources for filter icons representing different location types
Reviewed changes
Copilot reviewed 18 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/drawable/*.xml | New vector drawable icons for different location type filters (printer, other, library, gym, eatery, and main filter icon) |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/FavoritesFilterSheetState.kt | New enum class defining the available filter types with their icons and labels |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/FilterState.kt | Added trailing comma to last enum entry for consistency |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt | Added state management for filter sheet visibility and selected/applied filters |
| app/src/main/java/com/cornellappdev/transit/ui/theme/Color.kt | Added MutedTransitBlue color for active filter background |
| app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt | Integrated filter sheet state into the home screen |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/FilterRow.kt | New component displaying the filter button and selected filter tags in a flow row |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/FilterButton.kt | New button component that triggers the filter bottom sheet |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/FavoritesFilterSheetItem.kt | New component for individual filter options in the bottom sheet grid |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/FavoritesFilterBottomSheet.kt | Modal bottom sheet component displaying filter options with cancel/apply actions |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt | Integrated filter UI into the favorites view with conditional rendering |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/RoundedImagePlaceCard.kt | Removed horizontal and vertical padding to allow for spacing in parent component |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesButton.kt | Added vertical content padding parameter |
| .DS_Store, app/.DS_Store, app/src/.DS_Store, app/src/main/.DS_Store, app/src/main/res/.DS_Store, app/src/main/res/drawable/.DS_Store | macOS system files that should not be committed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import androidx.compose.material3.ExperimentalMaterial3Api | ||
| import androidx.compose.material3.HorizontalDivider | ||
| import androidx.compose.material3.ModalBottomSheet | ||
| import androidx.compose.material3.OutlinedTextFieldDefaults.contentPadding |
There was a problem hiding this comment.
This import is unused and should be removed. The contentPadding from OutlinedTextFieldDefaults is not used in this file. The contentPadding being used at line 166 comes from the PaddingValues function parameter.
| import androidx.compose.material3.OutlinedTextFieldDefaults.contentPadding |
| onRemoveFilter = {} | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
There are two blank lines at the end of this preview function. This is inconsistent with the codebase style, where preview functions typically have only one blank line after the closing brace (see line 98 or other preview functions in the codebase).
|
|
||
| } |
There was a problem hiding this comment.
There is an extra blank line after the closing brace. This is inconsistent with the codebase style where preview functions typically have only one blank line after the closing brace (see other preview functions in the codebase).
| onClick : () -> Unit, | ||
| text: String, | ||
| modifier : Modifier = Modifier |
There was a problem hiding this comment.
Extra spaces around the colon in parameter declarations. The codebase consistently uses no spaces before the colon (e.g., 'onClick:' not 'onClick :'). Please remove the spaces before the colons in both parameter declarations for consistency with the codebase style (see examples in CurrentLocationItem.kt:29, MenuItem.kt:31, SearchCategoryHeader.kt:21).
| onClick : () -> Unit, | |
| text: String, | |
| modifier : Modifier = Modifier | |
| onClick: () -> Unit, | |
| text: String, | |
| modifier: Modifier = Modifier |
There was a problem hiding this comment.
^^. Try to run the keyboard shortcut after you are finished with the file
| color = if(isActive) TransitBlue else MetadataGray, | ||
| shape = RoundedCornerShape(8.dp) | ||
| ) | ||
| .background( | ||
| color = if(isActive) MutedTransitBlue | ||
| else Color.Transparent, | ||
| shape = RoundedCornerShape(8.dp) | ||
| ) | ||
| .clickable(onClick = itemOnClick) | ||
| .padding(vertical = 16.dp), | ||
| horizontalAlignment = Alignment.CenterHorizontally | ||
| ) { | ||
| Icon( | ||
| painterResource(id = iconId), | ||
| contentDescription = label, | ||
| modifier = Modifier | ||
| .size(34.dp), | ||
| tint = if(isActive) TransitBlue else Color.Unspecified | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(8.dp)) | ||
|
|
||
| Text( | ||
| label, | ||
| color = if(isActive) TransitBlue else MetadataGray, |
There was a problem hiding this comment.
Missing space after 'if' keyword. The codebase consistently uses 'if (' with a space (see examples in BottomSheetFilterItem.kt:41, FilterRow.kt:48, EcosystemBottomSheetContent.kt:116). Please change 'if(isActive)' to 'if (isActive)' for consistency with the codebase style.
|
|
||
| @Preview(showBackground = true) | ||
| @Composable | ||
| private fun InactiveFavoritesFilterSheetItemInactive() { |
There was a problem hiding this comment.
The preview function name 'InactiveFavoritesFilterSheetItemInactive' is inconsistent with the preview naming convention used elsewhere in the codebase. Based on other preview functions like 'PreviewBottomSheetFilterItem' and 'InactivePreviewBottomSheetFilterItem' (from BottomSheetFilterItem.kt), the name should be 'InactivePreviewFavoritesFilterSheetItem' or 'PreviewFavoritesFilterSheetItemInactive' to maintain consistency.
| private fun InactiveFavoritesFilterSheetItemInactive() { | |
| private fun PreviewFavoritesFilterSheetItemInactive() { |
|
|
||
| } |
There was a problem hiding this comment.
There is an extra blank line after the closing brace. This is inconsistent with the codebase style where preview functions typically have only one blank line after the closing brace (see other preview functions in the codebase).
| backgroundColor = if(isCancel) Color.White else TransitBlue, | ||
| contentColor = if(isCancel) TransitBlue else Color.White | ||
| ), | ||
| shape = RoundedCornerShape(16.dp), | ||
| border = if(isCancel) BorderStroke(1.dp, TransitBlue) else null, |
There was a problem hiding this comment.
Missing space after 'if' keyword. The codebase consistently uses 'if (' with a space (see examples in BottomSheetFilterItem.kt:41, FilterRow.kt:48, EcosystemBottomSheetContent.kt:116). Please change 'if(isCancel)' to 'if (isCancel)' for consistency with the codebase style.
| backgroundColor = if(isCancel) Color.White else TransitBlue, | |
| contentColor = if(isCancel) TransitBlue else Color.White | |
| ), | |
| shape = RoundedCornerShape(16.dp), | |
| border = if(isCancel) BorderStroke(1.dp, TransitBlue) else null, | |
| backgroundColor = if (isCancel) Color.White else TransitBlue, | |
| contentColor = if (isCancel) TransitBlue else Color.White | |
| ), | |
| shape = RoundedCornerShape(16.dp), | |
| border = if (isCancel) BorderStroke(1.dp, TransitBlue) else null, |
|
|
||
| ) { |
There was a problem hiding this comment.
Extra blank line in function signature. There's an unnecessary blank line between the parameter list and the closing brace. Remove line 28 for cleaner code formatting.
| ) { | |
| ) { |
| FilterState.FAVORITES -> { | ||
| favoriteList(favorites, navigateToPlace, onAddFavoritesClick) | ||
| val appliedFilters by homeViewModel.appliedFavoritesFilters.collectAsStateWithLifecycle() | ||
| Column() { |
There was a problem hiding this comment.
Empty parentheses on Column constructor. When there are no parameters being passed, the parentheses can be omitted in Kotlin for cleaner syntax. Change 'Column()' to 'Column' for consistency with Kotlin conventions.
| Column() { | |
| Column { |
|
Remove the .DS_Store files |
2 participants
Overview
Changes Made
Test Coverage
Next Steps (delete if not applicable)
Screenshots (delete if not applicable)
The last button press was 'cancel' (e.g. do nothing)
Filter UI Screen Recording
filter_ui_demo.webm