-
Notifications
You must be signed in to change notification settings - Fork 1
[Home/Search Screen UI] - Implement Home/Search Screen UI #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements the Home/Search Screen UI for the Hustle Android app, introducing a two-mode interface with smooth animations between main browsing and search states.
Key Changes:
- Created a dual-mode home screen with animated transitions between main content and search views
- Implemented category-based navigation with button row and clickable section headers
- Added search functionality with recent searches and recently viewed services
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| TestingConstants.kt | Enhanced test data with diverse service examples across multiple categories |
| HomeScreen.kt | Main screen composable with state management and animated content switching |
| HomeNavigation.kt | Added navigation handlers for service details and category subpages |
| Routes.kt | Introduced CategoryType enum with serialization support and typeName mapping |
| HustleNavigation.kt | Removed unused currentRoute variable |
| SearchHeader.kt | Animated header with search bar that slides up when search is active |
| SearchContent.kt | Search view with recent searches and recently viewed services sections |
| MainContent.kt | Main view with category buttons and multiple service carousels |
| CategoryButtonRow.kt | Horizontal scrollable row of category filter buttons |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
| displayImageUrl = "https://news.cornell.edu/sites/default/files/styles/full_size/public/06_2023_1114_sh_005-n_1.jpg?itok=E3ecxgYl", | ||
| priceUnit = "/hour", | ||
| displayImageUrl = "https://s3-media0.fl.yelpcdn.com/bphoto/1KwtwltxdEYVz4TIHAzaow/1000s.jpg" |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing priceUnit field for Service id=3. While the Service model has a default value of \"\", this service should have priceUnit = \"/hour\" for consistency with the other test services.
| displayImageUrl = "https://s3-media0.fl.yelpcdn.com/bphoto/1KwtwltxdEYVz4TIHAzaow/1000s.jpg" | |
| displayImageUrl = "https://s3-media0.fl.yelpcdn.com/bphoto/1KwtwltxdEYVz4TIHAzaow/1000s.jpg", | |
| priceUnit = "/hour", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some services might not be by hour
| companion object { | ||
| fun fromTypeName(typeName: String): CategoryType { | ||
| return entries.firstOrNull { it.typeName == typeName } | ||
| ?: throw IllegalArgumentException("No CategoryType with typeName $typeName found.") |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message could be more helpful by including the valid typeName options. Consider: \"Unknown CategoryType '$typeName'. Valid types: ${entries.map { it.typeName }}\"
| ?: throw IllegalArgumentException("No CategoryType with typeName $typeName found.") | |
| ?: throw IllegalArgumentException("Unknown CategoryType '$typeName'. Valid types: ${entries.map { it.typeName }}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will end up removing this function
| onClick = { | ||
| navigateToCategorySubpage(CategoryType.POPULAR_RIGHT_NOW) | ||
| }, | ||
| modifier = Modifier.padding(start = 32.dp) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded padding value 32.dp should use HustleSpacing.large (which is 24.dp) or another appropriate spacing constant for consistency. This value appears on lines 58, 73, 88, and 103.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good thought, but would need to consult design
zachseidner1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think we should get rid of the fromTypeName function but looks good otherwise
| popularRightNowListings: List<Service>, | ||
| newOnHustleListings: List<Service>, | ||
| servicesNearYouListings: List<Service>, | ||
| availableThisWeekListings: List<Service>, | ||
| recentSearches: List<String>, | ||
| recentlyViewedServices: List<Service>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This is a lot of data to be passing in as separate parameters. To avoid parameter drilling, it could be nicer to make a data class HomeScreenViewState that contains the data the home screen needs to render
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, I'll add it to the HomeScreen file
| AVAILABLE_THIS_WEEK("Available this week"); | ||
|
|
||
| companion object { | ||
| fun fromTypeName(typeName: String): CategoryType { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think we should just make this fromTypeNameOrNull and have the caller handle the failure case to avoid the potential for crashing the app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of refactoring this such that the composable that will use this won't need to use this function, so I'll probably remove it.
| onSearch = onSearch, | ||
| modifier = Modifier.padding(horizontal = HustleSpacing.large) | ||
| ) | ||
| AnimatedContent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You found a way to state the two states of this screen very simply so we don't have to navigate to a new screen when this is pressed. This looks great, nice job!
| val popularRightNowListings = TEST_SERVICES | ||
| val newOnHustleListings = TEST_SERVICES | ||
| val servicesNearYouListings = TEST_SERVICES | ||
| val availableThisWeekListings = TEST_SERVICES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe we just pass in TEST_SERVICES directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol, I kinda forgot about that.
| enter = fadeIn() + scaleIn(), | ||
| exit = fadeOut(animationSpec = tween(150)) + shrinkHorizontally( | ||
| shrinkTowards = Alignment.Start, | ||
| animationSpec = tween(150) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this animation!
| ) { | ||
| items(SERVICE_CATEGORIES, key = { it.name }) { category -> | ||
| HustleButton( | ||
| onClick = { onCategoryClick(CategoryType.fromTypeName(category.name)) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just directly pass in category to onCategoryClick, removing the need for the fromTypeName function entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm planning on moving the categorytype definition to HustleConstants and replacing the ServiceCategory type's name field with categoryType: CategoryType, so that should remove the need for the extra function
Overview
Implement the ui for the Home/Search screen
https://www.figma.com/design/tRFDgfOFDXGXREx6q33p4P/Hustle?node-id=577-3679&t=ekSdFryK6xeB87PV-0
Changes Made
Test Coverage
Next Steps
Related PRs
Screenshots
Details
Screen.Recording.2025-11-16.at.1.21.47.AM.mov