π :: (#804) new λμμΈ μμ€ν μ μ©#805
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. WalkthroughIntroduces product flavor support (dev/prod) across modules, adds a Compose-based design system (colors, typography, theme) with dev/prod variants, moves widget glance color schemes into flavors, adds material3 dependencies, removes Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 |
There was a problem hiding this comment.
Pull request overview
This PR applies a new design system to the DMS Android application, implementing updated fonts and colors. The changes introduce product flavors (dev/prod) across multiple modules to support environment-specific design system implementations, add Material3 dependencies, and include a new meal widget feature with Glance support.
Key changes:
- Added Material3 and extended icons dependencies for enhanced UI components
- Implemented environment-specific product flavors (dev/prod) across core and feature modules
- Created separate design system implementations for dev and prod environments with distinct color schemes and typography
- Introduced a new meal widget with work manager integration for periodic updates
Reviewed changes
Copilot reviewed 15 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Added Material3 1.4.0 and extended icons dependencies |
| feature/build.gradle.kts | Added dev/prod product flavors and extended icons dependency |
| core/widget/build.gradle.kts | Added dev/prod product flavors for widget module |
| core/ui/build.gradle.kts | Added dev/prod product flavors for UI module |
| core/notification/build.gradle.kts | Added dev/prod product flavors for notification module |
| core/design-system/build.gradle.kts | Added dev/prod product flavors and Material3 dependency |
| core/design-system/src/prod/java/.../Typography.kt | Implemented prod typography with Noto Sans font family and 12 style variants |
| core/design-system/src/prod/java/.../Colors.kt | Implemented prod color scheme with comprehensive light/dark mode support |
| core/design-system/src/prod/java/.../Theme.kt | Updated prod theme to integrate MaterialTheme with custom typography |
| core/design-system/src/dev/java/.../Typography.kt | Implemented dev typography with updated design system properties and extension functions |
| core/design-system/src/dev/java/.../Colors.kt | Implemented dev color scheme with DmsColor sealed class for light/dark modes |
| core/design-system/src/dev/java/.../Theme.kt | Created dev theme with Material3 color scheme integration |
| core/widget/src/dev/designsystem/DmsWidgetGlanceColorScheme.kt | Added dev widget color scheme with fully qualified imports |
| core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt | Added prod widget color scheme with fully qualified imports |
| core/widget/src/main/.../DmsWidgetGlanceColorScheme.kt | Removed main widget color scheme (moved to flavor-specific sources) |
| core/widget/src/dev/meal/*.kt | Implemented meal widget components with Glance, WorkManager, and DataStore support |
| app/src/prod/kotlin/.../MainActivity.kt | Removed mainViewModel parameter from DmsApp call |
| app/src/prod/kotlin/.../MainActivityViewModel.kt | Added missing flow imports for StateFlow implementation |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lineHeight = 19.1.sp, | ||
| ), | ||
| bodyM: TextStyle = TextStyle( | ||
| fontWeight = FontWeight.Bold, |
There was a problem hiding this comment.
The bodyM parameter has an incorrect font weight of FontWeight.Bold. According to the naming convention (M for Medium), it should use FontWeight.Medium to be consistent with other "M" suffixed styles.
| fontWeight = FontWeight.Bold, | |
| fontWeight = FontWeight.Medium, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
π§Ή Nitpick comments (3)
core/widget/src/dev/designsystem/DmsWidgetGlanceColorScheme.kt (1)
25-61: Simplify by removing redundant fully qualified names.The color constants are already imported at the top of the file (lines 6-23), but the code uses fully qualified names throughout the color scheme definitions. This reduces readability and increases line length unnecessarily.
Apply this diff to use the imported names directly:
private val LightColorScheme = androidx.compose.material3.lightColorScheme( - primary = team.aliens.dms.android.core.designsystem.PrimaryDefault, - onPrimary = team.aliens.dms.android.core.designsystem.Gray1, - primaryContainer = team.aliens.dms.android.core.designsystem.PrimaryLighten2, - onPrimaryContainer = team.aliens.dms.android.core.designsystem.PrimaryDarken2, - error = team.aliens.dms.android.core.designsystem.ErrorDefault, - onError = team.aliens.dms.android.core.designsystem.Gray1, - errorContainer = team.aliens.dms.android.core.designsystem.ErrorLighten2, - onErrorContainer = team.aliens.dms.android.core.designsystem.ErrorDarken2, - background = team.aliens.dms.android.core.designsystem.Gray2, - onBackground = team.aliens.dms.android.core.designsystem.Gray10, - inverseSurface = team.aliens.dms.android.core.designsystem.Gray7, - inverseOnSurface = team.aliens.dms.android.core.designsystem.Gray3, - surface = team.aliens.dms.android.core.designsystem.Gray1, - onSurface = team.aliens.dms.android.core.designsystem.Gray9, - surfaceVariant = team.aliens.dms.android.core.designsystem.Gray6, - onSurfaceVariant = team.aliens.dms.android.core.designsystem.Gray5, + primary = PrimaryDefault, + onPrimary = Gray1, + primaryContainer = PrimaryLighten2, + onPrimaryContainer = PrimaryDarken2, + error = ErrorDefault, + onError = Gray1, + errorContainer = ErrorLighten2, + onErrorContainer = ErrorDarken2, + background = Gray2, + onBackground = Gray10, + inverseSurface = Gray7, + inverseOnSurface = Gray3, + surface = Gray1, + onSurface = Gray9, + surfaceVariant = Gray6, + onSurfaceVariant = Gray5, ) private val DarkColorScheme = androidx.compose.material3.darkColorScheme( - primary = team.aliens.dms.android.core.designsystem.PrimaryDefault, - onPrimary = team.aliens.dms.android.core.designsystem.Gray1, - primaryContainer = team.aliens.dms.android.core.designsystem.PrimaryLighten1, - onPrimaryContainer = team.aliens.dms.android.core.designsystem.PrimaryDarken2, - error = team.aliens.dms.android.core.designsystem.ErrorDefault, - onError = team.aliens.dms.android.core.designsystem.Gray1, - errorContainer = team.aliens.dms.android.core.designsystem.ErrorLighten1, - onErrorContainer = team.aliens.dms.android.core.designsystem.ErrorDarken2, - background = team.aliens.dms.android.core.designsystem.Gray9, - onBackground = team.aliens.dms.android.core.designsystem.Gray1, - inverseSurface = team.aliens.dms.android.core.designsystem.Gray3, - inverseOnSurface = team.aliens.dms.android.core.designsystem.Gray7, - surface = team.aliens.dms.android.core.designsystem.Gray8, - onSurface = team.aliens.dms.android.core.designsystem.Gray2, - surfaceVariant = team.aliens.dms.android.core.designsystem.Gray4, - onSurfaceVariant = team.aliens.dms.android.core.designsystem.Gray5, + primary = PrimaryDefault, + onPrimary = Gray1, + primaryContainer = PrimaryLighten1, + onPrimaryContainer = PrimaryDarken2, + error = ErrorDefault, + onError = Gray1, + errorContainer = ErrorLighten1, + onErrorContainer = ErrorDarken2, + background = Gray9, + onBackground = Gray1, + inverseSurface = Gray3, + inverseOnSurface = Gray7, + surface = Gray8, + onSurface = Gray2, + surfaceVariant = Gray4, + onSurfaceVariant = Gray5, )core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt (1)
25-61: Redundant fully qualified names.The imports (lines 6-23) are declared but unused; the code uses fully qualified names instead. Use the imports directly for cleaner code.
private val LightColorScheme = androidx.compose.material3.lightColorScheme( - primary = team.aliens.dms.android.core.designsystem.PrimaryDefault, - onPrimary = team.aliens.dms.android.core.designsystem.Gray1, - primaryContainer = team.aliens.dms.android.core.designsystem.PrimaryLighten2, + primary = PrimaryDefault, + onPrimary = Gray1, + primaryContainer = PrimaryLighten2, // ... apply same pattern to all other color assignments )core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
248-263: Extension properties create newTextStyleinstances on each access.
sLabelBandsLabelMextension properties allocate a newTextStyleon every getter call. This is fine for infrequent use but consider caching if performance becomes a concern.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (15)
app/src/prod/kotlin/team/aliens/dms/android/app/MainActivity.kt(0 hunks)app/src/prod/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt(1 hunks)core/design-system/build.gradle.kts(2 hunks)core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.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(1 hunks)core/design-system/src/prod/java/team/aliens/dms/android/core/designsystem/Theme.kt(2 hunks)core/notification/build.gradle.kts(1 hunks)core/ui/build.gradle.kts(1 hunks)core/widget/build.gradle.kts(1 hunks)core/widget/src/dev/designsystem/DmsWidgetGlanceColorScheme.kt(1 hunks)core/widget/src/main/java/team/aliens/dms/android/core/widget/designsystem/DmsWidgetGlanceColorScheme.kt(0 hunks)core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt(1 hunks)feature/build.gradle.kts(2 hunks)gradle/libs.versions.toml(2 hunks)
π€ Files with no reviewable changes (2)
- app/src/prod/kotlin/team/aliens/dms/android/app/MainActivity.kt
- core/widget/src/main/java/team/aliens/dms/android/core/widget/designsystem/DmsWidgetGlanceColorScheme.kt
π§° Additional context used
𧬠Code graph analysis (2)
core/design-system/src/prod/java/team/aliens/dms/android/core/designsystem/Theme.kt (1)
core/design-system/src/main/java/team/aliens/dms/android/core/designsystem/Toast.kt (2)
ToastLayout(151-167)rememberToastState(143-149)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (3)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.kt (2)
darkColors(273-312)lightColors(232-271)core/design-system/src/prod/java/team/aliens/dms/android/core/designsystem/Colors.kt (2)
darkColors(240-279)lightColors(199-238)core/design-system/src/main/java/team/aliens/dms/android/core/designsystem/Toast.kt (2)
ToastLayout(151-167)rememberToastState(143-149)
β° 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). (2)
- GitHub Check: Agent
- GitHub Check: build
π Additional comments (16)
app/src/prod/kotlin/team/aliens/dms/android/app/MainActivityViewModel.kt (1)
4-26: LGTM! Clean StateFlow implementation for update failure tracking.The implementation follows Android best practices:
- Proper encapsulation with private mutable state and public read-only exposure
- Thread-safe state management using StateFlow
- Clear consume pattern for handling the failure state
The boolean flag approach is well-suited for this use case where the state needs to be set, observed, and then reset after handling.
core/notification/build.gradle.kts (1)
42-52: LGTM! Flavor configuration aligns with project-wide strategy.The addition of the "environment" flavor dimension with dev and prod flavors is consistent with the flavor setup across other modules in this PR.
core/design-system/build.gradle.kts (1)
46-56: LGTM! Design system module properly configured.The flavor configuration is consistent with the project-wide strategy, and the Material3 dependency addition supports the new design system implementation.
Also applies to: 75-75
core/ui/build.gradle.kts (1)
45-55: LGTM! Flavor configuration is consistent.The environment flavor dimension setup aligns with the project-wide flavor strategy.
core/widget/build.gradle.kts (1)
44-54: LGTM! Flavor configuration is consistent.The environment flavor dimension setup aligns with the project-wide flavor strategy.
core/design-system/src/prod/java/team/aliens/dms/android/core/designsystem/Theme.kt (1)
5-5: LGTM! Typography mapping is comprehensive.The introduction of MaterialTypography alias and the mapping of custom typography roles (headline1/2/3, title1/2/3, body1/2/3, button, caption, overline) to Material 3 typography slots is well-structured and provides proper integration between the custom design system and Material Theme.
Also applies to: 25-47
feature/build.gradle.kts (1)
47-57: LGTM! Feature module properly configured for new design system.The flavor configuration is consistent with the project-wide strategy, and the addition of material-icons-extended dependency supports the enhanced iconography requirements of the new design system.
Also applies to: 88-88
gradle/libs.versions.toml (1)
50-50: Material3 version 1.4.0 is current and appropriate.The specified version is the latest stable release as of December 2025 and aligns with the December 2025 Jetpack Compose BOM (2025.12.00). No compatibility concerns identified.
core/widget/src/prod/designsystem/DmsWidgetGlanceColorScheme.kt (1)
63-68: LGTM!The
DmsWidgetGlanceColorSchemeobject correctly exposesColorProvidersfor Glance widgets with both light and dark theme support.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Theme.kt (3)
14-72: LGTM!The light and dark color schemes are well-structured with comprehensive Material 3 color slot mappings, including secondary, tertiary, outline, and surfaceTint colors.
74-117: LGTM!The
DmsThemecomposable correctly:
- Selects color scheme based on system theme
- Provides custom colors, shapes, and typography via
CompositionLocalProvider- Maps custom typography to Material 3 typography slots
- Wraps content with
ToastLayoutfor toast support
119-135: LGTM!The
DmsThemeobject provides proper read-only composable accessors to the current theme values via composition locals.core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
194-225: LGTM!The
notoSansFontFamilyis well-defined with comprehensive font weight variants (Black, Bold, Light, Medium, Normal, Thin).core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.kt (3)
214-222: Duplicate color values in dark theme.
red200andred300share the same value (0xFFFE0F0F), andblue200andblue300share the same value (0xFF0F6EFE). Verify this is intentional.
11-76: LGTM!The
Colorsclass follows the standard Compose pattern for observable color schemes usingmutableStateOfwithstructuralEqualityPolicy()and internal setters.
232-314: LGTM!The
lightColors()anddarkColors()factory functions provide sensible defaults fromDmsColortokens, andLocalColorsis correctly initialized.
| override fun toString(): String = "Colors(" + | ||
| "primary=$primary, " + | ||
| "onPrimary=$onPrimary, " + | ||
| "primaryContainer=$primaryContainer, " + | ||
| "onPrimaryContainer=$onPrimaryContainer, " + | ||
| "error=$error, " + | ||
| "onError=$onError, " + | ||
| "errorContainer=$errorContainer, " + | ||
| "onErrorContainer=$onErrorContainer, " + | ||
| "background=$background, " + | ||
| "onBackground=$onBackground, " + | ||
| "backgroundVariant=$backgroundVariant, " + | ||
| "onBackgroundVariant=$onBackgroundVariant, " + | ||
| "surface=$surface, " + | ||
| "onSurface=$onSurface, " + | ||
| "surfaceVariant=$surfaceVariant, " + | ||
| "onSurfaceVariant=$onSurfaceVariant, " + | ||
| ")" | ||
| } |
There was a problem hiding this comment.
toString() is missing icon and line properties.
The toString() method omits the icon, line, and isLight properties that are part of the class.
"surfaceVariant=$surfaceVariant, " +
"onSurfaceVariant=$onSurfaceVariant, " +
+ "icon=$icon, " +
+ "line=$line, " +
+ "isLight=$isLight" +
")"π 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.
| override fun toString(): String = "Colors(" + | |
| "primary=$primary, " + | |
| "onPrimary=$onPrimary, " + | |
| "primaryContainer=$primaryContainer, " + | |
| "onPrimaryContainer=$onPrimaryContainer, " + | |
| "error=$error, " + | |
| "onError=$onError, " + | |
| "errorContainer=$errorContainer, " + | |
| "onErrorContainer=$onErrorContainer, " + | |
| "background=$background, " + | |
| "onBackground=$onBackground, " + | |
| "backgroundVariant=$backgroundVariant, " + | |
| "onBackgroundVariant=$onBackgroundVariant, " + | |
| "surface=$surface, " + | |
| "onSurface=$onSurface, " + | |
| "surfaceVariant=$surfaceVariant, " + | |
| "onSurfaceVariant=$onSurfaceVariant, " + | |
| ")" | |
| } | |
| override fun toString(): String = "Colors(" + | |
| "primary=$primary, " + | |
| "onPrimary=$onPrimary, " + | |
| "primaryContainer=$primaryContainer, " + | |
| "onPrimaryContainer=$onPrimaryContainer, " + | |
| "error=$error, " + | |
| "onError=$onError, " + | |
| "errorContainer=$errorContainer, " + | |
| "onErrorContainer=$onErrorContainer, " + | |
| "background=$background, " + | |
| "onBackground=$onBackground, " + | |
| "backgroundVariant=$backgroundVariant, " + | |
| "onBackgroundVariant=$onBackgroundVariant, " + | |
| "surface=$surface, " + | |
| "onSurface=$onSurface, " + | |
| "surfaceVariant=$surfaceVariant, " + | |
| "onSurfaceVariant=$onSurfaceVariant, " + | |
| "icon=$icon, " + | |
| "line=$line, " + | |
| "isLight=$isLight" + | |
| ")" | |
| } |
π€ Prompt for AI Agents
In
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Colors.kt
around lines 119 to 137, the toString() implementation omits the icon, line, and
isLight properties; update the returned string to include "icon=$icon, " and
"line=$line, " and "isLight=$isLight" (placed appropriately with the other
properties) so the string representation includes all class fields and maintains
the same comma/spacing style as the existing entries.
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt
Show resolved
Hide resolved
| sLabelB: TextStyle = TextStyle( | ||
| fontWeight = FontWeight.Bold, | ||
| fontSize = 10.sp, | ||
| lineHeight = 11.9.sp, | ||
| ), | ||
| sLabelM: TextStyle = TextStyle( | ||
| fontWeight = FontWeight.Medium, | ||
| fontSize = 10.sp, | ||
| lineHeight = 11.9.sp, | ||
| ), |
There was a problem hiding this comment.
Unused constructor parameters sLabelB and sLabelM.
The secondary constructor accepts sLabelB and sLabelM parameters but they are not mapped to any primary constructor properties. These styles are effectively discarded.
Either remove the unused parameters or add corresponding properties to the primary constructor if these styles are needed.
π€ Prompt for AI Agents
In
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt
around lines 88 to 97, the secondary constructor defines sLabelB and sLabelM
parameters that are never assigned to any primary-constructor properties and are
thus discarded; fix this by either removing those parameters from the secondary
constructor if the styles are not required, or (preferred if they are needed)
add corresponding properties to the primary constructor (with type TextStyle and
matching names) and ensure the secondary constructor passes its sLabelB and
sLabelM values to those primary properties so the styles are stored and used.
There was a problem hiding this comment.
Actionable comments posted: 0
β»οΈ Duplicate comments (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
88-97: Unused constructor parameters (duplicate issue).The
sLabelBandsLabelMparameters are accepted by the secondary constructor but are never passed to the primary constructor (lines 98-111), making them effectively unused. While lines 249-263 provide these styles as dev-only extension properties, accepting unused constructor parameters is misleading.
π§Ή Nitpick comments (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt (1)
249-263: Consider caching sLabelB and sLabelM for consistency and performance.Unlike the other typography styles that are stored in the
Typographyinstance,sLabelBandsLabelMare computed on every access, creating newTextStyleinstances each time. While this might be acceptable for infrequent access, it's inconsistent with the design pattern used for other styles and could cause unnecessary allocations if these properties are accessed frequently (e.g., during recomposition).Consider either:
- Adding
sLabelBandsLabelMas properties in the primary constructor (storing them like other styles), or- Document that these are intentionally computed fresh for dev-only debugging purposes
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
core/design-system/src/dev/java/team/aliens/dms/android/core/designsystem/Typography.kt(1 hunks)
β° 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
κ°μ
μμ μ¬ν
μΆκ° λ‘ ν λ§
Summary by CodeRabbit
New Features
UX / Reliability
Infrastructure
βοΈ Tip: You can customize this high-level summary in your review settings.