-
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: Implement Accordion component with storybook #16 #17
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.
애니메이션이나 UX, SEO까지 고려하신 점 대단하신 것 같아요.
코드 잘 보고 많이 배워갑니다 🙇🏻♀️
거정님은 고민의 폭이 넓고 추상화하고 설계하는 것도 정말 잘하시는 게 느껴집니다!!!
제가 도움을 줄 부분이 없어서 ... 필요하시다면 싱글토글 기능을 제안해봅니다ㅎㅎ~
>(({ className, children, ...restProps }: AccordionContentProps, ref?) => { | ||
const { toggle, handleToggle } = useAccordionContext(); | ||
const contentRef = useRef<HTMLDivElement>(null); | ||
useBeforeMatch<HTMLDivElement>(contentRef, handleToggle); |
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.
검색어 찾기 기능이군요 👍🏻
const contentHeight = useExpandableHeight<HTMLDivElement>(contentRef, toggle); | ||
|
||
return ( | ||
<Box | ||
ref={contentRef || ref} | ||
style={assignInlineVars({ | ||
[S.contentHeightVar]: `${contentHeight + 1}px`, | ||
})} | ||
className={S.content} | ||
HIDDEN={toggle ? undefined : 'until-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.
display none이 아닌 숨김으로써 UX를 고려한 부분들 너무 좋습니다 👍🏻
{Children.map(children, (child, index) => ( | ||
<div key={index} className={S.contentChild}> | ||
{child} | ||
</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.
아 children으로 내부 내용을 유연하게 받는 것도 좋은 것 같아요!
const { toggle, setToggle, handleToggle } = useToggle(); | ||
|
||
const value = useMemo( | ||
() => ({ toggle, setToggle, handleToggle }), | ||
[handleToggle, setToggle, toggle], | ||
); | ||
return ( | ||
<AccoridonContext.Provider value={value}> | ||
<Box ref={ref} className={S.item} {...restProps}> | ||
{children} | ||
</Box> | ||
</AccoridonContext.Provider> |
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.
비즈니스 로직은 여기에 담는 컴파운드패턴도 배워갑니다
maxHeight: contentHeightVar, | ||
transitionProperty: 'max-height', | ||
transitionDuration: '200ms', | ||
transitionDelay: '100ms', | ||
transitionTimingFunction: 'cubic-bezier(0.37, 0, 0.63, 1)', |
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.
높이로 컨텐츠를 가린 부분 아이디어가 좋고 고민 많이 하신게 느껴지네요
cubic-vezier도 배워갑니다 !!
넵 싱글 토글 기능이나 애니메이션 부분 개선을 위해 더 고려해봐야겠네요 감사합니다! |
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.
fix: Remove duplicate package
Motivation 🤔
Key Changes 🔑
To Reviewers 🙏🏻
@ebokyung