Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/apis/primitives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,17 @@ export async function request<T>(
if (axios.isAxiosError(error)) {
// If error is raised during API request,
// pass it as an APIError
const responseData = error.response?.data;
const message =
typeof responseData === 'string'
? responseData
: typeof responseData === 'object' && responseData !== null
? JSON.stringify(responseData)
: error.message;
Copy link
Contributor

Choose a reason for hiding this comment

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

훨씬 정교해졌네요 !!!!!! 짱짱짱 !!!

const apiError = new APIError(
error.response?.data || error.message,
message,
error.response?.status || 500,
error.response?.data,
responseData,
);
throw apiError;
}
Expand Down
89 changes: 46 additions & 43 deletions src/components/DialogModal/DialogModal.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
import { PropsWithChildren } from 'react';

interface DialogButton {
text: string;
onClick: () => void;
isBold?: boolean;
}

interface DialogModalProps extends PropsWithChildren {
left: {
text: string;
onClick: () => void;
isBold?: boolean;
};
right: {
text: string;
onClick: () => void;
isBold?: boolean;
};
left?: DialogButton;
right?: DialogButton;
}

export default function DialogModal({
children,
left,
right,
}: DialogModalProps) {
if (left.isBold === undefined || null) {
left.isBold = false;
}
if (right.isBold === undefined || null) {
right.isBold = false;
}
const leftIsBold = left?.isBold ?? false;
const rightIsBold = right?.isBold ?? false;
const hasButtons = Boolean(left || right);
const isSingleButton = Boolean(left && !right) || Boolean(right && !left);
const buttonWidthClass = isSingleButton ? 'w-full' : 'w-1/2';

return (
<div
Expand All @@ -34,34 +31,40 @@ export default function DialogModal({
{children}

{/** Buttons */}
<div className="w-full border-t border-neutral-300" />
<div className="flex w-full flex-row items-center justify-center">
{/** Left button */}
<button
data-testid="button-left"
className="w-1/2 py-4 hover:bg-neutral-300"
onClick={() => left.onClick()}
>
<p
className={`w-full ${left.isBold ? 'font-bold' : ''} text-neutral-1000`}
>
{left.text}
</p>
</button>
{hasButtons && (
<>
<div className="w-full border-t border-neutral-300" />
<div className="flex w-full flex-row items-center justify-center">
{left && (
<button
data-testid="button-left"
className={`${buttonWidthClass} py-4 hover:bg-neutral-300`}
onClick={left.onClick}
>
<p
className={`w-full ${leftIsBold ? 'font-bold' : ''} text-neutral-1000`}
>
{left.text}
</p>
</button>
)}

{/** Right button */}
<button
data-testid="button-right"
className="w-1/2 py-4 hover:bg-brand"
onClick={() => right.onClick()}
>
<p
className={`w-full ${right.isBold ? 'font-bold' : ''} text-neutral-1000`}
>
{right.text}
</p>
</button>
</div>
{right && (
<button
data-testid="button-right"
className={`${buttonWidthClass} py-4 hover:bg-brand`}
onClick={right.onClick}
>
<p
className={`w-full ${rightIsBold ? 'font-bold' : ''} text-neutral-1000`}
>
{right.text}
</p>
</button>
)}
</div>
</>
)}
</div>
);
}
37 changes: 33 additions & 4 deletions src/page/DebateVotePage/DebateVotePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import DefaultLayout from '../../layout/defaultLayout/DefaultLayout';
import { useGetPollInfo } from '../../hooks/query/useGetPollInfo';
import ErrorIndicator from '../../components/ErrorIndicator/ErrorIndicator';
import useFetchEndPoll from '../../hooks/mutations/useFetchEndPoll';
import GoToDebateEndButton from '../../components/GoToDebateEndButton/GoToDebateEndButton';
import { useModal } from '../../hooks/useModal';
import DialogModal from '../../components/DialogModal/DialogModal';
export default function DebateVotePage() {
const navigate = useNavigate();
const baseUrl =
Expand Down Expand Up @@ -37,7 +38,20 @@ export default function DebateVotePage() {
refetch,
isRefetchError,
} = useGetPollInfo(pollId, { refetchInterval: 5000, enabled: isArgsValid });
const { openModal, closeModal, ModalWrapper } = useModal();
const { mutate } = useFetchEndPoll(handleGoToResult);
const handleConfirmEnd = () => {
if (!isArgsValid) return;
mutate(pollId, {
onSuccess: () => {
closeModal();
},
onError: () => {
closeModal();
alert('투표 종료에 실패했습니다.');
},
});
};
Comment on lines 43 to 54
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

비동기 뮤테이션 완료 전 모달 닫기 문제

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.


const participants = data?.voterNames;
const isLoading = isFetching || isRefetching;
Expand Down Expand Up @@ -114,11 +128,10 @@ export default function DebateVotePage() {
</main>

<DefaultLayout.StickyFooterWrapper>
<div className="flex w-full max-w-[800px] flex-row items-center justify-center gap-2">
<GoToDebateEndButton tableId={tableId} className="flex-1" />
<div className="flex w-full max-w-[400px] flex-row items-center justify-center gap-2">
<button
type="button"
onClick={() => mutate(pollId)}
onClick={openModal}
className="button enabled brand flex flex-1 flex-row rounded-full p-[24px]"
>
투표 결과 보기
Expand All @@ -127,6 +140,22 @@ export default function DebateVotePage() {
</DefaultLayout.StickyFooterWrapper>
</div>
</DefaultLayout.ContentContainer>
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

궁금한 것이 버튼은 prop으로 들어가는데 모달 내부의 텍스트는 children으로 자유도가 있는데 모달 내부를 구성하는 요소들이 구분되어 들어가는 이유가 있나용?? just 궁금. 당장 리팩토링하자는 소리 아닙니다요 ~!!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

  • 구현 화면
image
  • 피그마 시안
image

시안과의 차이가 조금 있는 것 같아 확인 부탁드려요! 글자 위치가 살짝 올라가 있고, 크기도 조금 더 작은 것 같아요!

그리고 피그마 봤을 때 호버가 됐을 때 노란색인지 디폴트가 노란색인지 모르겠네욥 .. 써니와 이야기 해보셔요 !!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

'정말로 종료하시겠습니까?'가 현재 시안인데 '투표를 종료하시겠습니까?'가 무엇을 종료하는지 확실히 와닿을 것 같은데, 치코는 어떻게 생각하시나요? 공감하신다면 요 내용도 함께 이야기 해주시면 어떨까욥?!!

Copy link
Contributor

Choose a reason for hiding this comment

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

@useon 모달 버튼을 Prop으로 넣어주는 이유는 저거 구현할 당시 모달의 생김새를 딱 저렇게 정확히 구현하기로 합의를 봤었기 때문이에요! 어차피 버튼 형태는 고정되어 있을 테니 버튼들 디자인을 최대한 동일하게 유지하기 위해 컨트롤할 수 있는 범위를 최대한 줄이자는 접근이었던 것으로 기억합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

혹시 저 시안은 어디 있는 건가요? 피그마 dev에서는 비슷하게 구현되어 있는 것 같은데 확인 가능하신가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

회의에서 진행했던 문구로 수정 진행했습니다.

</ModalWrapper>
Comment on lines +143 to +158
Copy link

@coderabbitai coderabbitai bot Dec 13, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

확인 모달에 취소 버튼 추가 권장

현재 모달은 "마감하기" 버튼만 제공하고 명시적인 취소 버튼이 없습니다. 사용자는 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

요기 리뷰처럼 취소 버튼도 명시적으로 있는게 좋겠다는 생각이 드는데 어떻게 생각하시나욥 ?!!!?!

Copy link

Choose a reason for hiding this comment

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

@useon, 좋은 의견이라고 생각합니다! 👍

명시적인 취소 버튼을 추가하는 것이 더 나은 사용자 경험을 제공할 것 같습니다:

장점:

  1. 명확한 의도 전달 - X 버튼이나 오버레이 클릭보다 "취소" 버튼이 더 직관적입니다
  2. 일반적인 UX 패턴 - 대부분의 확인 모달은 "취소/확인" 또는 "아니오/예" 형태의 두 버튼을 제공합니다
  3. 실수 방지 - 투표 마감은 되돌릴 수 없는 중요한 액션이므로, 명확한 취소 옵션이 필요합니다
  4. 접근성 - 모바일 환경에서 X 버튼보다 큰 버튼이 더 터치하기 쉽습니다

다행히 DialogModal 컴포넌트가 이미 optional한 left prop을 지원하도록 수정되어서, CodeRabbit이 제안한 대로 쉽게 추가할 수 있습니다:

<DialogModal
  left={{
    text: '취소',
    onClick: closeModal,
    isBold: false,
  }}
  right={{
    text: '마감하기',
    onClick: handleConfirmEnd,
    isBold: true,
  }}
>

이렇게 수정하시면 좋을 것 같습니다! 🐰

</DefaultLayout>
);
}
17 changes: 14 additions & 3 deletions src/page/DebateVoteResultPage/DebateVoteResultPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import VoteDetailResult from './components/VoteDetailResult';
import { useGetPollInfo } from '../../hooks/query/useGetPollInfo';
import ErrorIndicator from '../../components/ErrorIndicator/ErrorIndicator';
import { TeamKey } from '../../type/type';
import { useState } from 'react';
import { useCallback, useEffect, useState } from 'react';
import DialogModal from '../../components/DialogModal/DialogModal';
export default function DebateVoteResultPage() {
// 매개변수 검증
Expand All @@ -32,6 +32,17 @@ export default function DebateVoteResultPage() {
const handleGoHome = () => {
navigate('/');
};
const handleGoToEndPage = useCallback(() => {
navigate(`/table/customize/${tableId}/end`, { replace: true });
}, [navigate, tableId]);

useEffect(() => {
if (!isArgsValid) return;

window.addEventListener('popstate', handleGoToEndPage);
return () => window.removeEventListener('popstate', handleGoToEndPage);
}, [handleGoToEndPage, isArgsValid]);

const isLoading = isFetching || isRefetching;
const isError = isFetchError || isRefetchError;
const { openModal, ModalWrapper, closeModal } = useModal({
Expand Down Expand Up @@ -108,11 +119,11 @@ export default function DebateVoteResultPage() {
<div className="flex w-full max-w-[400px] flex-col items-center justify-center gap-2 md:w-full md:max-w-[800px] md:flex-row">
<button
type="button"
onClick={() => navigate(-1)}
onClick={handleGoToEndPage}
className="button enabled neutral flex w-full flex-1 rounded-full p-[24px]"
disabled={isLoading}
>
뒤로 가기 ←
토론 종료화면으로
</button>
<button
type="button"
Expand Down