-
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: 공통컴포넌트 tabs 구현 #23
Conversation
const contextValue = useMemo( | ||
() => ({ activeTab, setActiveTab }), | ||
[activeTab, setActiveTab], | ||
); |
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.
여기서 Memo 하면 뭐가 좋은가요?
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.
activeTab 또는 setActiveTab이 변경될 때만 contextValue를 생성할 수 있도록 하여 불필요한 리렌더링을 방지합니당ㅎㅂㅎ 린트에러가 메모 쓰라고 알려줫어연🛠️
<TabsList> | ||
<TabsTrigger value="tab1">Tab 1</TabsTrigger> | ||
<TabsTrigger rounded="md" buttonColor="red" value="tab2"> | ||
Tab 2 | ||
</TabsTrigger> | ||
<TabsTrigger buttonColor="blue" value="tab3"> | ||
Tab 3 | ||
</TabsTrigger> | ||
</TabsList> | ||
<TabsContent value="tab1">Tab1의 내용입니다.</TabsContent> | ||
<TabsContent value="tab2">Tab2의 내용입니다.</TabsContent> | ||
<TabsContent className="mt-23" value="tab3"> | ||
Tab3의 내용입니다. | ||
</TabsContent> |
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.
context 활용도 하고 좋네여~~ prop으로 받게 한 거 좋은 것 같아요.
round랑 padding은 prop으로 받아서 사용하는 곳마다 통일감을 주는 게 좋은 것 같아요.
플러스로 className도 prop으로 받고, 그 받은 prop을 cn 후순위에 넣으면 어떨까요? 사용하는 사람이 커스텀이 필요하면 할 수 있어도 좋을 것 같아서요. 그러면 통일감을 주면서도 확장성도 😙
쪼아용~~! |
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에서 작업한 내용을 간략히 설명해주세요.
📸 스크린샷
✅ 체크 리스트
👩💻 공유 포인트 및 논의 사항
트리거 버튼의 css를 className으로 사용자가 알아서 지정하게 할지,
이렇게 하나하나 프롭으로 받게할지 고민하다가 후자로 했어요. 어떤 방법이 좋을지 얘기해주시면 감사하겠습니당