[setting/#7] design system #8
Conversation
WalkthroughComplete redesign of the design system architecture from Material Design 3 to a custom HsLink theme. Replaced legacy color palette with new SkyBlue, DeepBlue, Grey, and Red color families. Introduced strongly-typed HsLinkColor and HsLinkTypography data classes backed by CompositionLocals. Updated theme infrastructure to provide colors and typography through composition locals instead of static schemes. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant HsLinkTheme as HsLinkTheme<br/>(Object)
participant ProvideCompose as ProvideHsLinkColorsAndTypography
participant CompLocal as CompositionLocal<br/>(Colors & Typog)
participant Screen as HomeScreen
App->>HsLinkTheme: HsLinkTheme { content }
HsLinkTheme->>ProvideCompose: ProvideHsLinkColorsAndTypography<br/>(defaultHsLinkColor,<br/>defaultHsLinkTypography)
ProvideCompose->>CompLocal: CompositionLocalProvider<br/>(LocalHsLinkColors,<br/>LocalHsLinkTypography)
CompLocal->>Screen: Render with theme context
Screen->>HsLinkTheme: HsLinkTheme.colors<br/>HsLinkTheme.typography
HsLinkTheme-->>Screen: Returns current<br/>LocalHsLinkColors.current<br/>LocalHsLinkTypography.current
Screen->>Screen: Apply SkyBlue100 color<br/>+ body_16Normal style
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes involve significant architectural restructuring across three foundational theme files with multiple interdependent data classes and composition locals. The migration from Material Design 3 to a custom theme system requires careful review of the new typing contracts, color mappings, and theme provision logic. While changes are cohesive in purpose, the density of new public APIs and their interconnections across files demands moderate-to-high scrutiny. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Color.kt (2)
111-111: Use a crashing default for CompositionLocal to catch missing providers early.-val LocalHsLinkColors = staticCompositionLocalOf { defaultHsLinkColor } +val LocalHsLinkColors = staticCompositionLocalOf<HsLinkColor> { + error("No HsLinkColors provided") +}
45-76: Token and API ergonomics: consider semantic aliases and property casing.
- Add semantic roles (e.g., primary, onPrimary, background, onBackground, surface, error) mapped to scale tokens to decouple UI from raw scales and ease dark-mode.
- Prefer lowerCamelCase for data class properties to align with Kotlin style.
app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt (1)
15-17: Lift HsLinkTheme higher (e.g., App/NavHost) to avoid per-screen re-wrapping and status-bar flicker.app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
53-55: Bridge your tokens into Material3 ColorScheme/Typography for wider theming.Pass a Material3 ColorScheme and Typography to
MaterialTheme(...)so stock components adopt your DS. Suggest adding a small mapper (e.g.,toMaterialColorScheme(colors)) and supplying it here.app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Type.kt (2)
128-128: Use a crashing default for LocalHsLinkTypography to surface missing providers.-val LocalHsLinkTypography = staticCompositionLocalOf { defaultHsLinkTypography } +val LocalHsLinkTypography = staticCompositionLocalOf<HsLinkTypography> { + error("No HsLinkTypography provided") +}
31-34: Clarify button text style names.Rename
btm_L/M/Sto clearer API likebuttonLabelLarge/Medium/Smallfor readability and IDE discoverability.Also applies to: 105-125
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
app/src/main/res/font/pretendard_medium.otfis excluded by!**/*.otfapp/src/main/res/font/pretendard_regular.otfis excluded by!**/*.otfapp/src/main/res/font/pretendard_semibold.otfis excluded by!**/*.otf
📒 Files selected for processing (4)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Color.kt(1 hunks)app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt(1 hunks)app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Type.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
⏰ 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-and-test
| val Pink40 = Color(0xFF7D5260) No newline at end of file | ||
| val Common = Color(0xFFFFFFFF) | ||
|
|
||
| val Transparent = Color(0xFF2D2D2D) |
There was a problem hiding this comment.
"Transparent" is opaque (#2D2D2D); make it truly transparent.
Use 0x00000000 (or Color.Transparent) to avoid unexpected opaque backgrounds.
-val Transparent = Color(0xFF2D2D2D)
+val Transparent = Color(0x00000000) // or: Color.Transparent📝 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 Transparent = Color(0xFF2D2D2D) | |
| val Transparent = Color(0x00000000) // or: Color.Transparent |
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Color.kt around
line 10, the val Transparent is defined as an opaque color (0xFF2D2D2D) instead
of being transparent; replace that value with a true transparent color (e.g.,
0x00000000) or use Color.Transparent so the variable name matches its behavior
and avoids unexpected opaque backgrounds.
| val view = LocalView.current | ||
| if (!view.isInEditMode) { | ||
| SideEffect { | ||
| (view.context as Activity).window.run { | ||
| WindowCompat.getInsetsController(this, view).isAppearanceLightStatusBars = false | ||
| } | ||
| } |
There was a problem hiding this comment.
Status bar icon contrast is inverted for a light UI.
Setting isAppearanceLightStatusBars = false forces light icons; on a light background they become unreadable. Default to dark icons on light backgrounds, or compute dynamically.
- WindowCompat.getInsetsController(this, view).isAppearanceLightStatusBars = false
+ // For light background (e.g., white), use dark icons:
+ WindowCompat.getInsetsController(this, view).isAppearanceLightStatusBars = true🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt around
lines 45–51, the code unconditionally sets isAppearanceLightStatusBars = false
which forces light status bar icons (poor contrast on light backgrounds); change
it to compute the correct value from the current theme (e.g., use
isSystemInDarkTheme() or MaterialTheme.colors.isLight) and set
isAppearanceLightStatusBars to true when the UI is light (so dark icons are
used), otherwise set it to false; implement this conditional assignment inside
the existing SideEffect block and keep casting/view access unchanged.
| val HsLinkSemiBoldFont = FontFamily(Font(R.font.pretendard_semibold)) | ||
| val HsLinkRegularFont = FontFamily(Font(R.font.pretendard_regular)) | ||
| val HsLinkMediumFont = FontFamily(Font(R.font.pretendard_medium)) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify font resources exist (pretendard_*).
Ensure res/font/pretendard_{semibold,regular,medium}.* are present to avoid build errors.
🏁 Script executed:
#!/bin/bash
# List expected font files and confirm references.
fd -a -t f 'res/font/pretendard_*' || true
rg -nP 'R\.font\.pretendard_(semibold|regular|medium)' -g '!**/build/**' -SLength of output: 416
Missing font resource files will cause build failures.
The code references R.font.pretendard_semibold, R.font.pretendard_regular, and R.font.pretendard_medium at lines 13-15, but these font resource files do not exist in res/font/. Add the three font files:
res/font/pretendard_semibold.ttf(or .otf)res/font/pretendard_regular.ttf(or .otf)res/font/pretendard_medium.ttf(or .otf)
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Type.kt around
lines 13 to 15 the code references R.font.pretendard_semibold,
R.font.pretendard_regular, and R.font.pretendard_medium which are missing and
will break the build; add the corresponding font resource files into res/font
(e.g., pretendard_semibold.ttf or .otf, pretendard_regular.ttf or .otf,
pretendard_medium.ttf or .otf), ensure their filenames match the R identifiers
(lowercase, underscores), and sync/clean the project so the resources are
recognized.
| style = HsLinkTheme.typography.body_16Normal, | ||
| color = HsLinkTheme.colors.SkyBlue100, | ||
| modifier = modifier |
There was a problem hiding this comment.
Text color likely fails contrast; avoid very light SkyBlue100 for body text.
Use a darker token (e.g., DeepBlue700 or Grey700) or a semantic onBackground.
- style = HsLinkTheme.typography.body_16Normal,
- color = HsLinkTheme.colors.SkyBlue100,
+ style = HsLinkTheme.typography.body_16Normal,
+ color = HsLinkTheme.colors.DeepBlue700,📝 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.
| style = HsLinkTheme.typography.body_16Normal, | |
| color = HsLinkTheme.colors.SkyBlue100, | |
| modifier = modifier | |
| style = HsLinkTheme.typography.body_16Normal, | |
| color = HsLinkTheme.colors.DeepBlue700, | |
| modifier = modifier |
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt around
lines 27 to 29, the Text uses a very light color token
(HsLinkTheme.colors.SkyBlue100) which is inappropriate for body text; replace it
with a darker token such as HsLinkTheme.colors.DeepBlue700,
HsLinkTheme.colors.Grey700, or the semantic HsLinkTheme.colors.onBackground to
ensure readable contrast, and update any previews or tests to reflect the new
color.
ISSUE
❗ WORK DESCRIPTION
📸 SCREENSHOT
📢 TO REVIEWERS
Summary by CodeRabbit
New Features
Bug Fixes