PrezelButton 높이 고정 문제 해결을 위한 Surface → Box 구조 변경#78
Merged
Conversation
`PrezelButton`의 아이콘 크기 결정 로직을 `PrezelButtonAppearance`로 캡슐화하여 컴포넌트 구조를 개선했습니다. * `PrezelButtonAppearance` 데이터 클래스에 `iconSize` 프로퍼티를 추가했습니다. * `PrezelButton` 컴포저블 내부에서 `buttonSize`를 직접 참조하여 아이콘 크기를 계산하던 방식에서, 생성된 `appearance.iconSize`를 사용하도록 변경했습니다. * `prezelButtonIconSize` 함수의 가시성을 `private`으로 변경하고 위치를 조정했습니다. * `PrezelButtonIcon` 컴포저블의 불필요한 `buttonSize` 파라미터를 제거했습니다.
Contributor
Walkthrough이 변경은 PrezelButton을 Surface 기반에서 Box 기반으로 전환해 LocalMinimumInteractiveComponentSize 영향으로 인한 최소 높이 제약을 제거했고, 스타일 속성을 중앙화한 내부 데이터 클래스 Possibly related PRs
Suggested labels
🚥 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. 📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButton.kt`:
- Around line 42-50: The clickable is applied after applyButtonAppearance which
reduces the visual Box size (especially icon-only XSMALL/SMALL in
PrezelButtonStyle) and thus shrinks the hit target; change the modifier order or
add an explicit minimum touch target so the clickable hit area remains at least
48.dp by default. Specifically, in PrezelButton.kt ensure
Modifier.clickable(...) is applied before any size-restricting modifiers from
applyButtonAppearance or add a min touch target (e.g., sizeIn/minHeight 48.dp)
around the composable unless an explicit opt-out is provided in
PrezelButtonStyle; keep references to modifier, applyButtonAppearance,
clickable, and PrezelButtonStyle when making the change.
- Around line 41-43: The Box in PrezelButton.kt that starts with "Box(modifier =
modifier.applyButtonAppearance(appearance)" lacks a contentAlignment so when
callers use Modifier.fillMaxWidth() or fixed height the Row children stick to
the top-left; update that Box to set contentAlignment = Alignment.Center (or
Alignment.CenterStart/CenterEnd as desired) so its children (the inner Row and
its text/icon) are centered inside the Box while keeping the Row's internal
arrangement unchanged.
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonPreview.kt`:
- Around line 109-117: The ghost icon preview is using a wrong shape because
appearance is always computed with isIconOnly = false; update
PrezelButtonPreview to compute PrezelButtonAppearance via
PrezelButtonAppearance.of(...) using the actual content context (set isIconOnly
= true for icon-only previews) before passing it into content and the Modifier
chain (especially where PrezelButtonType.GHOST and Modifier.drawDashBorder(...)
are applied), so each preview item derives its appearance from the real content
rather than a common false value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7d36d01f-bac4-4942-918b-0ae291c96609
📒 Files selected for processing (7)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButton.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonPreview.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelIconButton.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelTextButton.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/util/DrawDashBorder.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/util/DrowDashBorder.kt
💤 Files with no reviewable changes (1)
- Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/util/DrowDashBorder.kt
...esignsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButton.kt
Show resolved
Hide resolved
...esignsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButton.kt
Show resolved
Hide resolved
...stem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonPreview.kt
Outdated
Show resolved
Hide resolved
`PrezelButton` 컴포넌트의 최상위 `Box` 컨테이너에 `contentAlignment = Alignment.Center` 속성을 추가하여 내부 콘텐츠가 중앙에 정렬되도록 수정하였습니다.
* `PrezelButtonAppearance` 데이터 클래스의 생성자에서 `private` 키워드를 제거하여 내부(internal) 패키지 내에서의 가시성을 확보했습니다.
`PrezelButton` 컴포넌트의 텍스트 색상 결정 로직에서 불필요한 테마 체크와 의존성을 제거하였습니다. * `FILLED` 타입 버튼의 텍스트 색상을 결정할 때 `isSystemInDarkTheme()` 체크 및 `PrezelColorScheme.Dark` 직접 참조를 제거하고, 현재 테마의 `colors.textLarge`를 사용하도록 수정했습니다. * 사용되지 않는 `isSystemInDarkTheme` 및 `PrezelColorScheme` 임포트 문을 정리했습니다.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt (2)
66-66: 데이터 클래스 구조 분해는 필드 순서에 의존적입니다.
PrezelButtonStyle의 필드 순서가 변경되면 구조 분해가 조용히 깨질 수 있습니다. 명시적 접근 방식이 더 안전할 수 있습니다.♻️ 제안된 수정
- val (buttonType, buttonHierarchy, buttonSize, isRounded) = style - - return PrezelButtonAppearance( - textStyle = prezelButtonTextStyle(size = buttonSize), + return PrezelButtonAppearance( + textStyle = prezelButtonTextStyle(size = style.buttonSize), contentColor = prezelButtonContentColor( - type = buttonType, - hierarchy = buttonHierarchy, + type = style.buttonType, + hierarchy = style.buttonHierarchy, enabled = enabled, ), contentPadding = prezelButtonContentPadding( - size = buttonSize, + size = style.buttonSize, isIconOnly = isIconOnly, ), - iconSpacing = prezelButtonIconSpacing(size = buttonSize), + iconSpacing = prezelButtonIconSpacing(size = style.buttonSize), shape = prezelButtonShape( isIconOnly = isIconOnly, - isRounded = isRounded, - buttonSize = buttonSize, + isRounded = style.isRounded, + buttonSize = style.buttonSize, ), containerColor = prezelButtonContainerColor( - type = buttonType, - hierarchy = buttonHierarchy, + type = style.buttonType, + hierarchy = style.buttonHierarchy, enabled = enabled, ), borderStroke = prezelButtonBorderStroke( - type = buttonType, - hierarchy = buttonHierarchy, + type = style.buttonType, + hierarchy = style.buttonHierarchy, enabled = enabled, ), - iconSize = prezelButtonIconSize(size = buttonSize), + iconSize = prezelButtonIconSize(size = style.buttonSize), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt` at line 66, The destructuring val (buttonType, buttonHierarchy, buttonSize, isRounded) = style is brittle because it depends on PrezelButtonStyle's field order; replace the destructuring with explicit property access (e.g., use style.buttonType, style.buttonHierarchy, style.buttonSize, style.isRounded) wherever those variables are needed so changes to PrezelButtonStyle's declaration won’t silently break behavior.
239-245: 아이콘 크기를 디자인 토큰으로 관리하는 것을 고려해 주세요.다른 치수들은
PrezelSpacing,PrezelShapes등의 디자인 토큰을 사용하지만, 아이콘 크기는 하드코딩되어 있습니다. 일관성을 위해 디자인 토큰으로 관리하는 것을 고려해 볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt` around lines 239 - 245, The icon sizes in prezelButtonIconSize currently use hardcoded dp values; extract these into a design token (e.g., add a PrezelIconSizes object or extend an existing token file like PrezelSpacing/PrezelShapes) and replace the hardcoded 14.dp, 16.dp, 20.dp with token properties (e.g., PrezelIconSizes.xsmall, .small, .regular) so prezelButtonIconSize(PrezelButtonSize) returns the token values; update the token definitions and any imports/usage in PrezelButtonStyle.kt to reference the new tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt`:
- Line 66: The destructuring val (buttonType, buttonHierarchy, buttonSize,
isRounded) = style is brittle because it depends on PrezelButtonStyle's field
order; replace the destructuring with explicit property access (e.g., use
style.buttonType, style.buttonHierarchy, style.buttonSize, style.isRounded)
wherever those variables are needed so changes to PrezelButtonStyle's
declaration won’t silently break behavior.
- Around line 239-245: The icon sizes in prezelButtonIconSize currently use
hardcoded dp values; extract these into a design token (e.g., add a
PrezelIconSizes object or extend an existing token file like
PrezelSpacing/PrezelShapes) and replace the hardcoded 14.dp, 16.dp, 20.dp with
token properties (e.g., PrezelIconSizes.xsmall, .small, .regular) so
prezelButtonIconSize(PrezelButtonSize) returns the token values; update the
token definitions and any imports/usage in PrezelButtonStyle.kt to reference the
new tokens.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17f3b7ca-b1d0-42cd-a2fa-246734540454
📒 Files selected for processing (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt`:
- Around line 194-195: The FILLED branch in the color-selection logic
incorrectly hardcodes PrezelColorScheme.Dark.textLarge causing dark-theme text
to be used even when a light theme is active; update the PrezelButtonStyle.kt
branch handling PrezelButtonType.FILLED to use the provided colors parameter
(use colors.solidWhite) instead of PrezelColorScheme.Dark.textLarge so the white
text contrasts with the interactiveRegular background in both themes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f6451fb-4a9f-4523-94fc-eedfdce8551d
📒 Files selected for processing (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt
Comment on lines
194
to
+195
| return when (type) { | ||
| PrezelButtonType.FILLED -> if (isSystemInDarkTheme()) colors.textLarge else PrezelColorScheme.Dark.textLarge | ||
| PrezelButtonType.FILLED -> PrezelColorScheme.Dark.textLarge |
Contributor
There was a problem hiding this comment.
테마 무시 버그: PrezelColorScheme.Dark.textLarge 하드코딩
colors 파라미터를 받으면서도 FILLED 타입에서 PrezelColorScheme.Dark.textLarge를 직접 참조하고 있어, 라이트 테마에서도 다크 테마 색상이 사용됩니다.
FILLED 버튼의 interactiveRegular(파란색) 배경에 대비되는 흰색 텍스트가 필요하다면, 두 테마 모두에서 일관된 값을 갖는 colors.solidWhite를 사용해야 합니다.
🐛 테마 일관성을 위한 수정 제안
return when (type) {
- PrezelButtonType.FILLED -> PrezelColorScheme.Dark.textLarge
+ PrezelButtonType.FILLED -> colors.solidWhite
PrezelButtonType.OUTLINED -> colors.interactiveRegular
PrezelButtonType.GHOST -> colors.interactiveRegular
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/button/PrezelButtonStyle.kt`
around lines 194 - 195, The FILLED branch in the color-selection logic
incorrectly hardcodes PrezelColorScheme.Dark.textLarge causing dark-theme text
to be used even when a light theme is active; update the PrezelButtonStyle.kt
branch handling PrezelButtonType.FILLED to use the provided colors parameter
(use colors.solidWhite) instead of PrezelColorScheme.Dark.textLarge so the white
text contrasts with the interactiveRegular background in both themes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 내용
PrezelButton렌더링 구조를Surface에서Box + clickable기반으로 변경하여 외형/동작 로직을 분리했습니다.PrezelButtonAppearance를 도입해 텍스트 스타일, 색상, 패딩, shape, border, icon size 계산을 한 곳에서 관리하도록 정리했습니다.DrawDashBorder.kt로 정리하고 기존 오타 파일(DrowDashBorder.kt)을 제거했습니다../gradlew :core-designsystem:compileDebugKotlin빌드 성공🧩 관련 이슈
Surface → Box구조 변경 #77📸 스크린샷
이전에 말씀드린 것처럼, 배경과 경계가 시각적으로 구분되지 않는 컴포넌트의 경우 프리뷰에서 식별이 어렵기 때문에 DashBorder를 적용했습니다. 프리뷰 환경에서만 컴포넌트의 경계를 명확하게 확인하기 위한 용도입니다.
📢 논의하고 싶은 내용
Summary by CodeRabbit
릴리스 노트
Refactor
Style
Chores