-
Notifications
You must be signed in to change notification settings - Fork 0
[design] #15 디자인 시스템 구현 #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough디자인 시스템에 테마(색상·타이포), CompositionLocal 기반 NekiTheme, 버튼/상단바/다이얼로그 컴포넌트, 클릭 디바운싱 Modifier, 재사용 가능한 Preview 애노테이션 및 20개 이상의 벡터 드로우블 자산을 추가합니다. 공용 데이터 클래스와 컴포저블 API들이 도입되었습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (2)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/button/TopBarTextButton.kt:
- Around line 18-25: The Text composable in TopBarTextButton uses
modifier.clickableSingle before modifier.padding, so the padded area isn't
clickable; swap the modifier order to apply padding (and optionally a min height
like heightIn(min = 48.dp)) before clickableSingle so the entire visual touch
area is interactive; update the modifier chain on the Text (or the enclosing
composable using buttonText and onClick) to place padding/heightIn before
clickableSingle to enlarge the touch target.
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/DoubleButtonAlertDialog.kt:
- Around line 54-58: The Icon call in DoubleButtonAlertDialog.kt currently sets
contentDescription = null which hides the icon from accessibility tools; if the
icon conveys meaning provide an accessible label by using a localized string
resource (e.g., contentDescription =
stringResource(R.string.dialog_alert_icon_description)) or, if it is purely
decorative, explicitly mark that intent (e.g., keep contentDescription = null
but add a comment and/or use Modifier.semantics { hidden() } for clarity).
Update the Icon invocation that uses ImageVector.vectorResource(...) accordingly
and ensure the string resource key (dialog_alert_icon_description) is added to
your resources when choosing the accessible label approach.
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/SingleButtonAlertDialog.kt:
- Around line 54-58: The Icon in SingleButtonAlertDialog (the Icon call using
ImageVector.vectorResource(R.drawable.icon_dialog_alert)) currently sets
contentDescription = null which hides it from screen readers; update this by
providing an appropriate localized string resource (e.g.,
R.string.alert_icon_description) and passing it to contentDescription when the
icon conveys meaning, or explicitly mark it as decorative via semantics if it is
purely decorative; ensure you update composable parameters or resources
accordingly so accessibility readers receive the correct description.
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/WarningDialog.kt:
- Around line 44-52: The close icon in WarningDialog.kt currently sets
contentDescription = null, which is inaccessible; update the Icon (the one using
ImageVector.vectorResource and clickableSingle(onClick = onDismissRequest)) to
provide a non-null, localized description (e.g., use
stringResource(R.string.close) or R.string.close_dialog) for contentDescription
and add the corresponding string resource; ensure the description communicates
that the control closes the dialog (e.g., "닫기" or "다이얼로그 닫기").
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt:
- Around line 67-72: The debouncing updates lastEventTimeMs regardless of
whether the event was actually processed, so a blocked click can advance the
timestamp and cause later valid clicks to be incorrectly blocked; modify
processEvent so that lastEventTimeMs is set only when the event is invoked (move
the assignment inside the if block after event.invoke()), keeping the existing
now calculation and using the same identifiers (processEvent, lastEventTimeMs,
now) so only successfully processed events update the timestamp.
- Around line 61-73: MultipleEventsCutterImpl's lastEventTimeMs is susceptible
to race conditions when accessed from multiple threads; make access atomic by
marking the field as @Volatile and synchronizing the read-check-write in
processEvent: annotate lastEventTimeMs with @Volatile and wrap the logic in
processEvent (the now check and update of lastEventTimeMs) inside a
synchronized(this) block so the read-verify-update is atomic; reference
MultipleEventsCutterImpl, lastEventTimeMs, and processEvent when applying the
change.
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/TitleTopBar.kt:
- Around line 34-90: Add accessible contentDescription strings to the leading
Icon instances in CloseTitleTextButtonTopBar and BackTitleTextButtonTopBar:
update the Icon call inside CloseTitleTextButtonTopBar (currently using
ImageVector.vectorResource(R.drawable.icon_close_24)) to provide a descriptive
contentDescription (e.g., "Close" or a localized string resource) and do the
same for the Icon in BackTitleTextButtonTopBar (using
R.drawable.icon_arrow_left) so both icons expose proper accessibility labels
instead of null.
- Around line 14-32: The Icon in CloseTitleTopBar currently sets
contentDescription = null which prevents screen readers from announcing the
close button; update CloseTitleTopBar (and the Icon passed to NekiTitleTopBar)
to provide a meaningful contentDescription (e.g., a localized string resource
like R.string.close or accept a new parameter closeContentDescription: String =
stringResource(R.string.close)) and pass that value into the Icon's
contentDescription instead of null so assistive technologies can describe the
button.
- Around line 92-139: The leading Icon composables in BackTitleTopBar and
BackTitleOptionTopBar have contentDescription set to null; update the Icon
inside the leadingIcon lambda of both NekiTitleTopBar to provide a meaningful
description (e.g., "뒤로 가기") and update the Icon in the actions lambda of
BackTitleOptionTopBar to provide a description (e.g., "더보기") so screen readers
can announce the buttons; modify the Icon calls referenced in BackTitleTopBar
and BackTitleOptionTopBar to pass the appropriate contentDescription strings
instead of null.
In @core/designsystem/src/main/res/drawable/icon_close_20.xml:
- Around line 1-5: The filename-to-size mismatch: files like icon_close_20.xml
declare android:width/height as 24dp (and icon_close_24.xml declares 20dp),
causing inconsistent naming; update the vector drawable files (e.g.,
icon_close_20.xml, icon_close_24.xml and other affected files such as
icon_heart_primary_24.xml, icon_heart_gray_stroke_24.xml) so the numeric suffix
matches the android:width and android:height attributes (or alternatively
standardize and document a naming rule), ensuring each file’s android:width and
android:height values reflect the filename suffix consistently.
In @core/designsystem/src/main/res/drawable/icon_close_24.xml:
- Around line 1-18: The file name ends with _24 but the vector is 20dp; either
rename the file to icon_close_20.xml or make the vector actually 24dp by
updating android:width, android:height, android:viewportWidth and
android:viewportHeight from 20 to 24 (and if you choose 24, scale or adjust the
pathData coordinates in the two <path> elements so the visual size remains
correct); locate the vector by the file name icon_close_24 and the attributes
android:width/android:height/android:viewportWidth/android:viewportHeight and
update accordingly.
In @core/designsystem/src/main/res/drawable/icon_heart_gray_stroke_24.xml:
- Around line 1-5: The file name icon_heart_gray_stroke_24.xml does not match
the vector size (android:width="20dp", android:height="20dp",
viewportWidth/Height="20"); either rename the file to
icon_heart_gray_stroke_20.xml to reflect the actual size or update the vector
attributes to 24dp and viewportWidth/Height to 24 (and then scale/adjust the
pathData and strokeWidth in the <vector> paths to suit a 24x24 viewport) so the
filename and actual icon dimensions remain consistent.
In @core/designsystem/src/main/res/drawable/icon_heart_primary_24.xml:
- Around line 1-5: The file name icon_heart_primary_24.xml and the vector's
android:width/android:height/android:viewportWidth/android:viewportHeight (all
set to 20) are inconsistent; either rename the file to icon_heart_primary_20.xml
to match the 20dp vector or update the vector attributes to 24
(width/height/viewportWidth/viewportHeight) and adjust the pathData to fit a
24x24 viewport; locate the vector element in icon_heart_primary_24.xml to apply
one of these two changes so filename and actual icon size match.
In @core/designsystem/src/main/res/drawable/icon_heart_white_24.xml:
- Around line 1-9: The drawable filename icon_heart_white_24 doesn't match the
vector size: the <vector> has android:width/height and
viewportWidth/viewportHeight = 20, causing an inconsistency; fix by either
renaming the file to icon_heart_white_20 or updating the <vector> attributes
(android:width, android:height, android:viewportWidth, android:viewportHeight)
to 24 (and verify pathData scales correctly), referencing the vector element in
icon_heart_white_24.xml to locate the attributes to change.
🧹 Nitpick comments (9)
core/designsystem/src/main/res/drawable/icon_download.xml (1)
7-8: 색상 리소스 참조 사용을 고려하세요.현재
fillColor가 하드코딩된 hex 값(#74788B)을 사용하고 있습니다. 디자인 시스템의 일관성과 테마 지원을 위해 색상 리소스를 참조하는 것이 좋습니다(예:@color/icon_default또는@color/gray_600).이렇게 하면 다크 모드 지원이나 전체 색상 팔레트 변경 시 유지보수가 용이해집니다.
♻️ 색상 리소스 참조로 변경하는 예시
<path android:pathData="M13.999,18.171C13.844,18.171 13.698,18.147 13.562,18.099C13.426,18.051 13.299,17.968 13.183,17.85L8.983,13.65C8.749,13.417 8.637,13.145 8.647,12.834C8.656,12.523 8.768,12.25 8.983,12.017C9.216,11.784 9.493,11.662 9.815,11.653C10.136,11.644 10.413,11.755 10.645,11.988L12.833,14.175V5.834C12.833,5.503 12.945,5.226 13.169,5.003C13.393,4.78 13.67,4.668 13.999,4.667C14.329,4.666 14.606,4.778 14.831,5.003C15.056,5.228 15.168,5.505 15.166,5.834V14.175L17.354,11.988C17.587,11.755 17.864,11.642 18.185,11.652C18.507,11.661 18.784,11.783 19.016,12.017C19.23,12.25 19.342,12.523 19.352,12.834C19.362,13.145 19.25,13.417 19.016,13.65L14.816,17.85C14.699,17.967 14.573,18.05 14.437,18.099C14.301,18.148 14.155,18.172 13.999,18.171ZM6.999,23.334C6.358,23.334 5.809,23.105 5.352,22.649C4.895,22.192 4.667,21.643 4.666,21V18.667C4.666,18.336 4.778,18.06 5.002,17.836C5.226,17.613 5.503,17.501 5.833,17.5C6.162,17.5 6.44,17.612 6.665,17.836C6.889,18.061 7.001,18.338 6.999,18.667V21H20.999V18.667C20.999,18.336 21.111,18.06 21.335,17.836C21.559,17.613 21.836,17.501 22.166,17.5C22.496,17.5 22.773,17.612 22.998,17.836C23.223,18.061 23.334,18.338 23.333,18.667V21C23.333,21.642 23.104,22.191 22.648,22.649C22.191,23.106 21.642,23.334 20.999,23.334H6.999Z" - android:fillColor="#74788B"/> + android:fillColor="@color/icon_default"/>core/designsystem/src/main/res/drawable/icon_check_primary.xml (1)
6-8: 런타임 틴팅을 활용한 더 유연한 디자인 시스템 구현을 고려하세요.현재
fillColor에 색상(#FF311F)이 하드코딩되어 있어 테마 변경 시 유연성이 제한됩니다. 디자인 시스템에서는 다음 접근 방식을 권장합니다:
- 아이콘을 단색(예:
#000000)으로 정의하고 Compose의Icon컴포저블에서tint파라미터로 색상 적용- 이를 통해 동적 테마, 다크 모드, 색상 변형을 코드 레벨에서 쉽게 처리 가능
파일명에
_primary접미사를 사용한 것으로 보아 의도적으로 색상별 변형을 만드신 것 같지만, 런타임 틴팅 방식이 유지보수성과 확장성 측면에서 더 유리합니다.♻️ 제안하는 리팩토링
아이콘을 단색으로 변경:
<path android:pathData="M9.55,15.15L18.025,6.675C18.225,6.475 18.458,6.375 18.725,6.375C18.992,6.375 19.225,6.475 19.425,6.675C19.625,6.875 19.725,7.113 19.725,7.388C19.725,7.663 19.625,7.901 19.425,8.1L10.25,17.3C10.05,17.5 9.817,17.6 9.55,17.6C9.284,17.6 9.05,17.5 8.85,17.3L4.55,13C4.35,12.8 4.254,12.563 4.262,12.288C4.27,12.013 4.375,11.776 4.575,11.575C4.776,11.374 5.014,11.274 5.288,11.275C5.563,11.276 5.8,11.376 6,11.575L9.55,15.15Z" - android:fillColor="#FF311F"/> + android:fillColor="#FF000000"/>Compose에서 사용 시:
Icon( painter = painterResource(id = R.drawable.icon_check), contentDescription = "Check", tint = NekiTheme.colors.primary // 또는 다른 테마 색상 )core/designsystem/src/main/res/drawable/icon_info_gray_stroke.xml (1)
1-9: 벡터 드로어블 리소스가 올바르게 정의되었습니다.정보 아이콘의 XML 구조가 정확하며, 28x28 크기와 그레이 색상(#74788B)이 적용되었습니다. 참고로 파일명에 "stroke"가 포함되어 있지만 실제로는
fillColor를 사용하고 있습니다. 이것이 의도된 네이밍이라면 문제없으나, 향후 혼동을 방지하기 위해 네이밍 규칙을 확인하는 것을 권장합니다.core/designsystem/src/main/res/drawable/icon_heart_primary_24.xml (1)
7-8: 테마 색상 참조 사용 권장색상 값
#FF5647이 하드코딩되어 있습니다. 디자인 시스템에서 NekiColorScheme을 도입한 만큼, 색상 리소스나 테마 속성을 참조하여 테마 변경 시 일관성을 유지할 수 있도록 하는 것이 좋습니다.
res/values/colors.xml에 색상을 정의하고@color/primary형태로 참조하거나, 테마 속성을 활용하는 것을 고려하세요.♻️ 권장 리팩토링
1단계: colors.xml에 색상 정의
<!-- res/values/colors.xml --> <color name="primary">#FF5647</color>2단계: 색상 참조로 변경
<path android:pathData="..." - android:fillColor="#FF5647"/> + android:fillColor="@color/primary"/>core/designsystem/src/main/res/drawable/icon_arrow_left.xml (1)
7-9: 테마 색상 참조 사용 권장fillColor에
#3C3E48이 하드코딩되어 있습니다. AI summary에 따르면 디자인 시스템에서 gray800과 같은 색상이 NekiColorScheme에 정의되어 있으므로, 색상 리소스를 참조하여 다크 모드 등 테마 변경에 대응할 수 있도록 개선하는 것이 좋습니다.색상 리소스 또는 테마 속성을 활용하여 일관된 색상 관리를 권장합니다.
♻️ 권장 리팩토링
<path android:pathData="..." - android:fillColor="#3C3E48" + android:fillColor="@color/gray800" android:fillType="evenOdd"/>core/designsystem/src/main/res/drawable/icon_heart_primary_28.xml (1)
7-8: 테마 색상 참조 사용 권장fillColor에
#FF5647이 하드코딩되어 있습니다.icon_heart_primary_24.xml과 동일한 색상 값을 사용하고 있으므로, 색상 리소스를 정의하여 여러 아이콘에서 일관되게 참조하도록 개선하는 것이 좋습니다.디자인 시스템의 NekiColorScheme에 정의된 primary 색상을 활용하여 테마 일관성을 유지하세요.
♻️ 권장 리팩토링
<path android:pathData="..." - android:fillColor="#FF5647"/> + android:fillColor="@color/primary"/>core/designsystem/src/main/res/drawable/icon_heart_gray_stroke_24.xml (1)
8-11: 테마 색상 참조 사용 권장strokeColor에
#A0A3B0이 하드코딩되어 있습니다. 이는 디자인 시스템의 gray 계열 색상으로 보이며, NekiColorScheme에 정의된 색상을 참조하도록 변경하여 테마 일관성과 유지보수성을 개선할 수 있습니다.색상 리소스를 정의하고 참조하는 방식으로 개선하세요.
♻️ 권장 리팩토링
<path android:pathData="..." android:strokeLineJoin="round" android:strokeWidth="1.42857" android:fillColor="#00000000" - android:strokeColor="#A0A3B0" + android:strokeColor="@color/gray400" android:strokeLineCap="round"/>core/designsystem/src/main/res/drawable/icon_heart_gray_stroke_28.xml (1)
8-11: 테마 색상 참조 사용 권장strokeColor에
#A0A3B0이 하드코딩되어 있습니다.icon_heart_gray_stroke_24.xml과 동일한 색상을 사용하고 있으므로, 색상 리소스를 정의하여 여러 아이콘에서 일관되게 참조하도록 개선하는 것이 좋습니다.NekiColorScheme에 정의된 색상을 활용하여 테마 일관성을 유지하세요.
♻️ 권장 리팩토링
<path android:pathData="..." android:strokeLineJoin="round" android:strokeWidth="2" android:fillColor="#00000000" - android:strokeColor="#A0A3B0" + android:strokeColor="@color/gray400" android:strokeLineCap="round"/>core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/NekiTopBar.kt (1)
44-51: 람다 파라미터명 섀도잉 주의
NekiTitleTopBar의modifier파라미터와 람다 내부의modifier파라미터가 동일한 이름을 사용하고 있습니다. 현재 동작에는 문제가 없지만, 가독성과 유지보수성을 위해 람다 파라미터명을 다르게 지정하는 것이 좋습니다.♻️ 제안하는 수정
NekiTopBar( modifier = modifier, leadingIcon = leadingIcon, actions = actions, - title = { modifier -> + title = { alignModifier -> Text( - modifier = modifier, + modifier = alignModifier, text = title, style = NekiTheme.typography.title18SemiBold, color = NekiTheme.colorScheme.gray900, ) }, )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
core/designsystem/src/main/res/font/bowlbyone_regular.ttfis excluded by!**/*.ttfcore/designsystem/src/main/res/font/pretendard_bold.ttfis excluded by!**/*.ttfcore/designsystem/src/main/res/font/pretendard_medium.ttfis excluded by!**/*.ttfcore/designsystem/src/main/res/font/pretendard_regular.ttfis excluded by!**/*.ttfcore/designsystem/src/main/res/font/pretendard_semibold.ttfis excluded by!**/*.ttf
📒 Files selected for processing (34)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ComponentPreview.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/DevicePreview.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/button/CTAButton.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/button/NekiButton.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/button/TopBarTextButton.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/DoubleButtonAlertDialog.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/SingleButtonAlertDialog.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/WarningDialog.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/NekiTopBar.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/TitleTopBar.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Color.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.ktcore/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Type.ktcore/designsystem/src/main/res/drawable/icon_align.xmlcore/designsystem/src/main/res/drawable/icon_arrow_down.xmlcore/designsystem/src/main/res/drawable/icon_arrow_left.xmlcore/designsystem/src/main/res/drawable/icon_arrow_right.xmlcore/designsystem/src/main/res/drawable/icon_bell.xmlcore/designsystem/src/main/res/drawable/icon_check_primary.xmlcore/designsystem/src/main/res/drawable/icon_close_20.xmlcore/designsystem/src/main/res/drawable/icon_close_24.xmlcore/designsystem/src/main/res/drawable/icon_dialog_alert.xmlcore/designsystem/src/main/res/drawable/icon_download.xmlcore/designsystem/src/main/res/drawable/icon_heart_gray_stroke_24.xmlcore/designsystem/src/main/res/drawable/icon_heart_gray_stroke_28.xmlcore/designsystem/src/main/res/drawable/icon_heart_primary_24.xmlcore/designsystem/src/main/res/drawable/icon_heart_primary_28.xmlcore/designsystem/src/main/res/drawable/icon_heart_white_24.xmlcore/designsystem/src/main/res/drawable/icon_heart_white_28.xmlcore/designsystem/src/main/res/drawable/icon_info_gray_stroke.xmlcore/designsystem/src/main/res/drawable/icon_info_primary_fill.xmlcore/designsystem/src/main/res/drawable/icon_kebab.xmlcore/designsystem/src/main/res/drawable/icon_person.xml
🧰 Additional context used
🧬 Code graph analysis (6)
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/SingleButtonAlertDialog.kt (2)
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/CTAButton.kt (1)
CTAButtonPrimary(44-61)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/TopBarTextButton.kt (1)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/CTAButton.kt (2)
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/NekiButton.kt (1)
NekiButton(15-43)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/WarningDialog.kt (1)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/DoubleButtonAlertDialog.kt (2)
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/CTAButton.kt (2)
CTAButtonGray(63-80)CTAButtonPrimary(44-61)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/TitleTopBar.kt (3)
core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/NekiTopBar.kt (1)
NekiTitleTopBar(33-53)core/designsystem/src/main/java/com/neki/android/core/designsystem/button/TopBarTextButton.kt (1)
TopBarTextButton(12-26)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)
⏰ 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: ci-build
🔇 Additional comments (34)
core/designsystem/src/main/res/drawable/icon_download.xml (1)
1-9: 벡터 드로어블 구조가 올바르게 정의되었습니다.XML 구조, 네임스페이스, 치수 및 뷰포트 설정이 모두 적절합니다.
core/designsystem/src/main/res/drawable/icon_check_primary.xml (1)
1-5: 벡터 드로어블 선언이 올바릅니다.표준 Android 벡터 드로어블 형식을 따르고 있으며, 크기와 뷰포트가 적절하게 정의되어 있습니다.
core/designsystem/src/main/res/drawable/icon_info_primary_fill.xml (1)
1-9: 벡터 드로어블 리소스가 올바르게 구현되었습니다.XML 벡터 드로어블에서 하드코딩된 색상값을 사용하는 것이 표준 Android 패턴이며, 코드베이스의 모든 아이콘 파일(icon_heart_primary_24.xml, icon_heart_primary_28.xml 등)이 동일한 방식을 따릅니다. 색상값 #FF5647은 디자인 시스템의 Color.kt에 정의된 primary400과 일치하므로 추가 변경이 필요 없습니다.
core/designsystem/src/main/res/drawable/icon_heart_white_28.xml (1)
1-9: LGTM! 벡터 드로어블 리소스가 올바르게 정의되었습니다.하트 아이콘의 XML 구조와 속성이 정확하며, 28x28 크기와 흰색 색상이 일관되게 적용되었습니다.
core/designsystem/src/main/res/drawable/icon_person.xml (1)
1-9: LGTM! 벡터 드로어블 리소스가 올바르게 정의되었습니다.사용자 아이콘의 XML 구조가 정확하며, 28x28 크기와 그레이 색상(#8A8E9E)이 일관되게 적용되었습니다.
core/designsystem/src/main/res/drawable/icon_align.xml (1)
1-13: LGTM! 벡터 드로어블 리소스가 올바르게 정의되었습니다.정렬 아이콘의 XML 구조가 정확하며, 20x20 크기와 스트로크 기반 디자인이 적절하게 구현되었습니다. round lineCap과 lineJoin 설정으로 부드러운 선 처리가 잘 적용되었습니다.
core/designsystem/src/main/res/drawable/icon_bell.xml (1)
1-9: LGTM! 벡터 드로어블 리소스가 올바르게 정의되었습니다.벨 아이콘의 XML 구조가 정확하며, 28x28 크기와 그레이 색상(#8A8E9E)이 일관되게 적용되었습니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/DevicePreview.kt (1)
6-13: 디바이스 프리뷰 어노테이션이 잘 구현되었습니다.ComponentPreview와 함께 사용할 수 있는 디바이스 레벨 프리뷰 어노테이션이 적절하게 추가되었습니다. 설정된 디바이스 스펙(375dp x 812dp)은 일반적인 모바일 디바이스 크기를 잘 나타냅니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/ComponentPreview.kt (1)
6-11: 컴포넌트 프리뷰 어노테이션이 잘 구현되었습니다.재사용 가능한 프리뷰 어노테이션으로 디자인 시스템의 일관된 프리뷰 환경을 제공합니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt (2)
15-22: 리플 효과 제거 Modifier가 올바르게 구현되었습니다.
composed와remember를 사용하여 적절하게 구현되었습니다.
27-50: 중복 클릭 방지 Modifier가 잘 구현되었습니다.
debugInspectorInfo를 사용하여 디버깅 정보를 제공하고, 리플 효과를 유지하면서 중복 클릭을 방지하는 좋은 구현입니다.core/designsystem/src/main/res/drawable/icon_kebab.xml (1)
1-9: LGTM!Kebab 아이콘이 올바르게 정의되었습니다. 24dp 크기와 viewport가 일치하며, TopBar 컴포넌트에서 사용될 준비가 되었습니다.
core/designsystem/src/main/res/drawable/icon_arrow_down.xml (1)
1-14: LGTM!아래 방향 화살표 아이콘이 올바르게 구현되었습니다. 크기와 viewport가 일치하며 디자인 시스템의 다른 화살표 아이콘들과 일관된 색상(#74788B)을 사용합니다.
core/designsystem/src/main/res/drawable/icon_dialog_alert.xml (1)
1-10: LGTM!다이얼로그 알림 아이콘이 적절한 48dp 크기로 정의되었습니다. Alert 다이얼로그 컴포넌트에서 사용하기에 적합합니다.
core/designsystem/src/main/res/drawable/icon_arrow_right.xml (1)
1-13: LGTM!오른쪽 화살표 아이콘이 잘 구현되었습니다. 다른 화살표 아이콘들과 일관된 색상(#74788B)을 사용하며, stroke 기반 접근 방식이 chevron 스타일에 적합합니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/NekiTopBar.kt (1)
14-31: LGTM!
NekiTopBar컴포저블이 깔끔하게 구현되었습니다. Box 기반의 슬롯 패턴을 사용하여leadingIcon,title,actions를 유연하게 배치할 수 있으며, 각 슬롯에 적절한 alignment modifier가 적용되었습니다.core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/WarningDialog.kt (1)
77-86: LGTM!프리뷰 함수가 적절하게 구현되어 있으며, ComponentPreview 어노테이션을 사용하여 IDE에서 미리보기를 지원합니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/SingleButtonAlertDialog.kt (1)
89-101: LGTM!프리뷰 함수가 적절하게 구현되어 있습니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/DoubleButtonAlertDialog.kt (2)
36-39: DialogProperties 기본값 일관성 확인
SingleButtonAlertDialog는DialogProperties의 기본값으로dismissOnBackPress=false, dismissOnClickOutside=false를 사용하지만, 이 다이얼로그는DialogProperties()를 사용하여 기본값(true, true)을 적용합니다. 이러한 차이가 의도된 것인지 확인이 필요합니다.일반적으로 단일 버튼 다이얼로그는 사용자의 명시적 액션을 요구하는 경우가 많고, 이중 버튼 다이얼로그는 선택지를 제공하므로 이러한 차이가 의도적일 수 있습니다. 하지만 팀 내에서 이에 대한 명확한 가이드라인이 있는지 확인하는 것이 좋습니다.
98-112: LGTM!프리뷰 함수가 적절하게 구현되어 있습니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/CTAButton.kt (4)
15-42: LGTM!
NekiButton을 래핑하여 CTA 버튼의 일관된 스타일(둥근 모서리, 패딩, 타이포그래피)을 제공하고, 디바운싱 기능을 상속받습니다. 깔끔한 구조입니다.
44-61: LGTM!Primary CTA 버튼이 적절한 색상 스키마와 disabled 상태 처리를 포함하고 있습니다.
63-80: Gray 버튼의 disabled 상태 시각적 구분 확인
CTAButtonGray는 enabled와 disabled 상태에서 동일한 색상을 사용합니다. 이것이 디자인 의도라면 문제없지만, 사용자가 버튼의 활성/비활성 상태를 시각적으로 구분하기 어려울 수 있습니다.
CTAButtonPrimary는 disabled 상태에서 alpha를 적용하여 시각적 구분을 제공하는데, 일관성을 위해 Gray 버튼도 유사한 처리가 필요한지 디자인팀과 확인하는 것을 권장합니다.
82-123: LGTM!프리뷰 함수들이 적절하게 구현되어 있으며, Primary 버튼의 enabled/disabled 상태를 모두 확인할 수 있습니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/NekiButton.kt (1)
28-28:remember를 제거해서는 안 됩니다 - 현재 구현이 올바릅니다.
MultipleEventsCutter.get()은 호출할 때마다 새로운MultipleEventsCutterImpl()인스턴스를 반환합니다. 이 클래스는 중복 클릭 방지를 위해lastEventTimeMs상태를 유지하며, 500ms 간격의 디바운싱 로직을 구현하고 있습니다.remember를 사용하지 않으면 리컴포지션 시마다 새로운 인스턴스가 생성되어 상태가 초기화되므로 디바운싱 기능이 동작하지 않습니다. 현재 코드는 Compose 상태 관리 패턴을 올바르게 따르고 있습니다.Likely an incorrect or invalid review comment.
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (3)
9-20: LGTM!CompositionLocalProvider를 사용하여 typography와 colors를 제공하는 패턴이 적절하게 구현되었습니다. Material3 테마 구조와 일관된 접근 방식입니다.
22-37: LGTM!NekiTheme composable이 적절하게 구현되었습니다. MaterialTheme을 래핑하면서 커스텀 색상 스킴을 제공하는 구조가 좋습니다.
39-49: LGTM!
@ReadOnlyComposable어노테이션을 사용하여LocalTypography와LocalColorScheme에 접근하는 방식이 적절합니다. 이 패턴은 Material3의MaterialTheme객체와 동일한 방식으로, Compose 컴파일러가 최적화할 수 있게 해줍니다.core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Color.kt (2)
7-38: LGTM!
@Immutable어노테이션이 적절하게 적용되었고, 색상 스킴 구조가 잘 정의되었습니다.향후 확장을 위해 error, warning, success 같은 시맨틱 컬러 추가가 필요할 수 있습니다. 현재 디자인 시스템 요구사항에 이러한 상태 색상이 포함되어 있는지 확인해 주세요.
40-72: LGTM!
staticCompositionLocalOf를 사용한LocalColorScheme정의가 적절합니다. 색상 스킴은 런타임에 자주 변경되지 않으므로 이 선택이 올바릅니다.core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Type.kt (3)
15-31: LGTM!
pretendardFamily와pretendardStyle정의가 적절합니다.includeFontPadding = false와LineHeightStyle.Alignment.Center설정은 일관된 텍스트 렌더링을 위한 좋은 선택입니다.
41-46:title24SemiBold의 letterSpacing 값 확인 필요
title24SemiBold의letterSpacing이0.sp로 설정되어 있는 반면, 다른 모든 스타일은(-0.02).em을 사용하고 있습니다. 이것이 의도된 디자인인지 확인해 주세요.title24SemiBold = pretendardStyle.copy( fontWeight = FontWeight.SemiBold, fontSize = 24.sp, lineHeight = 36.sp, - letterSpacing = 0.sp, + letterSpacing = (-0.02).em, ),
163-194: LGTM!
NekiTypography데이터 클래스에@Immutable어노테이션이 적절하게 적용되었고,staticCompositionLocalOf를 사용한LocalTypography정의도 올바릅니다. 타이포그래피 네이밍 컨벤션(title/body/caption + size + weight)이 일관되게 적용되어 있습니다.core/designsystem/src/main/java/com/neki/android/core/designsystem/topbar/TitleTopBar.kt (1)
141-201: LGTM!각 TopBar 변형에 대한 Preview가 잘 구성되어 있으며,
NekiTheme으로 래핑하여 테마 적용을 확인할 수 있습니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/button/TopBarTextButton.kt
Show resolved
Hide resolved
...ignsystem/src/main/java/com/neki/android/core/designsystem/dialog/DoubleButtonAlertDialog.kt
Show resolved
Hide resolved
...ignsystem/src/main/java/com/neki/android/core/designsystem/dialog/SingleButtonAlertDialog.kt
Show resolved
Hide resolved
core/designsystem/src/main/java/com/neki/android/core/designsystem/dialog/WarningDialog.kt
Show resolved
Hide resolved
core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt:
- Around line 33-39: The inspector name in the debug metadata is wrong: update
the debugInspectorInfo block used by the clickableSingle modifier so the name
property matches the function name (change name = "clickable" to name =
"clickableSingle") and keep the existing properties map (properties["enabled"],
properties["onClickLabel"], properties["role"], properties["onClick"]) unchanged
so inspector output correctly identifies the modifier.
- Around line 61-73: MultipleEventsCutterImpl's debouncing uses a
non-thread-safe var lastEventTimeMs and a non-atomic check-then-act in
processEvent, which can race across threads; make it thread-safe by replacing
lastEventTimeMs with a java.util.concurrent.atomic.AtomicLong (or otherwise
serializing access) and update processEvent to perform an atomic compare-and-set
or get-and-update so only one thread can advance the timestamp and invoke event
(reference MultipleEventsCutterImpl, lastEventTimeMs, and processEvent).
🧹 Nitpick comments (4)
core/designsystem/src/main/res/drawable/icon_close_24.xml (1)
1-18: 디자인 시스템의 일관성을 위해 색상 리소스 참조를 사용하세요.현재
strokeColor가 하드코딩된 색상 값(#3C3E48)을 사용하고 있습니다. 디자인 시스템 구현에서는 색상 리소스나 테마 속성을 참조하는 것이 권장됩니다. 이를 통해:
- 디자인 시스템 전체의 색상 일관성 유지
- 다크 모드 등 테마 변경 지원
- 색상 변경 시 중앙 집중식 관리 가능
♻️ 권장하는 수정 방안
res/values/colors.xml에 색상을 정의한 후:<color name="icon_default">#3C3E48</color>아이콘에서 참조하도록 수정:
<path android:pathData="M20,4L4,20" android:strokeWidth="2" android:fillColor="#00000000" - android:strokeColor="#3C3E48" + android:strokeColor="@color/icon_default" android:strokeLineCap="round"/> <path android:pathData="M20,20L4,4" android:strokeWidth="2" android:fillColor="#00000000" - android:strokeColor="#3C3E48" + android:strokeColor="@color/icon_default" android:strokeLineCap="round"/>또는 테마 속성을 사용하는 방법도 고려해보세요:
android:strokeColor="?attr/colorOnSurface"core/designsystem/src/main/res/drawable/icon_close_20.xml (1)
6-17: 디자인 시스템 일관성을 위해 색상 리소스 참조를 권장합니다.두 경로 모두
#3C3E48색상을 하드코딩하고 있습니다. 디자인 시스템의 일관성과 유지보수성을 위해colors.xml에 정의된 색상 리소스나 테마 속성을 참조하는 것이 좋습니다.♻️ 색상 리소스 참조 제안
colors.xml에 색상을 정의한 후:<color name="icon_stroke_default">#3C3E48</color>해당 색상을 참조하도록 수정:
<path android:pathData="M16.666,3.333L3.333,16.666" android:strokeWidth="1.66667" android:fillColor="#00000000" - android:strokeColor="#3C3E48" + android:strokeColor="@color/icon_stroke_default" android:strokeLineCap="round"/> <path android:pathData="M16.666,16.666L3.333,3.333" android:strokeWidth="1.66667" android:fillColor="#00000000" - android:strokeColor="#3C3E48" + android:strokeColor="@color/icon_stroke_default" android:strokeLineCap="round"/>core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt (1)
27-50: 디바운스 시간을 설정 가능하게 만드는 것을 고려해보세요.현재 디바운스 윈도우가 500ms로 하드코딩되어 있습니다. 컴포넌트별로 다른 디바운스 시간이 필요한 경우 파라미터로 설정 가능하게 하면 유연성이 향상됩니다. 다만 디자인 시스템의 일관성을 위해 기본값을 유지하는 현재 방식도 타당합니다.
♻️ 설정 가능한 디바운스 윈도우 구현 예시
fun Modifier.clickableSingle( enabled: Boolean = true, onClickLabel: String? = null, role: Role? = null, + debounceMillis: Long = 500L, onClick: () -> Unit, ) = composed( inspectorInfo = debugInspectorInfo { name = "clickableSingle" properties["enabled"] = enabled properties["onClickLabel"] = onClickLabel properties["role"] = role properties["onClick"] = onClick + properties["debounceMillis"] = debounceMillis }, ) { - val multipleEventsCutter = remember { MultipleEventsCutter.get() } + val multipleEventsCutter = remember(debounceMillis) { MultipleEventsCutter.get(debounceMillis) } Modifier.clickable( enabled = enabled, onClickLabel = onClickLabel, onClick = { multipleEventsCutter.processEvent { onClick() } }, role = role, indication = ripple(), interactionSource = remember { MutableInteractionSource() }, ) }그리고
MultipleEventsCutter와 구현체를 업데이트하여 윈도우 시간을 받도록 수정:internal fun MultipleEventsCutter.Companion.get(debounceMillis: Long = 500L): MultipleEventsCutter = MultipleEventsCutterImpl(debounceMillis) private class MultipleEventsCutterImpl( private val debounceMillis: Long ) : MultipleEventsCutter { // ... override fun processEvent(event: () -> Unit) { val currentTime = now val lastTime = lastEventTimeMs.get() if (currentTime - lastTime >= debounceMillis) { if (lastEventTimeMs.compareAndSet(lastTime, currentTime)) { event.invoke() } } } }core/designsystem/src/main/res/drawable/icon_heart_primary_20.xml (1)
6-9: XML 드로어블의 하드코딩된 색상 값은 의도적인 디자인 패턴입니다.색상 값 #FF5647는 NekiColorScheme의 primary400과 일치하며, 이는 icon_heart_primary_20.xml 외에도 icon_info_primary_fill.xml 등 여러 파일에서 일관되게 사용되고 있습니다. 색상별 별도 드로어블 파일을 생성하는 현재 접근 방식은 모든 아이콘에서 통일되게 적용되고 있으므로 설계 시스템 가이드라인을 따르고 있습니다.
다만 NekiColorScheme은 Compose 환경에서 정의되고, XML 드로어블은 하드코딩된 색상을 사용하고 있다는 점을 고려하면, 런타임 테마 변경 시 XML 드로어블의 색상은 자동으로 업데이트되지 않습니다. 향후 동적 테마 지원이 필요한 경우 색상 리소스를 별도로 정의하고 참조하는 방식으로 개선할 수 있습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.ktcore/designsystem/src/main/res/drawable/icon_close_20.xmlcore/designsystem/src/main/res/drawable/icon_close_24.xmlcore/designsystem/src/main/res/drawable/icon_heart_gray_stroke_20.xmlcore/designsystem/src/main/res/drawable/icon_heart_primary_20.xmlcore/designsystem/src/main/res/drawable/icon_heart_white_20.xml
✅ Files skipped from review due to trivial changes (2)
- core/designsystem/src/main/res/drawable/icon_heart_gray_stroke_20.xml
- core/designsystem/src/main/res/drawable/icon_heart_white_20.xml
⏰ 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: ci-build
🔇 Additional comments (3)
core/designsystem/src/main/res/drawable/icon_close_20.xml (1)
1-5: 벡터 드로우블 설정이 올바릅니다.20dp 크기의 아이콘이 올바르게 정의되었으며, 뷰포트 크기와 물리적 크기가 일치합니다.
core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt (1)
15-22: LGTM! 리플 효과 제거 구현이 적절합니다.
composed와remember를 사용하여 상태를 올바르게 관리하고 있으며,indication = null설정으로 리플 효과를 제거하는 표준 패턴을 따르고 있습니다.core/designsystem/src/main/res/drawable/icon_heart_primary_20.xml (1)
1-5: 벡터 드로어블 선언이 올바릅니다.width/height와 viewport 크기가 일치하고, 파일명의 크기(_20)와도 정확히 매칭됩니다.
| inspectorInfo = debugInspectorInfo { | ||
| name = "clickable" | ||
| properties["enabled"] = enabled | ||
| properties["onClickLabel"] = onClickLabel | ||
| properties["role"] = role | ||
| properties["onClick"] = onClick | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인스펙터 이름이 모디파이어 함수명과 불일치합니다.
debugInspectorInfo의 name 속성이 "clickable"로 설정되어 있지만, 실제 모디파이어 함수명은 clickableSingle입니다. 디버깅 시 혼란을 줄 수 있습니다.
🔧 제안하는 수정
inspectorInfo = debugInspectorInfo {
- name = "clickable"
+ name = "clickableSingle"
properties["enabled"] = enabled
properties["onClickLabel"] = onClickLabel
properties["role"] = role
properties["onClick"] = onClick
},📝 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.
| inspectorInfo = debugInspectorInfo { | |
| name = "clickable" | |
| properties["enabled"] = enabled | |
| properties["onClickLabel"] = onClickLabel | |
| properties["role"] = role | |
| properties["onClick"] = onClick | |
| }, | |
| inspectorInfo = debugInspectorInfo { | |
| name = "clickableSingle" | |
| properties["enabled"] = enabled | |
| properties["onClickLabel"] = onClickLabel | |
| properties["role"] = role | |
| properties["onClick"] = onClick | |
| }, |
🤖 Prompt for AI Agents
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt
around lines 33 - 39, The inspector name in the debug metadata is wrong: update
the debugInspectorInfo block used by the clickableSingle modifier so the name
property matches the function name (change name = "clickable" to name =
"clickableSingle") and keep the existing properties map (properties["enabled"],
properties["onClickLabel"], properties["role"], properties["onClick"]) unchanged
so inspector output correctly identifies the modifier.
| private class MultipleEventsCutterImpl : MultipleEventsCutter { | ||
| private val now: Long | ||
| get() = System.currentTimeMillis() | ||
|
|
||
| private var lastEventTimeMs: Long = 0 | ||
|
|
||
| override fun processEvent(event: () -> Unit) { | ||
| if (now - lastEventTimeMs >= 500L) { | ||
| lastEventTimeMs = now | ||
| event.invoke() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동시성 문제: 디바운싱이 스레드 안전하지 않습니다.
lastEventTimeMs가 일반 var로 선언되어 있어 여러 스레드에서 동시에 접근할 경우 경쟁 조건(race condition)이 발생할 수 있습니다. 68-71번 줄의 check-then-act 패턴이 원자적이지 않아 두 스레드가 모두 시간 검사를 통과하여 중복 클릭을 허용할 수 있습니다.
🔒 AtomicLong을 사용한 스레드 안전 구현
+import java.util.concurrent.atomic.AtomicLong
+
private class MultipleEventsCutterImpl : MultipleEventsCutter {
private val now: Long
get() = System.currentTimeMillis()
- private var lastEventTimeMs: Long = 0
+ private val lastEventTimeMs = AtomicLong(0)
override fun processEvent(event: () -> Unit) {
- if (now - lastEventTimeMs >= 500L) {
- lastEventTimeMs = now
+ val currentTime = now
+ val lastTime = lastEventTimeMs.get()
+ if (currentTime - lastTime >= 500L) {
+ if (lastEventTimeMs.compareAndSet(lastTime, currentTime)) {
- event.invoke()
+ event.invoke()
+ }
}
}
}🤖 Prompt for AI Agents
In
@core/designsystem/src/main/java/com/neki/android/core/designsystem/extension/Modifier.kt
around lines 61 - 73, MultipleEventsCutterImpl's debouncing uses a
non-thread-safe var lastEventTimeMs and a non-atomic check-then-act in
processEvent, which can race across threads; make it thread-safe by replacing
lastEventTimeMs with a java.util.concurrent.atomic.AtomicLong (or otherwise
serializing access) and update processEvent to perform an atomic compare-and-set
or get-and-update so only one thread can advance the timestamp and invoke event
(reference MultipleEventsCutterImpl, lastEventTimeMs, and processEvent).
Ojongseok
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 !
| showBackground = true, | ||
| backgroundColor = 0xFFFFFFFF, | ||
| uiMode = UI_MODE_NIGHT_NO, | ||
| device = "spec:width=375dp,height=812dp,dpi=411", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
디바이스의 해상도(dpi)를 411로 설정하신 이유가 있을까요?
xxhdpi(480), xxxhdpi(640)이 가장 일반적으로..? 플래그십..? 모델의 해상도로 알고 있는데 411로 설정하신 이유가 궁금합니다
| ) { | ||
| Column( | ||
| modifier = Modifier | ||
| .width(320.dp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다이어로그의 width를 320.dp로 고정한 것이 디자인 시스템 상 335.dp를 반영한 것이 맞을까요?
디바이스 해상도에 따라 큰 차이가 있을 수 있어 .modifier.fillMaxWidth() 후 좌우 패딩으로 정의하는 것이 어떨까요!?
(SingleButtonAlertDialog, WarningDialog도 동일합니다)
| Box( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .height(54.dp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로 TopBar도 height를 고정하지 않고 vertical방향 패딩으로 조절하는게 좋을 것 같습니다, 텍스트의 사이즈가 시스템 글자크기 설정에 따라 달라질 수 있어 극단적인 상황에서 깨질 것 같습니다!
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선
✏️ Tip: You can customize this high-level summary in your review settings.