[FEAT] 피드백 타이머 구현 #361
[FEAT] 피드백 타이머 구현 #361
Conversation
Walkthrough피드백 타이머 기능이 추가되었습니다. 훅(useFeedbackTimer), 피드백 전용 UI(FeedbackTimer, CircularTimer/TimerController 확장), 페이지(FeedbackTimerPage), 스토리, 라우트, 타입(TimeBoxType에 FEEDBACK) 및 홈 이동 버튼(GoToHomeButton)이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant Router as 라우터
participant Page as FeedbackTimerPage
participant Hook as useFeedbackTimer
participant UI as FeedbackTimer
participant CT as CircularTimer
participant TC as TimerController
User->>Router: /table/customize/:id/end/feedback 요청
Router->>Page: FeedbackTimerPage 렌더
Page->>Hook: useFeedbackTimer() 호출
Page->>Hook: setTimer(0), setDefaultTimer(0)
Page->>UI: <FeedbackTimer feedbackTimerInstance=... />
UI->>CT: progress, time, boxType=FEEDBACK 전달
UI->>TC: 제어 핸들러(start/pause/reset) 전달
User->>TC: 시작 클릭
TC->>Hook: startTimer()
Hook-->>UI: timer/isRunning 업데이트 (interval)
UI->>CT: 진행률 갱신
User->>TC: 일시정지/리셋 클릭
TC->>Hook: pauseTimer()/resetTimer()
Hook-->>UI: 상태 반영
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
src/components/GoToHomeButton/GoToHomeButton.tsx (2)
10-16: 버튼 기본 type 및 접근성 속성 추가 권장폼 내부 사용 시 원치 않는 submit을 방지하고, 스크린리더 용 레이블을 부여해 주세요.
- <button + <button + type="button" + aria-label="홈으로 이동" onClick={handleClick} className="flex h-[72px] w-[492px] items-center justify-center gap-[12px] rounded-full border-[2px] border-default-disabled/hover bg-default-white px-[16px] py-[11px] font-semibold text-default-black opacity-80 transition-colors duration-200 hover:bg-default-disabled/hover" >
1-16: 정적 이동은 Link 사용이 더 적합 (선택 사항)단순 홈 이동은 Link가 시맨틱/접근성/포커스 관리에 유리합니다. 추후 투표 화면으로의 흐름이 생기면
to를 prop으로 받아 재사용 가능하게 확장하세요(기본값'/').-import { useNavigate } from 'react-router-dom'; +import { Link } from 'react-router-dom'; export default function GoToHomeButton() { - const navigate = useNavigate(); - - const handleClick = () => { - navigate('/'); - }; + // static navigation: Link 권장 return ( - <button - onClick={handleClick} + <Link + to="/" className="flex h-[72px] w-[492px] items-center justify-center gap-[12px] rounded-full border-[2px] border-default-disabled/hover bg-default-white px-[16px] py-[11px] font-semibold text-default-black opacity-80 transition-colors duration-200 hover:bg-default-disabled/hover" - > + aria-label="홈으로 이동" + role="button" + > <span>홈으로 돌아가기 →</span> - </button> + </Link> ); }src/page/TimerPage/components/TimerController.tsx (1)
32-40: FEEDBACK 배경색 분기 추가 LGTM + 클래스 매핑 단순화 제안조건 객체 여러 개 대신 매핑으로 단순화하면 유지보수가 수월합니다.
- <button - className={clsx( - 'flex size-[76px] items-center justify-center rounded-full p-[20px] xl:size-[92px]', - { 'bg-camp-blue': stance === 'PROS' }, - { 'bg-camp-red': stance === 'CONS' }, - { 'bg-default-neutral': stance === 'NEUTRAL' }, - { 'bg-brand': stance === 'FEEDBACK' }, - )} + <button + className={clsx( + 'flex size-[76px] items-center justify-center rounded-full p-[20px] xl:size-[92px]', + ({ PROS: 'bg-camp-blue', CONS: 'bg-camp-red', NEUTRAL: 'bg-default-neutral', FEEDBACK: 'bg-brand' } as const)[stance] + )}src/page/TimerPage/components/CircularTimer.tsx (1)
25-33: FEEDBACK 색상 분기 추가 LGTM + 하드코딩 제거/가독성 개선 제안색상 상수를 매핑으로 바꾸고, 가능하면 테마 토큰/변수로 일원화하세요(동일 매핑이 여러 컴포넌트에 퍼져 있음).
- const PROGRESS_COLOR = - stance === 'NEUTRAL' - ? '#A3A3A3' // bg-default-neutral - : stance === 'PROS' - ? '#1E91D6' // bg-camp-blue - : stance === 'FEEDBACK' - ? '#FECD4C' // bg-brand - : '#E14666'; // bg-camp-red + const stanceToProgressHex: Record<Stance, string> = { + PROS: '#1E91D6', // camp-blue + CONS: '#E14666', // camp-red + NEUTRAL: '#A3A3A3', // default-neutral + FEEDBACK: '#FECD4C', // brand + }; + const PROGRESS_COLOR = stanceToProgressHex[stance];src/routes/routes.tsx (1)
42-46: :id 파라미터 미사용 — 경로에서 제거 권장
FeedbackTimerPage 컴포넌트에서 useParams로 id를 전혀 참조하지 않으므로 불필요합니다. GA 경로 카디널리티 감소를 위해 아래처럼 수정해주세요.{ - path: '/debateEnd/:id/feedback', + path: '/debateEnd/feedback', element: <FeedbackTimerPage />, requiresAuth: true, },src/page/TimerPage/FeedbackTimerPage.tsx (1)
7-17: 초기 시간 0초 하드코딩 — 확장성을 위해 진입 파라미터/설정 연동 고려INITIAL_TIME=0은 데모에는 적합하나, 라우트 쿼리(?t=)나 설정값과 연동하면 재사용성이 올라갑니다. 최소한 0 이하일 때는 startTimer 가드도 가능.
원한다면 쿼리스트링 기반 초기화 예시를 제안하겠습니다.
src/page/TimerPage/stories/FeedbackTimer.stories.tsx (1)
20-24: 컨트롤 추가 제안initialTime에 대해 controls(range/step) 제공하면 디자이너/QA 사용성이 좋아집니다.
예: argTypes에 control: { type: 'number', min: 0, max: 1800, step: 30 }
src/page/TimerPage/components/FeedbackTimer.tsx (4)
55-55: Tailwind 클래스 순서 경고 — 정렬 니트픽정적 분석 경고(Invalid Tailwind CSS classnames order)를 해소하려면 클래스 순서를 정렬하세요.
적용 diff:
- <div className="flex flex-row space-x-[80px] items-center"> + <div className="flex items-center flex-row space-x-[80px]">
61-76: 시간 조절 버튼 접근성 보강아이콘이 아닌 텍스트이긴 하지만 스크린리더 명확성을 위해 aria-label 추가를 권장합니다. 또한 disabled 사유를 aria-disabled로 전달하면 좋습니다.
적용 예시:
- <button + <button key={label} type="button" onClick={() => adjustTime(value)} disabled={isRunning} + aria-label={`시간 ${value > 0 ? '추가' : '감소'} ${Math.abs(value / 60)}분`} + aria-disabled={isRunning} className={clsx(
80-109: 블록 컨테이너에 span 사용 — div로 교체 권장레이아웃 래퍼는 div가 더 적합합니다. 의미론/일관성 측면 개선.
적용 diff:
- <span className="flex w-[360px] flex-col space-y-[16px] xl:min-w-[480px]"> + <div className="flex w-[360px] flex-col space-y-[16px] xl:min-w-[480px]"> ... - </span> + </div>
59-59: 하드코딩 텍스트의 i18n 추출 제안'피드백 타이머' 문자열을 i18n 리소스로 분리하면 타이머군 컴포넌트 전반의 번역 일관성이 좋아집니다.
src/page/TimerPage/hooks/useFeedbackTimer.ts (4)
14-14: 타이머 ref 타입 교정브라우저 환경 호환을 위해 NodeJS.Timeout 대신 ReturnType 사용을 권장합니다.
적용 diff:
- const intervalRef = useRef<NodeJS.Timeout | null>(null); + const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
28-34: 일시정지 시 내부 상태 초기화재시작 시 혼동을 줄이기 위해 targetTimeRef도 null로 초기화하는 편이 명확합니다.
적용 diff:
const pauseTimer = useCallback(() => { if (intervalRef.current !== null) { clearInterval(intervalRef.current); intervalRef.current = null; } setIsRunning(false); + targetTimeRef.current = null; }, []);
41-42: 0초에서는 타이머 시작 방지 가드 추가timer가 0 이하일 때는 setInterval을 만들지 않도록 가드하면 불필요한 tick을 줄일 수 있습니다.
적용 diff:
- const startTimer = useCallback(() => { - if (intervalRef.current !== null || timer === null) return; + const startTimer = useCallback(() => { + if (intervalRef.current !== null || timer === null || timer <= 0) return;
123-123: 인터페이스/구현 시그니처 불일치resetTimer가 인터페이스에선
(value?: number)지만 구현은 무인자입니다. 혼선을 막기 위해 인터페이스를 구현에 맞춰 정리하세요.적용 diff:
- resetTimer: (value?: number) => void; + resetTimer: () => void;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/components/GoToHomeButton/GoToHomeButton.tsx(1 hunks)src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx(1 hunks)src/page/TimerPage/FeedbackTimerPage.tsx(1 hunks)src/page/TimerPage/components/CircularTimer.tsx(1 hunks)src/page/TimerPage/components/FeedbackTimer.tsx(1 hunks)src/page/TimerPage/components/TimerController.tsx(1 hunks)src/page/TimerPage/hooks/useFeedbackTimer.ts(1 hunks)src/page/TimerPage/stories/FeedbackTimer.stories.tsx(1 hunks)src/routes/routes.tsx(2 hunks)src/type/type.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe#335
File: src/page/TimerPage/components/NormalTimer.tsx:50-54
Timestamp: 2025-08-08T14:21:17.745Z
Learning: Progress 퍼센트(원형 게이지)는 컴포넌트가 아닌 훅(src/page/TimerPage/hooks/useCircularTimerAnimation.ts)에서 0–100으로 clamp한다. 향후 리뷰에서는 컴포넌트 레벨에서의 중복 clamp 제안 대신 훅 사용을 권장한다.
Applied to files:
src/page/TimerPage/components/CircularTimer.tsxsrc/page/TimerPage/components/FeedbackTimer.tsx
🧬 Code graph analysis (4)
src/page/TimerPage/FeedbackTimerPage.tsx (3)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
useFeedbackTimer(10-114)src/page/TimerPage/components/FeedbackTimer.tsx (1)
FeedbackTimer(20-111)src/components/GoToHomeButton/GoToHomeButton.tsx (1)
GoToHomeButton(3-18)
src/page/TimerPage/components/FeedbackTimer.tsx (6)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
FeedbackTimerLogics(116-126)src/util/formatting.ts (1)
Formatting(1-12)src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
useCircularTimerAnimation(4-18)src/hooks/useBreakpoint.ts (1)
useBreakpoint(11-36)src/page/TimerPage/components/CircularTimer.tsx (1)
CircularTimer(17-63)src/page/TimerPage/components/TimerController.tsx (1)
TimerController(15-58)
src/routes/routes.tsx (1)
src/page/TimerPage/FeedbackTimerPage.tsx (1)
FeedbackTimerPage(9-29)
src/page/TimerPage/stories/FeedbackTimer.stories.tsx (2)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
useFeedbackTimer(10-114)src/page/TimerPage/components/FeedbackTimer.tsx (1)
FeedbackTimer(20-111)
🪛 GitHub Check: test
src/page/TimerPage/components/FeedbackTimer.tsx
[warning] 55-55:
Invalid Tailwind CSS classnames order
🔇 Additional comments (7)
src/type/type.ts (1)
14-19: StanceToString에 FEEDBACK 추가 LGTM매핑 일관성 확보에 적절합니다.
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (1)
49-55: The requested snippet ofsrc/type/type.ts(lines 1–30) is being retrieved for inspection.src/routes/routes.tsx (1)
12-12: FeedbackTimerPage import 적절라우트 추가와 경로가 일치합니다. 별도 이슈 없습니다.
src/page/TimerPage/FeedbackTimerPage.tsx (1)
19-28: UX 흐름 확인 필요: 홈 이동 버튼만 존재현재 피드백 종료 후 홈으로만 이동합니다. 곧바로 투표 페이지로 연결하는 플로우가 요구되면 라우팅/네비게이션 전략 합의가 필요합니다.
src/page/TimerPage/stories/FeedbackTimer.stories.tsx (1)
7-15: 스토리 초기화 로직 적절args 변경 시 setTimer/setDefaultTimer로 동기화하는 패턴 깔끔합니다. 성능상 이슈도 없습니다.
src/page/TimerPage/components/FeedbackTimer.tsx (1)
33-43: 진행률 계산과 클램프 위치 적절훅(useCircularTimerAnimation)에서 0–100 클램프 처리하므로 컴포넌트 레벨 중복 클램프가 없고, 기존 합의와 일치합니다.
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
48-65: 타이머 갱신 주기와 반올림 정책 적절200ms 주기로 Date.now 기반 남은 초를 Math.ceil 처리해 드리프트를 억제하는 접근은 합리적입니다.
jaeml06
left a comment
jaeml06
left a comment
There was a problem hiding this comment.
정상작동확인 했습니다. 구현하느라 고생하셨습니다. 피드백 타이머는 아직 사용자가 접근이 불가능한 기능이 맞는거죠?
피드백 타이머의 경로
리뷰 남긴것 처럼 전 id가 필요하지 않다고 생각합니다. id로 인해 사용자마다 다르게 보일 필요가 없다고 생각하기 때문입니다.
피드백 타이머의 파일 위치
몇가지 방법이 있을 것 같습니다.
- 타이머 관련된 컴포넌트를 공통 컴포넌트로 분리하고 FeedbackTimerPage와 TimerPage로 구분해서 관리한다.
- 지금처럼 하나의 TimerPage의 흐름으로보고 내부에 같이 둔다.
저는 1번 흐름이 적절해보인다고 생각합니다. 하지만 분리하는 것이 어려울 수도 있다고 판단되는 이번 이슈에서 해결할 필요는 없다고 생각합니다. 7일까지 어쨋든 PR이 마감되어야하기도 하고요
홈으로 이동하기 버튼 컴포넌트 분리
좋은 것 같습니다. 문제는 각 페이지에서 같은 형태의 디자인이 될것이냐인데 그렇기만 하면 같이 관리해도 좋을 것 같습니다. 여러 홈으로 가는 버튼도 통합을 진행해야하는지 고민이되네요. 헤더도 어쨋던 홈으로 가는 버튼이고 에러 페이지의 홈으로 가기도 있으니 그 부분까지 통합을 하거나 컴포넌트 명을 좀 더 수정할 필요가 있을 것 같습니다
구현하면서 추가로 떠오른 점
나중에 버튼을 두개 만들어서 '홈으로 이동하기', '투표하러가기' 두개를 만들면 해결될 것 같습니다. 다만 이 경우에는 첫 질문인 라우터에 :id값을 추가해야 할 것 같습니다.
i-meant-to-be
left a comment
i-meant-to-be
left a comment
There was a problem hiding this comment.
확인했습니다! 빠른 시간 내에 많은 작업 쳐내시느라 고생 많으셨어요 😭
몇 가지 리뷰가 있어 RC를 남깁니다!
export type Stance = 'PROS' | 'CONS' | 'NEUTRAL' | 'FEEDBACK';추가로, 현재 코드 구조상 피드백 타이머를 Stance 타입에 추가하는 게 훨씬 개발이 수월하여 이 방향으로 진행하신 것으로 생각됩니다. 그러나, 세부 PR에서 설명을 드리겠지만 FEEDBACK이 과연 찬성/반대/중립처럼 팀(stance)에 해당하는지를 묻는다면 저는 'FEEDBACK'이 팀에 들어가기엔 적합하지 않다고 대답할 것 같아요..
export type TimeBoxType = 'NORMAL' | 'TIME_BASED' | 'FEEDBACK';따라서, 위 코드처럼 Stance 타입보다는 더 아래에 있는 TimeBoxType에 피드백 타이머를 추가하는 게 타입의 목적상 더 적절해 보입니다. 그러나, 제가 대충 봐도 Stance에 피드백 타이머 타입이 들어가는 게 훨씬 더 편한 구현 방향일 것 같기는 해서, 살펴보시고 할 만 하면 FEEDBACK을 TimeBoxType으로 옮겨주시되 그렇지 않다면 지금 방향으로 유지하는 것, 이렇게 2가지 선택지를 같이 고려해주시면 감사하겠습니다!
src/type/type.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| // Types | |||
| export type Stance = 'PROS' | 'CONS' | 'NEUTRAL'; | |||
| export type Stance = 'PROS' | 'CONS' | 'NEUTRAL' | 'FEEDBACK'; | |||
There was a problem hiding this comment.
권장: 피드백 타이머를 다른 타입으로 이동
현재 피드백 타이머가 추가된 타입은 Stance입니다. 이건 타이머의 팀을 나타내는 타입으로, 찬성/반대/중립 중 하나여야 합니다. 그러나 새로 추가된 FEEDBACK은 타이머의 팀이라고 보기에는 다소 무리가 있습니다.
export type TimeBoxType = 'NORMAL' | 'TIME_BASED' | 'FEEDBACK';따라서, 위 코드처럼 Stance 타입보다는 더 아래에 있는 TimeBoxType에 피드백 타이머를 추가하는 게 타입의 목적상 더 적절해 보입니다. CircularTimer에서 타이머 색상을 설정하기 위한 조건문 분기도 이거에 맞추는 게 좋아 보여요.
There was a problem hiding this comment.
맞습니다 .. 입장으로 보기에는 무리가 있는 것 같습니다 .. !!!! TimeBoxType에다가 추가했고 관련된 로직도 맞춰서 수정했습니다!
src/routes/routes.tsx
Outdated
| requiresAuth: false, | ||
| }, | ||
| { | ||
| path: '/debateEnd/:id/feedback', |
There was a problem hiding this comment.
대안: URL 수정
개인적으로 토론 종료 과정은 토론의 연장선상에 있다고 생각합니다. 따라서 URL을 아래와 같이 수정하는 부분을 제안드려요:
- 기존: /debateEnd/:id/feedback
- 제안: /table/customize/:id/finish/feedback
이렇게 할 경우, 추후 토론 종료 페이지에 추가될 투표 기능도 /table/customize/:id/finish/vote 이런 식으로 함께 묶어버릴 수 있어 장점이 있다고 생각해요.
There was a problem hiding this comment.
경로에 대한 고민이 있었는데 숀이 제시한 방향 좋네요 !!!
finish냐 end냐 고민하다가 제미나이의 추천 경로로 end로 했습니다 !
URL 경로 또한 사용자나 개발자에게
애플리케케이션의 흐름을 알려주는 역할을
합니다.
* /end/feedback: '[토론의] 끝에 있는 피드백'
이라고 자연스럽게 읽힙니다. end가 토론의 한
단계가 끝나는 '지점'을 잘 나타내줍니다.
* /finish/feedback: '[토론] 완료 후 피드백'
으로 읽힙니다. '완료'라는 단어가 주는 모든
것이 끝났다는 느낌 때문에, 뒤에 피드백이 또
나오는 것이 약간 어색하게 느껴질 수
있습니다.
따라서, 더 명확하고 논리적인 흐름을 위해서는
`/table/customize/:id/end/feedback` 경로가
가장 좋은 선택입니다.`
There was a problem hiding this comment.
질문: 새로운 훅을 생성한 이유
저는 저번에 작전 시간 타이머를 새로 작업하면서 기존의 useNormalTimer 로직을 그대로 변경 없이 재활용했습니다. 피드백 타이머도 기능이 상당히 단순한 편이고 useNormalTimer 로직에 그대로 포섭될 것 같은데, 별도로 훅을 분리하신 이유가 있나요.
There was a problem hiding this comment.
- 기존의 useNormalTimer에는 피드백 타이머의 버튼으로 시간이 추가되고 차감되는 함수가 없고, 그럴 책임이 없는 훅입니다.
그래서 작전시간 구현시에는 보니까 컴포넌트에서 setTimer로 시간을 조절하셨더라고요. 저는 따로 해당 일을 하는 함수까지 useFeedbackTimer의 책임으로 보고 훅으로 분리했습니다. - NormalTimer에서만 쓰고 FeedbackTimer에서는 필요없는 작전 시간 타이머 작동 여부, 작전 시간 동작 중에 기존 타이머를 저장하는 함수 등이 필요 없기 때문에 필요한 책임으로만 구성된 훅을 만들었습니다. 따라서 피드백 타이머에 필요한 유지보수가 더 쉬울 것 같아서 선택했습니다 !!
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/page/TimerPage/components/TimerController.tsx (3)
36-45: 배경색 클래스 충돌 가능성 — 단일 bg 클래스 계산으로 명시화하세요.
stance와 boxType 조건이 동시에 참이면 Tailwind bg-* 클래스가 중복 적용됩니다. 현재 순서상 FEEDBACK이 우선 적용되지만, 중복을 없애면 의도가 더 명확하고 유지보수성이 좋아집니다.다음과 같이 단일 bgClass를 계산해 적용하세요.
export default function TimerController({ onStart, onPause, onReset, isRunning, stance, boxType, }: TimerControllerProps) { + const bgClass = + boxType === 'FEEDBACK' + ? 'bg-brand' + : stance === 'PROS' + ? 'bg-camp-blue' + : stance === 'CONS' + ? 'bg-camp-red' + : 'bg-default-neutral'; return ( @@ - <button - className={clsx( - 'flex size-[76px] items-center justify-center rounded-full p-[20px] xl:size-[92px]', - { 'bg-camp-blue': stance === 'PROS' }, - { 'bg-camp-red': stance === 'CONS' }, - { 'bg-brand': boxType === 'FEEDBACK' }, - { - 'bg-default-neutral': - stance === 'NEUTRAL' && boxType !== 'FEEDBACK', - }, - )} + <button + className={clsx( + 'flex size-[76px] items-center justify-center rounded-full p-[20px] xl:size-[92px]', + bgClass, + )}
28-28: 오타로 인한 정렬 깨짐: items-cent → items-center
버튼 정렬이 틀어질 수 있습니다. 즉시 수정 권장.- className="items-cent flex size-[76px] justify-center rounded-full bg-default-black2 p-[20px] xl:size-[92px]" + className="items-center flex size-[76px] justify-center rounded-full bg-default-black2 p-[20px] xl:size-[92px]"
27-32: 조작 버튼 접근성/폼 안전성 보강(aria-label, type)
폼 내부에서도 안전하게 동작하도록 type="button"을 지정하고, 스크린리더용 라벨을 추가해주세요.- <button + <button + type="button" + aria-label="타이머 초기화" className="items-center flex size-[76px] justify-center rounded-full bg-default-black2 p-[20px] xl:size-[92px]" onClick={onReset} > @@ - <button + <button + type="button" + aria-label={isRunning ? '일시정지' : '재생'} + aria-pressed={isRunning} className={clsx( 'flex size-[76px] items-center justify-center rounded-full p-[20px] xl:size-[92px]', /* bgClass 혹은 기존 조건 */ )} onClick={() => {Also applies to: 35-60
🧹 Nitpick comments (6)
src/page/TimerPage/components/FeedbackTimer.tsx (6)
33-37: 시간 포맷팅 util 재사용으로 중복 제거
동일 로직이 util에 이미 존재합니다. 일관성과 테스트 용이성을 위해 util을 사용하세요.- const totalTime = timer ?? 0; - const minute = Formatting.formatTwoDigits( - Math.floor(Math.abs(totalTime) / 60), - ); - const second = Formatting.formatTwoDigits(Math.abs(totalTime % 60)); + const totalTime = timer ?? 0; + const { minutes, seconds } = Formatting.formatSecondsToMinutes(totalTime); + const minute = Formatting.formatTwoDigits(minutes); + const second = Formatting.formatTwoDigits(seconds);
90-98: 실시간 시간 변화를 스크린리더가 읽을 수 있도록 role/aria 속성 추가
타이머 값 변경을 보조기기가 감지하도록 role="timer", aria-live를 부여하세요.- <span + <span + role="timer" + aria-live="polite" + aria-atomic="true" className={clsx( 'flex w-full flex-row items-center justify-center space-x-[16px] p-[16px] text-[70px] font-bold tabular-nums text-default-black xl:text-[110px]', )} >
81-81: 블록 컨테이너는 div 사용 권장(span → div)
현재 span에 블록 레이아웃 클래스를 적용하고 있습니다. 시맨틱/일관성 측면에서 div가 적합합니다.- <span className="flex w-[360px] flex-col space-y-[16px] xl:min-w-[480px]"> + <div className="flex w-[360px] flex-col space-y-[16px] xl:min-w-[480px]"> @@ - </span> + </div>Also applies to: 110-110
62-77: 조절 버튼 접근성 라벨 제안
'-5분'처럼 기호가 포함된 텍스트는 스크린리더가 덜 친절할 수 있습니다. 예: aria-label="시간 5분 감소", "시간 1분 증가" 등으로 구체화 권장.
55-55: Tailwind 클래스 정렬 경고 해결
정적 분석 경고가 있습니다. Prettier Tailwind 플러그인으로 정렬하거나 클래스 순서를 정돈해 주세요. 예: "flex items-center flex-row space-x-[80px]".
33-41: adjustTime에서 defaultTimer 초과 방지를 위한 상한 클램프 추가를 고려하세요
현재 adjustTime은 음수는newTime < 0 ? 0 : newTime로 처리하지만, defaultTimer를 초과하는 값은 제한하지 않습니다. 필요 시newTime > defaultTimer ? defaultTimer : newTime로 상한 클램프 로직을 추가하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/page/TimerPage/components/CircularTimer.tsx(3 hunks)src/page/TimerPage/components/FeedbackTimer.tsx(1 hunks)src/page/TimerPage/components/TimerController.tsx(4 hunks)src/routes/routes.tsx(2 hunks)src/type/type.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/routes.tsx
- src/type/type.ts
- src/page/TimerPage/components/CircularTimer.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe#335
File: src/page/TimerPage/components/NormalTimer.tsx:50-54
Timestamp: 2025-08-08T14:21:17.745Z
Learning: Progress 퍼센트(원형 게이지)는 컴포넌트가 아닌 훅(src/page/TimerPage/hooks/useCircularTimerAnimation.ts)에서 0–100으로 clamp한다. 향후 리뷰에서는 컴포넌트 레벨에서의 중복 clamp 제안 대신 훅 사용을 권장한다.
Applied to files:
src/page/TimerPage/components/FeedbackTimer.tsx
🧬 Code graph analysis (2)
src/page/TimerPage/components/TimerController.tsx (1)
src/type/type.ts (1)
TimeBoxType(4-4)
src/page/TimerPage/components/FeedbackTimer.tsx (6)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
FeedbackTimerLogics(116-126)src/util/formatting.ts (1)
Formatting(1-12)src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
useCircularTimerAnimation(4-18)src/hooks/useBreakpoint.ts (1)
useBreakpoint(11-36)src/page/TimerPage/components/CircularTimer.tsx (1)
CircularTimer(18-65)src/page/TimerPage/components/TimerController.tsx (1)
TimerController(16-63)
🪛 GitHub Check: test
src/page/TimerPage/components/FeedbackTimer.tsx
[warning] 55-55:
Invalid Tailwind CSS classnames order
🔇 Additional comments (2)
src/page/TimerPage/components/TimerController.tsx (1)
13-14: boxType prop 추가는 역호환을 해치지 않는 안전한 확장입니다. LGTM.
기존 호출부에 영향 없이 스타일 제어만 확장되었습니다.Also applies to: 22-23
src/page/TimerPage/components/FeedbackTimer.tsx (1)
38-43: 원형 진행률 계산과 훅 클램프 사용이 적절합니다.
defaultTimer 0 분기 처리와 useCircularTimerAnimation에서의 0–100 clamp 활용이 일관됩니다. 이전 학습 노트(컴포넌트 레벨 중복 clamp 지양)와도 부합합니다. 굿.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/page/TimerPage/components/TimerController.tsx (4)
24-31: 중첩 삼항 제거로 가독성을 높이세요.간단한 매핑으로 의도를 더 분명히 표현할 수 있습니다.
- const bgClass = - boxType === 'FEEDBACK' - ? 'bg-brand' - : stance === 'PROS' - ? 'bg-camp-blue' - : stance === 'CONS' - ? 'bg-camp-red' - : 'bg-default-neutral'; + const bgClass = + boxType === 'FEEDBACK' + ? 'bg-brand' + : ( + { + PROS: 'bg-camp-blue', + CONS: 'bg-camp-red', + NEUTRAL: 'bg-default-neutral', + } as const + )[stance];
36-41: 하드코딩된 색상(#676767) 대신 토큰/필터를 사용하세요.디자인 토큰 일관성/다크모드 적응 측면에서 하드코딩 hex는 지양이 좋습니다. 간단히 brightness 필터로 대체 가능합니다.
- { 'hover:bg-[#676767]': boxType === 'FEEDBACK' }, + { 'hover:brightness-110': boxType === 'FEEDBACK' },
49-56: FEEDBACK 이외 컨텍스트에도 hover 피드백을 주는 게 좋습니다.브랜드 hover는 FEEDBACK에만 적용되어 있어, 나머지 상태에도 가벼운 시각 피드백을 주면 일관성이 좋아집니다.
- { 'hover:bg-brand-hover': boxType === 'FEEDBACK' }, + { + 'hover:bg-brand-hover': boxType === 'FEEDBACK', + 'hover:brightness-110': boxType !== 'FEEDBACK', + },
44-45: 아이콘을 보조기술에서 숨겨 접근성을 개선하세요.버튼에 aria-label이 있으므로 내부 아이콘은 aria-hidden 처리하는 것이 좋습니다.
- <DTReset className="size-full text-default-white" /> + <DTReset aria-hidden="true" className="size-full text-default-white" /> @@ - <GiPauseButton className="size-full text-default-white" /> + <GiPauseButton aria-hidden="true" className="size-full text-default-white" /> @@ - <DTPlay className="size-full translate-x-[4px] text-default-white" /> + <DTPlay aria-hidden="true" className="size-full translate-x-[4px] text-default-white" />Also applies to: 66-70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/GoToHomeButton/GoToHomeButton.tsx(1 hunks)src/page/TimerPage/components/FeedbackTimer.tsx(1 hunks)src/page/TimerPage/components/TimerController.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/GoToHomeButton/GoToHomeButton.tsx
- src/page/TimerPage/components/FeedbackTimer.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/page/TimerPage/components/TimerController.tsx (1)
src/type/type.ts (1)
TimeBoxType(4-4)
🔇 Additional comments (2)
src/page/TimerPage/components/TimerController.tsx (2)
4-4: TimeBoxType import 적절합니다.FEEDBACK 컨텍스트 반영 준비가 깔끔해요.
13-13: TimerController에 boxType 기본값 'NORMAL' 지정
다음과 같이 기본값을 설정해 undefined 분기 의존을 줄이고 타입 의도를 드러내세요:export default function TimerController({ onStart, onPause, onReset, isRunning, stance, - boxType, + boxType = 'NORMAL', }: TimerControllerProps) {FEEDBACKTimer.tsx 등 사용처에서 boxType='FEEDBACK'처럼 필요한 컨텍스트에 맞게 명시적으로 전달되었는지 확인하세요.
i-meant-to-be
left a comment
i-meant-to-be
left a comment
There was a problem hiding this comment.
확인했습니다! 제가 까먹고 빼놓은 리뷰가 하나 있어서 그것만 추가로 달아요. 복잡한 변경 사항은 아니라서 승인 남깁니다! 고생 많으셨어요 😢
| * 이미 동작중이면 재시작하지 않음 | ||
| */ | ||
| const startTimer = useCallback(() => { | ||
| if (intervalRef.current !== null || timer === null) return; |
There was a problem hiding this comment.
권장: 남은 시간이 0초일 때 타이머가 시작하지 않게 차단
| if (intervalRef.current !== null || timer === null) return; | |
| if (intervalRef.current !== null || timer === null || timer <= 0) return; |
오... 이거 제가 처음 리뷰할 때 달았던 것 같은데 누락됐었군요... 버그가 있습니다. 타이머가 0초일 때 시작 버튼을 누르면...
- 시작 버튼 클릭
- 타이머가 동작하기 시작하며 시작 버튼이 일시정지 버튼으로 바뀜
- 그리고 아주 짧은 시간 후에 타이머가 정지되며 일시정지 버튼이 시작 버튼으로 바뀜
이렇게 동작하게 됩니다. 다시 말하면, 시작해서는 안 되는 상황에서 타이머가 시작되는 문제가 있는 거죠. 아마 저렇게 수정하시면 시작 버튼을 눌러도 시작이 되지 않을 거예요. 제가 테스트해봐서 아마 문제 없을 겁니다!
There was a problem hiding this comment.
꼼꼼하게 확인해 주셔서 감사합니다 !!!!! 바로 해결했습니다 👍
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
41-41: 0초/미설정 시작 방지 가드 반영 — 좋습니다
timer === null || timer <= 0체크로 시작 버튼 비정상 동작이 차단되었습니다.
🧹 Nitpick comments (4)
src/page/TimerPage/hooks/useFeedbackTimer.ts (4)
48-66: 동일 값 setState 방지로 렌더 수 줄이기200ms 주기로 동일 초(second)가 반복 계산될 때 불필요한
setTimer호출을 피하세요.- setTimer(remainingSeconds); + setTimer((prev) => (prev === remainingSeconds ? prev : remainingSeconds));
28-34: 일시정지 시 타깃 시간도 초기화하여 상태 일관성 강화
pauseTimer에서targetTimeRef를 비워두면 다음 시작 시 의도가 더 명확해집니다.const pauseTimer = useCallback(() => { if (intervalRef.current !== null) { clearInterval(intervalRef.current); intervalRef.current = null; } + targetTimeRef.current = null; setIsRunning(false); }, []);
83-98: adjustTime가 defaultTimer까지 변경: 의도 확인 필요사용자가 시간 가감할 때 “리셋 시 원래 기본값”을 유지해야 한다면
defaultTimer변경은 surprising입니다. 요구사항에 따라 분리 고려 바랍니다.- setDefaultTimer((prevDefault) => { - const newDefault = prevDefault + amount; - return newDefault < 0 ? 0 : newDefault; - }); + // 기본값을 유지하려면 위 블록 제거설계 대안:
- UI에서만 표시 시간(
timer) 조정,defaultTimer는 초기화 기준으로 고정.- 혹은 별도 API
adjustDefault(amount)로 명시적 변경.
103-113: raw setter 노출 제거 → adjustTime/resetTimer만 노출
targetTimeRef재계산 없이setTimer/setDefaultTimer를 직접 호출하면 UI가 예상치 못하게 꼬일 수 있습니다. hook 반환값에서 raw setter를 제거하고 운영 단계에는adjustTime·resetTimer만 사용하도록 API를 축소하세요.
변경 시 아래 위치 등에서 호출 중인setTimer/setDefaultTimer를 모두adjustTime/resetTimer호출로 전환해야 합니다.
- src/page/TimerPage/FeedbackTimerPage.tsx
- src/page/TimerPage/components/CompactTimeoutTimer.tsx
- src/page/TimerPage/hooks/useTimerPageState.ts
- src/page/TimerPage/hooks/useNormalTimer.ts
- 스토리북(src/page/TimerPage/stories/FeedbackTimer.stories.tsx)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/page/TimerPage/hooks/useFeedbackTimer.ts(1 hunks)
🔇 Additional comments (1)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
100-102: 언마운트 클린업 적절 — 메모리 누수 우려 낮음
pauseTimer로 인터벌 정리하는 패턴 적절합니다.
| // 타이머에 표시되는 현재 남은 시간 | ||
| const [timer, setTimer] = useState<number | null>(null); | ||
| // 타이머 요청에 대한 ID를 저장 | ||
| const intervalRef = useRef<NodeJS.Timeout | null>(null); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
브라우저 타이핑 이슈: NodeJS.Timeout 대신 ReturnType 사용 권장
프론트엔드(브라우저) 빌드에서 NodeJS.Timeout은 @types/node 의존에 묶여 실패할 수 있습니다. 환경 독립적으로 아래처럼 교체해 주세요.
- const intervalRef = useRef<NodeJS.Timeout | null>(null);
+ const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);검증 스크립트:
🏁 Script executed:
#!/bin/bash
# node 타입 의존 여부와 NodeJS.Timeout 사용처 점검
rg -n '"types"\s*:\s*\[[^]]*node[^]]*\]' tsconfig*.json
rg -n '"@types/node"' package.json
rg -n --type=ts --type=tsx 'NodeJS\.Timeout'Length of output: 287
🏁 Script executed:
#!/bin/bash
# 전체 .ts 및 .tsx 파일에서 NodeJS.Timeout 사용 위치 검색
rg -n 'NodeJS\.Timeout' -g '*.ts' -g '*.tsx'Length of output: 366
NodeJS.Timeout → ReturnType로 교체 (3개 훅)
다음 훅들의 intervalRef 타입을
- useRef<NodeJS.Timeout | null>(null)
+ useRef<ReturnType<typeof setInterval> | null>(null)로 변경하세요.
- src/page/TimerPage/hooks/useFeedbackTimer.ts (14)
- src/page/TimerPage/hooks/useNormalTimer.ts (17)
- src/page/TimerPage/hooks/useTimeBasedTimer.ts (32)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const intervalRef = useRef<NodeJS.Timeout | null>(null); | |
| // before | |
| - const intervalRef = useRef<NodeJS.Timeout | null>(null); | |
| // after | |
| const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null); |
🤖 Prompt for AI Agents
In src/page/TimerPage/hooks/useFeedbackTimer.ts (line 14),
src/page/TimerPage/hooks/useNormalTimer.ts (line 17), and
src/page/TimerPage/hooks/useTimeBasedTimer.ts (line 32), replace the intervalRef
type from NodeJS.Timeout | null to ReturnType<typeof setInterval>; update the
useRef generics accordingly so intervalRef is declared as
useRef<ReturnType<typeof setInterval> | null>(null) in each hook to avoid
NodeJS-specific typing and ensure correct browser timer types.
| const resetTimer = useCallback(() => { | ||
| if (intervalRef.current !== null) { | ||
| clearInterval(intervalRef.current); | ||
| intervalRef.current = null; | ||
| } | ||
| setTimer(defaultTimer); | ||
| setIsRunning(false); | ||
| }, [defaultTimer]); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
resetTimer 시그니처/동작 불일치: value 인자를 무시함
인터페이스는 resetTimer(value?: number)인데 구현은 인자를 받지 않고 항상 defaultTimer로만 리셋합니다. 호출부가 값 전달 시 오작동합니다.
- const resetTimer = useCallback(() => {
+ const resetTimer = useCallback((value?: number) => {
if (intervalRef.current !== null) {
clearInterval(intervalRef.current);
intervalRef.current = null;
}
- setTimer(defaultTimer);
+ targetTimeRef.current = null;
+ setTimer(value ?? defaultTimer);
setIsRunning(false);
}, [defaultTimer]);호출부 점검:
🏁 Script executed:
#!/bin/bash
# resetTimer 인자 전달 사용처 확인
rg -n --type=ts --type=tsx '\bresetTimer\s*\('Length of output: 76
🏁 Script executed:
#!/bin/bash
# resetTimer 인자 전달 사용처 확인 (타입 필터 제거)
rg -n 'resetTimer\s*\(' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 240
🏁 Script executed:
#!/bin/bash
# 확인: useFeedbackTimer 훅 선언부 및 resetTimer 정의
rg -n 'export function useFeedbackTimer' -C3 src/page/TimerPage/hooks/useFeedbackTimer.ts
rg -n 'resetTimer' -C3 src/page/TimerPage/hooks/useFeedbackTimer.tsLength of output: 994
🏁 Script executed:
#!/bin/bash
# useFeedbackTimer 훅 전체 확인
sed -n '1,200p' src/page/TimerPage/hooks/useFeedbackTimer.tsLength of output: 2968
resetTimer 구현에 value 파라미터 반영 및 targetTimeRef 초기화 필요
src/page/TimerPage/hooks/useFeedbackTimer.ts (70–78):
- const resetTimer = useCallback(() => {
+ const resetTimer = useCallback((value?: number) => {
if (intervalRef.current !== null) {
clearInterval(intervalRef.current);
intervalRef.current = null;
}
- setTimer(defaultTimer);
+ targetTimeRef.current = null;
+ setTimer(value ?? defaultTimer);
setIsRunning(false);
}, [defaultTimer]);인터페이스에 선언된 resetTimer(value?: number) 시그니처와 동작이 불일치하므로, 구현부도 파라미터를 받아 해당 값으로 리셋하도록 수정해야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resetTimer = useCallback(() => { | |
| if (intervalRef.current !== null) { | |
| clearInterval(intervalRef.current); | |
| intervalRef.current = null; | |
| } | |
| setTimer(defaultTimer); | |
| setIsRunning(false); | |
| }, [defaultTimer]); | |
| const resetTimer = useCallback((value?: number) => { | |
| if (intervalRef.current !== null) { | |
| clearInterval(intervalRef.current); | |
| intervalRef.current = null; | |
| } | |
| targetTimeRef.current = null; | |
| setTimer(value ?? defaultTimer); | |
| setIsRunning(false); | |
| }, [defaultTimer]); |
🤖 Prompt for AI Agents
In src/page/TimerPage/hooks/useFeedbackTimer.ts around lines 70 to 78, the
resetTimer implementation ignores the optional parameter declared in the
interface and fails to reset targetTimeRef; update resetTimer to accept an
optional value?: number and use that value (or fallback to defaultTimer) when
calling setTimer, and reinitialize targetTimeRef.current to the correct future
timestamp based on the chosen timer value (e.g., Date.now() + valueMillis) after
clearing the interval and setting isRunning to false.
3 participants
🚩 연관 이슈
closed #354
📝 작업 내용
피드백 타이머
미리 구현되어 있던 CircularTimer컴포넌트와 TimerController를 이용해서 구현했습니다.
타입을 설정하고 해당되는 컬러등을 정의해서 재사용할 수 있어서 너무 좋았어요 !!!!!!!!
피드백 타이머의 경로
경로가 토론 종료 화면에서 진입하는 것이라 다음과 같이 설정했습니다. 또한 회원만 접속할 수 있는 페이지이기 때문에 requiresAuth를 true설정했습니다. id가 필요할지에 대해서는 의견 주세요!!!
피드백 타이머의 파일 위치
현재는 타이머페이지 폴더 안에 위치하고 있습니다. 피드백 타이머에서 사용되는 CircularTimer, TimerController가 전부 타이머 페이지 폴더의 내부 컴포넌트에 위치하고 있기 때문입니다. 하지만 고민되는 점이 모든 페이지 폴더 내에 대표하는 Page는 하나만 있기 때문에 일관성이 깨지고 토론 종료 화면, 투표 화면은 다 별도의 페이지 컴포넌트를 갖게 될 것이라 이 부분이 고민입니다 ! 편하게 의견 주세요. 어떻게 구성하는 것이 더 찾기 편하고 이해하기 편할지 😉
홈으로 이동하기 버튼 컴포넌트 분리
똑같은 UI의 홈으로 이동하기 버튼이 토론종료화면, 피드백화면 추후 추가될 투표화면에서도 필요할 것이라 예상했고 해당 컴포넌트를 components에 별도로 정의했는데 어떻게 생각하시는지 궁금합니다! 제가 이렇게 작성한 이유는 복잡한 코드가 아니지만 여러 페이지를 걸쳐 중복으로 작성해야 하고 추후 수정하게 된다면 3군데를 다 수정해야 하는 경우가 생긴다고 생각해서 입니다!
구현하면서 추가로 떠오른 점
피드백 타이머에서 아직 투표 기능이 구현되지 않았기 때문에 지금은 홈으로 이동하기가 맞다고 생각합니다. 하지만 피드백을 작성하고 투표로 넘어가는 경우는 없을까요? 그런 경우에는 홈으로 이동하기만 있다면 다시 투표하는 플로우까지 도달하기에 불편할 것 같다고 생각이 듭니다. 이 부분에 대해서도 인지하고 있어야 할 것 같습니다!
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
New Features
Style
Documentation