๐ :: (#808) ๋ก๊ทธ์ธ ํ๋ฉด ๊ฐ๋ฐ #809
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRestructures app navigation architecture, migrates design system (colors, typography, icons), introduces snackbar infrastructure with state management, removes legacy main feature screens (home, application, announcement, mypage), adds new SignIn flow with device token integration, and refactors build flavors with manifest and resource updates. Changes
Estimated code review effort๐ฏ 4 (Complex) | โฑ๏ธ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touchesโ Failed checks (1 warning)
โ Passed checks (2 passed)
โจ Finishing touches
๐งช 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 |
Summary of ChangesHello @uson1004, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! ์ด PR์ ๋ก๊ทธ์ธ ํ๋ฉด ๊ฐ๋ฐ๊ณผ ํจ๊ป ์ ํ๋ฆฌ์ผ์ด์ ์ ๋์์ธ ์์คํ ์ ๋๋์ ์ผ๋ก ๋ฆฌํฉํ ๋งํ๋ ๋ฐ ์ค์ ์ ๋ก๋๋ค. ์๋ก์ด ์ปค์คํ ๋ฒํผ, ํ ์คํธ ํ๋, ์ค๋ต๋ฐ ์์คํ ๊ณผ ๊ฐ์ UI ์ปดํฌ๋ํธ๋ค์ ๋์ ํ๊ณ , ์์ ์คํค๋ง์ ํ์ดํฌ๊ทธ๋ํผ๋ฅผ ์ ๋ฐ์ดํธํ์ต๋๋ค. ๋ํ, AndroidManifest ํ์ผ์ ๊ฐ์ํํ๊ณ ๋ ์ด์ ์ฌ์ฉ๋์ง ์๋ MVI ๊ด๋ จ ๊ธฐ๋ณธ ํด๋์ค๋ค์ ์ ๊ฑฐํ์ฌ ๋ ๊น๋ํ๊ณ ๋ชจ๋ํ๋ ์ํคํ ์ฒ๋ฅผ ๋ชฉํ๋ก ํฉ๋๋ค. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with ๐ and ๐ on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
๐งน Nitpick comments (27)
core/design-system/src/prod/res/drawable/ic_warning.xml (1)
6-12: Icon implementation looks good.The warning icon is properly structured with correct path data and a standard black fill color (appropriate for a design system base icon where tinting is applied at usage).
Optional optimization: The clip-path at lines 7-8 defines the full 24ร24 viewport and is typically redundant (paths are already clipped to the viewport by default). This is often auto-generated by design tools and can be safely removed to simplify the XML:
<group> - <clip-path - android:pathData="M0,0h24v24h-24z"/> <path android:pathData="M12.866,3L22.392,19.5C22.48,19.652 22.526,19.824 22.526,20C22.526,20.176 22.48,20.348 22.392,20.5C22.304,20.652 22.178,20.778 22.026,20.866C21.874,20.954 21.702,21 21.526,21H2.474C2.299,21 2.126,20.954 1.974,20.866C1.822,20.778 1.696,20.652 1.608,20.5C1.52,20.348 1.474,20.176 1.474,20C1.474,19.824 1.52,19.652 1.608,19.5L11.134,3C11.222,2.848 11.348,2.722 11.5,2.634C11.652,2.546 11.825,2.5 12,2.5C12.176,2.5 12.348,2.546 12.5,2.634C12.652,2.722 12.778,2.848 12.866,3ZM4.206,19H19.794L12,5.5L4.206,19ZM11,16H13V18H11V16ZM11,9H13V14H11V9Z" android:fillColor="#000000"/> </group>This applies to the other icon drawables being added in this PR as well (ic_error.xml, ic_success.xml, etc.) if they follow the same pattern.
core/ui/src/prod/kotlin/PaddingDefaults.kt (2)
9-9: Track the screen direction TODO.The TODO for screen direction handling highlights the need for RTL/LTR support. The current implementation uses
start/endinstead ofleft/right, which is already RTL-friendly, but additional logic may be required for complete bidirectional text support.Do you want me to open an issue to track this TODO or help draft a solution for screen direction handling?
56-98: LGTM with a minor note on default values.The Modifier extension functions are well-structured and follow a consistent pattern. One observation:
verticalPaddingusesSmall(8.dp) as the default, while most other vertical padding defaults useMedium(12.dp). This may be intentional for specific UI scenarios but could be worth documenting if there's a deliberate design rationale.core/design-system/src/prod/res/drawable/ic_backward.xml (2)
8-8: Consider using theme color attributes for better theming support.The hardcoded fill color
#555555limits the icon's adaptability to different themes (e.g., light/dark mode). Consider referencing a theme color attribute instead, or rely on runtime tinting in Compose where this icon is used.For example, if using this drawable with
Iconin Compose, you can apply tint dynamically:Icon( painter = painterResource(R.drawable.ic_backward), contentDescription = "Navigate back", tint = MaterialTheme.colorScheme.onSurface )Alternatively, define the icon without fillColor and always apply tint at the usage site for maximum flexibility.
1-9: These are distinct icons serving different purposesโnot duplicates.The
ic_backwardicon (chevron shape) andic_backicon (arrow shape) have different geometries and use cases.ic_backwardis used as a media control in navigation flows, whileic_backfollows the standard Android back navigation pattern. Both are correctly registered inDmsIcon.ktas separate icons.Consider using theme attributes instead of hardcoded colors to support multiple app themes or different build variants, such as
?attr/colorControlNormalfor better dark mode and theming flexibility.core/design-system/src/prod/res/drawable/ic_home.xml (1)
1-9: Use semantic colors for consistency with the design system.The icon uses a hard-coded fill color
#1C1C1Einstead of referencing the semantic color palette defined incolors.xml(e.g.,@color/gray_800or@color/gray_900). This pattern affects multiple icons in the set. For better maintainability and theming support, reference colors from the design system or apply tinting viaandroid:tint="?attr/colorControlNormal"at the consumption point.core/ui/src/prod/kotlin/mvi/Intent.kt (2)
3-3: Add KDoc to document the Intent base type.This base class serves as the foundation for MVI intent handling but lacks documentation explaining its purpose, usage patterns, and relationship to the MVI architecture.
Apply this diff to add documentation:
+/** + * Base type for all MVI intents in the application. + * Intents represent user actions or events that trigger state changes. + */ abstract class Intent
3-3: Consider using sealed interface for exhaustive type checking.For MVI patterns,
sealed interface(orsealed class) often provides better type safety thanabstract class, enabling exhaustive when expressions and preventing unintended subclasses outside the module.If your architecture would benefit from exhaustive type checking, consider:
-abstract class Intent +sealed interface Intentcore/ui/src/prod/kotlin/navigation/Destination.kt (1)
1-4: Consider removing deprecated unused code.The
Destinationclass is marked as deprecated with "No usage". Since it's not being used, consider removing it entirely to reduce technical debt and simplify the codebase.If there are no references to this class, apply this removal:
-package ui.navigation - -@Deprecated("No usage") -abstract class Destination(val route: String)core/design-system/src/prod/res/drawable/ic_edit.xml (1)
1-13: Consider using theme colors instead of hardcoded values.The icon uses a hardcoded fill color
#999999. For better theming support and consistency across light/dark modes, consider using a theme color reference or usingandroid:tintat the usage site instead ofandroid:fillColor.Example approach:
<path android:pathData="M9.175,5.65L8.35,4.825L2.917,10.259V11.083H3.741L9.175,5.65ZM10,4.825L10.824,4.001L10,3.176L9.175,4.001L10,4.825ZM4.225,12.25H1.75V9.775L9.587,1.938C9.696,1.829 9.845,1.767 10,1.767C10.154,1.767 10.302,1.829 10.412,1.938L12.062,3.588C12.172,3.698 12.233,3.846 12.233,4.001C12.233,4.155 12.172,4.304 12.062,4.413L4.225,12.25H4.225Z" - android:fillColor="#999999"/> + android:fillColor="@android:color/black"/>Then apply tint at the usage site for theme-aware coloring.
core/design-system/src/prod/res/drawable/ic_check.xml (1)
1-13: Consider using theme colors for consistency.The icon uses a hardcoded fill color
#000000. For better theming support across light/dark modes, consider using black as the fill and applying tint at the usage site, or use a theme color reference.The current approach works but may require duplicate icon files for different themes. Using tint in Compose (e.g.,
Icon(tint = DmsTheme.colorScheme.onSurface, ...)) provides better flexibility.core/design-system/src/prod/res/drawable/ic_back.xml (1)
1-9: Consider using theme colors for consistency.Similar to other new icons in this PR, this icon uses a hardcoded fill color
#000000. Consider using black as the base and applying theme-aware tint at the usage site for better dark mode support.This approach allows a single icon file to adapt to different color schemes through runtime tinting.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/modifier/DmsDropShadow.kt (1)
15-22: Consider adding input validation and documentation.The function accepts
Dpparameters but doesn't validate for negative values, which could produce unexpected rendering results. Additionally, thespreadparameter affects layout size but this behavior isn't documented.Consider adding:
- KDoc explaining the shadow behavior, especially how
spreadaffects the shadow size- Parameter validation or constraints for negative values
+/** + * Applies a drop shadow effect to the composable. + * + * @param shape The shape of the shadow outline + * @param color Shadow color with alpha + * @param blur Blur radius (software rendering, impacts performance) + * @param offsetY Vertical shadow offset + * @param offsetX Horizontal shadow offset + * @param spread Expands shadow size beyond the composable bounds + */ fun Modifier.dmsDropShadow( shape: Shape, color: Color = Color.Black.copy(0.25f), blur: Dp = 4.dp, offsetY: Dp = 4.dp, offsetX: Dp = 0.dp, spread: Dp = 0.dp, ) = this.drawBehind { + require(blur >= 0.dp) { "Blur must be non-negative" } val shadowSize = Size(size.width + spread.toPx(), size.height + spread.toPx())core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsSymbol.kt (1)
17-21: Consider localizing the content description for accessibility.The
contentDescriptionis hardcoded as"symbol", which won't be localized for international users relying on screen readers.Apply this diff to use a string resource:
+import androidx.compose.ui.res.stringResource + @Composable fun DmsSymbol(modifier: Modifier = Modifier) { val symbol = if (isSystemInDarkTheme()) { DmsIcon.SymbolDark } else { DmsIcon.SymbolLight } Image( modifier = modifier, painter = painterResource(symbol), - contentDescription = "symbol", + contentDescription = stringResource(R.string.content_description_symbol), ) }Then add the string resource to your strings.xml files for each supported locale.
core/design-system/src/prod/res/drawable/ic_blue_breakfast.xml (1)
2-3: Consider standardizing icon dimensions across the design system.This icon uses 26dp ร 26dp dimensions, while other icons like
ic_applicate.xmlandic_close.xmluse 24dp ร 24dp. The design system currently has multiple dimension groups (24dp for most utility icons, 26dp for blue meal icons, 28dp for regular meal icons), and mixing these sizes in the same UI can cause visual misalignment.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/DmsIconButton.kt (1)
28-32: Consider modifier ordering for size.Applying
.size(size)after themodifierparameter will override any size modifiers passed by the caller. If thesizeparameter should always take precedence, this is correct. Otherwise, consider allowing the caller's modifier to take precedence or document this behavior explicitly.If flexible sizing is desired, apply this diff:
- IconButton( - modifier = modifier.size(size), + IconButton( + modifier = modifier, enabled = enabled, onClick = onClick, ) { Icon( - modifier = Modifier.padding(contentPaddingValues), + modifier = Modifier + .size(size) + .padding(contentPaddingValues), painter = painterResource(resource), tint = tint, contentDescription = contentDescription, ) }core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
162-174: Fix variable name typo in hashCode().The variable name
Rultappears to be a typo and should beresultfor consistency and readability.Apply this diff:
override fun hashCode(): Int { - var Rult = headline1.hashCode() - Rult = 31 * Rult + headline2.hashCode() - Rult = 31 * Rult + headline3.hashCode() - Rult = 31 * Rult + title1.hashCode() - Rult = 31 * Rult + title2.hashCode() - Rult = 31 * Rult + title3.hashCode() - Rult = 31 * Rult + body1.hashCode() - Rult = 31 * Rult + body2.hashCode() - Rult = 31 * Rult + body3.hashCode() - Rult = 31 * Rult + caption.hashCode() - Rult = 31 * Rult + overline.hashCode() - Rult = 31 * Rult + button.hashCode() - return Rult + var result = headline1.hashCode() + result = 31 * result + headline2.hashCode() + result = 31 * result + headline3.hashCode() + result = 31 * result + title1.hashCode() + result = 31 * result + title2.hashCode() + result = 31 * result + title3.hashCode() + result = 31 * result + body1.hashCode() + result = 31 * result + body2.hashCode() + result = 31 * result + body3.hashCode() + result = 31 * result + caption.hashCode() + result = 31 * result + overline.hashCode() + result = 31 * result + button.hashCode() + return result }app/src/dev/AndroidManifest.xml (1)
19-21: Clarify the purpose of emptylib_namemeta-data.The
android.app.lib_namemeta-data with an empty value is typically used to specify a native library to load. If no native library is needed, this meta-data entry can be removed entirely. If it's intentionally empty to override a parent manifest, please add a comment for clarity.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/SnackBar.kt (2)
48-51: Add accessibility support for the snackbar icon.The icon
ImagehascontentDescription = null, which means screen readers won't announce the snackbar type (success/error). Consider providing a meaningful content description based on the snackbar type.- Image( - painter = painterResource(snackBarType.iconRes), - contentDescription = null, - ) + Image( + painter = painterResource(snackBarType.iconRes), + contentDescription = snackBarType.name.lowercase(), + )
69-70: TODO: Yellow variant not implemented.The TODO indicates a yellow snackbar variant is pending. When ready to implement, you'll need to add a new enum value (e.g.,
WARNING) with an appropriate icon resource.Would you like me to help implement the yellow/warning variant or open an issue to track this task?
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt (3)
146-148: Unreachableelsebranch.All
ButtonColorenum values (Primary,Gray,Error) are handled in thewhenexpression. Theelsebranch is unreachable and will cause a compile-time warning in Kotlin when exhaustivewhenis used. Remove theelsebranch to leverage exhaustivewhenchecking, which will catch missing cases if new enum values are added.ButtonColor.Error -> ButtonState( enabled = ButtonTheme( textColor = DmsTheme.colorScheme.inversePrimary, ), pressed = ButtonTheme( textColor = DmsTheme.colorScheme.primaryContainer, ), disabled = ButtonTheme( textColor = DmsTheme.colorScheme.onPrimary, ), ) - - else -> throw IllegalArgumentException("Unhandled ButtonColor: $this") }
186-189: Unreachableelsebranch (same issue).Same as aboveโremove this
elsebranch to enable exhaustivewhenchecking.ButtonColor.Error -> ButtonState( enabled = ButtonTheme( textColor = DmsTheme.colorScheme.outline, ), pressed = ButtonTheme( textColor = DmsTheme.colorScheme.errorContainer, ), disabled = ButtonTheme( textColor = DmsTheme.colorScheme.onError, ), ) - - else -> throw IllegalArgumentException("Unhandled ButtonColor: $this") }
198-198: Unused parameterbuttonType.The
buttonTypeparameter is passed toBasicButtonbut never used within the function body. Either remove it or add a TODO if it's planned for future use.core/design-system/src/prod/res/drawable/ic_visible.xml (1)
1-9: Consider theme-aware fill color.The hardcoded
fillColor="#1C1C1E"won't adapt to dark theme. If this icon should support theming, consider usingandroid:tintat the usage site or defining the color as a theme attribute.If you want the icon to be theme-aware:
- android:fillColor="#1C1C1E"/> + android:fillColor="?attr/colorOnSurface"/>Alternatively, apply tint when using the icon in composables via
colorFilterortintmodifier.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt (1)
28-31: Clarify the distinction betweenNotificationandNotice.Both
Notification(line 28) andNotice(line 31) map to the same drawable resourceR.drawable.ic_notice. If these represent the same concept, consider removing the duplicate. If they serve different purposes, use distinct drawable resources.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/textfield/DmsTextField.kt (1)
152-158: Consider simplifying the maxLength enforcement.The
maxLengthis enforced in two places:value.take(maxLength)on line 153 and the conditional check on lines 155-157. While this is safe, the inner check is redundant sincetake()already limits the string length.You could simplify to:
BasicTextField( - value = value.take(maxLength), + value = value, onValueChange = { newValue -> - if (newValue.length <= maxLength) { - onValueChange(newValue) - } + onValueChange(newValue.take(maxLength)) },core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (1)
92-94: Track the TODO comments for future implementation.Two features are marked as TODO: shape implementation and ToastLayout. Ensure these are tracked in your backlog.
Would you like me to create GitHub issues to track these pending implementations?
๐ Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (19)
app/src/prod/ic_launcher-playstore.pngis excluded by!**/*.pngcore/design-system/src/dev/res/drawable/ic_logo.pngis excluded by!**/*.pngcore/design-system/src/dev/res/font/pretendard_bold.otfis excluded by!**/*.otfcore/design-system/src/dev/res/font/pretendard_medium.otfis excluded by!**/*.otfcore/design-system/src/dev/res/font/pretendard_regular.otfis excluded by!**/*.otfcore/design-system/src/dev/res/font/pretendard_semi_bold.otfis excluded by!**/*.otfcore/design-system/src/prod/res/drawable-hdpi/ic_notification.pngis excluded by!**/*.pngcore/design-system/src/prod/res/drawable-mdpi/ic_notification.pngis excluded by!**/*.pngcore/design-system/src/prod/res/drawable-xhdpi/ic_notification.pngis excluded by!**/*.pngcore/design-system/src/prod/res/drawable-xxhdpi/ic_notification.pngis excluded by!**/*.pngcore/design-system/src/prod/res/drawable-xxxhdpi/ic_notification.pngis excluded by!**/*.pngcore/design-system/src/prod/res/drawable/ic_logo.pngis excluded by!**/*.pngcore/design-system/src/prod/res/font/noto_sans_kr_black.ttfis excluded by!**/*.ttfcore/design-system/src/prod/res/font/noto_sans_kr_bold.ttfis excluded by!**/*.ttfcore/design-system/src/prod/res/font/noto_sans_kr_light.ttfis excluded by!**/*.ttfcore/design-system/src/prod/res/font/noto_sans_kr_medium.ttfis excluded by!**/*.ttfcore/design-system/src/prod/res/font/noto_sans_kr_regular.ttfis excluded by!**/*.ttfcore/design-system/src/prod/res/font/noto_sans_kr_thin.ttfis excluded by!**/*.ttffeature/src/prod/ic_launcher-playstore.pngis excluded by!**/*.png
๐ Files selected for processing (107)
app/src/dev/AndroidManifest.xml(1 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt(3 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/DmsAppState.kt(1 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/EdgeToEdgeConfig.kt(1 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/MainActivity.kt(1 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/res/values-night/themes.xml(1 hunks)app/src/main/res/values/themes.xml(2 hunks)app/src/prod/AndroidManifest.xml(1 hunks)app/src/prod/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt(1 hunks)app/src/prod/res/values-night/themes.xml(1 hunks)app/src/prod/res/values/strings.xml(1 hunks)app/src/prod/res/values/themes.xml(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.kt(0 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt(2 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt(4 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt(14 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/DmsIconButton.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsSymbol.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/modifier/DmsDropShadow.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/DmsSnackBarVisuals.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/SnackBar.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/textfield/DmsTextField.kt(1 hunks)core/design-system/src/dev/res/drawable/ic_cancel.xml(1 hunks)core/design-system/src/dev/res/drawable/ic_error.xml(1 hunks)core/design-system/src/dev/res/drawable/ic_success.xml(1 hunks)core/design-system/src/dev/res/drawable/ic_symbol_dark.xml(1 hunks)core/design-system/src/dev/res/drawable/ic_symbol_light.xml(1 hunks)core/design-system/src/prod/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt(1 hunks)core/design-system/src/prod/res/drawable/ic_applicate.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_back.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_backward.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_bell.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_blue_bell.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_blue_breakfast.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_blue_dinner.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_blue_lunch.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_blue_notice.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_calender.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_camera.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_check.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_circle_outline.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_close.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_down.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_edit.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_forward.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_home.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_information.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_logo_dark.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_logo_light.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_notice.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_palette.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_person.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_photo.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_photo_close.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_plus.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_profile_default.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_sort.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_up.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_visible.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_warning.xml(1 hunks)core/design-system/src/prod/res/drawable/ic_wrong.xml(1 hunks)core/notification/src/main/java/team/aliens/dms/android/core/notification/NotificationManager.kt(1 hunks)core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/Intent.kt(0 hunks)core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/SideEffect.kt(0 hunks)core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/UiState.kt(0 hunks)core/ui/src/main/java/team/aliens/dms/android/core/ui/util/Utils.kt(0 hunks)core/ui/src/prod/kotlin/Banner.kt(2 hunks)core/ui/src/prod/kotlin/CollectInLaunchedEffectWithLifecycle.kt(1 hunks)core/ui/src/prod/kotlin/PaddingDefaults.kt(1 hunks)core/ui/src/prod/kotlin/activity/BaseActivity.kt(1 hunks)core/ui/src/prod/kotlin/composable/AppLogo.kt(1 hunks)core/ui/src/prod/kotlin/composable/FloatingNotice.kt(3 hunks)core/ui/src/prod/kotlin/composable/TextFields.kt(2 hunks)core/ui/src/prod/kotlin/mvi/BaseMviViewModel.kt(2 hunks)core/ui/src/prod/kotlin/mvi/Intent.kt(1 hunks)core/ui/src/prod/kotlin/mvi/SideEffect.kt(1 hunks)core/ui/src/prod/kotlin/mvi/UiState.kt(1 hunks)core/ui/src/prod/kotlin/navigation/Destination.kt(1 hunks)core/ui/src/prod/kotlin/util/Brushes.kt(1 hunks)core/ui/src/prod/kotlin/util/Utils.kt(1 hunks)core/ui/src/prod/kotlin/viewmodel/BaseViewModel.kt(1 hunks)core/ui/src/prod/kotlin/viewmodel/Extensions.kt(1 hunks)core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt(0 hunks)core/widget/src/prod/kotlin/team/aliens/dms/android/core/widget/MealType.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/Main.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListScreen.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListViewModel.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/navigation/AnnouncementNavigator.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/ApplicationScreen.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/ApplicationViewModel.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/navigation/ApplicationNavigator.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/HomeScreen.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/HomeViewModel.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/navigation/HomeNavigator.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageScreen.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageViewModel.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/navigation/MyPageNavigator.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/main/navigation/MainNavigator.kt(0 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/component/CompleteScreen.kt(4 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/component/IntroScreen.kt(4 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/viewmodel/OnboardingViewModel.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/navigation/SignInRoute.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt(1 hunks)
โ Files not processed due to max files limit (49)
- feature/src/prod/kotlin/team/aliens/dms/android/feature/editpassword/ConfirmPasswordScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/editpassword/EditPasswordViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/editpassword/SetPasswordScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/editprofile/EditProfileImageScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/editprofile/EditProfileImageViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/findid/FindIdScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/findid/FindIdViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/Main.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/application/ApplicationScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/application/ApplicationViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/home/HomeScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/home/HomeViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notice/NoticeDetailsScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notice/NoticeDetailsViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notification/box/NotificationBoxScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notification/box/NotificationBoxViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notification/settings/NotificationSettingsScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/notification/settings/NotificationSettingsViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/outing/OutingApplicationScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/outing/OutingStatusScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/outing/OutingViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/point/PointHistoryScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/point/PointHistoryViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/remains/RemainsApplicationScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/remains/RemainsApplicationViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/resetpassword/AccountVerificationScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/resetpassword/ResetPasswordEnterEmailVerificationCodeScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/resetpassword/ResetPasswordViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/resetpassword/SetPasswordScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signin/SignInScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signin/SignInViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/EnterEmailScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/EnterEmailVerificationCodeScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/EnterSchoolVerificationCodeScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/EnterSchoolVerificationQuestionScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/SetIdScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/SetPasswordScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/SetProfileImageScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/SignUpViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/signup/TermsScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/studyroom/Common.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/studyroom/details/StudyRoomDetailsScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/studyroom/details/StudyRoomDetailsViewModel.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/studyroom/list/StudyRoomListScreen.kt
- feature/src/prod/kotlin/team/aliens/dms/android/feature/studyroom/list/StudyRoomListViewModel.kt
๐ค Files with no reviewable changes (20)
- core/ui/src/main/java/team/aliens/dms/android/core/ui/util/Utils.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/navigation/MyPageNavigator.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/Main.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/navigation/AnnouncementNavigator.kt
- core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/Intent.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/HomeScreen.kt
- core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/UiState.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/ApplicationViewModel.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/ApplicationScreen.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageViewModel.kt
- core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/application/navigation/ApplicationNavigator.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/navigation/HomeNavigator.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/navigation/MainNavigator.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListViewModel.kt
- core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/announcement/AnnouncementListScreen.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/mypage/MyPageScreen.kt
- core/ui/src/main/java/team/aliens/dms/android/core/ui/mvi/SideEffect.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/main/home/HomeViewModel.kt
๐งฐ Additional context used
๐งฌ Code graph analysis (4)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/textfield/DmsTextField.kt (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/DmsIconButton.kt (1)
DmsIconButton(17-40)
app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt (4)
app/src/dev/kotlin/team/aliens/dms/android/app/DmsAppState.kt (1)
rememberDmsAppState(18-35)feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/navigation/OnboardingRoute.kt (1)
OnboardingRoute(6-13)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/navigation/SignInRoute.kt (1)
SignInRoute(7-20)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/SnackBar.kt (1)
DmsSnackBar(22-58)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/indecator/DmsDotsLoadingIndicator.kt (1)
DmsDotsLoadingIndicator(27-68)
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt (3)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsSymbol.kt (1)
DmsSymbol(9-22)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt (1)
DmsButton(255-353)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/textfield/DmsTextField.kt (1)
DmsTextField(39-107)
โฐ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
๐ Additional comments (64)
core/ui/src/prod/kotlin/util/Utils.kt (1)
1-1: Clarify the purpose of this empty utility file.The file contains only a package declaration with no utility functions, extensions, or other definitions. Please clarify whether this is:
- An intentional placeholder for future production-specific utilities
- A file that should have initial utility content
If this file is not needed yet, consider removing it or adding an initial skeleton of utilities planned for the login screen flow.
core/design-system/src/prod/res/drawable/ic_palette.xml (1)
1-21: Verify color strategy aligns with design system theming approach.The drawable uses a hardcoded fill color (
#000000) across all paths. Given that the PR overhauls the design system with new theming support (as mentioned in the summary), ensure this approach is intentional and consistent with how other new drawable assets handle colorsโparticularly if the app supports dark mode or dynamic theming.If dynamic color application is required, consider whether colors should be:
- Applied at the component level (e.g., via
setTint()orandroid:tintattributes)- Defined as theme attributes and referenced indirectly
- Hardcoded as is, with tinting applied elsewhere in the UI layer
core/design-system/src/prod/res/drawable/ic_blue_bell.xml (1)
1-9: LGTM!The vector drawable is well-formed with consistent dimensions, valid path data, and follows Android naming conventions.
core/design-system/src/prod/res/drawable/ic_person.xml (1)
1-9: LGTM! Standard vector drawable icon.The XML is well-formed and follows Android vector drawable conventions. The 24ร24 dp dimensions are standard for material icons, and the path data correctly defines a person silhouette.
core/design-system/src/prod/res/drawable/ic_warning.xml (1)
1-5: LGTM: Standard vector drawable declaration.The dimensions (24dp) and viewport (24x24) are consistent and follow Android best practices for icon resources.
core/ui/src/prod/kotlin/CollectInLaunchedEffectWithLifecycle.kt (1)
19-36: LGTM! Implementation follows Compose best practices.The function correctly handles lifecycle-aware Flow collection using
repeatOnLifecycleand properly manages Composable state updates viarememberUpdatedState. The@SuppressLintannotation is justified given the intentional deviation from Composable naming conventions for this extension utility.core/design-system/src/prod/res/drawable/ic_forward.xml (1)
1-9: LGTM!The vector drawable is correctly structured with standard dimensions and proper XML syntax. The forward-arrow icon is a clean addition to the design system.
feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/viewmodel/OnboardingViewModel.kt (1)
36-38: Architectural simplification is sound and compatible.The removal of
UiStateinheritance fromOnboardingStateis valid becauseBaseStateViewModel<S, E>(line 25 of BaseStateViewModel.kt) has no type constraints requiringSto extendUiState. The data class works correctly with the generic type parameter, andOnboardingViewModelsuccessfully extendsBaseStateViewModel<OnboardingState, Unit>with the providedinitialState()function.core/ui/src/prod/kotlin/PaddingDefaults.kt (1)
1-7: LGTM!Package declaration and imports are clean and appropriate for padding utilities.
core/design-system/src/dev/res/drawable/ic_symbol_dark.xml (2)
7-77: LGTM!The vector drawable implementation is well-structured:
- Proper use of
aapt:attrfor gradient definitions- Clean separation of white text elements and blue-gradient symbol components
- Linear gradients with appropriate color stops for visual depth
- Hardcoded brand colors ensure visual consistency across themes
The complexity is typical and acceptable for a logo asset.
1-6: Verify the file location and naming approach for theme assets.The file is located in
src/dev/res/drawable/with "dark" in the filename. For theme-specific assets, Android typically uses resource qualifier directories (e.g.,drawable-night/ic_symbol.xmlfor dark theme,drawable/ic_symbol.xmlfor light theme) rather than suffix-based naming. The "dev" source set suggests this might be build-variant specific rather than theme-specific.Please confirm whether:
- This approach aligns with your project's theming strategy
- The "dev" source set is the intended location for variant-specific branding
- Runtime theme switching (light/dark mode) will properly resolve to the correct asset
#!/bin/bash # Description: Check the project's drawable organization and theme handling strategy # Find all symbol drawable variants echo "=== Symbol drawable files ===" fd -e xml "ic_symbol" core/design-system/src # Check if there are resource qualifier directories for night mode echo -e "\n=== Night mode drawable directories ===" fd -t d "drawable-night" core/design-system/src # Look for theme configuration or documentation echo -e "\n=== Theme-related configuration ===" rg -l "ic_symbol|Theme|AppTheme" --type=xml core/design-system/src -g "!ic_symbol*.xml" | head -20core/design-system/src/prod/res/drawable/ic_notice.xml (1)
1-9: Consider aligning icon dimensions with the design system standard.The icon uses 23dp dimensions, which differs from the standard 24dp used by most icons in the design system (including the related
ic_blue_notice.xml). Verify whether this dimension is intentional for this particular icon or should be standardized to 24dp for consistency.Likely an incorrect or invalid review comment.
core/design-system/src/prod/res/drawable/ic_down.xml (1)
1-5: LGTM!The vector drawable dimensions and viewport are properly configured.
app/src/main/res/values-night/themes.xml (1)
1-11: LGTM!This is a formatting-only change that removes an empty line. No functional impact.
core/ui/src/prod/kotlin/composable/FloatingNotice.kt (2)
42-42: Theic_noticedrawable resource has been properly added to the design system in both dev and prod configurations. No action needed.
1-1: Package refactoring toui.composableis complete.Verification confirms no stale references to the old package path
team.aliens.dms.android.core.ui.composableremain in the codebase. The new package declarationui.composableand updated imports for padding utilities (PaddingDefaults,horizontalPadding,verticalPadding) are correctly applied across all files.core/ui/src/prod/kotlin/activity/BaseActivity.kt (1)
1-1: Package refactoring is complete.All references to BaseActivity have been successfully migrated to the new package path
ui.activity. No stale imports of the old package pathteam.aliens.dms.android.core.ui.activityremain in the codebase.app/src/dev/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt (1)
3-3: No changes neededโBaseViewModel is an empty abstract class with no functionality. Both the prod and dev versions of MainActivityViewModel are functionally identical and will operate the same way regardless of whether they extend BaseViewModel or ViewModel directly, as BaseViewModel provides no error handling, event management, lifecycle hooks, or other critical functionality.Likely an incorrect or invalid review comment.
core/design-system/src/prod/res/drawable/ic_blue_dinner.xml (1)
1-9: LGTM! New blue dinner icon added.The drawable resource is properly structured. The 26dp size is consistent with other meal icons (ic_blue_breakfast, ic_blue_lunch) and the #3D8AFF color aligns with the blue icon family in the design system.
core/design-system/src/prod/res/drawable/ic_blue_notice.xml (1)
1-9: LGTM! New blue notice icon added.The vector drawable is correctly defined with appropriate dimensions (24dp) for a notice icon and uses the consistent blue color (#3D8AFF) from the design system's blue icon family.
core/design-system/src/prod/res/drawable/ic_information.xml (1)
1-13: LGTM! Information icon properly structured.The vector drawable is well-formed with appropriate use of clipping path and standard 24dp dimensions. The black fill color suggests this icon will be tinted at runtime according to the theme.
core/design-system/src/prod/res/drawable/ic_blue_lunch.xml (1)
1-9: LGTM! Blue lunch icon added.The drawable is properly defined with consistent sizing (26dp) matching other meal icons and uses the standard blue color (#3D8AFF) from the design system.
feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/component/CompleteScreen.kt (3)
31-33: LGTM! Button imports updated to subpackage.The imports have been correctly updated to reference the button subpackage, aligning with the design system reorganization.
114-114: Good UX improvement with full-width button.Adding
Modifier.fillMaxWidth()to the primary action button follows modern design patterns and improves the user experience.
97-97: Color token change is consistent with design system.The change to
onTertiaryContaineris used consistently across the codebase for headline and title text elements (IntroScreen, SigninScreen, and now CompleteScreen). The token is semantically appropriate and maps togray700in both light and dark themes, providing adequate contrast for readability.core/design-system/src/prod/res/drawable/ic_plus.xml (1)
1-9: LGTM! Plus icon added with appropriate sizing.The vector drawable is properly structured. The 30dp size provides an adequate touch target for an action button, and the white fill color indicates it's designed for use on colored backgrounds or with runtime tinting.
core/ui/src/prod/kotlin/composable/AppLogo.kt (1)
1-1: Package namespace update for AppLogo is complete and correct.All imports of AppLogo reference the new package path
ui.composable.AppLogo. Verified:
feature/src/prod/kotlin/team/aliens/dms/android/feature/main/home/HomeScreen.kt(line 87)core/ui/src/prod/kotlin/Banner.kt(line 20)No references to the old package path remain in the codebase.
core/ui/src/prod/kotlin/util/Brushes.kt (1)
1-1: Verify that all imports reference the new package.The package has been updated from
team.aliens.dms.android.core.ui.utiltoui.util. Ensure all files that importverticalFadeOutBrushorverticalFadeInBrushhave been updated to the new import path.#!/bin/bash # Description: Find all imports from the util package and verify they use the new path. # Expected: All imports should reference ui.util, not the old path. echo "=== Searching for brush imports ===" rg -n "import.*(verticalFadeOutBrush|verticalFadeInBrush)" --type kotlin echo -e "\n=== Checking for old package path references ===" rg -n "team\.aliens\.dms\.android\.core\.ui\.util" --type kotlincore/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/modifier/DmsDropShadow.kt (1)
30-34: Be aware of BlurMaskFilter performance implications.Using
BlurMaskFiltertriggers software rendering for the shadow layer, which can impact performance on complex layouts or when many shadows are rendered simultaneously. Consider documenting this behavior and testing performance on lower-end devices.Based on learnings, if performance becomes an issue, consider using Compose's built-in
graphicsLayer { shadowElevation = ... }for simpler cases, or document when to use this custom shadow vs. elevation-based shadows.app/src/prod/res/values-night/themes.xml (1)
4-11: LGTM!The night theme configuration is correct. Transparent system bars with dark status/navigation bar icons (
windowLightStatusBarandwindowLightNavigationBarset tofalse) are appropriate for dark mode.app/src/main/res/values/themes.xml (1)
2-2: LGTM!Formatting change only - namespace attribute order has no functional impact.
core/design-system/src/dev/res/drawable/ic_error.xml (1)
1-9: LGTM!The error icon is properly defined with a square aspect ratio (18x18dp) and an appropriate red color (#FE0F0F) for error states.
core/ui/src/prod/kotlin/mvi/BaseMviViewModel.kt (1)
1-1: Verify all subclasses and imports have been updated.The package namespace has changed from
team.aliens.dms.android.core.ui.mvitoui.mvi, and the BaseViewModel import path has been updated. Ensure all ViewModels extendingBaseMviViewModelhave updated their imports.core/ui/src/prod/kotlin/Banner.kt (1)
1-1: Verify all usages of Banner and related imports have been updated.The package namespace has changed from
team.aliens.dms.android.core.uitoui. Ensure all files importing or using theBannercomposable have updated their import statements to reflect this change.core/ui/src/prod/kotlin/viewmodel/Extensions.kt (1)
1-1: Verify all imports across the codebase have been updated.The package namespace has changed from
team.aliens.dms.android.core.ui.viewmodeltoui.viewmodel. Ensure that all files importing these extension functions have updated their import statements to avoid compilation failures.core/notification/src/main/java/team/aliens/dms/android/core/notification/NotificationManager.kt (1)
11-11: LGTM!The import path update aligns with the design system refactoring that moves
DmsIconto the foundation package.core/design-system/src/dev/res/drawable/ic_success.xml (1)
1-9: LGTM!The success icon drawable is well-formed and follows standard Android vector drawable conventions. The resource will be used by the new snackbar component.
core/widget/src/prod/kotlin/team/aliens/dms/android/core/widget/MealType.kt (1)
3-3: LGTM!The import path update is consistent with the design system refactoring moving
DmsIconto the foundation package.core/design-system/src/prod/res/drawable/ic_sort.xml (1)
1-9: LGTM!The sort icon drawable is well-formed and will be exposed through the
DmsIconAPI surface.core/design-system/src/prod/res/drawable/ic_bell.xml (1)
1-9: LGTM!The bell icon drawable is well-formed and follows standard Android vector drawable conventions.
feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/component/IntroScreen.kt (2)
38-44: LGTM!The color theming updates align with the design system refactoring, using Material 3 color roles (
onPrimaryContainerandonTertiaryContainer) from the newDmsTheme.colorScheme.
83-83: LGTM!The text color updates consistently use
onTertiaryContainerfrom the new theme system, ensuring visual consistency across the intro screens.Also applies to: 104-104, 117-117
app/src/prod/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt (1)
8-8: Verify the import path for BaseViewModel.The import
ui.viewmodel.BaseViewModelneeds verification against the actual package location of the BaseViewModel class definition. Confirm whether this is a complete package path, an aliased import, or if it's configured via a package structure setting that would make this import valid.app/src/dev/kotlin/team/aliens/dms/android/app/MainActivity.kt (1)
48-48: LGTM!The
setEdgeToEdgeConfig()call is correctly placed beforesetDecorFitsSystemWindows, ensuring proper edge-to-edge UI configuration with transparent system bars.app/src/prod/res/values/strings.xml (1)
3-4: LGTM!The string resources are properly defined. The
app_nameis correctly marked as non-translatable, and thedefault_notification_channel_idfollows Android naming conventions.core/design-system/src/prod/res/drawable/ic_applicate.xml (1)
1-9: LGTM!The vector drawable follows standard Android conventions with appropriate dimensions and viewport settings.
core/design-system/src/prod/res/drawable/ic_close.xml (1)
1-9: LGTM!The vector drawable is well-formed and uses consistent dimensions (24dp ร 24dp) with other icons in the design system.
app/src/dev/kotlin/team/aliens/dms/android/app/EdgeToEdgeConfig.kt (1)
10-13: LGTM!The explicit
SystemBarStyle.autoconfiguration with transparent colors properly configures edge-to-edge rendering for both status and navigation bars. The implementation follows Android edge-to-edge best practices.app/src/prod/res/values/themes.xml (1)
4-4: Remove inconsistentandroid:fitsSystemWindows="true"from theme style.The theme sets
android:fitsSystemWindows="true"in prod themes but not in night themes, creating an inconsistency. While the codebase properly overrides this viaWindowCompat.setDecorFitsSystemWindows(window, false)in MainActivity and uses Material3 Compose components (which auto-handle insets), the redundant theme attribute should be removed for clarity:- <style name="Theme.DMS" parent="Theme.Material3.DayNight.NoActionBar" android:fitsSystemWindows="true"> + <style name="Theme.DMS" parent="Theme.Material3.DayNight.NoActionBar">Apply this change to both
app/src/main/res/values/themes.xmlandapp/src/prod/res/values/themes.xmlto match the night theme variants.app/src/main/AndroidManifest.xml (1)
1-2: The empty main manifest is a valid multi-variant configuration pattern.The variant-specific manifests (dev/prod) override the main manifest through Gradle's established priority order, allowing each build variant to define different components while reusing common configuration. The dev and prod manifests contain all necessary attributes (namespaces, application definitions, permissions), and the build.gradle.kts properly configures product flavors with corresponding sourceSets. The package namespace is set in the build configuration (namespace = "team.aliens.dms" in build.gradle.kts line 17), so the missing package attribute in the main manifest is not a concern. This pattern is correct and requires no changes.
core/design-system/src/prod/res/drawable/ic_circle_outline.xml (1)
1-11: LGTM!The vector drawable is correctly structured with valid dimensions, stroke, and path definition.
core/design-system/src/dev/res/drawable/ic_symbol_light.xml (1)
1-77: LGTM!The vector drawable correctly uses
aapt:attrfor inline gradient definitions, and all gradient configurations are properly specified.core/design-system/src/prod/res/drawable/ic_logo_light.xml (1)
1-79: LGTM!The logo vector drawable is correctly structured with proper gradient definitions using
aapt:attr.core/design-system/src/prod/res/drawable/ic_logo_dark.xml (1)
1-86: LGTM!The dark logo vector drawable is correctly structured with proper gradient definitions.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/DmsSnackBarVisuals.kt (1)
6-12: LGTM!The data class correctly implements
SnackbarVisualswith appropriate defaults and adds the customsnackBarTypeproperty for typed snackbar rendering.core/ui/src/prod/kotlin/composable/TextFields.kt (2)
1-1: All imports have been successfully updated.Verification confirms no remaining imports use the old package path
team.aliens.dms.android.core.ui.composable. All five files importingPasswordTextField(SetPasswordScreen.kt in editpassword, signup, and resetpassword features; ConfirmPasswordScreen.kt; and SignInScreen.kt) correctly use the new package pathui.composable.PasswordTextField.
35-35: DmsIcon relocation verified as consistent across the codebase.The import path change to include the
foundationsubpackage is fully applied across all 18 files in the repository. No lingering old imports remain, and all instances uniformly useimport team.aliens.dms.android.core.designsystem.foundation.DmsIcon. The relocation is complete and consistent.feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/navigation/SignInRoute.kt (1)
16-17: Empty navigation handlers for find ID/password flows.
navigateToFindIdandnavigateToFindPasswordare hardcoded as empty lambdas. If these features are planned, consider either:
- Exposing them as parameters in
SignInRoutefor proper wiring- Adding a TODO comment to track the incomplete implementation
app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt (1)
67-115: LGTM! Well-structured app-level snackbar integration.The Box wrapper with SnackbarHost positioned at the top center is cleanly implemented. The safe cast on line 108 with early return is a good defensive pattern.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/textfield/DmsTextField.kt (1)
39-107: LGTM! Well-designed text field component.The implementation provides comprehensive functionality including animated labels, error states, password visibility toggle, and clear button. The focus management and state-driven styling are well executed.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (1)
16-97: LGTM! Theme restructuring is well-implemented.The migration to
staticCompositionLocalOfand Material3'sColorSchemeis correctly done. The color token mappings for light and dark themes are comprehensive.app/src/prod/AndroidManifest.xml (1)
1-81: LGTM! Production manifest is properly configured.The manifest correctly declares:
- Runtime permissions with appropriate uses-feature for camera (non-required)
- MainActivity with portrait orientation lock
- Firebase messaging service with proper intent filter
- Widget receiver and initialization provider
All configurations follow Android best practices.
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt (1)
52-142: Well-structured sign-in screen implementation.The composable hierarchy is clean with good separation of concerns:
SignInhandles ViewModel wiring and side effectsSignInScreenmanages UI layout- Private composables (
UserInformationInputs,SignupActions) encapsulate reusable sectionsThe focus management with tap-to-dismiss is a nice UX touch.
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt (1)
67-94: Good error handling with user-friendly messages.The sign-in flow correctly:
- Manages loading state and button enabled state
- Maps specific exceptions to localized error messages
- Emits appropriate side effects for navigation and error display
The TODO on line 83 suggests future refactoring to centralize error message mapping, which would be a good improvement.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt
Outdated
Show resolved
Hide resolved
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt
Show resolved
Hide resolved
.../design-system/src/prod/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt
Show resolved
Hide resolved
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt
Outdated
Show resolved
Hide resolved
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt
Outdated
Show resolved
Hide resolved
| private fun getDeviceToken() { | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| runCatching { | ||
| notificationRepository.getDeviceToken() | ||
| }.onSuccess { deviceToken -> | ||
| setState { it.copy(deviceToken = deviceToken) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
#!/bin/bash
# First, find the SignInViewModel file to understand its structure
find . -name "SignInViewModel.kt" -type fRepository: team-aliens/DMS-Android
Length of output: 243
๐ Script executed:
#!/bin/bash
# Search for NotificationRepository implementation
find . -name "*NotificationRepository*" -type fRepository: team-aliens/DMS-Android
Length of output: 270
๐ Script executed:
#!/bin/bash
# Search for calls to getDeviceToken() to see where it's invoked
rg "getDeviceToken\(\)" --type kotlin -B 3 -A 3Repository: team-aliens/DMS-Android
Length of output: 7380
๐ Script executed:
#!/bin/bash
# Read NotificationRepository interface
cat -n ./data/src/main/java/team/aliens/dms/android/data/notification/repository/NotificationRepository.ktRepository: team-aliens/DMS-Android
Length of output: 1595
๐ Script executed:
#!/bin/bash
# Read NotificationRepositoryImpl implementation
cat -n ./data/src/main/java/team/aliens/dms/android/data/notification/repository/NotificationRepositoryImpl.ktRepository: team-aliens/DMS-Android
Length of output: 3416
๐ Script executed:
#!/bin/bash
# Search for deviceDataStoreDataSource.loadDeviceToken implementation
rg "loadDeviceToken" --type kotlin -B 2 -A 5Repository: team-aliens/DMS-Android
Length of output: 4659
๐ Script executed:
#!/bin/bash
# Search for notification permission handling
rg "permission\|Permission" --type kotlin | grep -i notifRepository: team-aliens/DMS-Android
Length of output: 49
๐ Script executed:
#!/bin/bash
# Find DeviceTokenNotFoundException
rg "DeviceTokenNotFoundException" --type kotlin -B 2 -A 2Repository: team-aliens/DMS-Android
Length of output: 2056
๐ Script executed:
#!/bin/bash
# Check the full dev SignInViewModel to see if deviceToken is required
cat -n ./feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.ktRepository: team-aliens/DMS-Android
Length of output: 5443
๐ Script executed:
#!/bin/bash
# Search for SignInState to understand what deviceToken is used for
rg "SignInState" --type kotlin -A 5 | head -50Repository: team-aliens/DMS-Android
Length of output: 3661
Add error handling for device token retrieval failure.
The getDeviceToken() call in init {} lacks an onFailure handler. If loadDeviceToken() throws DeviceTokenNotFoundException, the error is silently caught and deviceToken remains an empty string. Sign-in then proceeds with an empty token, causing it to fail at the server level without explicit user feedback. Add .onFailure { } to either display an error message, retry, or set a sensible default.
Additionally, verify that notification permissions are checked before attempting token retrieval, or confirm the repository handles missing permissions gracefully.
๐ค Prompt for AI Agents
In
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt
around lines 57 to 65, the runCatching call that fetches the device token has no
onFailure handler so a thrown DeviceTokenNotFoundException is swallowed and the
app proceeds with an empty token; add .onFailure { } to handle errors by
detecting DeviceTokenNotFoundException (or other exceptions) and then either
update state with a sensible default or error flag, show a user-facing
error/notification, or trigger a retry flow; also ensure you check notification
permission before calling notificationRepository.getDeviceToken() (or confirm
the repository already handles missing permissions) and log the error for
debugging.
| sendEffect(SignInSideEffect.NavigateToMain) | ||
| }.onFailure { | ||
| setState { this.copy(isLoading = false, buttonEnabled = true) } | ||
| Log.d("TEST", it.toString()) |
There was a problem hiding this comment.
Remove debug logging before production.
The Log.d("TEST", ...) statement should be removed or replaced with proper error tracking (e.g., Firebase Crashlytics, Sentry).
- Log.d("TEST", it.toString())๐ 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.
| Log.d("TEST", it.toString()) |
๐ค Prompt for AI Agents
In
feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt
around line 82, remove the temporary debug log statement Log.d("TEST",
it.toString()); replace it either by deleting the line entirely or by reporting
the error via the project's production error-tracking mechanism (e.g., send to
Crashlytics/Sentry) with a clear message and relevant context, ensuring no
debug-only tags remain in production code.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
๋ก๊ทธ์ธ ํ๋ฉด ๊ฐ๋ฐ๊ณผ ํจ๊ป ์ ๋ฐ์ ์ธ ๋์์ธ ์์คํ ๋ฆฌํฉํ ๋ง์ด ์ด๋ฃจ์ด์ง PR๋ก ๋ณด์ ๋๋ค. ์๋ก์ด ๋ก๊ทธ์ธ UI, ์ค๋ต๋ฐ ์์คํ , Edge-to-Edge ์ ์ฉ ๋ฑ ๋ง์ ๋ถ๋ถ์ด ๊ฐ์ ๋์์ต๋๋ค. ์ฝ๋ ๋ณ๊ฒฝ์ ๋ค์ ๊ฒํ ํ ๊ฒฐ๊ณผ, ๋ช ๊ฐ์ง ์์ ์ด ํ์ํ ๋ถ๋ถ์ ๋ฐ๊ฒฌํ์ต๋๋ค. ํนํ DmsTheme์์ MaterialTheme์ด ์ ๊ฑฐ๋ ์ ์ ์ฑ์ UI ๋ ๋๋ง์ ์น๋ช ์ ์ธ ์ํฅ์ ์ค ์ ์์ด ์์ ์ด ํ์ํฉ๋๋ค. ๋ํ, ๋ฒํผ์ ํ ๋๋ฆฌ๊ฐ ์ ์ฉ๋์ง ์๋ ๋ฌธ์ ์ ์ปฌ๋ฌ, ํฐํธ ๊ด๋ จ ์ฌ์ํ ๋ฒ๊ทธ๋ค์ด ํ์ธ๋์์ต๋๋ค. ์์ธํ ๋ด์ฉ์ ๊ฐ ํ์ผ์ ๋จ๊ธด ๋ฆฌ๋ทฐ ์ฝ๋ฉํธ๋ฅผ ์ฐธ๊ณ ํด์ฃผ์ธ์.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt
Show resolved
Hide resolved
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt
Outdated
Show resolved
Hide resolved
| var Rult = headline1.hashCode() | ||
| Rult = 31 * Rult + headline2.hashCode() | ||
| Rult = 31 * Rult + headline3.hashCode() | ||
| Rult = 31 * Rult + title1.hashCode() | ||
| Rult = 31 * Rult + title2.hashCode() | ||
| Rult = 31 * Rult + title3.hashCode() | ||
| Rult = 31 * Rult + body1.hashCode() | ||
| Rult = 31 * Rult + body2.hashCode() | ||
| Rult = 31 * Rult + body3.hashCode() | ||
| Rult = 31 * Rult + caption.hashCode() | ||
| Rult = 31 * Rult + overline.hashCode() | ||
| Rult = 31 * Rult + button.hashCode() | ||
| return Rult |
There was a problem hiding this comment.
๋ณ์๋ช
์ ์คํ๊ฐ ์์ต๋๋ค. Rult๋ฅผ result๋ก ์์ ํ๋ ๊ฒ์ด ์ข๊ฒ ์ต๋๋ค.
| var Rult = headline1.hashCode() | |
| Rult = 31 * Rult + headline2.hashCode() | |
| Rult = 31 * Rult + headline3.hashCode() | |
| Rult = 31 * Rult + title1.hashCode() | |
| Rult = 31 * Rult + title2.hashCode() | |
| Rult = 31 * Rult + title3.hashCode() | |
| Rult = 31 * Rult + body1.hashCode() | |
| Rult = 31 * Rult + body2.hashCode() | |
| Rult = 31 * Rult + body3.hashCode() | |
| Rult = 31 * Rult + caption.hashCode() | |
| Rult = 31 * Rult + overline.hashCode() | |
| Rult = 31 * Rult + button.hashCode() | |
| return Rult | |
| var result = headline1.hashCode() | |
| result = 31 * result + headline2.hashCode() | |
| result = 31 * result + headline3.hashCode() | |
| result = 31 * result + title1.hashCode() | |
| result = 31 * result + title2.hashCode() | |
| result = 31 * result + title3.hashCode() | |
| result = 31 * result + body1.hashCode() | |
| result = 31 * result + body2.hashCode() | |
| result = 31 * result + body3.hashCode() | |
| result = 31 * result + caption.hashCode() | |
| result = 31 * result + overline.hashCode() | |
| result = 31 * result + button.hashCode() | |
| return result |
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt
Show resolved
Hide resolved
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt
Show resolved
Hide resolved
| val errorMessage = when (it) { // TODO :: onFailure ์์ฒด์์ ๋ฐ์์ฌ ์ ์๊ฒ ๊ตฌ์กฐ ๋ณ๊ฒฝ | ||
| is BadRequestException -> "์๋ชป๋ ํ์์ด์์" | ||
| is UnprocessableEntityException -> "๋น๋ฐ๋ฒํธ๊ฐ ์ผ์นํ์ง ์์์" | ||
| is NotFoundException -> "๋ก๊ทธ์ธ ์ ๋ณด๋ฅผ ๋ค์ ํ์ธํด์ฃผ์ธ์" | ||
| is InternalServerErrorException -> "์๋ฒ์ ๋ฌธ์ ๊ฐ ๋ฐ์ํ์ด์" | ||
| else -> "์ผ์์ ์ธ ์ค๋ฅ๋ก ๋ก๊ทธ์ธ์ ํ ์ ์์ด์" | ||
| } |
There was a problem hiding this comment.
์ค๋ฅ ๋ฉ์์ง๋ฅผ ํ๋์ฝ๋ฉํ๋ ๋์ ๋ฌธ์์ด ๋ฆฌ์์ค๋ก ๊ด๋ฆฌํ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์ธ์. ์ด๋ ๊ฒ ํ๋ฉด ๋ค๊ตญ์ด ์ง์ ๋ฐ ๋ฌธ์์ด ๊ด๋ฆฌ๊ฐ ์ฉ์ดํด์ง๋๋ค.
There was a problem hiding this comment.
Actionable comments posted: 1
โป๏ธ Duplicate comments (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
162-174: Typo in variable name:Rultshould beresult.This was flagged in a previous review. The local variable should follow Kotlin naming conventions.
๐ Suggested fix
override fun hashCode(): Int { - var Rult = headline1.hashCode() - Rult = 31 * Rult + headline2.hashCode() - Rult = 31 * Rult + headline3.hashCode() - Rult = 31 * Rult + title1.hashCode() - Rult = 31 * Rult + title2.hashCode() - Rult = 31 * Rult + title3.hashCode() - Rult = 31 * Rult + body1.hashCode() - Rult = 31 * Rult + body2.hashCode() - Rult = 31 * Rult + body3.hashCode() - Rult = 31 * Rult + caption.hashCode() - Rult = 31 * Rult + overline.hashCode() - Rult = 31 * Rult + button.hashCode() - return Rult + var result = headline1.hashCode() + result = 31 * result + headline2.hashCode() + result = 31 * result + headline3.hashCode() + result = 31 * result + title1.hashCode() + result = 31 * result + title2.hashCode() + result = 31 * result + title3.hashCode() + result = 31 * result + body1.hashCode() + result = 31 * result + body2.hashCode() + result = 31 * result + body3.hashCode() + result = 31 * result + caption.hashCode() + result = 31 * result + overline.hashCode() + result = 31 * result + button.hashCode() + return result }
๐งน Nitpick comments (6)
core/ui/src/dev/java/team/aliens/dms/android/core/ui/PaddingDefaults.kt (1)
9-9: Offer to implement screen orientation support.The TODO indicates that screen orientation support is planned but not yet implemented.
Would you like me to open a new issue to track this enhancement, or generate an implementation that dynamically adjusts padding based on screen orientation (portrait/landscape)?
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt (1)
28-28: Consolidate duplicate icon references.Both
Notification(line 28) andNotice(line 31) map to the same drawable resourceR.drawable.ic_notice. This duplication creates ambiguity about which property should be used. Consider keeping only one canonical property name.๐ Suggested approach:
If "Notice" is the preferred term, remove the
Notificationproperty:- val Notification = R.drawable.ic_notice val Lunch = R.drawable.ic_lunch val Person = R.drawable.ic_person val Notice = R.drawable.ic_noticeOr if "Notification" is preferred, remove the
Noticeproperty:val Notification = R.drawable.ic_notice val Lunch = R.drawable.ic_lunch val Person = R.drawable.ic_person - val Notice = R.drawable.ic_noticeAlso applies to: 31-31
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt (2)
144-146: Unreachableelsebranch in exhaustivewhenexpression.The
whenalready covers allButtonColorenum values (Primary, Gray, Error). Theelsebranch is dead code and can be removed. The same applies tounderlinecolorScheme()at line 186.๐ Suggested fix
ButtonColor.Error -> ButtonState( enabled = ButtonTheme( textColor = DmsTheme.colorScheme.inversePrimary, ), pressed = ButtonTheme( textColor = DmsTheme.colorScheme.primaryContainer, ), disabled = ButtonTheme( textColor = DmsTheme.colorScheme.onPrimary, ), ) - - else -> throw IllegalArgumentException("Unhandled ButtonColor: $this") }
189-201: UnusedbuttonTypeparameter inBasicButton.The
buttonTypeparameter is accepted but never referenced within the function body. Either remove the parameter or implement the intended logic that uses it.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (1)
15-75: Consider documenting the custom color slot mappings.The mapping between DmsColor tokens and Material3 color slots diverges from standard Material3 semantics (e.g.,
surfaceTintโcontainer,scrimโbutton). Adding inline comments or a design-system ADR would help future maintainers understand the rationale.app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt (1)
65-67: Consider usingfillMaxSize()for clarity.The
Boxcurrently usesfillMaxWidth(), which works because the content (NavDisplay) will take the available height. However, explicitly usingfillMaxSize()would make the intent clearer and ensure the container properly sizes to fill the entire screen.๐ Apply this diff for more explicit sizing:
Box( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxSize(), ) {
๐ Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (14)
app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt(3 hunks)app/src/dev/kotlin/team/aliens/dms/android/app/DmsAppState.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt(1 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt(4 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt(13 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt(1 hunks)core/design-system/src/dev/res/drawable/ic_profile_default.xml(1 hunks)core/design-system/src/dev/res/drawable/ic_wrong.xml(1 hunks)core/ui/src/dev/java/team/aliens/dms/android/core/ui/PaddingDefaults.kt(1 hunks)core/ui/src/dev/java/team/aliens/dms/android/core/ui/viewmodel/BaseStateViewModel.kt(0 hunks)core/ui/src/prod/kotlin/team/aliens/dms/android/core/ui/composable/FloatingNotice.kt(2 hunks)core/ui/src/prod/kotlin/team/aliens/dms/android/core/ui/composable/TextFields.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt(1 hunks)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt(1 hunks)
๐ค Files with no reviewable changes (1)
- core/ui/src/dev/java/team/aliens/dms/android/core/ui/viewmodel/BaseStateViewModel.kt
โ Files skipped from review due to trivial changes (1)
- core/design-system/src/dev/res/drawable/ic_profile_default.xml
๐ง Files skipped from review as they are similar to previous changes (2)
- feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/viewmodel/SignInViewModel.kt
- feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/ui/SigninScreen.kt
๐งฐ Additional context used
๐งฌ Code graph analysis (2)
core/ui/src/dev/java/team/aliens/dms/android/core/ui/PaddingDefaults.kt (1)
core/ui/src/main/java/team/aliens/dms/android/core/ui/PaddingDefaults.kt (1)
None(10-54)
app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt (4)
app/src/dev/kotlin/team/aliens/dms/android/app/DmsAppState.kt (1)
rememberDmsAppState(19-33)feature/src/dev/kotlin/team/aliens/dms/android/feature/onboarding/navigation/OnboardingRoute.kt (1)
OnboardingRoute(6-13)feature/src/dev/kotlin/team/aliens/dms/android/feature/signin/navigation/SignInRoute.kt (1)
SignInRoute(7-20)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/snackbar/SnackBar.kt (1)
DmsSnackBar(22-58)
โฐ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
๐ Additional comments (21)
core/ui/src/prod/kotlin/team/aliens/dms/android/core/ui/composable/TextFields.kt (1)
35-35: LGTM! Clean import path update.The import path update to reference
DmsIconfrom thefoundationpackage is correct and aligns with the design system restructuring. The usage ofDmsIconproperties remains unchanged throughout the file.core/ui/src/prod/kotlin/team/aliens/dms/android/core/ui/composable/FloatingNotice.kt (1)
17-17: LGTM! Clean refactor for design system integration.The import addition and simplified resource reference improve code readability while maintaining the same functionality. This aligns well with the design system migration described in the PR summary.
Also applies to: 42-42
core/design-system/src/dev/res/drawable/ic_wrong.xml (1)
1-9: LGTM! Well-formed vector drawable resource.The vector drawable is correctly structured with appropriate dimensions (96x96 dp) and uses a suitable red color (#FF4646) for an error/wrong indicator icon. The path data properly defines the X-shaped mark.
core/ui/src/dev/java/team/aliens/dms/android/core/ui/PaddingDefaults.kt (3)
12-31: LGTM! Well-structured padding tokens.The padding constants follow a clear progression and the predefined
HorizontalandVerticalPaddingValues use sensible defaults that align with typical screen layout patterns.
45-53: LGTM! Correctly fixes bug from main source set.This implementation correctly uses
bottom = verticalon line 52. The corresponding function in the main source set has a bug where it usesbottom = horizontal, which would incorrectly apply horizontal padding to the bottom edge.
80-82: Verify the verticalPadding default value.The
verticalPaddingfunction usesSmall(4.dp) as its default, while other vertical padding utilities useMedium(12.dp):
PaddingDefaults.VerticalusesMediumfor top/bottomscreen()functions default toMediumfor verticaltopPadding()andbottomPadding()default toMediumPlease confirm whether this smaller default is intentional for specific use cases, or if it should be aligned with the other vertical padding defaults.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt (1)
13-13: Typo fix confirmed.The previously flagged typo (
BlueLaunch) has been corrected toBlueLunch.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/button/Button.kt (2)
330-334: Height calculation may produce inconsistent button content sizing.Using
textStyle.fontSize.toDp() * 1.2fresults in heights of ~14.4dp forlabelM(12sp) and ~19.2dp forbodyM(16sp) at 1x density. Verify this produces the intended visual appearance, especially for the loading indicator which may not scale proportionally.
59-105: LGTM - Gray color scheme addition.The new
ButtonColor.Graycase is properly implemented with enabled, pressed, and disabled states for the contained color scheme. The exhaustivewhenhandling is correct.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (2)
194-215: LGTM - Font weight mapping corrected.
pretendard_regularis now correctly mapped toFontWeight.Normal(400), aligning with standard font weight conventions. This resolves the critical issue from the previous review where it was incorrectly mapped toFontWeight.Thin.
99-110: LGTM - Font family migration.All typography styles are consistently updated to use
pretendardFontFamilyviawithDefaultFontFamily().core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (3)
88-112: LGTM - MaterialTheme wrapper restored.The
content()is now properly wrapped withMaterialTheme, providingcolorSchemeandtypographyto Material components. This resolves the critical issue from the previous review.
77-77: LGTM - LocalColors default value corrected.
LocalColorsnow uses the customlightColorSchemedefined at line 15, rather than the default Material3lightColorScheme()function. This addresses the previous review feedback.
36-43: Unusual color token mappings - verify intentionality.The
outlineandoutlineVariantslots use red colors (DmsColor.Light.red400,red500), which is atypical since outlines are usually neutral. Additionally, semantic tokens likeblack,container,button,hover,pressedare mapped to Material slots (onBackground,surfaceTint,scrim,tertiary,onTertiary).Confirm these mappings align with the design system specification, as components accessing
DmsTheme.colorScheme.outlinewill receive a red color.app/src/dev/kotlin/team/aliens/dms/android/app/DmsApp.kt (3)
45-45: Good state management pattern with testable defaults.Adding the
appStateparameter with a default value follows best practices for Compose state management, allowing easy injection for testing while providing a sensible default for production use.
73-96: LGTM: Navigation flow and snackbar integration well-structured.The navigation entries are properly wired with clear flow (Onboarding โ SignIn โ Main), and the snackbar callback correctly delegates to
appState.showSnackBar(). The backstack management (clearing on transition from Onboarding to SignIn, adding on SignIn to Main) follows expected patterns.
98-112: LGTM: SnackbarHost properly configured with defensive casting.The SnackbarHost is correctly positioned at the top with appropriate padding and z-indexing. The safe cast on Line 106 with early return is good defensive programming that prevents crashes if unexpected visuals types are passed.
app/src/dev/kotlin/team/aliens/dms/android/app/DmsAppState.kt (4)
19-33: LGTM: Well-structured state factory following Compose best practices.The
rememberDmsAppStatefactory correctly usesrememberwith appropriate dependencies and provides sensible defaults for bothcoroutineScopeandsnackBarHostState. This follows the standard Compose state holder pattern and enables both ease of use and testability.
35-39: LGTM: Appropriate use of @stable annotation and clean state structure.The
@Stableannotation correctly marks this state holder, enabling Compose's smart recomposition optimizations. The public properties provide necessary access to the underlying state for consumers.
40-49: Implementation addresses previous review concerns.The current implementation using
withTimeoutOrNulleffectively addresses the manual job cancellation issues flagged in previous reviews. This approach properly cancels the coroutine and dismisses the snackbar after the specified duration, ensuring predictable snackbar behavior.Note: If multiple snackbars are triggered rapidly, the timeout begins when each coroutine launches (not when the snackbar actually appears). In typical usage patterns where snackbars are shown occasionally, this won't be an issue.
51-61: LGTM: Clean public API with good encapsulation.The public
showSnackBarmethod provides a convenient API by accepting simple parameters and encapsulating the creation ofDmsSnackBarVisuals. The delegation to the private method keeps the implementation clean and maintains a clear separation of concerns.
| val BlueDinner = R.drawable.ic_blue_dinner | ||
| val BlueLunch = R.drawable.ic_blue_lunch | ||
| val Breakfast = R.drawable.ic_breakfast | ||
| val Calender = R.drawable.ic_calender |
There was a problem hiding this comment.
Fix the spelling: "Calender" should be "Calendar".
The property name contains a typo. The correct spelling is "Calendar" (not "Calender"). If the underlying drawable resource is also misspelled, consider renaming it as well to maintain consistency.
๐ Apply this diff to fix the property name:
- val Calender = R.drawable.ic_calender
+ val Calendar = R.drawable.ic_calendarNote: If the drawable resource file is named ic_calender.xml, rename it to ic_calendar.xml as well.
๐ 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.
| val Calender = R.drawable.ic_calender | |
| val Calendar = R.drawable.ic_calendar |
๐ค Prompt for AI Agents
In
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.kt
around line 15, the property name "Calender" is misspelled; change the property
to "Calendar = R.drawable.ic_calendar" and update all usages to
DmsIcon.Calendar. If the drawable resource file is named ic_calender.xml, rename
it to ic_calendar.xml and update any references (R.drawable.ic_calender ->
R.drawable.ic_calendar). After renaming, run a Gradle sync/clean to update
generated R references and fix any remaining compilation errors.
๊ฐ์
์์ ์ฌํญ
์ถ๊ฐ ๋ก ํ ๋ง
Summary by CodeRabbit
Release Notes
New Features
UI & Design Updates
Architecture Improvements
โ๏ธ Tip: You can customize this high-level summary in your review settings.