-
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
feat(system): Dropdown 컴포넌트 구현 #26
Conversation
❌ Deploy Preview for bbogak failed.
|
bf9b3c1
to
5ff2f3e
Compare
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.
혹시 컴포넌트의 사용법은 어떻게 될까요? 공통 컴포넌트 제작시에 컴포넌트 사용법도 함께 공유해주시면 좋을것 같습니다! 코드 리뷰하기에도 더 수월할 것 같아요~!
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.
고생하셨습니다 !
몇가지 코멘트 남겨드립니다 👍
export const CheckboxItem = forwardRef<HTMLButtonElement, Props>(function CheckboxItem( | ||
{ checked, disabled, children, ...restProps }, | ||
ref, | ||
) { | ||
const { styles, logics } = useDropdownContext(); | ||
|
||
return ( | ||
<button ref={ref} disabled={disabled} {...mergeProps(styles['checkbox-item'], logics['checkbox-item'], restProps)}> | ||
{children} | ||
<If condition={checked}> | ||
<Icon name="check" color={color.neutral30} size={20} /> | ||
</If> | ||
</button> | ||
); | ||
}); |
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.
Dropdown에 Checkbox가 필요한 이유에 대해 고민한 시간이 존재했습니다.
이후 영상을 보고 어떤 컴포넌트인지 파악했어요
CheckedItem이라는 이름은 어떨까요??
코멘트 수준으로 남겨드립니다 👍
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.
좋습니다!
separator: {}, | ||
'trigger-arrow': {}, | ||
'checkbox-item': { | ||
onClick: () => onOpenChange(false), |
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.
chain
을 사용해서 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.
컴포넌트 단계에서 mergeProps로 합쳐주고 있어요!
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.
👀 수고하셨습니다!!
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.
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.
네!
PR예제 본문에 첨부했습니다! |
3. 관련 스크린샷을 첨부해주세요.
2024-08-21.1.29.50.mov
4. 완료 사항
5. 추가 사항 / 코드 리뷰 받고 싶은 부분