-
Notifications
You must be signed in to change notification settings - Fork 1
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: Chip 컴포넌트 구현 #137
feat: Chip 컴포넌트 구현 #137
Conversation
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.
슥 봤을 때 보이는 것만 먼저 코멘트 보내두겠습니다!!!!
사실 별 거 없음
내일 더 열심히 읽어볼게요....................진ㅉ진짜임...........
package.json
Outdated
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.
framer-motion 왜 설치됐는지는 지난주 모각코에서 들어서 알고 있는데! 지금은 안 쓰니까 멸종시켜버리죠
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.
멸종시켰습니다!
src/components/Chip/Chip.stories.tsx
Outdated
const DefaultUsageComponent = () => { | ||
return ( | ||
<Chip size="medium" role="suggestion"> | ||
sdsd |
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.
굿뜨임니다
overflow-x: ${({ $swipable }) => ($swipable ? 'auto' : 'visible')}; | ||
|
||
row-gap: ${({ theme }) => theme.primitive.number[10]}px; | ||
column-gap: ${({ theme }) => theme.primitive.number[8]}px; |
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.
권장하지 않는 사용법에 대해서도 명시해둔 점,,👍✨
$size: ChipSizeType; | ||
$selected: boolean; | ||
$disabled: boolean; | ||
$isRoleInput: boolean; |
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.
저는 3개 이상부터는 StyledComponentProps
형식의 인터페이스를 하나 만드는데요! (2개까진 그냥 페카처럼 함)
이거에 대한 기준도 정하면 작지만 코드 보는 사람 입장에서 통일성이 올라갈 거 같아요! 다음 회의때 얘기해보면 어떨까요
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.
1️⃣ 어떤 작업을 했나요? (Summary)
Chip 컴포넌트와 ChipGroup 컴포넌트를 구현하고, 문서를 작성했습니다.
개인적으로 최근 구현했던 모든 코드 통틀어서 가장 어려운 작업이었습니다
그래서 생각보다 오래걸렸던 것 같네요
추상화 레벨이 높아서 그냥 필요할 때 바로 사용해도 큰 문제는 없겠지만,
문서 한 번씩 로컬 서버 열어서 읽어보는거 추천드립니다.
Chip
Compound Component 방식으로 props를 줄이고, 이벤트 할당이나 컴포넌트의 구성을 더욱 선언적으로 구성했습니다.
ChipGroup
role에 따라서 자식 Chip 컴포넌트의 역할을 공통적으로 부여하고, 기능을 다르게 적용해야했기에
Chip을 더 편하게 관리할 수 있도록 하는 ChipGroup 컴포넌트를 추가로 구현했습니다.
자식 Chip 컴포넌트의 role 공통화 ( ChipGroup을 통해 상속받음 )
마우스로 가로 스와이프 기능 ( 모바일 스크롤처럼 smooth effect 구현 )
따로 state 없이 chip들의 selected 상태를 관리할 수 있도록 추상화
등등
4️⃣ 체크리스트 (Checklist)
main
브랜치의 최신 코드를pull
받았나요?