Skip to content
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

ask()함수에 debounce() 함수 추가 #42

Merged
merged 18 commits into from
Apr 1, 2024
Merged

Conversation

koremp
Copy link
Collaborator

@koremp koremp commented Mar 28, 2024

  • src/utils/debounce.js를 추가

  • feat/open-test 브랜치에 적용된 src/components/Ask/index.jsaskButtonask() 함수에 debounce를 추가

Copy link

vercel bot commented Mar 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
call22nd ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 10:24pm

@koremp
Copy link
Collaborator Author

koremp commented Mar 28, 2024

이 PR은 feat/open-test 브랜치가 먼저 merge 된 이후 merge 되어야 합니다.

@nestiank nestiank added the enhancement New feature or request label Mar 28, 2024
@nestiank
Copy link
Collaborator

질문 보내기를 눌러도 결과 팝업이 뜨지 않는 것 같습니다!

@koremp
Copy link
Collaborator Author

koremp commented Mar 28, 2024

버튼을 누르고 alert가 뜨는 timeout 시간이 너무 오래걸려서 수정이 필요합니다.

@koremp
Copy link
Collaborator Author

koremp commented Mar 29, 2024

#37
#45
를 동시에 해결합니다

@koremp
Copy link
Collaborator Author

koremp commented Mar 29, 2024

제가 force-push해서 지금 feat/add-debounce 브랜치 deployment는 버튼이 뜨지 않습니다.
이전 debounce 함수를 다시 추가해서 적용 후 리뷰 부탁드립니다.

@nestiank
Copy link
Collaborator

nestiank commented Mar 29, 2024

모달이 안 뜹니다!
모달이 순간적으로 활성화되는 것 같긴 한데 보이지가 않습니다.

@koremp
Copy link
Collaborator Author

koremp commented Mar 30, 2024

모달이 안 뜹니다! 모달이 순간적으로 활성화되는 것 같긴 한데 보이지가 않습니다.

네 저한테도 동일하게 뜹니다 맞습니다 우선 푸쉬만 해놨거든요... 미리 커멘트를 달 걸 그랬네요

@koremp
Copy link
Collaborator Author

koremp commented Mar 30, 2024

모달이 안 뜹니다! 모달이 순간적으로 활성화되는 것 같긴 한데 보이지가 않습니다.

네 저한테도 동일하게 뜹니다 맞습니다 우선 푸쉬만 해놨거든요... 미리 커멘트를 달 걸 그랬네요

완료하고 다시 멘션하겠습니다. @nestiank

@koremp
Copy link
Collaborator Author

koremp commented Mar 30, 2024

modal 확인되었습니다. 확인 부탁드려요. @nestiank @limkhl

@koremp koremp requested review from nestiank and limkhl March 30, 2024 05:17
@limkhl
Copy link
Collaborator

limkhl commented Mar 30, 2024

보통 debounce 걸면 연속 클릭이 끝나기 전에는 동작을 안하는 게 맞을텐데 지금은 첫 클릭에 바로 pending 상태로 바뀌고 ask 함수가 실행되는 것 같아요. 근데 지금 동작도 사실 광클이 불가능한 구조니까 상관 없을 것 같긴 하지만 참고차 남겨두어요. 그리고 개인적으로는 alert 확인 누른 다음에는 페이지를 한번 새로고침 한다든지 하는 식으로 업데이트된 화면을 보여주면 좋을 것 같긴 한데, 사실 alert에는 해당 동작을 넣을 수가 없긴 하니까 일단 이대로 가도 괜찮을 것 같아요. 요것도 참고차 남깁니다!

Comment on lines +16 to +18
makeResponse(res, {

})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

두번째 인자로 {} 이게 꼭 들어가야 하는 건가요? 아니라면 빼면 좋을 것 같아요. 뭔가 싶어서 계속 거슬리긴 하더라고요. 😂

<button className="askButton" disabled={true}>지금은 질문을 보낼 수 없습니다.</button>
</div> */}
<div>
<button className="askButton" onClick={() => { setPending(true) }} >
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<button className="askButton" onClick={() => { setPending(true) }} >
<button className="askButton" onClick={() => setPending(true)} >

요렇게 쓰면 더 깔끔할 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋아요!

@koremp
Copy link
Collaborator Author

koremp commented Mar 30, 2024

보통 debounce 걸면 연속 클릭이 끝나기 전에는 동작을 안하는 게 맞을텐데 지금은 첫 클릭에 바로 pending 상태로 바뀌고 ask 함수가 실행되는 것 같아요. 근데 지금 동작도 사실 광클이 불가능한 구조니까 상관 없을 것 같긴 하지만 참고차 남겨두어요. 그리고 개인적으로는 alert 확인 누른 다음에는 페이지를 한번 새로고침 한다든지 하는 식으로 업데이트된 화면을 보여주면 좋을 것 같긴 한데, 사실 alert에는 해당 동작을 넣을 수가 없긴 하니까 일단 이대로 가도 괜찮을 것 같아요. 요것도 참고차 남깁니다!

업데이트한 화면을 보여주려면 새로고침을 하기 보다 해당 후보에게 내가 질문을 보냈음을 표시하는 무언가가 있으면 좋을 것 같기는 하네요.

@limkhl limkhl merged commit ea489d1 into develop Apr 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants