-
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(@yourssu/react): create usePreventDuplicateClick #55
base: main
Are you sure you want to change the base?
Conversation
수고하셨어용! 제가 현재 노트북이 없는 관계로,, 리뷰는 11일 이후에 하도록 하겠숩니다😭 |
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.
👍
참고로 p4) 요거는 Pn 룰 입니다
export const usePreventDuplicateClick = () => { | ||
const [disabled, setDisabled] = useState(false); | ||
const loadingRef = useRef(false); | ||
|
||
const handleClick = useCallback(async (callback: () => Promise<void>) => { | ||
if (loadingRef.current) return; | ||
|
||
loadingRef.current = true; | ||
setDisabled(true); | ||
|
||
await callback(); | ||
|
||
loadingRef.current = false; | ||
setDisabled(false); | ||
}, []); | ||
|
||
return { disabled, handleClick }; | ||
}; |
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.
p4) 유틸리티의 관점에서는 usePreventDuplicateClick
보다는 usePromise
가 더 낫지 않나 싶네요.
export const usePreventDuplicateClick = () => { | |
const [disabled, setDisabled] = useState(false); | |
const loadingRef = useRef(false); | |
const handleClick = useCallback(async (callback: () => Promise<void>) => { | |
if (loadingRef.current) return; | |
loadingRef.current = true; | |
setDisabled(true); | |
await callback(); | |
loadingRef.current = false; | |
setDisabled(false); | |
}, []); | |
return { disabled, handleClick }; | |
}; | |
export const usePromise = () => { | |
const [isPending, setIsPending] = useState(false); | |
const callPromise = useCallback(async (callback: () => Promise<void>) => { | |
setIsPending(true); | |
await callback(); | |
setIsPending(false); | |
}, []); | |
return { isPending, callPromise }; | |
}; |
if (loadingRef.current) return; | ||
|
||
loadingRef.current = true; |
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.
p3) 굳이 ref를 사용할 필요가 없어보여요.
본래 목적이 이벤트 핸들러로 국한된다면 내부에서 상태와 세터를 같이 사용해도 문제 없을거예요.
1️⃣ 어떤 작업을 했나요? (Summary)
기존 코드에 영향을 미치지 않는 변경사항
usePreventDuplicateClick
훅 추가usePreventDuplicateClick.test
테스트 추가@testing-library/jest-dom
devDendency 추가2️⃣ 알아두시면 좋아요!
훅의 테스트를 위해 테스트 코드에서 더미 버튼을 만든 후 버튼의
disabled
속성 값에disabled
를 대입해서 이를 테스트 했습니다. 또한100ms
의 호출 시간을 가지는 콜백 함수를 mocking해서, 중복 실행 여부를 확인하였습니다.3️⃣ 추후 작업
usePreventDuplicateClick
훅 적용usePreventDuplicateClick
훅 적용4️⃣ 체크리스트 (Checklist)
main
브랜치의 최신 코드를pull
받았나요?