Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new Onboarding composable and preview, makes Onboarding the start destination and wires its login to navigate to Home (clearing back stack), introduces multiple vector drawable resources, removes one Gradle dependency, and updates the version catalog; MainActivity import updated but still renders MainNavigation(). Changes
Sequence Diagram(s)sequenceDiagram
participant App as AppStart
participant Nav as MainNavigation
participant Onb as Onboarding
participant NavCtrl as NavController
App->>Nav: launch (start destination = Onboarding)
Nav->>Onb: render Onboarding()
Onb->>Onb: run staged entrance animations
Onb-->>Onb: start continuous clock-hand animations (if triggered)
Onb->>NavCtrl: onLoginClick()
NavCtrl->>NavCtrl: clear back stack
NavCtrl->>Nav: navigate to Home
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/build.gradle.kts (1)
44-48:⚠️ Potential issue | 🟠 MajorRemove redundant
ui-graphicsdependencies.Lines 44, 47, and 48 all import variations of
androidx.compose.ui:ui-graphics:
libs.androidx.compose.ui.graphics— BOM-managed (correct)libs.androidx.ui.graphics— explicit version 1.10.2libs.ui.graphics— explicit version 1.10.4Only the BOM-managed version (line 44) should be retained. The others are redundant and introduce version conflicts.
🛠️ Suggested fix
implementation(libs.androidx.compose.ui.graphics) implementation(libs.androidx.compose.ui.tooling.preview) implementation(libs.androidx.compose.material3) - implementation(libs.androidx.ui.graphics) - implementation(libs.ui.graphics) testImplementation(libs.junit)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/build.gradle.kts` around lines 44 - 48, The build file includes duplicate ui-graphics dependencies: keep the BOM-managed implementation(libs.androidx.compose.ui.graphics) and remove the redundant explicit-version entries implementation(libs.androidx.ui.graphics) and implementation(libs.ui.graphics) to avoid version conflicts; update the dependencies block so only implementation(libs.androidx.compose.ui.graphics) remains for ui-graphics.
🧹 Nitpick comments (4)
app/src/main/res/drawable/ic_google_g.xml (1)
1-5: Consider reducing the intrinsic size of this vector drawable.The 600dp × 600dp dimensions are unusually large for a logo icon. While vector drawables scale well, using smaller intrinsic dimensions (e.g., 48dp or 64dp) would be more aligned with typical icon sizes and reduce memory footprint slightly. The visual output will be identical when scaled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/drawable/ic_google_g.xml` around lines 1 - 5, The vector drawable's intrinsic size is set to 600dp which is unnecessarily large for an icon; update the root <vector> attributes android:width and android:height to a typical icon size (for example 48dp or 64dp) while keeping android:viewportWidth and android:viewportHeight unchanged so the graphic scales identically — locate the <vector> element in ic_google_g.xml and change android:width and android:height to the smaller dp value (e.g., "48dp") to reduce memory footprint.app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt (3)
170-248: Consider extracting a reusable login button composable.The Google and Cornell login buttons share identical structure (bordered Button with icon + text Row). A shared composable would reduce duplication and simplify future maintenance when implementing actual login functionality.
♻️ Example extraction
`@Composable` private fun LoginButton( iconRes: Int, iconDescription: String, text: String, onClick: () -> Unit, modifier: Modifier = Modifier ) { val curve = 50.dp Button( modifier = modifier .height(54.dp) .border( width = 1.dp, color = Color(0XFFBCB2B2), shape = RoundedCornerShape(curve) ) .fillMaxWidth(), onClick = onClick, shape = RoundedCornerShape(curve), colors = ButtonDefaults.buttonColors( containerColor = Color.White, contentColor = Color.Black ), contentPadding = PaddingValues(horizontal = 20.dp) ) { Row( verticalAlignment = Alignment.CenterVertically, modifier = Modifier.fillMaxWidth() ) { Image( painter = painterResource(id = iconRes), contentDescription = iconDescription, modifier = Modifier.size(32.dp), contentScale = ContentScale.Fit ) Spacer(modifier = Modifier.width(16.dp)) Text( text = text, style = MaterialTheme.typography.titleMedium.copy( fontWeight = FontWeight.Normal, color = Color.Black ), fontSize = 14.sp ) } } }Usage:
LoginButton( iconRes = R.drawable.ic_google_g, iconDescription = "Google logo", text = "Log-in with Google", onClick = {} )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 170 - 248, Duplicate bordered Button + Row code in Onboarding.kt should be extracted into a reusable composable; create a private `@Composable` named LoginButton(iconRes: Int, iconDescription: String, text: String, onClick: () -> Unit, modifier: Modifier = Modifier) that encapsulates the rounded bordered Button, Row, Image and Text (use the same styles, sizes, curve value and contentPadding), then replace the two inline Button blocks with calls to LoginButton(R.drawable.ic_google_g, "Google logo", "Log-in with Google", {...}) and LoginButton(R.drawable.ic_cornell_logo, "Cornell logo", "Log-in with Cornell netID", {...}) passing any modifiers as needed.
317-343: Consider adding comments to explain the transform origin calculations.The magic numbers (
11f / 30f,2.5f/25f,16.5f/25f) represent pivot points for the clock hand rotation but are not self-documenting. Brief comments would improve maintainability.📝 Proposed improvement with documentation
Image( painter = painterResource(id = R.drawable.ic_hour_hand), contentDescription = "Hour hand", contentScale = ContentScale.FillBounds, modifier = Modifier .size(hourHandWidth, hourHandHeight) .graphicsLayer { + // Pivot point offset from center (11/30 of the hand dimensions) val value = 11f / 30f translationX = (0.5f - value) * size.width translationY = (0.5f - value) * size.height rotationZ = hourHandRotation transformOrigin = TransformOrigin(value, value) } ) Image( painter = painterResource(id = R.drawable.ic_minute_hand), contentDescription = "Minute hand", contentScale = ContentScale.FillBounds, modifier = Modifier .size(minuteHandWidth, minuteHandHeight) .graphicsLayer { + // Pivot point: x at 2.5/25, y at 16.5/25 of minute hand dimensions val xValue = 2.5f/25f val yValue = 16.5f/25f translationX = (0.5f - xValue) * size.width translationY = (0.5f - yValue) * size.height rotationZ = minuteHandRotation transformOrigin = TransformOrigin(xValue, yValue) } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 317 - 343, Add brief inline comments above the Modifier.graphicsLayer blocks for the hour and minute hand Images explaining the meaning and origin of the magic-number fractions (value = 11f/30f, xValue = 2.5f/25f, yValue = 16.5f/25f) and that they define the pivot/TransformOrigin for rotation; mention that these are normalized coordinates relative to the image size and tie them to the corresponding rotation variables (hourHandRotation, minuteHandRotation) so future maintainers understand why those specific fractions were chosen and how to adjust them if the asset or sizing changes.
92-111: Consider resetting rotation values to prevent unbounded float growth.The rotation values accumulate indefinitely (360, 720, 1080, ...). While float overflow is impractical, precision degrades for very large floats. If the app stays open for extended periods, this could eventually cause visual jitter.
♻️ Proposed fix using snapTo to reset after each rotation
LaunchedEffect(handsSpinStarted) { if (!handsSpinStarted) return@LaunchedEffect launch { while (isActive) { minuteHandRotation.animateTo( - minuteHandRotation.value + 360f, + 360f, animationSpec = tween(durationMillis = 2_000, easing = LinearEasing) ) + minuteHandRotation.snapTo(0f) } } launch { while (isActive) { hourHandRotation.animateTo( - hourHandRotation.value + 360f, + 360f, animationSpec = tween(durationMillis = 10_000, easing = LinearEasing) ) + hourHandRotation.snapTo(0f) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 92 - 111, The rotation values in LaunchedEffect (guarded by handsSpinStarted) accumulate indefinitely via minuteHandRotation and hourHandRotation animateTo calls; after each animateTo completes, reset the corresponding Animatable back into a small range (e.g., modulo 360) using snapTo to avoid unbounded float growth and precision loss. Specifically, inside each launched coroutine that animates minuteHandRotation and hourHandRotation, await the animateTo call completion and then call minuteHandRotation.snapTo(minuteHandRotation.value % 360f) and similarly for hourHandRotation (or compute the expected rotation % 360f) so the animation loop continues visually identical but keeps animatable values bounded. Ensure you perform the snapTo inside the same coroutine after animateTo and still respect isActive.
🤖 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/chimes/ui/screens/Onboarding.kt`:
- Around line 270-278: The Text composable using fontSize = 8.sp in
Onboarding.kt is too small for accessibility; update the Text (the "log in
without an account" Text call) to use a larger, accessible size (e.g., >=12.sp)
or use an appropriate MaterialTheme typography token like
MaterialTheme.typography.bodySmall or titleSmall instead of hardcoding 8.sp, and
preserve the existing fontWeight, color, and underline styling so the visual
intent remains.
In `@gradle/libs.versions.toml`:
- Line 16: Remove duplicate catalog entries for androidx.compose.ui:ui-graphics
and keep a single source of truth (prefer the Compose BOM-managed entry). In
libs.versions.toml, delete the redundant keys (e.g., remove ui-graphics =
"1.10.4" and/or androidx-ui-graphics = "1.10.2") so only
androidx-compose-ui-graphics (BOM-managed) remains; then update build.gradle.kts
to stop importing the removed aliases and only use the BOM-managed dependency
reference to avoid conflicting versions.
---
Outside diff comments:
In `@app/build.gradle.kts`:
- Around line 44-48: The build file includes duplicate ui-graphics dependencies:
keep the BOM-managed implementation(libs.androidx.compose.ui.graphics) and
remove the redundant explicit-version entries
implementation(libs.androidx.ui.graphics) and implementation(libs.ui.graphics)
to avoid version conflicts; update the dependencies block so only
implementation(libs.androidx.compose.ui.graphics) remains for ui-graphics.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt`:
- Around line 170-248: Duplicate bordered Button + Row code in Onboarding.kt
should be extracted into a reusable composable; create a private `@Composable`
named LoginButton(iconRes: Int, iconDescription: String, text: String, onClick:
() -> Unit, modifier: Modifier = Modifier) that encapsulates the rounded
bordered Button, Row, Image and Text (use the same styles, sizes, curve value
and contentPadding), then replace the two inline Button blocks with calls to
LoginButton(R.drawable.ic_google_g, "Google logo", "Log-in with Google", {...})
and LoginButton(R.drawable.ic_cornell_logo, "Cornell logo", "Log-in with Cornell
netID", {...}) passing any modifiers as needed.
- Around line 317-343: Add brief inline comments above the
Modifier.graphicsLayer blocks for the hour and minute hand Images explaining the
meaning and origin of the magic-number fractions (value = 11f/30f, xValue =
2.5f/25f, yValue = 16.5f/25f) and that they define the pivot/TransformOrigin for
rotation; mention that these are normalized coordinates relative to the image
size and tie them to the corresponding rotation variables (hourHandRotation,
minuteHandRotation) so future maintainers understand why those specific
fractions were chosen and how to adjust them if the asset or sizing changes.
- Around line 92-111: The rotation values in LaunchedEffect (guarded by
handsSpinStarted) accumulate indefinitely via minuteHandRotation and
hourHandRotation animateTo calls; after each animateTo completes, reset the
corresponding Animatable back into a small range (e.g., modulo 360) using snapTo
to avoid unbounded float growth and precision loss. Specifically, inside each
launched coroutine that animates minuteHandRotation and hourHandRotation, await
the animateTo call completion and then call
minuteHandRotation.snapTo(minuteHandRotation.value % 360f) and similarly for
hourHandRotation (or compute the expected rotation % 360f) so the animation loop
continues visually identical but keeps animatable values bounded. Ensure you
perform the snapTo inside the same coroutine after animateTo and still respect
isActive.
In `@app/src/main/res/drawable/ic_google_g.xml`:
- Around line 1-5: The vector drawable's intrinsic size is set to 600dp which is
unnecessarily large for an icon; update the root <vector> attributes
android:width and android:height to a typical icon size (for example 48dp or
64dp) while keeping android:viewportWidth and android:viewportHeight unchanged
so the graphic scales identically — locate the <vector> element in
ic_google_g.xml and change android:width and android:height to the smaller dp
value (e.g., "48dp") to reduce memory footprint.
🪄 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: 5ea5fe73-6939-45d3-be8a-54aa57811ec6
📒 Files selected for processing (12)
app/build.gradle.ktsapp/src/main/java/com/cornellappdev/chimes/MainActivity.ktapp/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.ktapp/src/main/res/drawable/ic_clock.xmlapp/src/main/res/drawable/ic_cornell_logo.xmlapp/src/main/res/drawable/ic_google_g.xmlapp/src/main/res/drawable/ic_hour_hand.xmlapp/src/main/res/drawable/ic_logo.xmlapp/src/main/res/drawable/ic_minute_hand.xmlapp/src/main/res/drawable/ic_onboarding_background.xmlgradle.propertiesgradle/libs.versions.toml
app/build.gradle.kts
Outdated
| compileSdk { | ||
| version = release(36) | ||
| } | ||
| compileSdk = 35 |
There was a problem hiding this comment.
Is there a reason we are changing to 35 instead of 36?
There was a problem hiding this comment.
There really isn't! I have changed it back to 36. I think I changed it to 35 as my computer was experiencing some bugs early on but now 36 does work.
| enableEdgeToEdge() | ||
| setContent { | ||
| MainNavigation() | ||
| Onboarding() |
There was a problem hiding this comment.
Might be good to merge changes from main and make use of the Navigation setup
|
|
||
| Spacer(modifier = Modifier.height(15.dp)) | ||
|
|
||
| Button( |
There was a problem hiding this comment.
This looks like you can create into a reusable composable since both buttons are virtually the same otherwise
| verticalAlignment = Alignment.CenterVertically, | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) { | ||
| Image( |
There was a problem hiding this comment.
Icon might be better here
| } | ||
| } | ||
|
|
||
| Box(modifier = Modifier.fillMaxSize().background(Color(0xFFFEF7F7))) { |
There was a problem hiding this comment.
Nit: For the color, see if there is a design system from figma and try to set one up for the colors with variables instead of defining them specifically every time
There was a problem hiding this comment.
Same for all color definitions if they are going to be reused for future screens
There was a problem hiding this comment.
I don't think there is a design system from Figma (I used an extension to extract the hex codes). I will create variables for the color hex codes though.
|
|
||
| Spacer(modifier = Modifier.height(45.dp)) | ||
|
|
||
| Button( |
There was a problem hiding this comment.
You can probably use a TextButton composable here
|
|
||
| launch { | ||
| while (isActive) { | ||
| minuteHandRotation.animateTo( |
There was a problem hiding this comment.
For this and the below animation, rememberInfiniteTransition is probably better for continuous animations.
There was a problem hiding this comment.
Actually, I just noticed that it changed when the effect started showing (the spin) for some reason, and so it wouldn't start at the right place (hour and minute hands). Working on it.
| modifier = Modifier | ||
| .alpha(loginAlpha.value) | ||
| .offset(y = loginOffsetY.value.dp) | ||
| .width(333.dp) |
There was a problem hiding this comment.
Might be better to have fillMaxWidth and use padding instead of specifying the width for diff device sizes
| modifier = Modifier | ||
| .size(hourHandWidth, hourHandHeight) | ||
| .graphicsLayer { | ||
| val value = 11f / 30f |
There was a problem hiding this comment.
Nit: Could probably benefit from a more descriptive name and can be a const. Same for other specific num values
| val chimesAlpha = remember { Animatable(0f) } | ||
| val headerOffsetY = remember { Animatable(0f) } | ||
| val loginAlpha = remember { Animatable(0f) } | ||
| val loginOffsetY = remember { Animatable(300f) } // Starts 300dp below |
There was a problem hiding this comment.
Is it possible for us to store dp directly instead of converting?
There was a problem hiding this comment.
Unfortunately I don't think so, as the Animatable function expects a float as input.
|
|
||
|
|
||
| @Composable | ||
| fun Onboarding () { |
There was a problem hiding this comment.
Nit: would be good to have a Preview function for the screen
| var handsSpinStarted by remember { mutableStateOf(false) } | ||
|
|
||
| LaunchedEffect(Unit) { | ||
| delay(300) |
There was a problem hiding this comment.
Are all these delays + launch blocks necessary or is there a way we can sequence most of the animations explicitly and reduce delays + launch until it is needed?
There was a problem hiding this comment.
I've simplified some of them by removing the launch and delay blocks, but some delays are still intentional to leave a small gap.
AndrewCheung360
left a comment
AndrewCheung360
left a comment
There was a problem hiding this comment.
Nice work! The animation looks really nice! Just left a couple comments for possible improvements to check out
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt (1)
197-211:⚠️ Potential issue | 🟠 MajorIncrease the guest-login CTA readability and touch target.
Line 210 (
8.sp) and Line 200 (25.dp) are below practical accessibility minimums for readable/tappable text controls.Suggested fix
TextButton( onClick = {}, shape = RoundedCornerShape(0.dp), - modifier = Modifier.height(25.dp), + modifier = Modifier.heightIn(min = 48.dp), contentPadding = PaddingValues(0.dp) ) { Text( text = "log in without an account", @@ - fontSize = 8.sp + fontSize = 12.sp ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 197 - 211, The guest-login CTA uses TextButton with a 25.dp height and 8.sp font which are below accessibility/touch-target/readability minimums; update the TextButton (the composable with text "log in without an account") to use a larger tappable height (e.g., >= 48.dp or apply Modifier.sizeIn(minHeight = 48.dp)) and increase the Text fontSize to a readable value (e.g., 14.sp) and adjust contentPadding to provide adequate touch padding; keep the visual styling (Underline, Thin weight) but ensure the combination meets contrast/readability standards.
🧹 Nitpick comments (3)
app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt (3)
268-268: Drop the lint suppression if no longer needed.
BoxWithConstraintsscope is used (maxWidth), so@SuppressLint("UnusedBoxWithConstraintsScope")appears unnecessary and can hide future signal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` at line 268, The `@SuppressLint`("UnusedBoxWithConstraintsScope") annotation in Onboarding.kt is likely unnecessary since BoxWithConstraints is used (e.g., maxWidth) — remove the suppression line so the linter can report real issues; specifically delete the `@SuppressLint`("UnusedBoxWithConstraintsScope") annotation residing above the composable where BoxWithConstraints and its scope (maxWidth) are referenced to allow the linter to function normally.
145-146: Move user-facing strings to resources for localization.These strings are hardcoded in composables, which makes i18n/l10n harder for upcoming screens.
Also applies to: 170-171, 179-180, 204-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 145 - 146, The hardcoded user-facing strings in Onboarding.kt (e.g., the Text composable with text = "chimes" and the other occurrences around lines referenced) must be moved into Android string resources and consumed via stringResource(...) in the composables; update each Text call (and any other places using literal strings at the noted locations) to use stringResource(R.string.<name>) and add corresponding entries in res/values/strings.xml with descriptive resource names (e.g., onboarding_app_title, onboarding_subtitle, onboarding_cta). Ensure you import androidx.compose.ui.res.stringResource and keep the existing style/typography usages unchanged.
49-50:whiteAlphais animated but never used in rendering.This adds animation/state work with no visual effect. Remove it or bind it to a composable’s alpha.
Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 49 - 50, The Animatable whiteAlpha declared in Onboarding.kt (alongside logoAlpha) is never applied to any composable so it should be removed or actually used; either delete the unused remember { Animatable(0f) } named whiteAlpha to avoid dead state, or bind it to a composable’s alpha (for example apply whiteAlpha.value to a Modifier.alpha(...) or the alpha parameter of a Box/Surface/Image in the Onboarding composable) and ensure any animation launched targets whiteAlpha; update both occurrences mentioned in the review.
🤖 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/chimes/ui/screens/Onboarding.kt`:
- Around line 169-174: The LoginButton instances currently wire onClick = {}
(no-op) which produces a dead-end UX; locate the LoginButton usages (and any
other interactive composables with empty onClick lambdas) and replace the empty
lambdas with real handlers — e.g., call the appropriate ViewModel/sign-in method
or navigation action (invoke your signInWithGoogle(), signInWithApple(), or
navController.navigate(...) functions) or explicitly mark the button disabled
(enabled = false) and provide a clear visual hint if the action isn’t
implemented yet; ensure every LoginButton/onClick and other clickable lambdas
mentioned are wired to a concrete handler or disabled so taps no longer do
nothing.
---
Duplicate comments:
In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt`:
- Around line 197-211: The guest-login CTA uses TextButton with a 25.dp height
and 8.sp font which are below accessibility/touch-target/readability minimums;
update the TextButton (the composable with text "log in without an account") to
use a larger tappable height (e.g., >= 48.dp or apply Modifier.sizeIn(minHeight
= 48.dp)) and increase the Text fontSize to a readable value (e.g., 14.sp) and
adjust contentPadding to provide adequate touch padding; keep the visual styling
(Underline, Thin weight) but ensure the combination meets contrast/readability
standards.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt`:
- Line 268: The `@SuppressLint`("UnusedBoxWithConstraintsScope") annotation in
Onboarding.kt is likely unnecessary since BoxWithConstraints is used (e.g.,
maxWidth) — remove the suppression line so the linter can report real issues;
specifically delete the `@SuppressLint`("UnusedBoxWithConstraintsScope")
annotation residing above the composable where BoxWithConstraints and its scope
(maxWidth) are referenced to allow the linter to function normally.
- Around line 145-146: The hardcoded user-facing strings in Onboarding.kt (e.g.,
the Text composable with text = "chimes" and the other occurrences around lines
referenced) must be moved into Android string resources and consumed via
stringResource(...) in the composables; update each Text call (and any other
places using literal strings at the noted locations) to use
stringResource(R.string.<name>) and add corresponding entries in
res/values/strings.xml with descriptive resource names (e.g.,
onboarding_app_title, onboarding_subtitle, onboarding_cta). Ensure you import
androidx.compose.ui.res.stringResource and keep the existing style/typography
usages unchanged.
- Around line 49-50: The Animatable whiteAlpha declared in Onboarding.kt
(alongside logoAlpha) is never applied to any composable so it should be removed
or actually used; either delete the unused remember { Animatable(0f) } named
whiteAlpha to avoid dead state, or bind it to a composable’s alpha (for example
apply whiteAlpha.value to a Modifier.alpha(...) or the alpha parameter of a
Box/Surface/Image in the Onboarding composable) and ensure any animation
launched targets whiteAlpha; update both occurrences mentioned in the review.
🪄 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: b7a45020-d41c-4674-be2d-ad1bdac1f5fa
📒 Files selected for processing (3)
app/build.gradle.ktsapp/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (1)
- gradle/libs.versions.toml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt (1)
269-279: Suppress annotation may be unnecessary.
BoxWithConstraintsScopeis actively used (maxWidthat lines 280-285), so the@SuppressLint("UnusedBoxWithConstraintsScope")annotation might not be needed. However, this is harmless if it was added to silence a false positive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt` around lines 269 - 279, The `@SuppressLint`("UnusedBoxWithConstraintsScope") annotation on the ClockLogo Composable appears unnecessary because the BoxWithConstraints scope is used (e.g., reading maxWidth inside the BoxWithConstraints); remove the `@SuppressLint` annotation from the ClockLogo declaration so the linter can correctly reflect usage, leaving the BoxWithConstraints and its internal maxWidth references intact.
🤖 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/chimes/ui/screens/Onboarding.kt`:
- Around line 45-51: Remove the file-level mutable var onLoginClick and instead
pass the callback through the composable hierarchy: delete the top-level
onLoginClick var, update the Onboarding composable signature to accept
onLoginButtonClick and forward that parameter to LoginButton (do not reassign
global state), update the LoginButton composable to accept a lambda parameter
(e.g., onClick: () -> Unit) and invoke it instead of referencing the removed
global, and update all call sites that previously relied on the global (the
places where LoginButton is invoked around lines 173 and 181) to pass the
onLoginButtonClick lambda through.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt`:
- Around line 269-279: The `@SuppressLint`("UnusedBoxWithConstraintsScope")
annotation on the ClockLogo Composable appears unnecessary because the
BoxWithConstraints scope is used (e.g., reading maxWidth inside the
BoxWithConstraints); remove the `@SuppressLint` annotation from the ClockLogo
declaration so the linter can correctly reflect usage, leaving the
BoxWithConstraints and its internal maxWidth references intact.
🪄 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: 96c23e19-7182-4f42-a0ba-891b2fad7fa3
📒 Files selected for processing (5)
app/build.gradle.ktsapp/src/main/java/com/cornellappdev/chimes/MainActivity.ktapp/src/main/java/com/cornellappdev/chimes/ui/navigation/MainNavigation.ktapp/src/main/java/com/cornellappdev/chimes/ui/navigation/NavigationItem.ktapp/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt
💤 Files with no reviewable changes (1)
- app/build.gradle.kts
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/cornellappdev/chimes/MainActivity.kt
app/src/main/java/com/cornellappdev/chimes/ui/screens/Onboarding.kt
Outdated
Show resolved
Hide resolved
3 participants
Overview
Implemented the complete display of the Onboarding Screen
All of the buttons in the log-in interface are currently display-only
Changes Made
Added
Onboarding.ktas the class that will include all the code for the onboarding screenAdded
ic_clock.xmlas the vector asset representing the chimes logo without both minute and hour handsAdded
ic_cornell_logo.xmlas the vector asset representing the Cornell University logoAdded
ic_google_g.xmlas the vector asset representing the square Google "G" logoAdded
ic_hour_hand.xmlas the vector asset representing the hour hand in the chimes logoAdded
ic_logo.xmlas the vector asset representing the full chimes logo with hour and minute hands in their respective positionsAdded
ic_minute_hand.xmlas the vector asset representing the minute hand in the chimes logoAdded
ic_onboarding_background.xmlas the vector asset representing the first screen of onboarding's backgroundAltered
MainActivity.ktto display only the onboarding for nowAltered
build.gradle.ktsas my gradle was not building correctly under previous settingsDemo
screen-20260326-012656.mp4
Test Method
Motorola Edge 2022
Summary by CodeRabbit
New Features
Chores