Open
Conversation
Member
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds a Profile and Edit Profile feature (screens, avatar picker, carousel), wires new navigation routes, updates game model/time parsing and score conversion, adjusts GameDetails routing, adds drawables and theme color, and updates build/dependency versions. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant NavHost as NavigationHost
participant Profile as ProfileScreen
participant Edit as EditProfileScreen
participant Picker as AvatarPickerBottomSheet
participant Carousel as ProfileGameCarousel
User->>NavHost: navigate to Profile
NavHost->>Profile: render ProfileScreen
activate Profile
Profile->>Carousel: render carousels
User->>Profile: tap Edit Profile
Profile->>NavHost: navigate(EditProfile)
deactivate Profile
NavHost->>Edit: render EditProfileScreen
activate Edit
User->>Edit: tap Edit Photo
Edit->>Picker: show AvatarPicker
activate Picker
User->>Picker: select avatar
Picker->>Edit: onAvatarSelected(avatar)
deactivate Picker
User->>Edit: modify fields, tap Save
Edit->>NavHost: onBackClick / navigateUp()
deactivate Edit
NavHost->>Profile: return to ProfileScreen
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 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.
Actionable comments posted: 4
🧹 Nitpick comments (8)
app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt (3)
81-84: PlaceholderonClickhandlers need implementation.The
onClickcallbacks for bothGamesCarouselinstances contain placeholder comments. These should be wired to navigate to game details (similar to other screens usingnavigateToGameDetails).Would you like me to suggest how to wire these to navigation, similar to
HomeScreenandPastGamesScreen?Also applies to: 117-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt` around lines 81 - 84, The two GamesCarousel components in ProfileScreen (the one at games = bookmarkedGames and the other at games = createdGames) use placeholder onClick handlers; replace those comments by invoking the app's navigation helper (e.g., call navigateToGameDetails(gameId) or the equivalent navigation function used by HomeScreen/PastGamesScreen) so clicking a game navigates to its detail screen, passing the gameId parameter through the existing navigateToGameDetails method used elsewhere.
51-84: Extract duplicate section pattern into a reusable composable.The "Bookmarks" and "Games You Might Like" sections share identical structure (header row with title, count, chevron, and GamesCarousel). Consider extracting:
♻️ Suggested extraction
`@Composable` private fun GameSection( title: String, games: List<GameCardData>, onClick: (String) -> Unit, modifier: Modifier = Modifier ) { Row( modifier = modifier .fillMaxWidth() .padding(horizontal = 20.dp), horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically ) { Text( text = title, fontSize = 18.sp, fontWeight = FontWeight.Bold, color = Color.Black ) Row(verticalAlignment = Alignment.CenterVertically) { Text( text = "${games.size} Results", fontSize = 14.sp, color = Color.Gray ) Icon( imageVector = Icons.Outlined.ChevronRight, contentDescription = null, tint = Color.Gray, modifier = Modifier.size(18.dp) ) } } Spacer(modifier = Modifier.height(12.dp)) GamesCarousel(games = games, onClick = onClick) }Also applies to: 87-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt` around lines 51 - 84, Extract the duplicated header + carousel UI into a private composable (e.g., GameSection) and replace both the "Bookmarks" block (which currently uses bookmarkedGames and GamesCarousel) and the "Games You Might Like" block (which uses the other games list) to call this composable; GameSection should take title: String, games: List<GameCardData>, onClick: (String) -> Unit, and optional modifier, render the Row with title, count ("${games.size} Results"), chevron Icon, a Spacer, then call GamesCarousel(games = games, onClick = onClick); update usages so bookmarkedGames and the other games list pass through and remove the duplicated Row/Spacer/GamesCarousel blocks.
192-207: Hardcoded user data inProfileRow.The user name "Audrey Wu" and handle "@audreywuu" are hardcoded. This is acceptable for UI scaffolding, but these values should eventually come from a ViewModel or data source.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt` around lines 192 - 207, ProfileRow currently hardcodes the display name and handle ("Audrey Wu" and "@audreywuu"); update it to accept and render dynamic data by wiring the Text composables to source values instead of literals. Modify the ProfileRow (or enclosing ProfileScreen) composable to take parameters like userName: String and userHandle: String (or read them from a ViewModel property) and replace the hardcoded strings in the two Text calls with those parameters/observed state so the UI reflects the real user data.app/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.kt (1)
150-151: UsingAnytype for icons loses type safety.Changing
unselectedIconandselectedIconfromInttoAnyallows both resource IDs andImageVector, but sacrifices compile-time type checking. Consider a sealed interface:♻️ Type-safe alternative
sealed interface NavIcon { data class Resource(val resId: Int) : NavIcon data class Vector(val imageVector: ImageVector) : NavIcon } data class NavItem( val screen: ScoreScreens, val label: String, val unselectedIcon: NavIcon, val selectedIcon: NavIcon )This prevents accidental misuse and makes the API self-documenting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.kt` around lines 150 - 151, Replace the two Any-typed icon properties on NavItem (unselectedIcon and selectedIcon) with a sealed interface NavIcon and use concrete implementations (e.g., Resource(resId: Int) and Vector(imageVector: ImageVector)) so callers must supply either a resource id or an ImageVector explicitly; update NavItem to take NavIcon for both unselectedIcon and selectedIcon and refactor any creation/consumption sites to pattern-match on NavIcon (Resource vs Vector) when rendering.app/src/main/java/com/cornellappdev/score/components/GamesCarousel.kt (1)
30-48: Consider adding akeyparameter for stable item identity.Using
items(games)without a key may cause unnecessary recompositions when the list updates. SinceGameCardDatahas anidfield (used inonClick), consider:- items(games) { game -> + items(games, key = { it.id }) { game ->This helps Compose efficiently diff and animate list changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/components/GamesCarousel.kt` around lines 30 - 48, The carousel uses items(games) without a stable key which can cause unnecessary recompositions; update the call to items(...) to provide a unique key (use the game's id) so Compose can track identities and diff efficiently — locate the items(games) lambda in GamesCarousel.kt where FeaturedGameCard is created and change the items invocation to include key = { game -> game.id } (or the appropriate unique id property from GameCardData) so list updates and animations are stable.app/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.kt (1)
42-55: Consider adding anelsebranch or using a sealed type for icons.The current implementation silently ignores unsupported icon types. If
NavItem.unselectedIcon/selectedIcon(typed asAnyinRootNavigation.kt) receives an unexpected type, no icon renders. Consider either:
- Adding a logging/warning
elsebranch for debugging- Using a sealed class/interface for icon types to ensure compile-time safety
This is low-risk since the current usages are controlled, but would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.kt` around lines 42 - 55, The code in ScoreNavigationBar.kt currently handles two runtime icon types (Int and ImageVector) for item.selectedIcon/item.unselectedIcon but silently does nothing for any other type; update the implementation to either (A) add a clear else branch after the existing checks that logs a warning via your logger (or calls a fallback Icon) mentioning the NavItem id/title and the unexpected icon type, or (B) refactor NavItem.selectedIcon/unselectedIcon in RootNavigation.kt to use a sealed class (e.g., IconResource with subclasses ResourceId(val id: Int) and VectorIcon(val vector: ImageVector)) and update ScoreNavigationBar to pattern-match on that sealed type so unsupported cases are impossible at compile time; reference the item variable, NavItem.selectedIcon/unselectedIcon, and the Icon composable when making the change.app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt (2)
178-179: Save buttononClickis not implemented.The Save button has an empty
onClick = { }handler. This should eventually persist the edited name and username, then navigate back.Would you like me to help outline the save logic integration with a ViewModel?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around lines 178 - 179, The Save button in EditProfileScreen has an empty onClick; wire it to the screen's save flow by calling the ViewModel's save/update method (e.g., EditProfileViewModel.saveProfile or updateProfile) with the current editedName/editedUsername state, perform simple validation (non-empty, username uniqueness check via the ViewModel/repository), await the result, surface errors via the ViewModel's error state, and on success invoke navigation back (e.g., navController.popBackStack() or provided onNavigateUp callback). Ensure the Button's onClick references the exact state variables used in EditProfileScreen (editedName, editedUsername) and the ViewModel method name (saveProfile/updateProfile) so the persistence and navigation happen atomically.
129-142:BasicTextFieldlacks visual feedback.
BasicTextFieldwithout adecorationBoxprovides no visual indication of focus state or field boundaries. Users may not realize the field is editable. Consider adding adecorationBoxparameter or usingOutlinedTextField/TextFieldfor better UX.Also applies to: 160-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around lines 129 - 142, The BasicTextField usage (the one bound to variable name with modifier Modifier.weight(1f)) has no visual affordance; replace it with an OutlinedTextField or TextField (or add a decorationBox) to provide focus/border/placeholder and proper padding, and ensure you forward the same textStyle, value/onValueChange, and modifier; do the same for the other BasicTextField instance referenced (lines ~160-173) so both fields show focus state and editable bounds.
🤖 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/build.gradle.kts`:
- Line 84: Remove the duplicate Foundation dependency that causes a version
conflict: delete the implementation(libs.foundation) entry and keep the newer
alias implementation(libs.androidx-foundation) (or vice versa keep only the one
that resolves to version 1.10.3). Ensure only a single
androidx.compose.foundation dependency remains in the dependencies block to
avoid mixing libs.foundation and libs.androidx-foundation.
- Line 107: Remove the explicit version suffix from the implementation
dependency for "androidx.compose.material:material-icons-extended:1.6.0" so it
inherits the version from the Compose BOM (the compose BOM declaration in the
build script), preventing the BOM from being downgraded; locate the dependency
string
"implementation(\"androidx.compose.material:material-icons-extended:1.6.0\")"
and change it to the form without a version so it aligns with the BOM-managed
Compose version.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt`:
- Around line 61-68: The back Icon in EditProfileScreen
(Icons.Outlined.ChevronLeft) uses a 24.dp visual size and a clickable modifier,
yielding a touch target smaller than the 48.dp accessibility recommendation;
replace the plain Icon clickable with an IconButton (or wrap the Icon in a Box
with a minimum touch target) so the clickable area meets 48.dp, keep the visual
icon size at 24.dp (e.g., set Icon size to 24.dp while the IconButton/Box
provides the larger touch area) and call onBackClick() from the IconButton's
onClick to preserve behavior.
In `@gradle/libs.versions.toml`:
- Line 22: You have two duplicate aliases for the same
artifact—androidx-foundation and foundation—pointing to different versions;
remove one alias from libs.versions.toml and update the build script imports to
use only the remaining alias (or switch the build to rely on the Compose BOM
instead of an explicit alias); ensure the kept alias/version for
androidx.compose.foundation matches the intended BOM alignment or is
intentionally decoupled so you don’t get conflicting versions on the classpath.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/score/components/GamesCarousel.kt`:
- Around line 30-48: The carousel uses items(games) without a stable key which
can cause unnecessary recompositions; update the call to items(...) to provide a
unique key (use the game's id) so Compose can track identities and diff
efficiently — locate the items(games) lambda in GamesCarousel.kt where
FeaturedGameCard is created and change the items invocation to include key = {
game -> game.id } (or the appropriate unique id property from GameCardData) so
list updates and animations are stable.
In `@app/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.kt`:
- Around line 150-151: Replace the two Any-typed icon properties on NavItem
(unselectedIcon and selectedIcon) with a sealed interface NavIcon and use
concrete implementations (e.g., Resource(resId: Int) and Vector(imageVector:
ImageVector)) so callers must supply either a resource id or an ImageVector
explicitly; update NavItem to take NavIcon for both unselectedIcon and
selectedIcon and refactor any creation/consumption sites to pattern-match on
NavIcon (Resource vs Vector) when rendering.
In `@app/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.kt`:
- Around line 42-55: The code in ScoreNavigationBar.kt currently handles two
runtime icon types (Int and ImageVector) for
item.selectedIcon/item.unselectedIcon but silently does nothing for any other
type; update the implementation to either (A) add a clear else branch after the
existing checks that logs a warning via your logger (or calls a fallback Icon)
mentioning the NavItem id/title and the unexpected icon type, or (B) refactor
NavItem.selectedIcon/unselectedIcon in RootNavigation.kt to use a sealed class
(e.g., IconResource with subclasses ResourceId(val id: Int) and VectorIcon(val
vector: ImageVector)) and update ScoreNavigationBar to pattern-match on that
sealed type so unsupported cases are impossible at compile time; reference the
item variable, NavItem.selectedIcon/unselectedIcon, and the Icon composable when
making the change.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt`:
- Around line 178-179: The Save button in EditProfileScreen has an empty
onClick; wire it to the screen's save flow by calling the ViewModel's
save/update method (e.g., EditProfileViewModel.saveProfile or updateProfile)
with the current editedName/editedUsername state, perform simple validation
(non-empty, username uniqueness check via the ViewModel/repository), await the
result, surface errors via the ViewModel's error state, and on success invoke
navigation back (e.g., navController.popBackStack() or provided onNavigateUp
callback). Ensure the Button's onClick references the exact state variables used
in EditProfileScreen (editedName, editedUsername) and the ViewModel method name
(saveProfile/updateProfile) so the persistence and navigation happen atomically.
- Around line 129-142: The BasicTextField usage (the one bound to variable name
with modifier Modifier.weight(1f)) has no visual affordance; replace it with an
OutlinedTextField or TextField (or add a decorationBox) to provide
focus/border/placeholder and proper padding, and ensure you forward the same
textStyle, value/onValueChange, and modifier; do the same for the other
BasicTextField instance referenced (lines ~160-173) so both fields show focus
state and editable bounds.
In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt`:
- Around line 81-84: The two GamesCarousel components in ProfileScreen (the one
at games = bookmarkedGames and the other at games = createdGames) use
placeholder onClick handlers; replace those comments by invoking the app's
navigation helper (e.g., call navigateToGameDetails(gameId) or the equivalent
navigation function used by HomeScreen/PastGamesScreen) so clicking a game
navigates to its detail screen, passing the gameId parameter through the
existing navigateToGameDetails method used elsewhere.
- Around line 51-84: Extract the duplicated header + carousel UI into a private
composable (e.g., GameSection) and replace both the "Bookmarks" block (which
currently uses bookmarkedGames and GamesCarousel) and the "Games You Might Like"
block (which uses the other games list) to call this composable; GameSection
should take title: String, games: List<GameCardData>, onClick: (String) -> Unit,
and optional modifier, render the Row with title, count ("${games.size}
Results"), chevron Icon, a Spacer, then call GamesCarousel(games = games,
onClick = onClick); update usages so bookmarkedGames and the other games list
pass through and remove the duplicated Row/Spacer/GamesCarousel blocks.
- Around line 192-207: ProfileRow currently hardcodes the display name and
handle ("Audrey Wu" and "@audreywuu"); update it to accept and render dynamic
data by wiring the Text composables to source values instead of literals. Modify
the ProfileRow (or enclosing ProfileScreen) composable to take parameters like
userName: String and userHandle: String (or read them from a ViewModel property)
and replace the hardcoded strings in the two Text calls with those
parameters/observed state so the UI reflects the real user data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0ea26fe-4d28-463a-b8d2-d944df5f3a18
📒 Files selected for processing (8)
app/build.gradle.ktsapp/src/main/java/com/cornellappdev/score/components/GamesCarousel.ktapp/src/main/java/com/cornellappdev/score/nav/ScoreNavHost.ktapp/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.ktapp/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.ktapp/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.ktapp/src/main/java/com/cornellappdev/score/screen/ProfileScreen.ktgradle/libs.versions.toml
EmilJiang
reviewed
Mar 25, 2026
Contributor
EmilJiang
left a comment
EmilJiang
left a comment
There was a problem hiding this comment.
hello this is some awesome work. sorry for the late review! let me know if you have an questions. also u can mostly ignore the ai comments unless they look reasonable
Comment on lines
+42
to
+55
| val icon = if (isSelected) item.selectedIcon else item.unselectedIcon | ||
| if (icon is Int) { | ||
| Icon( | ||
| painter = painterResource(id = icon), | ||
| contentDescription = null, | ||
| tint = Color.Unspecified | ||
| ) | ||
| } else if (icon is ImageVector) { | ||
| Icon( | ||
| imageVector = icon, | ||
| contentDescription = null, | ||
| tint = if (isSelected) CrimsonPrimary else GrayPrimary | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Is it possible to import the Profile Icon as a Drawable in Resource Manager on the left. This way you do not need this additional if statement. I can show you next time!
Comment on lines
+174
to
+180
| NavItem( | ||
| label = "Profile", | ||
| unselectedIcon = Icons.Outlined.Person, | ||
| selectedIcon = Icons.Filled.Person, | ||
| screen = ScoreScreens.Profile, | ||
| ), | ||
| ) |
Contributor
There was a problem hiding this comment.
I think all the other Icons are Drawable so here u would make the change!
Comment on lines
+150
to
+151
| val unselectedIcon: Any, | ||
| val selectedIcon: Any |
Comment on lines
+52
to
+65
| composable<ScoreScreens.Profile> { | ||
| ProfileScreen( | ||
| navigateToEditProfile = { | ||
| navController.navigate(ScoreScreens.EditProfile) | ||
| } | ||
| ) | ||
| } | ||
| composable<ScoreScreens.EditProfile> { | ||
| EditProfileScreen( | ||
| onBackClick = { | ||
| navController.navigateUp() | ||
| } | ||
| ) | ||
| } |
Comment on lines
+48
to
+49
| var name by remember { mutableStateOf("Audrey Wu") } | ||
| var username by remember { mutableStateOf("audreywuu") } |
Contributor
There was a problem hiding this comment.
this is fine for now but when u add a view model it should be from that!
Comment on lines
+75
to
+77
| } | ||
| HorizontalDivider(color = Color(0xFFF0F0F0)) | ||
|
|
Contributor
There was a problem hiding this comment.
i think for all the Color() you should use theme/Color.kt, it helps so if there is ever a theme change this will also be changed!
Comment on lines
+96
to
+105
| Text( | ||
| text = "Edit photo", | ||
| color = Color(0xFFA5210D), | ||
| fontSize = 18.sp, | ||
| fontFamily = poppinsFamily, | ||
| fontWeight = FontWeight.SemiBold, | ||
| lineHeight = 18.sp, | ||
| letterSpacing = 0.sp, | ||
| textAlign = TextAlign.Center | ||
| ) |
Contributor
There was a problem hiding this comment.
I think all of these should use theme/TextStyle.kt. you could prob quickly do this by asking chat or claude to modify it :)
Comment on lines
+58
to
+62
| Text( | ||
| text = "Bookmarks", | ||
| fontSize = 18.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = Color.Black |
Contributor
There was a problem hiding this comment.
same thing fro ProfileScreen could all of these use theme/style.kt?
| Column( | ||
| modifier = modifier | ||
| .fillMaxSize() | ||
| .background(Color.White) |
Contributor
There was a problem hiding this comment.
Could this screen use theme/color.kt?
| Icon( | ||
| imageVector = Icons.Outlined.ChevronRight, | ||
| contentDescription = null, | ||
| tint = Color.Gray, |
Contributor
There was a problem hiding this comment.
could this use components/GamesCarousel.kt? it looks to be the same :)
amjiao
requested changes
Apr 6, 2026
Contributor
amjiao
left a comment
amjiao
left a comment
There was a problem hiding this comment.
really great work overall and congrats on your first PR 🥳🥳🥳 pls take a look over the comments and re-request reviews from emil and me when you finish addressing them! and ofc lmk if you have any questions!!
Contributor
There was a problem hiding this comment.
since GamesCarousel is used on the home screen for another feature, changing it here for the profile page affects how the home screen looks. The changes look great but we just need to move them to a new composable so we can tweak them specifically for the profile page without accidentally changing other parts of the UI! you'll want to revert the changes in this file after you've moved them to a new composable!
also, the bookmarks lazy column might be pretty similar to the HighlightsCardRow composable (in terms of the header (Bookmarks n Results >). feel free to take a look at that/copy some code from there into the new component for this profile page row!
| screen = ScoreScreens.ScoresScreen, | ||
| ), | ||
| ) No newline at end of file | ||
| NavItem( |
Contributor
There was a problem hiding this comment.
i think we might've talked ab this after you opened this PR so no worries, but just a note that this icon will be in the top right corner of the home page!
| Text( | ||
| text = "Edit profile", | ||
| fontSize = 16.sp, | ||
| fontWeight = FontWeight.SemiBold, |
Contributor
There was a problem hiding this comment.
for text, specific styles are specified in the figma that correspond to styles in TextStyle.kt that you can reference so that you don't have to specify a new style every time! i think for this it'd be heading2, and you can write that as Text(text = "Edit profile", style = heading2)
| .background(Color(0xFF7EDAFF)), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text("🏓", fontSize = 85.sp) |
Contributor
There was a problem hiding this comment.
nit: ultimately it'd be best practice to display this as an icon rather than text but we can fix that when you network!
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/score/screen/EditProfileScreen.kt`:
- Around line 84-91: When showing AvatarPickerBottomSheet (when showAvatarPicker
is true) seed its initial selection from currentAvatar so reopening doesn't
clear the prior choice; pass currentAvatar into the sheet (or initialize the
sheet's selectedAvatar from currentAvatar) and keep the
onAvatarSelected/onDismiss logic unchanged so Save remains enabled if the user
reopens without changing the avatar. Reference: showAvatarPicker,
AvatarPickerBottomSheet, currentAvatar, selectedAvatar.
- Around line 288-301: The close touch target is too small (Modifier.size(32.dp)
on the Box containing the Icon), making dismissal hard; replace the Box
clickable with a larger accessible touch target by using IconButton (or changing
the modifier to .size(48.dp) or .requiredSizeIn(minWidth = 48.dp, minHeight =
48.dp)) while keeping the CircleShape border/clip and the Icon size the same,
and invoke onDismiss from the IconButton's onClick to preserve behavior
(reference Box, Modifier.size(32.dp), Icon and onDismiss in the current code).
- Around line 73-82: EditProfileScreen currently passes a no-op lambda to
EditProfileContent's onSaveClick, leaving the Save CTA enabled but doing
nothing; update the screen boundary so a real save flow is wired or make the CTA
inert until there are changes. Specifically, add a save callback parameter
(e.g., onSave: (name:String, username:String, avatar:Uri?) -> Unit) to
EditProfileScreen and forward it to EditProfileContent's onSaveClick, or
alternatively change the state tracking in EditProfileScreen (track isDirty when
name/username/avatar differ from initial values) and pass a disabled/hidden
state to EditProfileContent so the Save button is not enabled when the handler
is a no-op; adjust ScoreNavHost (or the caller) to provide the concrete
persistence implementation when routing to EditProfileScreen.
In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt`:
- Around line 56-85: The root Column in the ProfileScreen should be made
vertically scrollable so the lower ProfileGameCarousel rows aren't clipped on
small devices; wrap the Column's modifier with a remembered scroll state
(rememberScrollState()) and apply verticalScroll(scrollState) to it (update the
modifier used by the root Column in ProfileScreen), and add the necessary
imports for rememberScrollState and verticalScroll; keep existing children like
ProfileTopBar, ProfileRow, and ProfileGameCarousel unchanged.
🪄 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: fb30f976-6140-4f70-a4af-af4df972ce71
⛔ Files ignored due to path filters (13)
app/src/main/res/drawable/badminton_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/baseball_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/basketball_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/billiards_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/bowling_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/football_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/golf_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/pingpong.pngis excluded by!**/*.pngapp/src/main/res/drawable/pingpong_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/soccer_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/tennis_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/volleyball_profile.pngis excluded by!**/*.pngapp/src/main/res/drawable/weightlifting_profile.pngis excluded by!**/*.png
📒 Files selected for processing (15)
app/build.gradle.ktsapp/src/main/java/com/cornellappdev/score/components/ProfileGameCarousel.ktapp/src/main/java/com/cornellappdev/score/nav/ScoreNavHost.ktapp/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.ktapp/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.ktapp/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.ktapp/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.ktapp/src/main/java/com/cornellappdev/score/screen/ProfileScreen.ktapp/src/main/java/com/cornellappdev/score/theme/Color.ktapp/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.ktapp/src/main/res/drawable/back_arrow.xmlapp/src/main/res/drawable/ic_check.xmlapp/src/main/res/drawable/ic_profile.xmlapp/src/main/res/drawable/ic_profile_filled.xmlgradle/libs.versions.toml
✅ Files skipped from review due to trivial changes (5)
- app/src/main/res/drawable/ic_profile.xml
- app/src/main/res/drawable/back_arrow.xml
- app/src/main/java/com/cornellappdev/score/theme/Color.kt
- app/src/main/res/drawable/ic_profile_filled.xml
- app/src/main/res/drawable/ic_check.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/com/cornellappdev/score/nav/ScoreNavigationBar.kt
- app/build.gradle.kts
- gradle/libs.versions.toml
- app/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.kt
Comment on lines
+73
to
+82
| EditProfileContent( | ||
| name = name, | ||
| onNameChange = { name = it }, | ||
| username = username, | ||
| onUsernameChange = { username = it }, | ||
| currentAvatar = currentAvatar, | ||
| onBackClick = onBackClick, | ||
| onEditPhotoClick = { showAvatarPicker = true }, | ||
| onSaveClick = { /* Handle save */ } | ||
| ) |
There was a problem hiding this comment.
Don’t expose an enabled Save button with a no-op handler.
This route is already reachable from ScoreNavHost, so tapping Save silently drops the user’s edits. Either wire a real save callback through the screen boundary or disable/hide the CTA until persistence exists.
💾 Suggested fix
-@Composable
-fun EditProfileScreen(onBackClick: () -> Unit) {
+@Composable
+fun EditProfileScreen(
+ onBackClick: () -> Unit,
+ onSaveClick: (String, String, AvatarOption) -> Unit,
+) {
@@
EditProfileContent(
name = name,
onNameChange = { name = it },
username = username,
onUsernameChange = { username = it },
currentAvatar = currentAvatar,
onBackClick = onBackClick,
onEditPhotoClick = { showAvatarPicker = true },
- onSaveClick = { /* Handle save */ }
+ onSaveClick = { onSaveClick(name, username, currentAvatar) }
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around
lines 73 - 82, EditProfileScreen currently passes a no-op lambda to
EditProfileContent's onSaveClick, leaving the Save CTA enabled but doing
nothing; update the screen boundary so a real save flow is wired or make the CTA
inert until there are changes. Specifically, add a save callback parameter
(e.g., onSave: (name:String, username:String, avatar:Uri?) -> Unit) to
EditProfileScreen and forward it to EditProfileContent's onSaveClick, or
alternatively change the state tracking in EditProfileScreen (track isDirty when
name/username/avatar differ from initial values) and pass a disabled/hidden
state to EditProfileContent so the Save button is not enabled when the handler
is a no-op; adjust ScoreNavHost (or the caller) to provide the concrete
persistence implementation when routing to EditProfileScreen.
Comment on lines
+84
to
+91
| if (showAvatarPicker) { | ||
| AvatarPickerBottomSheet( | ||
| onDismiss = { showAvatarPicker = false }, | ||
| onAvatarSelected = { | ||
| currentAvatar = it | ||
| showAvatarPicker = false | ||
| } | ||
| ) |
There was a problem hiding this comment.
Preserve the current avatar when reopening the picker.
selectedAvatar always starts as null, so reopening the sheet clears the existing choice and disables Save until the user taps the same avatar again. Seed the sheet from currentAvatar.
🖼️ Suggested fix
AvatarPickerBottomSheet(
+ initialAvatar = currentAvatar,
onDismiss = { showAvatarPicker = false },
onAvatarSelected = {
currentAvatar = it
showAvatarPicker = false
}
)
@@
fun AvatarPickerBottomSheet(
+ initialAvatar: AvatarOption,
onDismiss: () -> Unit,
onAvatarSelected: (AvatarOption) -> Unit
) {
- var selectedAvatar by remember { mutableStateOf<AvatarOption?>(null) }
+ var selectedAvatar by remember(initialAvatar) { mutableStateOf(initialAvatar) }Also applies to: 256-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around
lines 84 - 91, When showing AvatarPickerBottomSheet (when showAvatarPicker is
true) seed its initial selection from currentAvatar so reopening doesn't clear
the prior choice; pass currentAvatar into the sheet (or initialize the sheet's
selectedAvatar from currentAvatar) and keep the onAvatarSelected/onDismiss logic
unchanged so Save remains enabled if the user reopens without changing the
avatar. Reference: showAvatarPicker, AvatarPickerBottomSheet, currentAvatar,
selectedAvatar.
Comment on lines
+288
to
+301
| Box( | ||
| modifier = Modifier | ||
| .size(32.dp) | ||
| .border(1.dp, GrayLight, CircleShape) | ||
| .clip(CircleShape) | ||
| .clickable { onDismiss() }, | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.Default.Close, | ||
| contentDescription = "Close", | ||
| modifier = Modifier.size(16.dp), | ||
| tint = GrayMedium | ||
| ) |
There was a problem hiding this comment.
Expand the bottom-sheet close target.
The dismiss control is only 32.dp wide, which is noticeably harder to hit than the usual 48.dp target. Using IconButton here keeps the icon size while fixing the touch area.
♿ Suggested fix
- Box(
- modifier = Modifier
- .size(32.dp)
- .border(1.dp, GrayLight, CircleShape)
- .clip(CircleShape)
- .clickable { onDismiss() },
- contentAlignment = Alignment.Center
- ) {
+ IconButton(
+ onClick = onDismiss,
+ modifier = Modifier
+ .size(48.dp)
+ .border(1.dp, GrayLight, CircleShape)
+ .clip(CircleShape)
+ ) {
Icon(
imageVector = Icons.Default.Close,
contentDescription = "Close",
modifier = Modifier.size(16.dp),
tint = GrayMedium
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Box( | |
| modifier = Modifier | |
| .size(32.dp) | |
| .border(1.dp, GrayLight, CircleShape) | |
| .clip(CircleShape) | |
| .clickable { onDismiss() }, | |
| contentAlignment = Alignment.Center | |
| ) { | |
| Icon( | |
| imageVector = Icons.Default.Close, | |
| contentDescription = "Close", | |
| modifier = Modifier.size(16.dp), | |
| tint = GrayMedium | |
| ) | |
| IconButton( | |
| onClick = onDismiss, | |
| modifier = Modifier | |
| .size(48.dp) | |
| .border(1.dp, GrayLight, CircleShape) | |
| .clip(CircleShape) | |
| ) { | |
| Icon( | |
| imageVector = Icons.Default.Close, | |
| contentDescription = "Close", | |
| modifier = Modifier.size(16.dp), | |
| tint = GrayMedium | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around
lines 288 - 301, The close touch target is too small (Modifier.size(32.dp) on
the Box containing the Icon), making dismissal hard; replace the Box clickable
with a larger accessible touch target by using IconButton (or changing the
modifier to .size(48.dp) or .requiredSizeIn(minWidth = 48.dp, minHeight =
48.dp)) while keeping the CircleShape border/clip and the Icon size the same,
and invoke onDismiss from the IconButton's onClick to preserve behavior
(reference Box, Modifier.size(32.dp), Icon and onDismiss in the current code).
Comment on lines
+56
to
+85
| Column( | ||
| modifier = modifier | ||
| .fillMaxSize() | ||
| .background(White) | ||
| ) { | ||
| ProfileTopBar( | ||
| modifier = Modifier.padding(horizontal = 20.dp) | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(20.dp)) | ||
|
|
||
| ProfileRow( | ||
| modifier = Modifier.padding(horizontal = 20.dp), | ||
| onEditClick = navigateToEditProfile | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(24.dp)) | ||
|
|
||
| ProfileGameCarousel( | ||
| title = "Bookmarks", | ||
| games = bookmarkedGames, | ||
| onClick = { gameId -> navigateToGameDetails(gameId) } | ||
| ) | ||
|
|
||
| ProfileGameCarousel( | ||
| title = "Games You Might Like", | ||
| games = recommendedGames, | ||
| onClick = { gameId -> navigateToGameDetails(gameId) } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Make the profile screen vertically scrollable.
This screen stacks two carousels in a fixed-height Column, so the lower section can be clipped on smaller devices. A simple vertical scroll on the root container avoids hiding content.
📱 Suggested fix
Column(
modifier = modifier
.fillMaxSize()
.background(White)
+ .verticalScroll(rememberScrollState())
) {Add the corresponding rememberScrollState / verticalScroll imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/ProfileScreen.kt` around
lines 56 - 85, The root Column in the ProfileScreen should be made vertically
scrollable so the lower ProfileGameCarousel rows aren't clipped on small
devices; wrap the Column's modifier with a remembered scroll state
(rememberScrollState()) and apply verticalScroll(scrollState) to it (update the
modifier used by the root Column in ProfileScreen), and add the necessary
imports for rememberScrollState and verticalScroll; keep existing children like
ProfileTopBar, ProfileRow, and ProfileGameCarousel unchanged.
* git issue fix * advanced fitler first iteration * center icon * google-services.json * fix scrollable * pagination * changes for baseball scores * Delete app/src/google-services.json
There was a problem hiding this comment.
Actionable comments posted: 3
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/score/util/GameDataUtil.kt (1)
99-110:⚠️ Potential issue | 🔴 CriticalCrash risk:
scorePart[1]is accessed before the size guard.Line 100 indexes
scorePart[1]before theif (scorePart.size != 2) return nullcheck on line 101. For anyresultwhose comma-tail lacks a-(e.g."W, 5","L,forfeit", or even",()"),scorePart.size == 1and this will throwIndexOutOfBoundsException, whichScoreRepository.fetchGamesdoes not catch in its innermapNotNull— one malformed row blows up the whole page map.Also,
Log.d("HIHI", ...)at line 105 is a dev-leftover log (noisy tag, prints to logcat for every game parse). Please remove.🛡️ Proposed fix
val scorePart = parts[1].split("-") - val secondScorePartEdge = scorePart[1].split("(") if (scorePart.size != 2) return null + val secondScorePartEdge = scorePart[1].split("(") val homeScore = scorePart[0].toIntOrNull() - val oppScore = secondScorePartEdge[0].toIntOrNull() - Log.d("HIHI", oppScore.toString()) + val oppScore = secondScorePartEdge[0].toIntOrNull() if (homeScore != null && oppScore != null) {And drop the now-unused
import android.util.Logat line 3 if no other usages remain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/util/GameDataUtil.kt` around lines 99 - 110, Reorder and harden the parsing so you check scorePart.size before accessing scorePart[1]: move the guard if (scorePart.size != 2) return null to immediately after val scorePart = parts[1].split("-"), then only compute secondScorePartEdge = scorePart[1].split("(") and parse homeScore/oppScore after that check; also remove the dev Log.d("HIHI", ...) call and delete the now-unused android.util.Log import if it becomes unused.
♻️ Duplicate comments (3)
app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt (3)
84-91:⚠️ Potential issue | 🟡 MinorSeed the avatar sheet with the current selection.
Reopening the picker starts
selectedAvatarasnull, so Save is disabled until the user reselects an avatar.🖼️ Proposed fix
if (showAvatarPicker) { AvatarPickerBottomSheet( + initialAvatar = currentAvatar, onDismiss = { showAvatarPicker = false }, onAvatarSelected = { currentAvatar = it showAvatarPicker = false } @@ fun AvatarPickerBottomSheet( + initialAvatar: AvatarOption, onDismiss: () -> Unit, onAvatarSelected: (AvatarOption) -> Unit ) { - var selectedAvatar by remember { mutableStateOf<AvatarOption?>(null) } + var selectedAvatar by remember(initialAvatar) { mutableStateOf<AvatarOption?>(initialAvatar) }Also applies to: 256-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around lines 84 - 91, When opening the AvatarPickerBottomSheet the internal selection starts null so Save is disabled; seed its selected avatar with the current avatar by passing the currentAvatar into the picker (or setting the picker's initialSelectedAvatar state) so the sheet initializes with selectedAvatar = currentAvatar. Update the AvatarPickerBottomSheet invocation (and any other call sites like the one at lines 256-260) to accept and use an initialSelectedAvatar param and ensure its Save button checks that selectedAvatar is non-null after seeding.
73-82:⚠️ Potential issue | 🟠 MajorWire Save to a real action or disable it.
Line 81 still leaves an enabled Save CTA with a no-op handler, so edits appear to save but are silently discarded. Pass a real
onSaveClickthrough the screen boundary, or disable/hide the button until persistence is available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around lines 73 - 82, The Save button is wired to a no-op handler in EditProfileScreen; wire a real persistence action or disable the CTA. Propagate an onSaveClick callback into EditProfileScreen (or accept a ViewModel/saveUser function) and pass it into EditProfileContent’s onSaveClick instead of the empty lambda, or alternatively change EditProfileContent to hide/disable the Save button when onSaveClick is null and pass null until persistence is implemented; update references: EditProfileScreen, EditProfileContent, onSaveClick, and any caller that composes EditProfileScreen to provide the actual save handler (using name, username, currentAvatar state).
288-301:⚠️ Potential issue | 🟡 MinorExpand the close button touch target.
The dismiss target is still only 32.dp, below the usual 48.dp accessible touch target.
♿ Proposed fix
- Box( + IconButton( + onClick = onDismiss, modifier = Modifier - .size(32.dp) + .size(48.dp) .border(1.dp, GrayLight, CircleShape) - .clip(CircleShape) - .clickable { onDismiss() }, - contentAlignment = Alignment.Center + .clip(CircleShape) ) { Icon( imageVector = Icons.Default.Close, contentDescription = "Close", modifier = Modifier.size(16.dp), tint = GrayMedium ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around lines 288 - 301, The close button's clickable area is only Modifier.size(32.dp) which is below the 48.dp accessibility target; wrap or modify the Box so the visual circle stays 32.dp but the touch target is at least 48.dp (e.g., use Modifier.sizeIn(minWidth = 48.dp, minHeight = 48.dp) or add transparent padding around the clickable area and apply .clickable { onDismiss() } to the larger container). Update the Box (the composable using Modifier.size(32.dp) and onDismiss) so the border/clip/Icon keep 32.dp visual size while the outer modifier ensures a minimum touch target of 48.dp.
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt (2)
136-141: HoistparseResultScore(graphqlGame.result)to avoid parsing the same string twice.The fallback invokes
parseResultScore(graphqlGame.result)independently forcornellScoreandotherScore, parsing the sameresultstring up to twice per game across potentially many paged games. Compute once and reuse both.firstand.second.♻️ Proposed refactor
- .mapNotNull { graphqlGame -> - val scores = graphqlGame.result?.split(",")?.getOrNull(1)?.split("-") - val cornellScore = scores?.getOrNull(0)?.toNumberOrNull() - ?: parseResultScore(graphqlGame.result)?.first - val otherScore = scores?.getOrNull(1)?.toNumberOrNull() ?: parseResultScore( - graphqlGame.result - )?.second + .mapNotNull { graphqlGame -> + val scores = graphqlGame.result?.split(",")?.getOrNull(1)?.split("-") + val parsedFallback by lazy { parseResultScore(graphqlGame.result) } + val cornellScore = + scores?.getOrNull(0)?.toNumberOrNull() ?: parsedFallback?.first + val otherScore = + scores?.getOrNull(1)?.toNumberOrNull() ?: parsedFallback?.secondNote:
fetchGamesPrevat lines 71-73 was not updated with the same fallback. If it’s still reachable, it will have inconsistent null behavior vs.fetchGames; otherwise consider removing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt` around lines 136 - 141, Hoist the call to parseResultScore(graphqlGame.result) into a local val (e.g., val parsedResult = parseResultScore(graphqlGame.result)) and use parsedResult?.first and parsedResult?.second as fallbacks for cornellScore and otherScore instead of calling parseResultScore twice; update the block that sets cornellScore and otherScore to first try the split-based numbers and then fall back to parsedResult?.first / parsedResult?.second, and make the same change in fetchGamesPrev so both fetchGames and fetchGamesPrev have consistent null/fallback behavior.
180-180: Debug log leaks game IDs on every fetch.
Log.d("ScoreRepository", "Fetching game with id: $id")fires on every detail open. Consider gating behindBuildConfig.DEBUGor removing before release to keep production logs clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt` at line 180, The debug log in ScoreRepository (the Log.d call that prints "Fetching game with id: $id") leaks game IDs; guard or remove it to avoid exposing IDs in production by wrapping the Log.d call behind a BuildConfig.DEBUG check (or removing it entirely), i.e., update the fetch method in ScoreRepository to only emit this debug message when BuildConfig.DEBUG is true (or delete the Log.d line) so production builds won't log game IDs.
🤖 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/score/screen/EditProfileScreen.kt`:
- Around line 262-275: The upload glyph (R.drawable.upload) is included in the
avatars list and is being treated as a selectable AvatarOption, so remove or
exclude the upload placeholder from any selectable avatar collections (the
avatars list and the other avatar lists referenced around the same places) and
handle it separately in the UI; locate where AvatarOption(R.drawable.upload) is
added and either (a) remove that entry and render the upload button/placebo
outside the selectable avatar lists, or (b) add a distinguishing flag/property
to AvatarOption (e.g., isPlaceholder or isUpload) and update the selection logic
to ignore options with that flag so users cannot save the upload glyph as their
profile photo.
- Around line 191-222: The name and username BasicTextField components allow
multi-line input and lack accessibility labels; update the two BasicTextField
calls (the one bound to name/onNameChange and the one bound to
username/onUsernameChange) to set singleLine = true and add a semantics modifier
with contentDescription (e.g., semantics { contentDescription = "Name input" }
and "Username input") so screen readers can identify them; keep the existing
textStyle and modifier (e.g., Modifier.weight(1f)) and add semantics via
Modifier.semantics(...) chained into the modifier for each field.
In `@app/src/main/java/com/cornellappdev/score/screen/HomeScreen.kt`:
- Around line 163-169: The profile Image in HomeScreen.kt currently has a 28.dp
clickable area; increase the touch target by wrapping or altering the modifiers
so the clickable container is 48.dp while keeping the visual image 28.dp (e.g.,
make a parent Box or Surface with
modifier.size(48.dp).clip(CircleShape).clickable { navigateToProfile() } and
place the Image inside with .size(28.dp) and centered). Update the modifiers on
the Image and/or its parent so navigateToProfile() remains the click handler and
the visual appearance is unchanged while the accessibility target becomes 48.dp.
---
Outside diff comments:
In `@app/src/main/java/com/cornellappdev/score/util/GameDataUtil.kt`:
- Around line 99-110: Reorder and harden the parsing so you check scorePart.size
before accessing scorePart[1]: move the guard if (scorePart.size != 2) return
null to immediately after val scorePart = parts[1].split("-"), then only compute
secondScorePartEdge = scorePart[1].split("(") and parse homeScore/oppScore after
that check; also remove the dev Log.d("HIHI", ...) call and delete the
now-unused android.util.Log import if it becomes unused.
---
Duplicate comments:
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt`:
- Around line 84-91: When opening the AvatarPickerBottomSheet the internal
selection starts null so Save is disabled; seed its selected avatar with the
current avatar by passing the currentAvatar into the picker (or setting the
picker's initialSelectedAvatar state) so the sheet initializes with
selectedAvatar = currentAvatar. Update the AvatarPickerBottomSheet invocation
(and any other call sites like the one at lines 256-260) to accept and use an
initialSelectedAvatar param and ensure its Save button checks that
selectedAvatar is non-null after seeding.
- Around line 73-82: The Save button is wired to a no-op handler in
EditProfileScreen; wire a real persistence action or disable the CTA. Propagate
an onSaveClick callback into EditProfileScreen (or accept a ViewModel/saveUser
function) and pass it into EditProfileContent’s onSaveClick instead of the empty
lambda, or alternatively change EditProfileContent to hide/disable the Save
button when onSaveClick is null and pass null until persistence is implemented;
update references: EditProfileScreen, EditProfileContent, onSaveClick, and any
caller that composes EditProfileScreen to provide the actual save handler (using
name, username, currentAvatar state).
- Around line 288-301: The close button's clickable area is only
Modifier.size(32.dp) which is below the 48.dp accessibility target; wrap or
modify the Box so the visual circle stays 32.dp but the touch target is at least
48.dp (e.g., use Modifier.sizeIn(minWidth = 48.dp, minHeight = 48.dp) or add
transparent padding around the clickable area and apply .clickable { onDismiss()
} to the larger container). Update the Box (the composable using
Modifier.size(32.dp) and onDismiss) so the border/clip/Icon keep 32.dp visual
size while the outer modifier ensures a minimum touch target of 48.dp.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt`:
- Around line 136-141: Hoist the call to parseResultScore(graphqlGame.result)
into a local val (e.g., val parsedResult = parseResultScore(graphqlGame.result))
and use parsedResult?.first and parsedResult?.second as fallbacks for
cornellScore and otherScore instead of calling parseResultScore twice; update
the block that sets cornellScore and otherScore to first try the split-based
numbers and then fall back to parsedResult?.first / parsedResult?.second, and
make the same change in fetchGamesPrev so both fetchGames and fetchGamesPrev
have consistent null/fallback behavior.
- Line 180: The debug log in ScoreRepository (the Log.d call that prints
"Fetching game with id: $id") leaks game IDs; guard or remove it to avoid
exposing IDs in production by wrapping the Log.d call behind a BuildConfig.DEBUG
check (or removing it entirely), i.e., update the fetch method in
ScoreRepository to only emit this debug message when BuildConfig.DEBUG is true
(or delete the Log.d line) so production builds won't log game IDs.
🪄 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: 1495b11e-c2ea-48a9-ac97-2f306096fe75
📒 Files selected for processing (11)
app/build.gradle.ktsapp/src/main/graphql/Games.graphqlapp/src/main/java/com/cornellappdev/score/model/Game.ktapp/src/main/java/com/cornellappdev/score/model/ScoreRepository.ktapp/src/main/java/com/cornellappdev/score/nav/ScoreNavHost.ktapp/src/main/java/com/cornellappdev/score/nav/root/RootNavigation.ktapp/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.ktapp/src/main/java/com/cornellappdev/score/screen/HomeScreen.ktapp/src/main/java/com/cornellappdev/score/util/GameDataUtil.ktapp/src/main/res/drawable/profile_ic.xmlapp/src/main/res/drawable/upload.xml
✅ Files skipped from review due to trivial changes (3)
- app/src/main/graphql/Games.graphql
- app/src/main/res/drawable/profile_ic.xml
- app/src/main/res/drawable/upload.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/build.gradle.kts
Comment on lines
+191
to
+222
| BasicTextField( | ||
| value = name, | ||
| onValueChange = onNameChange, | ||
| textStyle = Style.heading5.copy( | ||
| color = GrayPrimary, | ||
| textAlign = TextAlign.End | ||
| ), | ||
| modifier = Modifier.weight(1f) | ||
| ) | ||
| } | ||
|
|
||
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(horizontal = 24.dp), | ||
| horizontalArrangement = Arrangement.SpaceBetween, | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| Text( | ||
| text = "Username", | ||
| style = Style.heading6 | ||
| ) | ||
|
|
||
| BasicTextField( | ||
| value = username, | ||
| onValueChange = onUsernameChange, | ||
| textStyle = Style.heading5.copy( | ||
| color = GrayPrimary, | ||
| textAlign = TextAlign.End | ||
| ), | ||
| modifier = Modifier.weight(1f) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "build.gradle*" -o -name "gradle.properties" | head -20Repository: cuappdev/score-android
Length of output: 127
🏁 Script executed:
cat ./app/build.gradle.kts | grep -A 5 -B 5 -i "compose\|foundation"Repository: cuappdev/score-android
Length of output: 3169
🏁 Script executed:
find . -name "libs.versions.toml" -o -name "versions.gradle" -o -name "gradle/libs.versions.toml" | xargs cat 2>/dev/nullRepository: cuappdev/score-android
Length of output: 2282
🏁 Script executed:
cat -n app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt | sed -n '185,230p'Repository: cuappdev/score-android
Length of output: 1820
🏁 Script executed:
head -50 app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.ktRepository: cuappdev/score-android
Length of output: 2424
🏁 Script executed:
rg "import.*semantics" app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.ktRepository: cuappdev/score-android
Length of output: 48
🏁 Script executed:
# Check Compose Foundation 1.10.3 release date and BasicTextField API
# Let's also verify the exact version we're dealing with
python3 << 'EOF'
# Compose Foundation 1.10.3 was released around mid-2024
# BasicTextField singleLine parameter was available from early versions
# The proposal uses androidx.compose.ui.semantics which should be available
print("Compose Foundation 1.10.3 Timeline:")
print("- Released: July 2024 (part of Compose 1.7.0)")
print("- BasicTextField with singleLine parameter: Available since Compose 1.0.0+")
print("- semantics API: Available since Compose 1.0.0+")
print("\nBased on Compose history:")
print("- singleLine parameter in BasicTextField: ✓ Available in 1.10.3")
print("- contentDescription in semantics: ✓ Available in 1.10.3")
EOFRepository: cuappdev/score-android
Length of output: 418
🏁 Script executed:
# Search for any existing usage of semantics or singleLine in the codebase
rg "singleLine\s*=" app/src/main/java/com/cornellappdev/score/Repository: cuappdev/score-android
Length of output: 183
🏁 Script executed:
# Check how semantics is used in the codebase
rg "contentDescription" app/src/main/java/com/cornellappdev/score/ -B 2 -A 2Repository: cuappdev/score-android
Length of output: 33818
🏁 Script executed:
# Check if semantics is used anywhere in the codebase
rg "semantics\s*{" app/src/main/java/com/cornellappdev/score/ -A 2Repository: cuappdev/score-android
Length of output: 155
🏁 Script executed:
# Check if semantics is used anywhere in the codebase
rg "semantics" app/src/main/java/com/cornellappdev/score/ -A 2Repository: cuappdev/score-android
Length of output: 48
🏁 Script executed:
# Check how singleLine is used in HighlightsSearchBar
cat -n app/src/main/java/com/cornellappdev/score/components/highlights/HighlightsSearchBar.kt | grep -A 10 "singleLine"Repository: cuappdev/score-android
Length of output: 666
Make the edit fields single-line and accessible.
The name and username fields currently allow newline input and lack accessibility labels. This breaks the row layout and makes fields inaccessible to screen readers. Add singleLine = true and expose field labels via semantics { contentDescription = ... }. The required APIs are available in Compose Foundation 1.10.3.
♿ Proposed fix
+import androidx.compose.ui.semantics.contentDescription
+import androidx.compose.ui.semantics.semantics
+
BasicTextField(
value = name,
onValueChange = onNameChange,
textStyle = Style.heading5.copy(
color = GrayPrimary,
textAlign = TextAlign.End
),
- modifier = Modifier.weight(1f)
+ singleLine = true,
+ modifier = Modifier
+ .weight(1f)
+ .semantics { contentDescription = "Name" }
)
@@
BasicTextField(
value = username,
onValueChange = onUsernameChange,
textStyle = Style.heading5.copy(
color = GrayPrimary,
textAlign = TextAlign.End
),
- modifier = Modifier.weight(1f)
+ singleLine = true,
+ modifier = Modifier
+ .weight(1f)
+ .semantics { contentDescription = "Username" }
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BasicTextField( | |
| value = name, | |
| onValueChange = onNameChange, | |
| textStyle = Style.heading5.copy( | |
| color = GrayPrimary, | |
| textAlign = TextAlign.End | |
| ), | |
| modifier = Modifier.weight(1f) | |
| ) | |
| } | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(horizontal = 24.dp), | |
| horizontalArrangement = Arrangement.SpaceBetween, | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| Text( | |
| text = "Username", | |
| style = Style.heading6 | |
| ) | |
| BasicTextField( | |
| value = username, | |
| onValueChange = onUsernameChange, | |
| textStyle = Style.heading5.copy( | |
| color = GrayPrimary, | |
| textAlign = TextAlign.End | |
| ), | |
| modifier = Modifier.weight(1f) | |
| ) | |
| BasicTextField( | |
| value = name, | |
| onValueChange = onNameChange, | |
| textStyle = Style.heading5.copy( | |
| color = GrayPrimary, | |
| textAlign = TextAlign.End | |
| ), | |
| singleLine = true, | |
| modifier = Modifier | |
| .weight(1f) | |
| .semantics { contentDescription = "Name" } | |
| ) | |
| } | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(horizontal = 24.dp), | |
| horizontalArrangement = Arrangement.SpaceBetween, | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| Text( | |
| text = "Username", | |
| style = Style.heading6 | |
| ) | |
| BasicTextField( | |
| value = username, | |
| onValueChange = onUsernameChange, | |
| textStyle = Style.heading5.copy( | |
| color = GrayPrimary, | |
| textAlign = TextAlign.End | |
| ), | |
| singleLine = true, | |
| modifier = Modifier | |
| .weight(1f) | |
| .semantics { contentDescription = "Username" } | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around
lines 191 - 222, The name and username BasicTextField components allow
multi-line input and lack accessibility labels; update the two BasicTextField
calls (the one bound to name/onNameChange and the one bound to
username/onUsernameChange) to set singleLine = true and add a semantics modifier
with contentDescription (e.g., semantics { contentDescription = "Name input" }
and "Username input") so screen readers can identify them; keep the existing
textStyle and modifier (e.g., Modifier.weight(1f)) and add semantics via
Modifier.semantics(...) chained into the modifier for each field.
Comment on lines
+262
to
+275
| val avatars = listOf( | ||
| AvatarOption(R.drawable.upload), | ||
| AvatarOption(R.drawable.pingpong_profile), | ||
| AvatarOption(R.drawable.soccer_profile), | ||
| AvatarOption(R.drawable.tennis_profile), | ||
| AvatarOption(R.drawable.badminton_profile), | ||
| AvatarOption(R.drawable.billiards_profile), | ||
| AvatarOption(R.drawable.basketball_profile), | ||
| AvatarOption(R.drawable.bowling_profile), | ||
| AvatarOption(R.drawable.volleyball_profile), | ||
| AvatarOption(R.drawable.golf_profile), | ||
| AvatarOption(R.drawable.weightlifting_profile), | ||
| AvatarOption(R.drawable.baseball_profile) | ||
| ) |
There was a problem hiding this comment.
Don’t make the upload placeholder a saveable avatar.
R.drawable.upload is treated like a normal avatar, so users can save the upload glyph as their profile photo even though uploaded photos are marked as future work.
🧹 Proposed short-term fix
val avatars = listOf(
- AvatarOption(R.drawable.upload),
AvatarOption(R.drawable.pingpong_profile),
AvatarOption(R.drawable.soccer_profile),
AvatarOption(R.drawable.tennis_profile),Also applies to: 314-317, 398-398
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/EditProfileScreen.kt` around
lines 262 - 275, The upload glyph (R.drawable.upload) is included in the avatars
list and is being treated as a selectable AvatarOption, so remove or exclude the
upload placeholder from any selectable avatar collections (the avatars list and
the other avatar lists referenced around the same places) and handle it
separately in the UI; locate where AvatarOption(R.drawable.upload) is added and
either (a) remove that entry and render the upload button/placebo outside the
selectable avatar lists, or (b) add a distinguishing flag/property to
AvatarOption (e.g., isPlaceholder or isUpload) and update the selection logic to
ignore options with that flag so users cannot save the upload glyph as their
profile photo.
Comment on lines
+163
to
+169
| Image( | ||
| painter = painterResource(id = R.drawable.profile_ic), | ||
| contentDescription = "Profile", | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(start = 24.dp) | ||
| .size(28.dp) | ||
| .clip(CircleShape) | ||
| .clickable { navigateToProfile() } |
There was a problem hiding this comment.
Increase the profile button touch target.
The clickable profile entry point is only 28.dp. Wrap the visual image in a 48.dp clickable container so it is easier to tap.
♿ Proposed fix
- Image(
- painter = painterResource(id = R.drawable.profile_ic),
- contentDescription = "Profile",
- modifier = Modifier
- .size(28.dp)
- .clip(CircleShape)
- .clickable { navigateToProfile() }
- )
+ Box(
+ modifier = Modifier
+ .size(48.dp)
+ .clip(CircleShape)
+ .clickable { navigateToProfile() },
+ contentAlignment = Alignment.Center
+ ) {
+ Image(
+ painter = painterResource(id = R.drawable.profile_ic),
+ contentDescription = "Profile",
+ modifier = Modifier
+ .size(28.dp)
+ .clip(CircleShape)
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Image( | |
| painter = painterResource(id = R.drawable.profile_ic), | |
| contentDescription = "Profile", | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(start = 24.dp) | |
| .size(28.dp) | |
| .clip(CircleShape) | |
| .clickable { navigateToProfile() } | |
| Box( | |
| modifier = Modifier | |
| .size(48.dp) | |
| .clip(CircleShape) | |
| .clickable { navigateToProfile() }, | |
| contentAlignment = Alignment.Center | |
| ) { | |
| Image( | |
| painter = painterResource(id = R.drawable.profile_ic), | |
| contentDescription = "Profile", | |
| modifier = Modifier | |
| .size(28.dp) | |
| .clip(CircleShape) | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/HomeScreen.kt` around lines
163 - 169, The profile Image in HomeScreen.kt currently has a 28.dp clickable
area; increase the touch target by wrapping or altering the modifiers so the
clickable container is 48.dp while keeping the visual image 28.dp (e.g., make a
parent Box or Surface with modifier.size(48.dp).clip(CircleShape).clickable {
navigateToProfile() } and place the Image inside with .size(28.dp) and
centered). Update the modifiers on the Image and/or its parent so
navigateToProfile() remains the click handler and the visual appearance is
unchanged while the accessibility target becomes 48.dp.
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
4 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.
Summary by CodeRabbit
New Features
Style