[Refactor/#55] Button tertiary varient 추가 - #56
Merged
Merged
Conversation
📝 WalkthroughWalkthroughButton 컴포넌트에 "tertiary" variant를 추가하고, 텍스트 스타일(heading1, heading3, heading4, body1)을 업데이트했습니다. Button의 children 렌더링 방식도 개선했습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
🧪 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 |
jjjsun
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/button/Button.tsx (1)
5-52:⚠️ Potential issue | 🟡 Minortertiary에서 size prop이 사실상 무시됩니다.
Line 31-52에서
variantClasses.tertiary가!h-7,px-5,rounded-component-lg,font-body2를 포함해sizeClasses[size]를 덮어써요. 의도된 고정 크기라면 size 옵션을 제한/명시하고, 의도 아니라면 size/variant 클래스를 분리하는 편이 안전합니다.🛠️ 제안: size 클래스 분리로 충돌 제거
- const variantClasses = { + const variantClasses = { primary: "bg-chart-3 text-white hover:opacity-80 disabled:bg-bg-disabled disabled:text-text-disabled disabled:hover:opacity-50", @@ - tertiary: - "!h-7 border border-gray-200 text-text-auth-sub px-5 rounded-component-lg bg-white font-body2 hover:bg-gray-100 transition-colors duration-200 ease-in-out", + tertiary: + "border border-gray-200 text-text-auth-sub bg-white hover:bg-gray-100 transition-colors duration-200 ease-in-out", custom: "", }; + + const resolvedSizeClasses = + variant === "tertiary" + ? "h-7 px-5 rounded-component-lg font-body2" + : sizeClasses[size]; @@ className={twMerge( "flex items-center justify-center gap-2 active-scale whitespace-nowrap", - sizeClasses[size], + resolvedSizeClasses, variantClasses[variant], fullWidth && "w-full relative", (disabled || isLoading) && "opacity-50 cursor-not-allowed", className, )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/button/Button.tsx` around lines 5 - 52, The tertiary variant currently includes size-related classes that override sizeClasses (see variantClasses.tertiary and sizeClasses in Button), causing the size prop to be ignored; fix by removing height/padding/rounded/font classes from variantClasses.tertiary so it only contains color/visual rules, or alternatively enforce the intent by applying sizeClasses conditionally (e.g., only skip sizeClasses when variant === 'tertiary' and explicitly document/limit allowed sizes) — update the Button component to either strip "!h-7 px-5 rounded-component-lg font-body2" from variantClasses.tertiary or add conditional logic around applying sizeClasses to preserve consistent sizing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/common/button/Button.tsx`:
- Around line 5-52: The tertiary variant currently includes size-related classes
that override sizeClasses (see variantClasses.tertiary and sizeClasses in
Button), causing the size prop to be ignored; fix by removing
height/padding/rounded/font classes from variantClasses.tertiary so it only
contains color/visual rules, or alternatively enforce the intent by applying
sizeClasses conditionally (e.g., only skip sizeClasses when variant ===
'tertiary' and explicitly document/limit allowed sizes) — update the Button
component to either strip "!h-7 px-5 rounded-component-lg font-body2" from
variantClasses.tertiary or add conditional logic around applying sizeClasses to
preserve consistent sizing behavior.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/common/button/Button.stories.tsxsrc/components/common/button/Button.tsxsrc/index.css
jjjsun
changed the title
Seojegyeong
approved these changes
Feb 25, 2026
YermIm
approved these changes
Feb 25, 2026
9 tasks
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
3 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.
🚨 관련 이슈
Closed #55
✨ 변경사항
✏️ 작업 내용
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
Summary by CodeRabbit
릴리스 노트