-
Notifications
You must be signed in to change notification settings - Fork 1
[Home/Search Screen UI] - Foundation: General Components, Icon Assets, Bottom Navigation Update, Constants #8
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
…ts, icon assets, composeBom dependency update, bottom navigation update, minor design system updates
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.
Fantastic work! I love how you made the UI super flat which made this easy to review. I left a couple nits but great work as always.
| val name: String, @DrawableRes | ||
| val iconResId: Int |
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: formatting is more clear as such:
data class ServiceCategory(
val name: String,
@DrawableRes val iconResId: Int
)Since it's more obvious that @DrawableRes is applied to the Int rather than the String
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.
whoops, seems like I missed it when I clicked reformat, will fix
| modifier: Modifier = Modifier | ||
| ) { | ||
| // TODO: Add loading and error states | ||
| SubcomposeAsyncImage( |
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.
Why SubcomposeAsyncImage over AsyncImage? Oh I see, I suppose you want custom loading and error states? I think that should be fine, if that's the case
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.
Yeah, it was mainly for the loading and error states in the future
| import com.cornellappdev.hustle.ui.theme.HustleTheme | ||
|
|
||
| @Composable | ||
| fun HustleButton( |
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 Slack Compose lint should be telling you that parameters are not in recommended order here, follow the lint please 🙏
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.
Yeah 😅, for some reason the lint was not being applied, but now it's working, so will fix!
| } | ||
|
|
||
| @Composable | ||
| fun ServiceCard( |
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.
Fix parameter order according to lint
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.
Same thing as above 😅 will fix
| } | ||
| } | ||
|
|
||
| data class ServiceCardPreviewParameters( |
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.
yay preview parameters 😌
| // Animate the button to scale up and then back to normal size | ||
| scale.animateTo(1.2f, spring(dampingRatio = Spring.DampingRatioMediumBouncy)) | ||
| scale.animateTo(1f, spring(dampingRatio = Spring.DampingRatioMediumBouncy)) |
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.
Awesome animation! but the comment is a little unnecessary lol
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.
ok, will remove!
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.
Looks great, seconding zach ofc, but otherwise no notes!
(looking over this was actually rlly beneficial, I just rediscovered IconButton lol 😃)
Overview
Set up foundation for home/search screen ui with general components, icon assets, constants, etc.
https://www.figma.com/design/tRFDgfOFDXGXREx6q33p4P/Hustle?node-id=577-3679&t=ekSdFryK6xeB87PV-0
Changes Made
Test Coverage
Next Steps
Screenshots
Details
Screen.Recording.2025-11-16.at.1.21.47.AM.mov