[FIX] 버그 수정 및 사소한 디자인 개선 #385
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough여러 컴포넌트의 UI/접근성 속성과 타이머 애니메이션/시간 조정 로직이 수정되었습니다: 홈 버튼 클래스 단순화 및 하단 고정, 헤더·도움말 버튼에 aria/title 추가, 로고 원형 크롭 해제, 팀 모달 버튼 배경 추가, CROSS_EXAM 발언 유형 추가, useCircularTimerAnimation에 isRunning 인자 추가 및 피드백 타이머 시간 업데이트 흐름 재정렬. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant C as 타이머 컴포넌트
participant H as useCircularTimerAnimation
participant A as CircularTimer
U->>C: 시작/일시정지/진행 상태 변경
C->>H: useCircularTimerAnimation(rawProgress, isRunning)
Note right of H #DDEBF7: isRunning=true → duration 0.7\nisRunning=false → duration 0
H-->>C: progressMotionValue
C->>A: progressMotionValue 전달
A-->>U: 원형 진행 애니메이션 갱신
sequenceDiagram
autonumber
actor U as 사용자
participant F as useFeedbackTimer
U->>F: adjustTime(delta)
alt isRunning === true
F-->>U: 즉시 반환 (변경 없음)
else
F->>F: newTime 계산 (prev + delta), 0 미만이면 0으로 클램프
F->>F: setTimer(newTime)
F->>F: setDefaultTimer(newTime)
F-->>U: 상태 업데이트 완료
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
83-101: 시간 동기화 로직에 잠재적 타이밍 문제가 있습니다.
adjustTime함수가defaultTimer와timer를 동기화하기 위해newTime변수를 사용하지만, 두 개의 독립적인setState호출에서 이 변수를 공유합니다.setState는 비동기적이며,setTimer콜백이 실행될 때newTime이 아직 할당되지 않았을 가능성이 있습니다.다음과 같이 수정하여 동기화를 보장하세요:
const adjustTime = useCallback( (amount: number) => { if (isRunning) { return; } - let newTime: number; - - setDefaultTimer((prevDefault) => { - newTime = prevDefault + amount; - return newTime < 0 ? 0 : newTime; - }); - - setTimer(() => { - return newTime < 0 ? 0 : newTime; - }); + setDefaultTimer((prevDefault) => { + const newTime = Math.max(0, prevDefault + amount); + setTimer(newTime); + return newTime; + }); }, [isRunning], );이렇게 하면
newTime계산이setDefaultTimer콜백 내부에서 완료되고, 즉시setTimer에 전달되어 동기화가 보장됩니다.
🧹 Nitpick comments (1)
src/components/GoToHomeButton/GoToHomeButton.tsx (1)
18-18: 버튼 내부 h1 사용 재검토 권장내비게이션 버튼 텍스트를
h1으로 마크업하는 것은 의미론적으로 부적절할 수 있습니다.h1은 페이지의 주요 제목을 나타내는 시맨틱 요소이며, 하나의 페이지에 여러h1이 존재하거나 버튼 내부에 위치하면 SEO와 스크린 리더 사용자에게 혼란을 줄 수 있습니다.다음과 같이
span으로 되돌리는 것을 권장합니다:- <h1>홈으로 돌아가기 →</h1> + <span>홈으로 돌아가기 →</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/template_logo/kondae_time.pngis excluded by!**/*.png
📒 Files selected for processing (13)
src/components/GoToHomeButton/GoToHomeButton.tsx(1 hunks)src/layout/components/header/StickyTriSectionHeader.tsx(2 hunks)src/page/DebateEndPage/DebateEndPage.tsx(3 hunks)src/page/LandingPage/components/TemplateCard.tsx(1 hunks)src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx(3 hunks)src/page/TableOverviewPage/components/TeamSelectionModal/TeamSelectionModal.tsx(1 hunks)src/page/TimerPage/FeedbackTimerPage.tsx(1 hunks)src/page/TimerPage/TimerPage.tsx(1 hunks)src/page/TimerPage/components/FeedbackTimer.tsx(1 hunks)src/page/TimerPage/components/NormalTimer.tsx(1 hunks)src/page/TimerPage/components/TimeBasedTimer.tsx(3 hunks)src/page/TimerPage/hooks/useCircularTimerAnimation.ts(1 hunks)src/page/TimerPage/hooks/useFeedbackTimer.ts(1 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/hooks/useCircularTimerAnimation.ts
🧬 Code graph analysis (6)
src/page/TimerPage/components/FeedbackTimer.tsx (1)
src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
useCircularTimerAnimation(4-21)
src/page/TimerPage/TimerPage.tsx (1)
src/components/icons/Help.tsx (1)
DTHelp(3-31)
src/page/TimerPage/components/NormalTimer.tsx (1)
src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
useCircularTimerAnimation(4-21)
src/layout/components/header/StickyTriSectionHeader.tsx (1)
src/util/accessToken.ts (1)
isLoggedIn(13-15)
src/page/TimerPage/FeedbackTimerPage.tsx (2)
src/page/TimerPage/components/FeedbackTimer.tsx (1)
FeedbackTimer(20-112)src/components/GoToHomeButton/GoToHomeButton.tsx (1)
GoToHomeButton(3-21)
src/page/TimerPage/components/TimeBasedTimer.tsx (1)
src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
useCircularTimerAnimation(4-21)
🔇 Additional comments (18)
src/page/TableOverviewPage/components/TeamSelectionModal/TeamSelectionModal.tsx (1)
163-163: 스타일 변경 확인 완료!
bg-default-white클래스 추가로 '토론 정보 수정하기' 버튼에 흰색 배경이 적용되었으며, 호버 스타일(hover:bg-default-disabled/hover)도 유지되어 PR 목표와 일치합니다.src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (4)
41-47: 새로운 발언 유형 추가가 적절합니다.
CROSS_EXAM타입이 기존 패턴과 일관되게 추가되었습니다.
54-54: 레이블 매핑이 올바릅니다.'교차 조사' 레이블이 정확하게 매핑되었습니다.
132-134: 띄어쓰기 변형을 모두 처리합니다."교차조사"와 "교차 조사" 두 가지 케이스를 처리하여 견고성을 확보했습니다.
247-247: 드롭다운 옵션에 정상적으로 추가되었습니다.
CROSS_EXAM이 선택 가능한 옵션으로 올바르게 추가되었습니다.src/page/LandingPage/components/TemplateCard.tsx (1)
36-36: 로고 원형 크롭 제거 승인
rounded-full클래스를 제거하여 사각형 또는 비원형 로고가 잘리지 않도록 수정한 것이 적절합니다.src/layout/components/header/StickyTriSectionHeader.tsx (1)
86-87: 접근성 속성 추가 승인헤더 버튼에
aria-label과title속성을 추가하여 스크린 리더 사용자와 일반 사용자 모두에게 명확한 정보를 제공하도록 개선했습니다. 로그인 상태에 따른 동적 레이블 처리도 적절합니다.Also applies to: 103-104
src/page/DebateEndPage/DebateEndPage.tsx (3)
24-24: h-screen 사용 시 뷰포트 오버플로 확인 필요컨테이너를
min-h-screen에서h-screen으로 변경했습니다. 이 경우 콘텐츠가 뷰포트 높이를 초과하면 잘리거나 스크롤이 불가능할 수 있습니다. 특히 모바일이나 작은 화면에서 카드와 버튼이 모두 표시되는지 확인이 필요합니다.다양한 뷰포트 높이(예: 모바일 landscape, 작은 태블릿)에서 레이아웃 테스트를 권장합니다.
41-41: 트랜지션 효과 추가 승인카드 버튼에
transition-all duration-300을 추가하여 호버 및 상태 변경 시 부드러운 애니메이션을 제공하도록 개선했습니다.Also applies to: 59-59
83-85: GoToHomeButton 고정 배치 승인
fixed포지셔닝으로 하단에 버튼을 고정하여 스크롤과 무관하게 일관된 위치를 제공하도록 개선했습니다. 반응형 여백(bottom-[8%]/xl:bottom-[12%])도 적절합니다.src/page/TimerPage/components/NormalTimer.tsx (1)
54-54: 타이머 애니메이션 개선 승인
useCircularTimerAnimation훅에isRunning인자를 전달하여, 타이머 정지 시 진행 바 애니메이션이 튕기는 문제를 해결했습니다. 훅 내부에서isRunning이 false일 때 duration을 0으로 설정하여 즉시 진행 상태를 반영하도록 한 것이 적절합니다.src/page/TimerPage/components/FeedbackTimer.tsx (1)
41-41: LGTM! 애니메이션 훅 호출이 올바르게 업데이트되었습니다.
isRunning인자를useCircularTimerAnimation훅에 전달하여 타이머 정지 상태에서 진행 바가 튕기는 애니메이션 문제를 해결합니다. 훅 시그니처와 일치하며 PR 목표에 부합합니다.src/page/TimerPage/TimerPage.tsx (1)
76-80: LGTM! 접근성 개선이 적절하게 적용되었습니다.도움말 버튼에
aria-label과title속성을 추가하여 스크린 리더 지원 및 툴팁을 제공합니다. 아이콘 클래스를size-full에서h-full로 변경하여DTHelp컴포넌트의aspect-[45/44]비율이 올바르게 유지되도록 합니다.src/page/TimerPage/FeedbackTimerPage.tsx (1)
22-28: LGTM! 레이아웃 개선이 적절하게 적용되었습니다.뷰포트 높이(
h-screen)를 사용하고GoToHomeButton을 고정 위치 컨테이너로 감싸 하단에 일관되게 배치합니다. 핵심 UI의 세로 중앙 정렬 및 버튼의 적응형 하단 배치 목표에 부합합니다.src/page/TimerPage/hooks/useCircularTimerAnimation.ts (1)
4-18: LGTM! 타이머 정지 상태에서의 애니메이션 제어가 올바르게 구현되었습니다.
isRunning파라미터를 추가하여 타이머 실행 상태에 따라 애니메이션 지속 시간을 조절합니다. 정지 상태에서는duration: 0으로 즉시 전환하고, 실행 중에는duration: 0.7로 부드러운 애니메이션을 적용하여 진행 바가 튕기는 문제를 해결합니다.src/page/TimerPage/components/TimeBasedTimer.tsx (3)
87-87: 타이머 상태 기반 애니메이션 제어가 올바르게 구현되었습니다.
useCircularTimerAnimation훅에isRunning파라미터를 전달하여, 타이머가 정지 상태일 때 애니메이션이 발생하지 않도록 하는 변경사항이 정확합니다. 이는 차례 전환 시 진행 바가 튕기는 문제를 해결합니다.
144-151: "전체 시간" 디스플레이의 너비 조정이 일관되게 적용되었습니다.분과 초 표시 영역의 너비를
xl:w-[100px]로 동일하게 축소하여 레이아웃을 개선했습니다. 변경사항이 대칭적으로 적용되어 시각적 일관성을 유지합니다.
164-172: "현재 시간" 디스플레이의 타이포그래피와 컨테이너 너비가 일관되게 개선되었습니다.글꼴 크기를
text-[80px]로 증가시키고 기본 브레이크포인트의 컨테이너 너비를w-[120px]로 조정하여 가독성과 레이아웃 균형을 개선했습니다. 분과 초 표시에 동일한 변경이 적용되어 시각적 일관성을 유지합니다.
jaeml06
left a comment
jaeml06
left a comment
There was a problem hiding this comment.
확인했습니다. 기능 정상적으로 동작확인했고 몇가지 간단한 피드백만 남겼습니다. approve드리겠습니다
| <div className="flex"> | ||
| <button | ||
| className="sm:text-lg sm:py-4 w-full border-[2px] border-default-disabled/hover py-3 text-lg font-semibold hover:bg-default-disabled/hover md:py-5 md:text-xl lg:py-[21px] lg:text-[22px]" | ||
| className="sm:text-lg sm:py-4 w-full border-[2px] border-default-disabled/hover bg-default-white py-3 text-lg font-semibold hover:bg-default-disabled/hover md:py-5 md:text-xl lg:py-[21px] lg:text-[22px]" |
There was a problem hiding this comment.
이것과는 다른 이야기 일 수 있으나, 제 기억에는 DialogModal을 구현해주신 것으로 알고 있는데 현재 디자인과 유사한 지점이 있는 것 같습니다. 차이점은 오른쪽 버튼의 색상에 차이가 있는데 해당 DialogModal을 변경해서 이용 가능한지 궁금하네요
There was a problem hiding this comment.
저도 DialogModal을 사용해서 현재 구현하고 있는데 버튼 색상을 변경할 수 있는 props를 추가할지 고민하고 있습니다.
There was a problem hiding this comment.
주신 의견대로 DialogModal로 대체를 시도해 봤는데요. 일단 DialogModal은 왼쪽 버튼과 오른쪽 버튼인 left와 right를 인자로 받고 있기에, TeamSelectionModal의 '동전 던지기' 버튼처럼 모달 버튼이 1개인 시나리오는 대응 불가능합니다. 또한 말씀해주신 대로 버튼 색상에 대한 변경 사항도 반영이 필요합니다.
따라서 현 단계에서는 작업해야 할 것이 꽤 있어 리팩터링을 시도하진 않겠습니다. 다만, 추후 DialogModal을 더 확장성 있게 개선하는 방안은 고려 가능할 것 같습니다. 예를 들어, 하단 버튼이 들어가는 영역에 대한 정보를 left와 right로 제공받지 않고, 그냥 JSX 컴포넌트를 그대로 인수로 받는 식으로요.
| navigate('/'); | ||
| }; | ||
|
|
||
| // "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" |
| className="button enabled neutral flex w-[492px] flex-row space-x-[12px] rounded-full p-[24px]" | ||
| > | ||
| <span>홈으로 돌아가기 →</span> | ||
| <h1>홈으로 돌아가기 →</h1> |
There was a problem hiding this comment.
h1 태그로 변경하신 이유가 있을까요? 적절한 태그 사용이 아닌 것 같습니다
There was a problem hiding this comment.
적절한 의견 주신 것 같아, 그냥 태그를 제거하고 텍스트만 넣어 두었습니다!
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 ,, 바쁘실텐데 버그 수정 및 디자인 개선 작업 정말 감사합니다 .. 꼼꼼하게 다 반영 및 PR에도 작성해 주셔서 확인하기 편했어요! dev환경에서도 확인 완료했습니다! 한 가지 피드백 타이머 부분만 확인 부탁드려요 짱입니다 ,, 😭👍
| setTimer((prevTimer) => { | ||
| const newTime = (prevTimer ?? 0) + amount; | ||
| setDefaultTimer((prevDefault) => { | ||
| newTime = prevDefault + amount; |
There was a problem hiding this comment.
2025-10-02.10.06.55.mov
현재 1분 설정 후 30초가 줄어들고 1분을 설정했을 때 1분 30초가 설정되어야 하는데 2분으로 설정되는 문제가 있습니다.
시간은 30초로 줄어들었는데 초기값에 더하는 방식이라 오류가 발생하는 것 같아요! 남은 시간에 추가 시간이 더해지는 방식으로 수정되어야 할 것 같습니다!
There was a problem hiding this comment.
이 부분 반영했습니다! 이제부터는 피드백 타이머 전체 시간이 아니라 현재 시간 기준으로 계산됩니다.
기존
- 전체 시간 1분 / 현재 시간 30초
- 1분 추가 버튼 클릭
- 전체 시간에 맞추어 전체 시간 2분 / 현재 시간 2분으로 설정
현재
- 전체 시간 1분 / 현재 시간 30초
- 1분 추가 버튼 클릭
- 현재 시간에 맞추어 전체 시간 1분 30초 / 현재 시간 1분 30초로 설정
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/GoToHomeButton/GoToHomeButton.tsx(1 hunks)src/page/TimerPage/hooks/useFeedbackTimer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/GoToHomeButton/GoToHomeButton.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/page/TimerPage/hooks/useFeedbackTimer.ts (1)
89-98: 코드 가독성 개선 제안: 초기화되지 않은 변수 패턴현재 구현은 기능적으로 올바르게 동작하며 과거 리뷰에서 지적된 버그들(null 처리, 시간 동기화)을 모두 수정했습니다. 다만
newTime변수가 선언 시 초기화되지 않고setTimer콜백 내부에서 할당된 후setDefaultTimer에서 사용되는 패턴은 코드 흐름을 파악하기 어렵게 만들 수 있습니다.더 명확한 대안으로, 값을 먼저 계산한 후 두 상태를 업데이트하는 방식을 고려해보세요:
const adjustTime = useCallback( (amount: number) => { if (isRunning) { return; } - let newTime: number; - setTimer((prevTimer) => { - newTime = (prevTimer ?? 0) + amount; - return newTime < 0 ? 0 : newTime; + const computed = (prevTimer ?? 0) + amount; + const newTime = Math.max(0, computed); + setDefaultTimer(newTime); + return newTime; }); - - setDefaultTimer(() => { - return newTime < 0 ? 0 : newTime; - }); }, [isRunning], );참고:
setDefaultTimer를setTimer콜백 내부에서 호출하는 것은 React의 상태 배치 처리와 관련하여 일반적인 패턴은 아니지만, 이 경우 두 상태가 동일한 값으로 동기화되어야 하므로 허용 가능합니다.
📜 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)
85-87: 동작 중 시간 조정 방지 로직이 올바르게 추가되었습니다.타이머가 실행 중일 때 시간 조정을 막는 early return 가드가 추가되어, 의도하지 않은 동작을 방지합니다.
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 ! 피드백 타이머 시간 추가 관련한 동작 확인했습니다 !! 꼼꼼한 반영 감사합니다 !!!
i-meant-to-be
temporarily deployed
to
DEPLOY_DEV
GitHub Actions
Inactive
🚩 연관 이슈
closed #369
📝 작업 내용
요약
title추가CircularTimer바가 살짝 튕기는 문제 수정RoundControlRow의 적응형 너비 관련 문제 수정세부 내용
피드백 타이머 시간 조정 시 오류 수정
오류 시나리오는 아래와 같습니다:
이 부분을 해결하기 위해 피드백 타이머에서 시간 조정 시, 현재 시간도 반드시 전체 시간과 동일하게 동기화되도록 하여, 현재 시간과 전체 시간에 차이가 생기는 문제를 수정했습니다.
홈으로 돌아가기 버튼 스타일 수정
피드백 타이머와 토론 종료 화면의 '홈으로 돌아가기' 버튼 스타일을 우리 디자인 컨벤션의 그것으로 수정했습니다. 정확히는, src/index.css에 지정된
button-enabled-neutral스타일을 적용했습니다.동전 던지기 모달 누락된 배경 색상 설정
동전 던지기 모달의 '토론 정보 수정하기' 버튼에 누락된 배경 색상 흰색을 설정했습니다.
신규 발언 유형으로 교차 조사 추가
제곧내.
헤더 버튼에 타이틀 추가
헤더 일부 버튼에 접근성 및 사용자의 기능 이해를 돕기 위해 aria 태그와 호버 시 뜨는 도움말을 추가했습니다.
일부 기관 로고 잘리는 문제 수정
문제가 발생한 이유는 기존 로고를 담는 컨테이너에
rounded-full이 적용되어 있어, 원형이 아닌 로고들은 모서리가 잘리기 때문이었습니다. 따라서 컨테이너의rounded-full태그를 삭제하고, 이에 더해 투명 처리가 되지 않아 어색하게 출력되었던 건국대 로고도 배경을 제거하여 자연스럽게 보이도록 개선했습니다.시간 총량제 타이머 전환 시 애니메이션 튕기는 문제 수정
2025-10-01.115610.mp4
기존 문제 상황은 아래와 같습니다:
이 현상은 순서를 전환할 때에만 발생합니다. 따라서, 순서가 전환될 때 타이머가 정지된다는 점에서 착안하여, 타이머 정지 시에는 애니메이션이 적용되지 않도록 하여 문제를 수정하였습니다.
RoundControlRow적응형 태그 문제 수정❗ Before
✅ After
지난 경희대 이감과의 UT에서 식별되었던, 시간 총량제 타이머의 현재 시간 적응형 레이아웃이 어색했던 문제를 수정했습니다.
(변경될 수 있음) 피드백 타이머, 토론 종료 화면 UI 가운데 고정
2가지 변경 사항을 적용하였습니다:
다만 이 부분 관련하여 디자인적 변경이 있을 수 있어, 일단은 알려만 드립니다. 아마 바뀌어도 수치 정도만 바뀌지 않을까 하여 미리 PR에 포함하여 올려드립니다.
🗣️ 리뷰 요구사항 (선택)
없음
Summary by CodeRabbit