-
Notifications
You must be signed in to change notification settings - Fork 1
PrezelBadge 구현 #38
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
PrezelBadge 구현 #38
Conversation
PrezelAvatar의 크기를 디자인 가이드에 맞춰 상향 조정하고, 기본 아이콘의 색상을 변경했습니다. * Avatar 크기 조정: REGULAR (48dp -> 64dp), SMALL (24dp -> 32dp) * 기본 아이콘(ic_person)의 tint 색상을 `borderMedium`에서 `iconDisabled`로 변경했습니다.
AndroidManifest.xml에 `android.permission.INTERNET` 권한을 추가했습니다.
디자인 시스템에 배지(Badge) 컴포넌트를 새롭게 추가했습니다. * `PrezelBadge` 컴포넌트 및 관련 Preview 구현 * 배지 크기 설정을 위한 `PrezelBadgeSize` (SMALL, REGULAR) 추가 * 배지 상태 제어를 위한 `PrezelBadgeActive`, `PrezelBadgeDisabled` 추가
`PrezelBadge` 컴포넌트 내 `padding` 메서드 호출 시 불필요한 줄바꿈을 제거하여 체이닝 스타일을 수정했습니다.
WalkthroughAndroidManifest.xml에 Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🤖 Fix all issues with AI agents
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/badge/PrezelBadge.kt`:
- Around line 49-67: The REGULAR branch uses .size(dimension) then .padding(...)
which constrains the content area; change it to allow the badge to size to its
text by replacing the fixed .size(dimension) with a minimum height constraint
(use defaultMinSize(minHeight = dimension) or defaultMinSize(minHeight = 20.dp))
and apply .background(...) then .padding(horizontal = 6.dp, vertical = 2.dp) so
the modifier on the PrezelBadgeSize.REGULAR case (look for
PrezelBadgeSize.REGULAR, modifier, dimension, PrezelTheme.shapes.V1000, and the
Text block) uses defaultMinSize + background + padding to avoid clipping and let
the badge expand with its text.
🧹 Nitpick comments (2)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/badge/PrezelBadgeSize.kt (1)
8-16:PrezelBadgeActive와PrezelBadgeDisabledenum 대신 Boolean 사용을 고려하세요.이진 상태(ON/OFF)에 대해 enum을 사용하는 것은 불필요한 복잡성을 추가합니다. Compose의 표준 패턴은
enabled: Boolean = true와 같은 Boolean 파라미터를 사용하는 것입니다.현재 API:
PrezelBadge(active = PrezelBadgeActive.ON, disabled = PrezelBadgeDisabled.OFF)권장 API:
PrezelBadge(isActive = true, enabled = true)Boolean을 사용하면 호출부가 더 간결해지고, Compose 컴포넌트의 관례(
Button,TextField등에서enabled: Boolean사용)와 일치합니다.♻️ 제안된 리팩토링
-enum class PrezelBadgeActive { - ON, - OFF, -} - -enum class PrezelBadgeDisabled { - ON, - OFF, -}그리고
PrezelBadge.kt에서:`@Composable` fun PrezelBadge( modifier: Modifier = Modifier, text: String? = null, size: PrezelBadgeSize = PrezelBadgeSize.REGULAR, isActive: Boolean = false, enabled: Boolean = true, )Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/badge/PrezelBadge.kt (1)
82-84: Preview 함수를private으로 변경하세요.다른 컴포넌트의 Preview 함수들(
PrezelAvatarSizePreview,PrezelAvatarTypePreview)과 일관성을 위해private접근 제한자를 추가하세요.♻️ 제안된 수정
`@ThemePreview` `@Composable` -fun PrezelBadgePreview() { +private fun PrezelBadgePreview() {
.../designsystem/src/main/java/com/team/prezel/core/designsystem/component/badge/PrezelBadge.kt
Outdated
Show resolved
Hide resolved
PrezelBadge의 크기별 구현을 별도의 private composable 함수로 분리하여 코드 가독성을 개선했습니다. * `PrezelBadgeSize.SMALL` 대응을 위한 `PrezelDotBadge` 분리 * `PrezelBadgeSize.REGULAR` 대응을 위한 `PrezelNumberBadge` 분리
`PrezelBadge` 컴포넌트의 위치를 이동하고, 불필요한 Enum 타입을 Boolean으로 대체하여 구조를 단순화했습니다. * `PrezelBadge.kt` 위치 변경: `component/badge/` -> `component/` * `PrezelBadgeActive`, `PrezelBadgeDisabled` Enum 삭제 및 `Boolean` 타입으로 변경 * `PrezelBadgeSize` Enum을 `PrezelBadge.kt` 파일 내부로 이동 및 `PrezelBadgeSize.kt` 삭제 * 패키지 경로 변경에 따른 import 문 수정 및 프리뷰 코드 업데이트
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
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt`:
- Around line 80-87: The Box modifier currently applies size before padding
(modifier.size(badgeSize).background(...).padding(...)), which makes padding
consume space inside the fixed badgeSize; change this so padding is outside the
fixed size or make the badge size content-aware: either move .padding(...)
before .size(badgeSize) to treat padding as external margin, or replace
.size(badgeSize) with a content-flexible constraint (e.g.,
defaultMinSize/minWidth/minHeight using badgeSize as minimum) and keep padding
inside; update the modifier chain in PrezelBadge (the Box modifier) accordingly
so text layout matches the intended behavior.
- Around line 110-171: The PrezelBadgePreview function exceeds Detekt's 40-line
limit; refactor it by either splitting the single PrezelBadgePreview into
multiple smaller `@Composable` previews (e.g., PrezelBadgePreviewSmallStates,
PrezelBadgePreviewRegularStates) each rendering a subset of the Rows, or
simplify the layout by removing per-Row padding and using
Column(verticalArrangement = Arrangement.spacedBy(8.dp)) inside PrezelTheme to
reduce lines; update or remove redundant Row wrappers as needed to bring each
preview under the 40-line threshold while keeping the same badge examples and
using the existing PrezelBadge, PrezelBadgeSize and PrezelTheme symbols.
🧹 Nitpick comments (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt (1)
33-48: 고정 크기로 인한 텍스트 오버플로우 가능성 검토 필요REGULAR 배지의 크기가 20.dp로 고정되어 있어, 두 자릿수 이상의 텍스트(예: "99", "100")가 표시될 경우 텍스트가 잘리거나 레이아웃이 깨질 수 있습니다.
PR 논의 항목에서 언급된 "최댓값" 관련 결정에 따라, 다음 중 하나를 고려해 주세요:
- 텍스트 길이에 따라 동적으로 너비가 조절되도록
widthIn(min = 20.dp)사용- 최댓값 초과 시 "9+" 또는 "99+" 형식으로 표시하는 로직 추가
♻️ 동적 너비 적용 예시
val badgeSize = when (size) { PrezelBadgeSize.SMALL -> 8.dp - PrezelBadgeSize.REGULAR -> 20.dp + PrezelBadgeSize.REGULAR -> 20.dp // height만 사용, width는 동적으로 처리 }
PrezelNumberBadge에서Modifier.size(badgeSize)대신Modifier.height(badgeSize).widthIn(min = badgeSize)를 사용하는 방안을 검토해 주세요.
...l/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt
Outdated
Show resolved
Hide resolved
...l/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt
Show resolved
Hide resolved
`PrezelBadge`의 프리뷰 코드를 가독성 있게 개선하고, 사이즈별로 프리뷰를 분리했습니다. * `PrezelBadgePreview`를 `PrezelSmallBadgePreview`와 `PrezelRegularBadgePreview`로 분리 * 공통 레이아웃을 위한 `BadgeRowPreview` 컴포넌트 추가 * 프리뷰 내 상태별(Active, Disabled) 비교가 용이하도록 레이아웃 수정
`PrezelBadge` 컴포넌트의 가독성과 유지보수성을 위해 내부 함수명을 변경하고, 하드코딩된 패딩 값을 테마 시스템 기반의 `PaddingValues`로 분리하였습니다. * `badgeColors`를 `prezelBadgeColors`로 이름 변경 * `prezelBadgeContentPadding` 함수를 추가하여 테마의 spacing 시스템(`V4`, `V2`) 적용 * 컴포넌트의 패딩 설정 방식을 `PaddingValues` 사용으로 변경
`PrezelBadge`의 활성화 및 기본 상태에 따른 배경색과 텍스트 색상 값을 수정했습니다. * `disabled` 상태의 배경색을 `bgDisabled`에서 `bgLarge`로 변경 * `active` 및 기본 상태의 텍스트 색상을 `textLarge`에서 `PrezelColorScheme.Light.bgRegular`로 변경
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: 1
🤖 Fix all issues with AI agents
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt`:
- Around line 103-110: The active and default branches of prezelBadgeColors
currently hard-code PrezelColorScheme.Light.bgRegular for the second color,
which ignores dark theme; replace those uses of
PrezelColorScheme.Light.bgRegular with the theme-aware value from
PrezelTheme.colors (e.g., PrezelTheme.colors.bgRegular or the appropriate
PrezelTheme.colors.text* value if that second element is intended as text color)
so both active and else branches use PrezelTheme.colors instead of
PrezelColorScheme.Light.
♻️ Duplicate comments (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt (1)
83-90: 고정 size 내부에 padding 적용으로 콘텐츠 영역 축소 가능
size()뒤에padding()을 적용하면 내부 텍스트 영역이 줄어듭니다. 의도한 동작인지 확인해 주세요.
...l/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt
Outdated
Show resolved
Hide resolved
* `PrezelBadge`의 파라미터를 `text` 대신 `count`를 받도록 변경하고 필수로 수정했습니다. * `PrezelBadgeSize` enum의 위치를 파일 상단으로 이동했습니다. * `PrezelDotBadge`에 고정 크기(8.dp)를 적용하고 `badgeSize` 파라미터를 제거했습니다. * `PrezelNumberBadge`에 `defaultMinSize(20.dp)`를 적용하여 가변 텍스트에 대응하도록 수정했습니다. * 배지의 상태별 배경색 및 텍스트 색상 로직을 조정했습니다. * 배지 컴포넌트 변화에 따른 프리뷰 코드를 업데이트했습니다.
- `PrezelDotBadge` 및 `PrezelNumberBadge`의 파라미터 순서를 변경하여 `modifier`를 뒤로 이동했습니다. - `PrezelNumberBadge`의 패딩 설정을 `prezelBadgeContentPadding` 함수 대신 직접 지정하도록 수정했습니다. - 불필요해진 `prezelBadgeContentPadding` 함수를 제거했습니다.
`BadgeRowPreview` 컴포넌트의 파라미터 타입을 `List<@composable () -> Unit>`에서 `RowScope.() -> Unit`으로 변경하여 가독성을 높였습니다. * `BadgeRowPreview`에서 `RowScope`를 사용하도록 수정 * 리스트 순회 방식에서 선언적 방식으로 프리뷰 구성 변경 * `Arrangement.spacedBy`를 사용하여 아이템 간 간격 설정 적용
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: 1
🤖 Fix all issues with AI agents
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt`:
- Around line 75-82: 현재 Box의 modifier에서 defaultMinSize(20.dp)가 minWidth만 20.dp로
설정하고 있어 최소 높이가 지정되지 않습니다; PrezelBadge의 높이도 보장하려면 modifier 체인에서 defaultMinSize를
명시적 매개변수로 호출하여 minWidth와 minHeight를 모두 20.dp로 설정하거나 defaultMinSize(minWidth =
20.dp, minHeight = 20.dp)처럼 수정하고, 변경은 PrezelBadge.kt의 Box 블록과 해당 modifier 호출을
업데이트하세요.
♻️ Duplicate comments (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt (1)
92-101: 하드코딩된PrezelColorScheme.Dark.textLarge사용에 대한 의도 확인 필요이전 리뷰에서
PrezelColorScheme.Light하드코딩이 지적되었고, 현재Dark로 변경되었지만 여전히 테마 시스템(PrezelTheme.colors)을 우회하고 있습니다.배지 배경이 항상 어두운 색상(
interactiveRegular,solidBlack)이라 밝은 텍스트가 필요한 것으로 보이지만,textOnPrimary나textInverse같은 시맨틱 컬러 토큰이 있다면 그것을 사용하는 것이 테마 일관성에 더 좋습니다.의도한 디자인이라면 주석으로 이유를 남겨주시면 유지보수에 도움이 됩니다.
🧹 Nitpick comments (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt (1)
28-51:count파라미터가SMALL사이즈에서 불필요하게 요구됨
PrezelBadgeSize.SMALL인 경우count값이 사용되지 않지만, 현재 API에서는 항상 전달해야 합니다. PR 설명에 언급된 "Size enum을 제거하고 텍스트 존재 여부로 Small/Regular 결정" 논의와도 관련이 있습니다.🔧 대안 1: count를 nullable로 변경
`@Composable` fun PrezelBadge( active: Boolean, - count: Int, + count: Int? = null, modifier: Modifier = Modifier, size: PrezelBadgeSize = PrezelBadgeSize.REGULAR, disabled: Boolean = false, ) {🔧 대안 2: size 제거 후 count 유무로 결정
`@Composable` fun PrezelBadge( active: Boolean, modifier: Modifier = Modifier, count: Int? = null, disabled: Boolean = false, ) { // count가 null이면 SMALL(dot), 있으면 REGULAR(number) }
...l/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelBadge.kt
Show resolved
Hide resolved
`PrezelBadge` 컴포넌트의 `defaultMinSize` 설정을 가로와 세로 모두 20.dp가 적용되도록 명시적으로 수정했습니다.
📌 작업 내용
PrezelBadge 컴포저블 구현
Badge 상태 처리
Preview 6종 추가
🧩 관련 이슈
📸 스크린샷
📢 논의하고 싶은 내용
Summary by CodeRabbit
새로운 기능
개선 사항
권한 변경
✏️ Tip: You can customize this high-level summary in your review settings.