-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 투표 에러 처리 + 종료 시 모달창 #407
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
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough투표 종료를 모달 확인 흐름으로 변경하고, 결과 페이지에서 뒤로가기를 종료 화면으로 강제 리다이렉트하도록 네비게이션을 조정했으며, DialogModal에 재사용 가능한 DialogButton 타입을 추가하고 API 에러 메시지 생성 로직을 개선했습니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant VotePage as DebateVotePage
participant Modal as DialogModal
participant API as EndPollAPI
participant ResultPage as DebateVoteResultPage
User->>VotePage: "종료" 버튼 클릭
VotePage->>Modal: openModal()
Modal->>User: 확인 모달 표시
User->>Modal: "마감하기" 클릭
Modal->>VotePage: handleConfirmEnd 실행
VotePage->>API: end-poll 뮤테이션 호출
API-->>VotePage: 응답(성공/오류)
VotePage->>Modal: closeModal()
VotePage->>ResultPage: 결과 화면으로 네비게이션
Note over ResultPage: mount 시 popstate 리스너 등록\n뒤로가기 시 /table/.../end 로 리다이렉트
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/apis/primitives.ts (1)
45-51: API 에러 응답에서message필드 추출 고려일반적으로 API 에러 응답은
{ message: "..." }형태의 구조를 가집니다. 현재 구현은 객체 전체를 JSON.stringify하여{"message":"투표가 만료되었습니다"}형태로 표시될 수 있습니다. 사용자에게 더 깔끔한 메시지를 표시하려면 message 필드를 우선 추출하는 것이 좋습니다.const responseData = error.response?.data; const message = typeof responseData === 'string' ? responseData - : typeof responseData === 'object' && responseData !== null - ? JSON.stringify(responseData) - : error.message; + : typeof responseData === 'object' && responseData !== null + ? (responseData as Record<string, unknown>).message?.toString() ?? + JSON.stringify(responseData) + : error.message;src/page/DebateVoteResultPage/DebateVoteResultPage.tsx (1)
39-50: 불필요한 래퍼 함수 제거 가능
redirectToEndPage함수는handleGoToEndPage를 단순히 호출하는 래퍼입니다. 이벤트 리스너에handleGoToEndPage를 직접 전달하면 코드가 더 간결해집니다.useEffect(() => { if (!isArgsValid) return; - const redirectToEndPage = () => { - handleGoToEndPage(); - }; - window.addEventListener('popstate', redirectToEndPage); + window.addEventListener('popstate', handleGoToEndPage); return () => { - window.removeEventListener('popstate', redirectToEndPage); + window.removeEventListener('popstate', handleGoToEndPage); }; }, [handleGoToEndPage, isArgsValid]);src/page/DebateVotePage/DebateVotePage.tsx (1)
136-148: 단일 버튼 모달에 취소 옵션 부재현재 모달에는 "종료하기" 버튼만 있고 취소 버튼이 없습니다. 사용자가 실수로 모달을 열었을 때 닫기(X) 버튼이나 오버레이 클릭으로만 취소할 수 있습니다.
DebateVoteResultPage처럼 "아니오"/"네" 형태의 양방향 선택을 제공하는 것이 UX 일관성 측면에서 좋을 수 있습니다.<DialogModal + left={{ + text: '취소', + onClick: closeModal, + }} right={{ text: '종료하기', onClick: handleConfirmEnd, isBold: true, }} >src/components/DialogModal/DialogModal.tsx (1)
3-11: DialogButton 인터페이스 개선 LGTM버튼을 선택적으로 받을 수 있도록 변경하여 컴포넌트의 유연성이 향상되었습니다.
isBold옵션으로 강조 표시도 지원하여 다양한 사용 사례에 대응할 수 있습니다.다른 컴포넌트에서
DialogButton타입을 재사용할 수 있도록 export하는 것을 고려해 보세요:-interface DialogButton { +export interface DialogButton { text: string; onClick: () => void; isBold?: boolean; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/apis/primitives.ts(1 hunks)src/components/DialogModal/DialogModal.tsx(2 hunks)src/page/DebateVotePage/DebateVotePage.tsx(4 hunks)src/page/DebateVoteResultPage/DebateVoteResultPage.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/page/DebateVotePage/DebateVotePage.tsx (3)
src/hooks/useModal.tsx (1)
useModal(15-87)src/hooks/mutations/useFetchEndPoll.ts (1)
useFetchEndPoll(5-12)src/components/DialogModal/DialogModal.tsx (1)
DialogModal(14-70)
🔇 Additional comments (2)
src/page/DebateVoteResultPage/DebateVoteResultPage.tsx (1)
35-37: 뒤로가기 동작 변경 구현 LGTM
replace: true옵션을 사용하여 히스토리 스택을 교체하고, popstate 이벤트 리스너로 브라우저 뒤로가기 버튼을 처리하는 방식이 적절합니다. PR 목표인 "투표 완료 화면에서 뒤로가기 방지"를 잘 구현했습니다.Also applies to: 127-131
src/components/DialogModal/DialogModal.tsx (1)
19-23: 버튼 레이아웃 로직 LGTM단일 버튼과 이중 버튼 시나리오를 처리하는
isSingleButton과buttonWidthClass로직이 깔끔합니다.
| const handleConfirmEnd = () => { | ||
| if (!isArgsValid) return; | ||
| mutate(pollId); | ||
| closeModal(); | ||
| }; |
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.
비동기 뮤테이션 완료 전 모달 닫기 문제
mutate(pollId)는 비동기 작업이지만 closeModal()이 즉시 호출됩니다. 뮤테이션이 실패하면 모달이 이미 닫혀 있어 사용자가 에러를 인지하기 어렵습니다. 만료된 투표 처리가 이 PR의 목표 중 하나이므로, 에러 핸들링을 고려해 주세요.
const handleConfirmEnd = () => {
if (!isArgsValid) return;
- mutate(pollId);
- closeModal();
+ mutate(pollId, {
+ onSuccess: () => {
+ closeModal();
+ },
+ onError: () => {
+ closeModal();
+ // 에러 토스트 표시 등 사용자에게 피드백 제공
+ },
+ });
};🤖 Prompt for AI Agents
In src/page/DebateVotePage/DebateVotePage.tsx around lines 43-47, the code calls
mutate(pollId) and immediately closeModal(), which closes the modal before the
async mutation completes; change this so the modal is closed only after a
successful mutation and remains open (and shows an error) if the mutation fails.
Use the mutation's async API (e.g., mutateAsync) or provide onSuccess/onError
callbacks: await mutateAsync(pollId) or call closeModal() in onSuccess, and
handle errors in onError by showing an error toast/message and keeping the modal
open; also disable the confirm button while the mutation is loading to prevent
duplicate submissions.
useon
left a comment
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.
치코 ~!! 꼼꼼 에러 처리 정말 굿입니다!!! 몇 가지 코멘트 남겼으니 편하게 확인해 주셔요 ~!! 감사합니다 ^.^!!!!
| ? responseData | ||
| : typeof responseData === 'object' && responseData !== null | ||
| ? JSON.stringify(responseData) | ||
| : error.message; |
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.
훨씬 정교해졌네요 !!!!!! 짱짱짱 !!!
| <div className="text-neutral-1000 p-20 text-center text-lg font-semibold"> | ||
| 정말로 종료하시겠습니까? | ||
| </div> | ||
| </DialogModal> |
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.
궁금한 것이 버튼은 prop으로 들어가는데 모달 내부의 텍스트는 children으로 자유도가 있는데 모달 내부를 구성하는 요소들이 구분되어 들어가는 이유가 있나용?? just 궁금. 당장 리팩토링하자는 소리 아닙니다요 ~!!!!
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.
@useon 모달 버튼을 Prop으로 넣어주는 이유는 저거 구현할 당시 모달의 생김새를 딱 저렇게 정확히 구현하기로 합의를 봤었기 때문이에요! 어차피 버튼 형태는 고정되어 있을 테니 버튼들 디자인을 최대한 동일하게 유지하기 위해 컨트롤할 수 있는 범위를 최대한 줄이자는 접근이었던 것으로 기억합니다.
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.
회의에서 진행했던 문구로 수정 진행했습니다.
| if (!isArgsValid) return; | ||
|
|
||
| const redirectToEndPage = () => { | ||
| handleGoToEndPage(); |
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.
지금 redirectToEndPage라는 함수가 handleGoToEndPage함수를 부르는 행위밖에 하지 않는데 새롭게 감싼 이유가 있는지 궁금합니다!! 그냥 바로 handleGoToEndPage를 호출해도 되지 않을까 했어요!
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.
넵 제가 생각해도 굳이 필요가 없네요. handleGoToEndPage를 바로 사용할 수 있도록 변경했습니다
i-meant-to-be
left a comment
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.
늦게 리뷰 남겨서 미안합니다 😭 요청하셨던 기능이 제대로 동작하는지에 대해서는 다 확인해봤는데 잘 돌아가는 것 같아요. 다만, 한 가지 의견을 주셨으면 하는 리뷰가 있어서 RC 남깁니다. 고생하셨습니당
체크리스트
- 투표 종료 시 한 번 더 물어보기
- 만료된 투표 뒤로가기 금지 에러
- 만료된 투표일 경우, 투표 시 오류가 잘 전달되도록 변경
| <div className="text-neutral-1000 p-20 text-center text-lg font-semibold"> | ||
| 정말로 종료하시겠습니까? | ||
| </div> | ||
| </DialogModal> |
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.
@useon 모달 버튼을 Prop으로 넣어주는 이유는 저거 구현할 당시 모달의 생김새를 딱 저렇게 정확히 구현하기로 합의를 봤었기 때문이에요! 어차피 버튼 형태는 고정되어 있을 테니 버튼들 디자인을 최대한 동일하게 유지하기 위해 컨트롤할 수 있는 범위를 최대한 줄이자는 접근이었던 것으로 기억합니다.
Co-authored-by: Shawn Kang <77564014+i-meant-to-be@users.noreply.github.com>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/page/DebateVotePage/DebateVotePage.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/page/DebateVotePage/DebateVotePage.tsx (3)
src/hooks/useModal.tsx (1)
useModal(15-87)src/hooks/mutations/useFetchEndPoll.ts (1)
useFetchEndPoll(5-12)src/components/DialogModal/DialogModal.tsx (1)
DialogModal(14-70)
🔇 Additional comments (5)
src/page/DebateVotePage/DebateVotePage.tsx (5)
8-9: LGTM!모달 기능 구현을 위한 필요한 import가 정확하게 추가되었습니다.
41-41: LGTM!useModal 훅이 올바르게 초기화되었습니다.
43-54: LGTM! 이전 리뷰 피드백이 반영되었습니다.
onSuccess와onError콜백을 사용하여 비동기 mutation 완료 후 모달을 닫도록 개선되었습니다. 이전 리뷰에서 지적된 문제가 해결되었습니다.
131-131: LGTM!단일 버튼 레이아웃에 맞게 footer 최대 너비가 적절히 조정되었습니다.
134-134: LGTM!버튼 클릭 시 모달을 열도록 변경하여 확인 단계가 추가되었습니다.
| <ModalWrapper> | ||
| <DialogModal | ||
| right={{ | ||
| text: '마감하기', | ||
| onClick: handleConfirmEnd, | ||
| isBold: true, | ||
| }} | ||
| > | ||
| <div className="px-16 py-24 text-center text-black"> | ||
| <p className="text-xl font-semibold">투표를 마감하시겠습니까?</p> | ||
| <p className="mt-2 text-sm text-default-neutral"> | ||
| 투표를 마감하면 더이상 표를 받을 수 없습니다! | ||
| </p> | ||
| </div> | ||
| </DialogModal> | ||
| </ModalWrapper> |
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.
확인 모달에 취소 버튼 추가 권장
현재 모달은 "마감하기" 버튼만 제공하고 명시적인 취소 버튼이 없습니다. 사용자는 X 버튼이나 오버레이 클릭으로만 취소할 수 있는데, 이는 일반적인 확인 모달 패턴과 다릅니다. 명확한 취소 옵션을 제공하는 것이 더 나은 사용자 경험을 제공합니다.
다음과 같이 left 버튼을 추가하는 것을 권장합니다:
<ModalWrapper>
<DialogModal
+ left={{
+ text: '취소',
+ onClick: closeModal,
+ isBold: false,
+ }}
right={{
text: '마감하기',
onClick: handleConfirmEnd,
isBold: true,
}}
>🤖 Prompt for AI Agents
In src/page/DebateVotePage/DebateVotePage.tsx around lines 143-158, the
confirmation modal only provides a "마감하기" action and lacks an explicit cancel
button; add a left button for cancellation by supplying the DialogModal's left
prop with text "취소" and an onClick that calls the modal close handler (use the
existing close/onClose/toggle handler in this component or create handleClose to
set modal state false), ensure the cancel button is not bold and maintains
accessibility (aria-label or proper focus handling) so users can clearly dismiss
the modal.
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.
@useon, 좋은 의견이라고 생각합니다! 👍
명시적인 취소 버튼을 추가하는 것이 더 나은 사용자 경험을 제공할 것 같습니다:
장점:
- 명확한 의도 전달 - X 버튼이나 오버레이 클릭보다 "취소" 버튼이 더 직관적입니다
- 일반적인 UX 패턴 - 대부분의 확인 모달은 "취소/확인" 또는 "아니오/예" 형태의 두 버튼을 제공합니다
- 실수 방지 - 투표 마감은 되돌릴 수 없는 중요한 액션이므로, 명확한 취소 옵션이 필요합니다
- 접근성 - 모바일 환경에서 X 버튼보다 큰 버튼이 더 터치하기 쉽습니다
다행히 DialogModal 컴포넌트가 이미 optional한 left prop을 지원하도록 수정되어서, CodeRabbit이 제안한 대로 쉽게 추가할 수 있습니다:
<DialogModal
left={{
text: '취소',
onClick: closeModal,
isBold: false,
}}
right={{
text: '마감하기',
onClick: handleConfirmEnd,
isBold: true,
}}
>이렇게 수정하시면 좋을 것 같습니다! 🐰



🚩 연관 이슈
closed #406
📝 작업 내용
[투표] 투표 종료 시 한번 더 물어보기
투표 결과 보기 클릭 시, 모달이 출력되도록 변경했습니다.
이를 위해 src/components/DialogModal/DialogModal.tsx 컴포넌트의 props인 left와 right를 선택적으로 받을 수 있도록 변경하였습니다.
만료된 투표 뒤로가기 금지 에러 처리
src/page/DebateVoteResultPage/DebateVoteResultPage.tsx에서
뒤로가기 버튼대신 토론 종료화면으로 버튼으로 변경하였습니다. 또한 브라우저 상 뒤로가기 버튼을 누르더라도 토론 종료화면으로 이동하도록 변경하였습니다.
만료된 투표일 경우, 투표시 에러 처리
기존에 에러메시지가 Object로 정확한 에러메세지가 표시되지 않는 문제가 있었습니다. 이를 수정해서 사용자가 만료된 투표라는 것을 정확히 인지 할 수 있도록 변경했습니다.
🗣️ 리뷰 요구사항 (선택)
다들 한번씩 정상작동하는지 확인 부탁드립니다.
Summary by CodeRabbit
새로운 기능
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.