Knji #19
Knji #19
Conversation
WalkthroughThe changes implement a Material Design 3 theming system with edge-to-edge display support, rename the BottomAppBar component to NavigationBar, modify the Settings composable's public API by removing the viewModel parameter, systematically replace hard-coded colors throughout the codebase with theme-aware color schemes, and introduce new UI composables for account management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Rationale: The diff spans 11 files with a mix of repetitive color/theming replacements and heterogeneous structural changes. Key areas requiring careful review include: systematic MaterialTheme color migration across multiple files; public API change to Settings (viewModel removal) requiring verification of caller updates; component renaming and state management adjustments; edge-to-edge display configuration with DisposableEffect; and a potential onClick bug in PassKeyDetail (onClick = { onClick } vs onClick = { onClick() }). Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive theme overhaul for the Hashin Android application, transitioning from hardcoded colors to a Material3-based theming system with proper dark/light mode support and edge-to-edge display capabilities.
Key Changes
- Implemented Material3 color schemes with custom color definitions for both dark and light themes
- Enabled edge-to-edge display with transparent system bars
- Refactored all UI components to use MaterialTheme color scheme instead of hardcoded colors
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Theme.kt | Added dark/light color schemes with custom colors, edge-to-edge support with system bar styling, and ripple theme formatting |
| Color.kt | Defined custom color palette including surface variants, selection colors, and renamed color constants |
| MainActivity.kt | Configured window insets for edge-to-edge display and disabled dynamic color theming |
| Settings.kt | Simplified UI by removing animations, restructured profile card, and updated to use theme colors |
| PassKeyDetail.kt | Replaced hardcoded colors with MaterialTheme references and cleaned up unused variables |
| PassKey.kt | Updated text field styling to use theme colors and added custom cursor color |
| Vault.kt | Added background color from theme and commented out debug logging |
| Home.kt | Renamed BottomAppBar to NavigationBar and switched to saveable state |
| NavigationBar.kt | Renamed component, adjusted layout for edge-to-edge display, and improved preview |
| Element.kt | Converted Image to Icon with theme tint and enhanced preview configurations |
| NewProfile.kt | Created new empty file for future profile functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import androidx.compose.animation.core.* | ||
| import androidx.compose.foundation.background | ||
| import androidx.compose.foundation.clickable | ||
| import androidx.compose.foundation.gestures.scrollable |
There was a problem hiding this comment.
The import androidx.compose.foundation.gestures.scrollable is unused. Remove this import as the code uses verticalScroll modifier instead.
| import androidx.compose.foundation.gestures.scrollable |
| if (visible) { | ||
| AlertDialog( | ||
| onDismissRequest = onDismiss, | ||
| title = { Text("Really !?") }, |
There was a problem hiding this comment.
Corrected spelling of 'Really !?' to 'Really!?' - remove the space before the exclamation mark.
| title = { Text("Really !?") }, | |
| title = { Text("Really!?") }, |
| AlertDialog( | ||
| onDismissRequest = onDismiss, | ||
| title = { Text("Really !?") }, | ||
| text = { Text("Are you sure you want to sign out ?") }, |
There was a problem hiding this comment.
Remove space before the question mark in 'sign out ?'.
| text = { Text("Are you sure you want to sign out ?") }, | |
| text = { Text("Are you sure you want to sign out?") }, |
| modifier = Modifier.size(50.dp) | ||
| ) | ||
| } | ||
| Spacer(modifier = Modifier.height(20.dp)) |
There was a problem hiding this comment.
Using Spacer(modifier = Modifier.height(20.dp)) inside a horizontal Row has no effect. Use Modifier.width() instead or remove this Spacer.
| Spacer(modifier = Modifier.height(20.dp)) | |
| Spacer(modifier = Modifier.width(20.dp)) |
| import android.content.ClipboardManager | ||
| import android.content.Context | ||
| import android.graphics.drawable.Icon | ||
| import android.text.Layout |
There was a problem hiding this comment.
The import android.text.Layout is unused and should be removed.
| import android.text.Layout |
| colors = CardDefaults.cardColors( | ||
| containerColor = Color(0xFF3F51B5) | ||
| containerColor = Color(0xFF3F51B5) // TODO(should be dynamic) | ||
|
|
There was a problem hiding this comment.
[nitpick] Empty line 169 inside the CardDefaults.cardColors() call should be removed for cleaner code formatting.
| modifier: Modifier = Modifier | ||
| ) { | ||
| val iconTint = iconTintDark | ||
| val iconTint by remember { mutableStateOf(iconTintDark) } |
There was a problem hiding this comment.
Using remember { mutableStateOf() } for a constant value is unnecessary. Either use val iconTint = iconTintDark directly, or if this needs to be dynamic based on theme, derive it from the current theme instead of hardcoding to iconTintDark.
| val iconTint by remember { mutableStateOf(iconTintDark) } | |
| val iconTint = iconTintDark |
|
|
||
| private lateinit var navController: NavHostController | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Remove the unnecessary blank line 35.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/java/com/karan/hashin/screens/home/PassKeyDetail.kt (1)
472-483: Critical bug: onClick handler not invoked.Line 473 has
onClick = { onClick }which creates a lambda that captures theonClickparameter without calling it. The button click will not trigger the intended action.Apply this diff to fix:
Button( - onClick = { onClick }, + onClick = onClick, shape = RoundedCornerShape(12.dp),app/src/main/java/com/karan/hashin/screens/home/PassKey.kt (2)
160-187: Disable or replace visibility toggle when editing.In edit mode the field is disabled and value is literal "••••••••"; toggling visibility does nothing but still changes state. Hide the trailing icon or repurpose it (e.g., copy password).
- trailingIcon = { - val image = if (isPasswordVisible) R.drawable.visibility else R.drawable.visibility_off - Icon( - painter = painterResource(id = image), - contentDescription = "Visibility state of password", - modifier = Modifier.clickable { isPasswordVisible = !isPasswordVisible } - ) - }, + trailingIcon = if (!doEdit) { + { + val image = if (isPasswordVisible) R.drawable.visibility else R.drawable.visibility_off + Icon( + painter = painterResource(id = image), + contentDescription = "Toggle password visibility", + modifier = Modifier.clickable { isPasswordVisible = !isPasswordVisible } + ) + } + } else null,
261-290: Fix sticky validation booleans; propagate error UI.serviceError/passError only ever set true; they aren’t reset when inputs become valid, blocking submits. Also, fields never show error state.
- if (service.isBlank()) serviceError = true - if (pass.isBlank()) passError = true - - if (!serviceError) { + serviceError = service.isBlank() + passError = pass.isBlank() + + if (!serviceError) { if (doEdit) { ... } else { - if (!passError) { + if (!passError) { viewModel.addPassKey(service, username, pass, desc, label) } } // reset input fields service = "" username = "" pass = "" desc = "" label = "" }And add error visuals to fields:
OutlinedTextField( value = service, ... - colors = OutlinedTextFieldDefaults.colors( ... ) + isError = serviceError, + colors = OutlinedTextFieldDefaults.colors( ... ) ) OutlinedTextField( value = if (doEdit) "••••••••" else pass, ... - colors = OutlinedTextFieldDefaults.colors( ... ) + isError = !doEdit && passError, + colors = OutlinedTextFieldDefaults.colors( ... ) )
🧹 Nitpick comments (13)
app/src/main/java/com/karan/hashin/MainActivity.kt (1)
3-25: Remove unused imports.Many imports are not used in this file:
Activity,TRANSPARENT,View,Window,WindowInsets,WindowInsetsController,WindowManager,SystemBarStyle,LocalActivity,enableEdgeToEdge,RequiresApi,isSystemInDarkTheme,Composable,SideEffect,Color,toArgb,LocalView, andWindowInsetsControllerCompat.app/src/main/java/com/karan/hashin/ui/theme/Theme.kt (2)
108-108: Unsafe cast to ComponentActivity.The cast
LocalActivity.current as ComponentActivitywill throw aClassCastExceptionif the activity is not aComponentActivity. SinceMainActivityextendsComponentActivity, this works in the current codebase, but consider using a safe cast or null check for robustness.Apply this diff for a safer approach:
- val activity = LocalActivity.current as ComponentActivity + val activity = LocalActivity.current as? ComponentActivity ?: return
135-135: Empty onDispose block.The
onDisposecleanup is empty. If this is intentional (no cleanup needed), consider adding a comment to clarify. Otherwise, if edge-to-edge state should be reverted when the effect is disposed, implement the appropriate cleanup.app/src/main/java/com/karan/hashin/screens/home/Vault.kt (1)
73-73: Remove commented-out code.Instead of commenting out the debug log, consider removing it entirely if it's no longer needed.
Apply this diff:
val data = viewModel.passkeys.also { -// Log.d("#ined", "data: ${it.toList()}") }app/src/main/java/com/karan/hashin/components/NavigationBar.kt (1)
50-50: Simplify static icon tint.
iconTintis initialized withmutableStateOf(iconTintDark)but never changes. Consider using a simplevalinstead.Apply this diff:
- val iconTint by remember { mutableStateOf(iconTintDark) } + val iconTint = iconTintDarkapp/src/main/java/com/karan/hashin/screens/home/PassKey.kt (5)
123-132: Use onSurfaceVariant for placeholders; align focused/unfocused colors with M3.
- Placeholder color should be onSurfaceVariant with opacity, not primary.
- Focused border generally primary; unfocused border outline. Current focusedBorderColor=outline reverses the cue.
- placeholder = { Text("eg. Github", color = MaterialTheme.colorScheme.primary.copy(alpha = 0.4f)) }, + placeholder = { Text("eg. Github", color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.6f)) }, - colors = OutlinedTextFieldDefaults.colors( - unfocusedBorderColor = Unfocused, - unfocusedLeadingIconColor = Unfocused, - focusedLeadingIconColor = MaterialTheme.colorScheme.primary, - focusedBorderColor = MaterialTheme.colorScheme.outline, - focusedLabelColor = MaterialTheme.colorScheme.primary - ) + colors = OutlinedTextFieldDefaults.colors( + focusedBorderColor = MaterialTheme.colorScheme.primary, + focusedLabelColor = MaterialTheme.colorScheme.primary, + unfocusedBorderColor = MaterialTheme.colorScheme.outline, + unfocusedLeadingIconColor = MaterialTheme.colorScheme.onSurfaceVariant + )
151-156: Mirror the text field color adjustments here too.Use primary for focused cues, outline/onSurfaceVariant for unfocused.
- colors = OutlinedTextFieldDefaults.colors( - unfocusedBorderColor = Unfocused, - unfocusedLeadingIconColor = Unfocused, - focusedLeadingIconColor = MaterialTheme.colorScheme.primary, - focusedBorderColor = MaterialTheme.colorScheme.outline, - focusedLabelColor = MaterialTheme.colorScheme.primary - ) + colors = OutlinedTextFieldDefaults.colors( + focusedBorderColor = MaterialTheme.colorScheme.primary, + focusedLabelColor = MaterialTheme.colorScheme.primary, + focusedLeadingIconColor = MaterialTheme.colorScheme.primary, + unfocusedBorderColor = MaterialTheme.colorScheme.outline, + unfocusedLeadingIconColor = MaterialTheme.colorScheme.onSurfaceVariant + )
296-314: Prefer theme colors and contentColorFor; avoid hard-coded white to preserve disabled/content contrast.Blue constant + forced white text can misread when disabled.
- colors = ButtonDefaults.buttonColors( - containerColor = Blue - ), + colors = ButtonDefaults.buttonColors( + containerColor = MaterialTheme.colorScheme.primary + ), ... - if (viewModel.processing) { - CircularProgressIndicator( - color = Color.White, + if (viewModel.processing) { + CircularProgressIndicator( + color = LocalContentColor.current, strokeWidth = 2.dp, ... ) } else { Text( text = if (doEdit) "Update" else "Add", style = MaterialTheme.typography.titleMedium, - color = Color.White + color = LocalContentColor.current ) }
371-396: LabelSelector: use theme colors; avoid fixed grays for text; compute content color.Also, selectedIndex should reflect existing label in edit mode.
- colors = CardDefaults.cardColors( - containerColor = if (isSelected) Purple else Gray - ), + colors = CardDefaults.cardColors( + containerColor = if (isSelected) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.surfaceVariant + ), ... - Text( - text = label, - color = if (isSelected) Color.White else Color(0xFF666666), - fontSize = 14.sp, - fontWeight = if (isSelected) FontWeight.Medium else FontWeight.Normal - ) + val content = if (isSelected) MaterialTheme.colorScheme.onPrimary + else MaterialTheme.colorScheme.onSurfaceVariant + Text( + text = label, + color = content, + fontSize = 14.sp, + fontWeight = if (isSelected) FontWeight.Medium else FontWeight.Normal + )And expose initial selection (hoistable):
-fun LabelSelector( - labels: List<String>, - modifier: Modifier = Modifier, - onLabelSelected: (String) -> Unit -) { - var selectedIndex by remember { mutableIntStateOf(0) } +fun LabelSelector( + labels: List<String>, + modifier: Modifier = Modifier, + initialSelectedLabel: String? = null, + onLabelSelected: (String) -> Unit +) { + val initialIndex = remember(labels, initialSelectedLabel) { + initialSelectedLabel?.let { labels.indexOf(it) }?.takeIf { it >= 0 } ?: 0 + } + var selectedIndex by remember { mutableIntStateOf(initialIndex) }Then call from Passkey with current label:
- LabelSelector( - listOf("Personal", "Work", "Business", "Social", "Other") - ) { + LabelSelector( + labels = listOf("Personal", "Work", "Business", "Social", "Other"), + initialSelectedLabel = label + ) {
328-369: Remove unused animation to cut recomposition/allocations.animatedProgress is updated but never read.
- val animatedProgress = remember { Animatable(0f) } ... - scope.launch { - animatedProgress.animateTo( - targetValue = 1f, - animationSpec = spring( - dampingRatio = Spring.DampingRatioMediumBouncy, - stiffness = Spring.StiffnessLow - ) - ) - } + // Removed unused animation; keep scale/elevation for feedback.app/src/main/java/com/karan/hashin/screens/home/Settings.kt (3)
46-46: Use val for scrollState.You don’t reassign it.
- var scrollState = rememberScrollState(0) + val scrollState = rememberScrollState(0)
247-258: Remove or implement AccountEditPopUp.Empty Popup has no UI and adds maintenance cost. Either flesh it out or delete until needed.
- @Composable - fun AccountEditPopUp( - visible: Boolean, - onConfirm: () -> Unit, - onDismiss: () -> Unit - ) { - if (visible) { - Popup() { - - } - } - } + // Removed unused AccountEditPopUp placeholder to avoid dead code. Reintroduce with actual content when needed.
288-291: Use your app theme in previews.Preview with HashinTheme to better match production styling.
- MaterialTheme { + HashinTheme { Settings() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/src/main/java/com/karan/hashin/MainActivity.kt(2 hunks)app/src/main/java/com/karan/hashin/components/Element.kt(6 hunks)app/src/main/java/com/karan/hashin/components/NavigationBar.kt(2 hunks)app/src/main/java/com/karan/hashin/screens/home/Home.kt(3 hunks)app/src/main/java/com/karan/hashin/screens/home/NewProfile.kt(1 hunks)app/src/main/java/com/karan/hashin/screens/home/PassKey.kt(9 hunks)app/src/main/java/com/karan/hashin/screens/home/PassKeyDetail.kt(14 hunks)app/src/main/java/com/karan/hashin/screens/home/Settings.kt(2 hunks)app/src/main/java/com/karan/hashin/screens/home/Vault.kt(4 hunks)app/src/main/java/com/karan/hashin/ui/theme/Color.kt(1 hunks)app/src/main/java/com/karan/hashin/ui/theme/Theme.kt(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/karan/hashin/MainActivity.kt (1)
app/src/main/java/com/karan/hashin/ui/theme/Theme.kt (1)
HashinTheme(101-149)
app/src/main/java/com/karan/hashin/screens/home/Home.kt (1)
app/src/main/java/com/karan/hashin/components/NavigationBar.kt (1)
NavigationBar(42-122)
app/src/main/java/com/karan/hashin/components/NavigationBar.kt (1)
app/src/main/java/com/karan/hashin/ui/theme/Theme.kt (1)
HashinTheme(101-149)
app/src/main/java/com/karan/hashin/components/Element.kt (1)
app/src/main/java/com/karan/hashin/ui/theme/Theme.kt (1)
HashinTheme(101-149)
🔇 Additional comments (19)
app/src/main/java/com/karan/hashin/screens/home/NewProfile.kt (1)
1-2: Empty stub file—verify intent and next steps.This file contains only a package declaration with no imports, classes, or implementations. Given the PR's broader Material Design 3 theming and UI updates, clarify whether:
- This is an intentional placeholder to be populated in a follow-up commit, or
- The NewProfile screen implementation should be included in this PR.
If implementation is expected, it should follow the Material Design 3 theming patterns established elsewhere in the codebase (using
MaterialThemecolors and consistent styling with the updated theme system).app/src/main/java/com/karan/hashin/ui/theme/Theme.kt (1)
24-92: LGTM!The expanded color schemes for both dark and light themes properly leverage Material Design 3 tokens and the new color constants from Color.kt. The theming structure is well-organized and comprehensive.
app/src/main/java/com/karan/hashin/components/Element.kt (3)
62-69: LGTM!The switch from
ImagetoIconwithMaterialTheme.colorScheme.primarytint properly integrates with the theming system. The size reduction from 52dp to 36dp improves visual balance.
75-87: LGTM!Typography adjustments (font size 22sp → 20sp, padding refinements) and the use of
MaterialTheme.colorScheme.onPrimaryContainerimprove consistency with Material Design 3 guidelines.
110-135: LGTM!The dual theme previews (dark and light) with
dynamicColor = falseare well-configured and facilitate design verification across both theme modes.app/src/main/java/com/karan/hashin/ui/theme/Color.kt (1)
9-20: LGTM!The new color constants (Purple, Blue, Gray, selection colors, and surface variants) are well-named and support the expanded Material Design 3 theming system.
app/src/main/java/com/karan/hashin/screens/home/PassKeyDetail.kt (2)
113-113: LGTM!The use of
MaterialTheme.colorScheme.backgroundproperly integrates with the theming system.
251-465: LGTM!The systematic replacement of hard-coded colors with
MaterialTheme.colorSchemeproperties (surface, surfaceVariant, primary) ensures proper theme adaptation. Icon sizing and tint adjustments are well-integrated.app/src/main/java/com/karan/hashin/screens/home/Vault.kt (1)
53-53: LGTM!The use of
MaterialTheme.colorScheme.backgroundproperly integrates the vault screen with the theming system.app/src/main/java/com/karan/hashin/screens/home/Home.kt (2)
32-32: LGTM!The change from
remembertorememberSaveableimproves user experience by preserving the selected tab across configuration changes (e.g., screen rotation).
34-69: LGTM!The migration from
BottomAppBartoNavigationBaris correctly implemented. The removal ofnavigationBarsPaddingfrom the Scaffold is appropriate sinceNavigationBarnow handles this internally (line 120 in NavigationBar.kt).app/src/main/java/com/karan/hashin/components/NavigationBar.kt (2)
51-121: LGTM!The theming update to
tertiaryContainer, layout refinements (wrapContentHeight, explicit dimensions), and the addition ofnavigationBarsPaddingproperly support the edge-to-edge theming system.
124-142: LGTM!The preview configuration effectively demonstrates the
NavigationBarin a dark theme with system UI, providing a comprehensive visual representation.app/src/main/java/com/karan/hashin/screens/home/PassKey.kt (2)
77-77: Strong move to theme-driven background.Using MaterialTheme.colorScheme.background aligns with M3 and dark mode.
191-194: Card surface theming looks good.Using colorScheme.surface standardizes elevation overlays with M3.
app/src/main/java/com/karan/hashin/screens/home/Settings.kt (4)
61-71: ElevatedCard theming and spacing look good.Consistent surface usage and rounded corners match the rest of the PR.
91-106: Dividers and descriptions align with outline/onSurfaceVariant patterns.Good M3 alignment.
139-141: Icon container uses primary tint/overlay correctly.Nice subtle emphasis without overpowering.
266-283: AlertDialog copy and actions look fine.Wire it to the Sign Out trigger (see ProfileCard diff).
| super.onCreate(savedInstanceState) | ||
| enableEdgeToEdge() | ||
|
|
||
| WindowCompat.setDecorFitsSystemWindows(window, false) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify redundant edge-to-edge configuration.
WindowCompat.setDecorFitsSystemWindows(window, false) is set here, but HashinTheme (Theme.kt lines 120-136) also calls enableEdgeToEdge in a DisposableEffect. The enableEdgeToEdge API internally manages window insets, making this manual call potentially redundant or conflicting.
Run the following script to confirm the edge-to-edge handling in Theme.kt:
🏁 Script executed:
#!/bin/bash
# Verify edge-to-edge handling in Theme.kt
rg -nP -A5 'enableEdgeToEdge' --type=ktLength of output: 26
🏁 Script executed:
#!/bin/bash
# Search for enableEdgeToEdge in all Kotlin files without type filter
rg -nP -A5 'enableEdgeToEdge' --type-list | grep -i kotlinLength of output: 389
🏁 Script executed:
#!/bin/bash
# Find Theme.kt and search for enableEdgeToEdge directly
fd -e kt -path '*Theme.kt' --exec grep -n 'enableEdgeToEdge' {} \;Length of output: 168
🏁 Script executed:
#!/bin/bash
# Broader search for enableEdgeToEdge without type filtering
rg -nP -A5 'enableEdgeToEdge'Length of output: 1729
🏁 Script executed:
#!/bin/bash
# Also verify WindowCompat.setDecorFitsSystemWindows usage in MainActivity
rg -nP -B3 -A3 'WindowCompat.setDecorFitsSystemWindows' Length of output: 563
🏁 Script executed:
#!/bin/bash
# Check if MainActivity actually calls enableEdgeToEdge (not just imports)
rg -nP 'enableEdgeToEdge\s*\(' app/src/main/java/com/karan/hashin/MainActivity.ktLength of output: 0
🏁 Script executed:
#!/bin/bash
# Get full context of Theme.kt lines 120-145 to see the DisposableEffect structure
sed -n '120,145p' app/src/main/java/com/karan/hashin/ui/theme/Theme.ktLength of output: 662
🏁 Script executed:
#!/bin/bash
# Get context around setContent in MainActivity to see when HashinTheme is called
rg -nP -B2 -A8 'setContent' app/src/main/java/com/karan/hashin/MainActivity.ktLength of output: 748
🌐 Web query:
androidx.activity enableEdgeToEdge setDecorFitsSystemWindows redundant
💡 Result:
Short answer: Yes — calling ComponentActivity.enableEdgeToEdge and then also calling WindowCompat.setDecorFitsSystemWindows(false) is usually redundant. enableEdgeToEdge is a convenience helper that sets up edge‑to‑edge for you (including turning off decor fitting and configuring system bars), so you normally only need enableEdgeToEdge. [1][2]
If you need custom behavior (different insets handling or bar styles) you can skip enableEdgeToEdge and call WindowCompat.setDecorFitsSystemWindows / WindowInsetsController APIs yourself. [1][2]
Sources:
- enableEdgeToEdge docs (androidx.activity ComponentActivity). [1]
- Explanation / guidance (Stack Overflow). [2]
Remove redundant window configuration at line 39.
Calling WindowCompat.setDecorFitsSystemWindows(window, false) and enableEdgeToEdge() together is redundant—enableEdgeToEdge() is a convenience helper that internally handles turning off decor fitting and configuring system bars. Since HashinTheme calls activity.enableEdgeToEdge() at Theme.kt lines 131-134, the manual call at line 39 in MainActivity should be removed.
🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/MainActivity.kt around line 39, remove the
redundant call to WindowCompat.setDecorFitsSystemWindows(window, false) since
HashinTheme already calls activity.enableEdgeToEdge() (Theme.kt lines 131-134)
which configures edge-to-edge and system bars; simply delete the line so
edge-to-edge configuration is only handled by the theme helper.
| textStyle = TextStyle( | ||
| color = MaterialTheme.colorScheme.primary, | ||
| fontSize = 16.sp | ||
| ), | ||
| cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), | ||
| decorationBox = { innerTextField -> | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .padding(16.dp) | ||
| .padding(vertical = 16.dp, horizontal = 20.dp) | ||
| ) { | ||
| if (desc.isEmpty()) { | ||
| Text( | ||
| text = "Add a description...", | ||
| style = TextStyle( | ||
| color = Color.Gray, | ||
| color = Unfocused, | ||
| fontSize = 16.sp |
There was a problem hiding this comment.
Use onSurface for text and onSurfaceVariant for placeholder to meet contrast.
Typing in primary on a surface can fail WCAG contrast in dark/light.
- textStyle = TextStyle(
- color = MaterialTheme.colorScheme.primary,
- fontSize = 16.sp
- ),
- cursorBrush = SolidColor(MaterialTheme.colorScheme.primary),
+ textStyle = TextStyle(
+ color = MaterialTheme.colorScheme.onSurface,
+ fontSize = 16.sp
+ ),
+ cursorBrush = SolidColor(MaterialTheme.colorScheme.primary),
- style = TextStyle(
- color = Unfocused,
- fontSize = 16.sp
- )
+ style = TextStyle(
+ color = MaterialTheme.colorScheme.onSurfaceVariant,
+ fontSize = 16.sp
+ )🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/screens/home/PassKey.kt around lines 215
to 231, the input text currently uses MaterialTheme.colorScheme.primary and the
placeholder uses Unfocused which can fail WCAG contrast on surfaces; change the
TextField textStyle color to MaterialTheme.colorScheme.onSurface, set the
placeholder Text color to MaterialTheme.colorScheme.onSurfaceVariant instead of
Unfocused, and update the cursorBrush to use
SolidColor(MaterialTheme.colorScheme.onSurface) so text, placeholder, and cursor
colors meet surface contrast guidelines.
| containerColor = Color(0xFF3F51B5) // TODO(should be dynamic) | ||
|
|
There was a problem hiding this comment.
Hard-coded color with TODO.
The containerColor uses a hard-coded Color(0xFF3F51B5) with a TODO comment indicating it should be dynamic. This breaks theme consistency and won't adapt to light/dark mode.
Consider using a theme color or creating a dynamic color based on the label. Would you like me to suggest an implementation that derives the header color from the label's color (similar to the badge)?
🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/screens/home/PassKeyDetail.kt around lines
168-169, the header uses a hard-coded Color(0xFF3F51B5) with a TODO; replace
this with a dynamic value so it respects the app theme and label-based coloring.
Use the label's color when available (falling back to
MaterialTheme.colorScheme.primaryContainer or primary) and compute a readable
content color via contentColorFor(...) or a contrast helper; apply the derived
containerColor and corresponding contentColor to the component and remove the
hard-coded value and TODO.
| // viewModel: HomeViewModel, | ||
| modifier: Modifier = Modifier | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
Signature change: ensure all call sites updated and no lost functionality.
Settings no longer accepts HomeViewModel; verify usages and migrate any logic that depended on it.
🏁 Script executed:
#!/bin/bash
# Find invocations of Settings that still pass a viewModel or expect one.
rg -nP '\bSettings\s*\(' -C2
rg -nP 'Settings\s*\(\s*viewModel\s*=' -C2
# Find direct references to HomeViewModel within Settings screen files (should be none now).
rg -nP --type=kt 'HomeViewModel' app/src/main/javaLength of output: 1808
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context around Home.kt line 99
cat app/src/main/java/com/karan/hashin/screens/home/Home.kt | sed -n '95,105p'
# Search for all Settings( calls without problematic --type flag
rg -n 'Settings\s*\(' --type-add 'kt:*.kt' -t kt -C3
Length of output: 2639
Fix compilation error: Home.kt still passes removed viewModel parameter to Settings()
Home.kt line 99 calls Settings(viewModel) but Settings() no longer accepts this parameter. Update the call to Settings().
Additionally, verify that HomeViewModel import in Settings.kt (line 37) is no longer needed; if unused internally, remove it.
🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/screens/home/Settings.kt around lines
41-43 the Settings composable signature no longer accepts a viewModel; update
the call site in app/src/main/java/com/karan/hashin/screens/home/Home.kt at line
99 to call Settings() with no arguments. Also check Settings.kt line 37 for the
HomeViewModel import and remove that import if it is now unused after the
parameter removal.
| var isDarkTheme by rememberSaveable { mutableStateOf(false) } | ||
| var scrollState = rememberScrollState(0) | ||
|
|
||
| Column( | ||
| verticalArrangement = Arrangement.spacedBy(8.dp), | ||
| modifier = modifier | ||
| .statusBarsPadding() | ||
| .fillMaxSize() | ||
| .background( | ||
| brush = Brush.verticalGradient( | ||
| colors = listOf( | ||
| MaterialTheme.colorScheme.background, | ||
| MaterialTheme.colorScheme.background.copy(alpha = 0.95f) | ||
| ) | ||
| ) | ||
| ) | ||
| .padding(16.dp), | ||
| verticalArrangement = Arrangement.spacedBy(20.dp) | ||
| .padding(horizontal = 16.dp) | ||
| .background(MaterialTheme.colorScheme.background) | ||
| .verticalScroll(scrollState) | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
Dark Theme switch is a no-op; wire it to the app theme.
Toggle updates state but doesn’t affect HashinTheme. Hoist to a shared state (DataStore/VM) or wrap content with your theme using that flag.
- var isDarkTheme by rememberSaveable { mutableStateOf(false) }
+ var isDarkTheme by rememberSaveable { mutableStateOf(false) }
...
- Column(
+ HashinTheme(darkTheme = isDarkTheme) {
+ Column(
...
- .verticalScroll(scrollState)
+ .verticalScroll(scrollState)
) {
...
}
+ }If theming is controlled higher up (e.g., MainActivity), replace the switch handler with a callback to update that source.
I'll help verify this review comment by examining the codebase. Let me search for the theme implementation and Settings.kt structure.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find HashinTheme implementation
find . -type f -name "*.kt" | xargs grep -l "fun HashinTheme\|object HashinTheme\|class HashinTheme" 2>/dev/null | head -10
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Find MainActivity to see app-level theme management
find . -type f -name "MainActivity.kt" 2>/dev/nullLength of output: 52
🏁 Script executed:
#!/bin/bash
# Search for isDarkTheme usage across the codebase
rg -n "isDarkTheme" --type=kotlin -C3 2>/dev/nullLength of output: 1627
🏁 Script executed:
#!/bin/bash
# Search for theme-related state management (DataStore, ViewModel, etc)
rg -n "DataStore|ThemeViewModel|ThemeRepository" --type=kotlin 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get full Settings.kt file to see complete context
find . -type f -name "Settings.kt" -path "*/screens/home/*" 2>/dev/nullLength of output: 61
---
🏁 Script executed:
```shell
#!/bin/bash
# View HashinTheme implementation
cat ./app/src/main/java/com/karan/hashin/ui/theme/Theme.kt
Length of output: 4250
🏁 Script executed:
#!/bin/bash
# View MainActivity to check app-level theme
cat ./app/src/main/java/com/karan/hashin/MainActivity.ktLength of output: 1585
🏁 Script executed:
#!/bin/bash
# View full Settings.kt file
cat ./app/src/main/java/com/karan/hashin/screens/home/Settings.ktLength of output: 10732
🏁 Script executed:
#!/bin/bash
# Check if isDarkTheme is referenced in lambda/callback elsewhere
rg -n "isDarkTheme|darkTheme" --type=kotlin app/src/main/java/com/karan/hashin/screens/home/ -A5 -B5Length of output: 2540
Wire the dark theme toggle to actually control theming in the app.
The isDarkTheme toggle in Settings.kt (line 45) is confirmed to be completely isolated—it updates local state but has no effect anywhere. It's not persisted, not hoisted to shared state, and the Column is not wrapped with HashinTheme. Currently, theming is controlled only at MainActivity level via the system dark mode default.
To fix, either:
- Screen-level: Wrap the Column with
HashinTheme(darkTheme = isDarkTheme)to affect Settings screen only - App-wide: Hoist isDarkTheme state to MainActivity or ViewModel, pass it through
HashinTheme(darkTheme = isDarkTheme), and add persistence (DataStore or SharedPreferences)
🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/screens/home/Settings.kt around lines 45
to 56, the local isDarkTheme state is updated but not applied to the UI or
persisted; either wrap the Settings UI with the app theme using
HashinTheme(darkTheme = isDarkTheme) so toggling affects only this screen, or
hoist isDarkTheme to a shared owner (MainActivity or a ViewModel), expose it to
HashinTheme at the app root and add persistence (DataStore/SharedPreferences) so
the toggle controls app-wide theming and survives restarts; implement one of
these approaches and remove the unused local-only state accordingly.
|
|
||
| @Composable | ||
| fun ProfileCard(modifier: Modifier = Modifier) { | ||
| ElevatedCard( | ||
| elevation = CardDefaults.elevatedCardElevation(defaultElevation = 8.dp), | ||
| shape = RoundedCornerShape(24.dp), | ||
| colors = CardDefaults.elevatedCardColors( | ||
| containerColor = MaterialTheme.colorScheme.surface | ||
| ), | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(vertical = 8.dp) | ||
| ) { | ||
| Column( | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| modifier = Modifier | ||
| .padding(top = 12.dp, bottom = 24.dp) | ||
| .padding(horizontal = 24.dp) | ||
| ) { | ||
| IconButton( | ||
| onClick = { }, | ||
| modifier = Modifier | ||
| .size(32.dp) | ||
| .align(Alignment.End) | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.Outlined.Edit, | ||
| contentDescription = "Edit info", | ||
| tint = MaterialTheme.colorScheme.primary | ||
| ) | ||
| } | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| ) { | ||
| Box( | ||
| contentAlignment = Alignment.Center, | ||
| modifier = Modifier | ||
| .size(80.dp) | ||
| .clip(CircleShape) | ||
| .background(MaterialTheme.colorScheme.tertiary) | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.Default.Person, | ||
| contentDescription = "Profile", | ||
| tint = Color.White, | ||
| modifier = Modifier.size(50.dp) | ||
| ) | ||
| } | ||
| Spacer(modifier = Modifier.height(20.dp)) | ||
| Column( | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| ) { | ||
| Text( | ||
| text = "John Doe", | ||
| style = MaterialTheme.typography.headlineMedium, | ||
| fontWeight = FontWeight.Bold, | ||
| color = MaterialTheme.colorScheme.onSurface | ||
| ) | ||
| Text( | ||
| text = "[email protected]", | ||
| style = MaterialTheme.typography.bodyLarge, | ||
| color = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.7f) | ||
| ) | ||
| } | ||
|
|
||
|
|
||
| } | ||
| Spacer(modifier = Modifier.height(24.dp)) | ||
| Button( | ||
| onClick = { /* Sign out */ }, | ||
| colors = ButtonDefaults.buttonColors( | ||
| containerColor = MaterialTheme.colorScheme.error | ||
| ), | ||
| shape = RoundedCornerShape(16.dp), | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(56.dp) | ||
| ) { | ||
| Text("Sign Out", style = MaterialTheme.typography.titleMedium, color = Color.White) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
ProfileCard: fix Row spacing; wire Sign Out; avoid hard-coded white.
- Spacer(height) inside Row is a no-op; use width.
- Hook "Sign Out" button to SignOutAlert.
- Prefer contentColorFor over fixed white.
- Spacer(modifier = Modifier.height(20.dp))
+ Spacer(modifier = Modifier.width(20.dp))
...
- Button(
- onClick = { /* Sign out */ },
+ var showSignOut by rememberSaveable { mutableStateOf(false) }
+ Button(
+ onClick = { showSignOut = true },
colors = ButtonDefaults.buttonColors(
containerColor = MaterialTheme.colorScheme.error
),
...
) {
- Text("Sign Out", style = MaterialTheme.typography.titleMedium, color = Color.White)
+ Text("Sign Out", style = MaterialTheme.typography.titleMedium, color = contentColorFor(MaterialTheme.colorScheme.error))
}
+ if (showSignOut) {
+ SignOutAlert(
+ visible = true,
+ onConfirm = { showSignOut = false /* TODO: trigger sign out */ },
+ onDismiss = { showSignOut = false }
+ )
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/java/com/karan/hashin/screens/home/Settings.kt around lines
159-245, the Row spacing, Sign Out action and hard-coded text color need fixes:
change the Spacer inside the Row from a vertical spacer to a horizontal one (use
width instead of height) so the avatar and texts separate correctly; wire the
"Sign Out" Button to the SignOutAlert by adding a boolean state (e.g.,
showSignOutDialog) toggled on button click and pass that state/handlers into the
SignOutAlert so the dialog is shown and handles confirm/dismiss actions; and
replace the hard-coded Color.White for the button text with a dynamic content
color (use contentColorFor(MaterialTheme.colorScheme.error) or LocalContentColor
with appropriate composition) so the text color follows the theme.
Summary by CodeRabbit
New Features
Style