-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature : Button, ButtonGroup, IconButton 컴포넌트 구현 #188
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #188 +/- ##
==========================================
- Coverage 1.09% 1.07% -0.02%
==========================================
Files 236 242 +6
Lines 2106 2140 +34
Branches 349 358 +9
==========================================
Hits 23 23
- Misses 1979 2009 +30
- Partials 104 108 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Bundle SizesCompared against none
Dynamic import: None found. |
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.
LGTM 👍
const validChildren = Children.toArray(children).filter( | ||
(child) => | ||
isValidElement(child) && | ||
( | ||
child.type as { | ||
name: string; | ||
} | ||
).name === 'Button' | ||
) as ReactElement[]; |
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.
react에 이런 요소를 검증하는 메소드가 있군요 !
유효한 children요소인지, 이름이 Button
인지 검사하는 로직이군요 😄
const renderElements = (elements: ReactElement[]) => { | ||
if (elements.length === 1) { | ||
const props = elements[0].props as ButtonProps; | ||
return cloneElement(elements[0], { | ||
className: cn('w-full', props.className), | ||
}); | ||
} | ||
|
||
return ( | ||
<div className="flex gap-8"> | ||
{elements.map((element, index) => { | ||
const props = elements[index].props as ButtonProps; | ||
|
||
if (index === 0) { | ||
return cloneElement(element, { | ||
className: cn('flex-shrink-0', props.className), | ||
variant: props.variant ?? 'solid-default', | ||
}); | ||
} | ||
return cloneElement(element, { className: cn('w-full', props.className) }); | ||
})} | ||
</div> | ||
); | ||
}; |
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.
자식요소로 만든 elements
인자를 받아서, 1개라면 혼자 w-full
로 영역을 모두 가지고, 2개 이상이면 영역을 나눠서 가지는 로지깅군요.
react의 몰랐던 메소드가 많네요. cloneElement
..
많이 배워갑니다 👍
Feature : Button, ButtonGroup, IconButton 컴포넌트 구현
Feature : Button, ButtonGroup, IconButton 컴포넌트 구현
Feature : Button, ButtonGroup, IconButton 컴포넌트 구현
💡 왜 PR을 올렸나요?
💁 무엇이 어떻게 바뀌나요?
56 사이즈는 medium, 48 사이즈는 small로 size를 두었습니다. 추후 large 또는 더 작은 small이 추가된다면 수정할 예정입니다.
아직 디자인에서 사용하지 않은 outline 버튼은 구현하지 않았습니다!
기존에 twMerge에서는
text-white
와text-h1
이 겹치는 이슈가 있었습니다.동일한 이슈를 참고하여 해결했습니다!
Group에 들어가는 버튼 컴포넌트가 하나일 때는 solid-primary + 너비 100%인 버튼이 보이게 되고,
두 개일 때는
그리고 flex로 구성되도록 설정해두었습니다!
사용하는 쪽에서는 따로 스타일을 정의하지 않아도 구현가능합니다.
ex)
버튼 컴포넌트 외에 다른 컴포넌트 및 태그가 들어가면 무시됩니다.
background는 사용하는 쪽에서
className
으로 정의하도록 하고, 해당 컴포넌트에서는 layout만 정의했습니다.💬 리뷰어분들께