-
Notifications
You must be signed in to change notification settings - Fork 4
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: useDisclosure 훅 구현 #22
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.
@dev2820
안녕하세요! 기여해주셔서 감사합니다!
오 그런데 useDisclosure의 코드가 toggle을 제외하면 대부분 useSwitch와 동일한데요, 함수들의 역할이 비슷한 것 같습니다. useSwitch로 통일하는 것은 어떨까요?
@d0422 의도적으로 나눈 것이긴 합니다. 스위치는 useSwitch를, 디스클로저엔 useDisclosure를 각각 사용하도록이요. 두 훅 모두 useBoolean을 컴포넌트에 맞춰 해석하고 있습니다. chakra의 useBoolean과 useDisclosure와 유사합니다 허나 라이브러리의 방향성이 심플함이나 가벼움이라면 useBoolean으로 통일하는 쪽은 괜찮을 것 같습니다. 어떻게 생각하시나요? |
@dev2820 구현부가 거의 겹치게 되어 말씀을드렸던 것인데, 남겨주신 의견을 통해 다시 생각해보니 확실히 useSwitch와 useDisclosure훅이 분리된 형태로 제공되는 것이 라이브러리 사용자(개발자)가 기능을 구현할때 더 유용할 것 같다는 생각이듭니다. 구현부가 같아도, 메서드 이름이 다르면 맥락이 완전히 달라질 수 있으니까요! 남겨주신 chakra의 useDisclosure 코드를 참고해보았는데요, 해당 훅에서는 좋은 의견과 기여 감사합니다! 코드는 통합하지 않아도 좋을것 같아요! |
저도 PR을 너무 대충올린 것 같아 뜨끔하네요. 의도 설명도 없이 코드만....주의하겠습니다. @d0422 님의 의견과 방향성 말해주셔서 감사합니다. 다음에 기여할때 참고할게요. |
@dev2820 |
useDisclosure 훅은 modal, disclosure 등의 열림/닫힘 상태를 가지는 컴포넌트를 제어하기 위해 생성된 커스텀 훅입니다. 자세한 설명은 storybook을 참고해주세요