[FEAT] 종소리 크기 조절 기능 #403
[FEAT] 종소리 크기 조절 기능 #403
Conversation
Walkthrough타이머에 볼륨 UI와 제어 로직을 추가했습니다. CustomRangeSlider, VolumeBar, DTVolume 아이콘을 도입하고 useBellSound과 useTimerPageState에 로컬스토리지 기반 볼륨(디바운스 포함) 및 토글 UI를 추가해 TimerPage에 통합했으며 헤더·모달의 z-index를 조정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as 사용자
participant TimerPage as TimerPage(UI)
participant State as useTimerPageState
participant VolumeBar as VolumeBar(UI)
participant Slider as CustomRangeSlider
participant BellHook as useBellSound
participant Storage as LocalStorage
Note right of TimerPage#00CC66: 볼륨 토글·렌더링 흐름
User->>TimerPage: 볼륨 버튼 클릭
TimerPage->>State: toggleVolumeBar()
State-->>TimerPage: isVolumeBarOpen 변경
TimerPage->>VolumeBar: render(volume, onVolumeChange)
Note right of Slider#FFD27F: 슬라이더 상호작용
User->>Slider: 슬라이더 조작(newValue 0..10)
Slider->>VolumeBar: onValueChange(newValue)
VolumeBar->>State: setVolume(newValue)
State->>BellHook: updateVolume(newValue / 10)
BellHook->>BellHook: volumeRef.current = (0.0..1.0)
BellHook--xStorage Storage: 500ms 디바운스 후 저장
Note over BellHook,User#A3D2CA: 벨 재생 시 볼륨 적용
TimerPage->>BellHook: playBell()
BellHook->>BellHook: audio.volume = volumeRef.current
BellHook-->>User: 소리 재생
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
🧹 Nitpick comments (7)
src/components/CustomRangeSlider/CustomRangeSlider.stories.tsx (1)
1-24: CustomRangeSlider 스토리 구성이 적절하며, 몇 가지 소소한 개선 여지가 있습니다.
- 기본
meta/Story정의와args값(0~10 범위, step 1)은 실제 사용처와 잘 맞습니다.- 다만
Meta,StoryObj는 이 파일에서는 타입으로만 사용되므로, 아래처럼typeimport 로 바꾸면 번들 최적화와 다른 스토리 파일(Icon.stories.tsx)과의 일관성에 도움이 됩니다.import type { Meta, StoryObj } from '@storybook/react';
onValueChange는console.log대신 Storybook의action('onValueChange')를 쓰면, 콘솔 오염 없이 스토리 패널에서 이벤트를 추적하기가 더 편해집니다.둘 다 필수는 아니고, 추후 스토리 정리 시 반영해도 충분해 보입니다.
src/components/VolumeBar/VolumeBar.stories.tsx (1)
1-19: VolumeBar 스토리는 동작에는 문제 없고, 스토리 이름/핸들러 네이밍만 조금 정리하면 좋겠습니다.
title: 'components/VolumeBar'는 다른 스토리(Components/CustomRangeSlider,Design System/Icons)와 케이스가 달라서,'Components/VolumeBar'정도로 맞춰두면 스토리 트리 구조가 더 정돈될 것 같습니다.onVolumeChange의 인자 이름을delta대신volume정도로 맞추면, 실제 props 의미와 일치해서 코드 읽기가 더 자연스럽습니다.- 여기서도
console.log대신 Storybookaction('onVolumeChange')를 사용하는 것을 고려해 볼 수 있습니다.기능에는 영향 없는 부분이라, 여유 있을 때 정리해도 될 수준으로 보입니다.
src/page/TimerPage/TimerPage.tsx (1)
18-19: 타이머 헤더에 볼륨 컨트롤을 붙인 구조가 기존 패턴과 자연스럽게 잘 통합되었습니다.
useTimerPageState에서isVolumeBarOpen,toggleVolumeBar,volume,setVolume을 꺼내서 우측 헤더에 배치한 흐름이 기존 도움말/전체 화면 토글 패턴과 일관돼서 이해하기 쉽습니다.- 볼륨 버튼을 감싸는 컨테이너를
relative로 두고, 그 안에absolute위치의<VolumeBar>를top-[48px],-right-[60px]에 배치한 레이아웃도 코드 상으로는 문제 없어 보입니다.StickyTriSectionHeader에z-50이 추가되어 있어서 스크롤 영역과의 레이어 충돌도 잘 피할 수 있을 것 같습니다.- 다만
top-[48px],-right-[60px]같은 매직 넘버는 헤더 높이나 버튼 크기가 바뀔 때 흐트러지기 쉬우니, 디자인이 고정되면 Tailwind spacing 토큰이나 상수화로 정리해 두면 유지보수에 도움이 될 것 같습니다.- 모바일/작은 화면 환경에서 VolumeBar가 화면 밖으로 잘리지 않고 충분히 보이는지만 실제 브라우저에서 한 번만 확인해 주시면 좋겠습니다.
Also applies to: 44-47, 113-131
src/components/VolumeBar/VolumeBar.tsx (1)
1-21: VolumeBar UI 구성은 명확하고, 접근성/문서 측면에서 몇 가지 작은 개선 포인트가 있습니다.
- SVG 배경 레이어와 실제 인터랙션 레이어를 분리한 구조(배경
<svg>+z-10컨텐츠)는 시각적으로 복잡한 컴포넌트를 깔끔하게 유지하는 데 도움이 됩니다.- 상단 주석이
// Integer 1-10, step = 1로 되어 있지만, 실제로는MIN_VOLUME = 0으로 0(음소거)~10 범위를 쓰고 있으니, 주석을0~10 (0은 음소거), step=1정도로 맞춰 두면 오해를 줄일 수 있습니다.- 음소거 버튼은 아이콘만 있는 버튼이라, 나중에라도
aria-label="음소거"정도를 추가해 두면 스크린리더 사용자 입장에서도 의미가 더 명확해집니다. (현재 상위 TimerPage에서 볼륨 토글 버튼에 라벨을 달고 있긴 하지만, 이 컴포넌트 단독 사용 가능성을 생각하면 여기에도 라벨을 고려해 볼 수 있습니다.)w-[234px]고정 폭은 디자인 스펙에 맞춘 것으로 보이지만, 향후 다른 레이아웃에서도 재사용할 계획이라면className으로 width를 완전히 오버라이드할 수 있다는 점을 팀 내에서 공유해 두는 것도 좋겠습니다.기능에는 영향이 없는 부분들이라, 추후 리팩터링 타이밍에 정리해도 무방해 보입니다.
Also applies to: 35-129
src/page/TimerPage/hooks/useBellSound.ts (1)
10-43: 벨 볼륨 상태/로컬스토리지 연동과volumeRef사용 패턴이 잘 설계되었고, 안정성을 조금 더 높일 수 있는 포인트가 있습니다.
useState초기값을localStorage에서 읽어오되,typeof window !== 'undefined'체크를 넣어서 SSR 환경을 잘 고려한 점, 그리고volumeRef.current에 최신 값을 sync 해 두고playBell안에서는 ref를 참조하는 패턴은setState비동기 특성을 피하는 좋은 접근입니다. (이전 학습에서 논의했던 “setState 직후 값을 바로 쓰지 않는다” 패턴을 잘 적용하신 것 같습니다. Based on learnings, ...)- 볼륨 변경 시 500ms 디바운스로
localStorage에 저장하는useEffect도 드래그 중 불필요한 쓰기를 줄이는 용도로 적절해 보입니다.추가로 고려해 볼 수 있는 점은:
저장 값 파싱 안정성
Number(saved)가NaN이 되는 경우(손상된 값 등)를 대비해 한 번 필터링해 두면 좋습니다.const saved = localStorage.getItem(STORAGE_KEY); if (saved !== null) { const parsed = Number(saved); return Number.isFinite(parsed) ? parsed : 1.0; } return 1.0;이렇게 해 두면 잘못된 값이 들어가 있어도 최소한
NaN으로 Audio.volume 을 설정하는 상황은 피할 수 있습니다.UI 볼륨(0
10)과 실제 오디오 볼륨 스케일(01) 일관성 확인
- 이 훅에서 관리하는
volume값이audio.volume에 그대로 들어가고 있고,- TimerPage 쪽에서는
VolumeBar가0~10범위로 볼륨을 표현/제어하고 있습니다.현재 설계가
useTimerPageState단계에서0~10 ↔ 0~1변환을 해 주는 구조라면 문제 없겠지만, 혹시라도 변환 없이 그대로 연결되어 있다면 실제 오디오 볼륨이 0~1 범위를 벗어나거나, UI 슬라이더의 값과 체감 볼륨이 선형적으로 맞지 않을 수 있습니다.useTimerPageState에서:
- UI에 노출하는
volume은 0~10 스케일인지,setVolume이 이 훅으로 전달될 때는 항상 0~1 범위로 정규화되는지한 번만 확인해 주시면 좋겠습니다.
리턴 타입 변경 영향
이 훅이
{ volume, setVolume }을 새로 리턴하게 되었으니,useBellSound를 사용하는 모든 곳(현재는useTimerPageState가 유력)에서 새 API를 기준으로 타입과 사용처가 정리되었는지도 함께 확인해 주세요.위 사항들은 기능을 막는 수준은 아니고, 안정성과 유지보수성을 조금 더 올려 주는 성격이라, 이번 PR에서 함께 반영해도 좋고 추후 리팩터링 때 반영해도 괜찮아 보입니다.
Also applies to: 70-71
src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
41-52: 툴팁 노출 방식에 키보드 포커스 대응을 추가하면 좋겠습니다지금은
onMouseEnter/onMouseLeave에만 의존해서 키보드로 포커스했을 때는 값 툴팁을 볼 수 없습니다. 접근성을 위해 포커스에도 동일하게 열리고 닫히도록 처리하는 것을 권장합니다. 또한title이pointer-events-none인 Thumb에만 달려 있어서 실제 브라우저 기본 툴팁은 거의 활용되지 않습니다.<input type="range" min={min} max={max} step={step} value={value} onChange={(e) => onValueChange(Number(e.target.value))} - onMouseEnter={() => setShowTooltip(true)} - onMouseLeave={() => setShowTooltip(false)} + onMouseEnter={() => setShowTooltip(true)} + onMouseLeave={() => setShowTooltip(false)} + onFocus={() => setShowTooltip(true)} + onBlur={() => setShowTooltip(false)} className="absolute inset-0 z-10 h-full w-full cursor-pointer opacity-0" />
title={value.toString()}는 제거하거나, 필요하다면 실제 포커스 가능한input쪽으로 옮기는 것도 고려해 볼 수 있습니다.Also applies to: 58-67
src/page/TimerPage/hooks/useTimerPageState.ts (1)
21-21: 볼륨 스케일 변환 로직은 명확하지만rawVolume에 대한 클램프를 검토해 보세요
VOLUME_SCALE = 10을 기준으로0.0~1.0 → 0~10으로 변환하는 구조는 이해하기 쉽습니다. 다만 localStorage에 의도치 않게 범위를 벗어난 값이 저장된 경우rawVolume이 1보다 크거나 0보다 작을 수 있어, UI상volume값이 기대 범위를 벗어날 여지가 있습니다.간단히 UI용 계산 단계에서 한 번만 clamp 해 두면 방어적으로 더 안전해집니다(오디오 재생은 그대로 두고, UI만 정규화).
- const volume = Math.round(rawVolume * VOLUME_SCALE); + const safeRawVolume = Math.min(1, Math.max(0, rawVolume)); + const volume = Math.round(safeRawVolume * VOLUME_SCALE);또는 아예
useBellSound내부에서 localStorage에서 읽은 값을0~1로 clamp하도록 바꾸는 것도 한 가지 방향입니다(이 PR 범위를 넘어가면 후속 PR로 빼셔도 될 것 같습니다).Also applies to: 65-75
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/components/CustomRangeSlider/CustomRangeSlider.stories.tsx(1 hunks)src/components/CustomRangeSlider/CustomRangeSlider.tsx(1 hunks)src/components/VolumeBar/VolumeBar.stories.tsx(1 hunks)src/components/VolumeBar/VolumeBar.tsx(1 hunks)src/components/icons/Icon.stories.tsx(2 hunks)src/components/icons/Volume.tsx(1 hunks)src/layout/components/header/StickyTriSectionHeader.tsx(1 hunks)src/page/TimerPage/TimerPage.tsx(3 hunks)src/page/TimerPage/hooks/useBellSound.ts(3 hunks)src/page/TimerPage/hooks/useTimerPageState.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
📚 Learning: 2025-07-29T13:58:40.950Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 334
File: src/page/TimerPage/hooks/useTimeBasedTimer.ts:0-0
Timestamp: 2025-07-29T13:58:40.950Z
Learning: In React hooks using useState, when you need to use an updated state value immediately after setting it, you should return the new value from the function rather than relying on the state variable, since useState setters are asynchronous. The pattern where resetTimerForNextPhase returns the calculated value and resetAndStartTimer uses that returned value directly is a valid solution to handle React's asynchronous state updates, and apparent code duplication in such cases serves a functional purpose.
Applied to files:
src/page/TimerPage/hooks/useBellSound.ts
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 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/useTimerPageState.ts
🧬 Code graph analysis (7)
src/components/icons/Icon.stories.tsx (1)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)
src/components/VolumeBar/VolumeBar.stories.tsx (1)
src/components/VolumeBar/VolumeBar.tsx (1)
VolumeBar(18-130)
src/components/VolumeBar/VolumeBar.tsx (2)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
CustomRangeSlider(14-71)
src/components/CustomRangeSlider/CustomRangeSlider.stories.tsx (1)
src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
CustomRangeSlider(14-71)
src/page/TimerPage/hooks/useTimerPageState.ts (1)
src/page/TimerPage/hooks/useBellSound.ts (1)
useBellSound(12-71)
src/components/icons/Volume.tsx (1)
src/components/icons/IconProps.tsx (1)
IconProps(3-6)
src/page/TimerPage/TimerPage.tsx (2)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)src/components/VolumeBar/VolumeBar.tsx (1)
VolumeBar(18-130)
🔇 Additional comments (4)
src/layout/components/header/StickyTriSectionHeader.tsx (1)
23-27: 헤더 z-index 조정으로 볼륨 바 등 오버레이와의 레이어 충돌을 잘 피하고 있습니다.
sticky헤더에z-50을 추가해서 아래 컨텐츠보다 항상 위에 오도록 만든 부분이, 타이머 헤더에 붙는 VolumeBar 같은 드롭다운 UI와도 자연스럽게 동작할 것 같습니다. 코드 상으로는 부작용 없이 안전한 변경으로 보입니다.src/components/icons/Volume.tsx (1)
1-22: DTVolume 아이콘 컴포넌트 구현이 일관적이고 재사용 가능하게 잘 되어 있습니다.
IconProps를 그대로 활용하면서color기본값,className확장,...props전달까지 다른 아이콘들과 동일한 패턴으로 보이고,fill={color}로 색상 제어도 명확합니다. 추가적인 이슈는 없어 보입니다.src/components/icons/Icon.stories.tsx (1)
20-20: 새 Volume 아이콘 스토리가 기존 아이콘 스토리 패턴과 잘 맞습니다.
OnVolume스토리가 다른 아이콘 스토리들과 동일한 구조(공통 args, 렌더 래퍼, 라벨 텍스트)를 따르고 있어서 디자인 시스템 내에서 일관되게 보일 것 같습니다. 스토리 타입 정의와 props 전달에도 특별한 문제는 없어 보입니다.Also applies to: 261-271
src/page/TimerPage/hooks/useTimerPageState.ts (1)
77-81: 볼륨 바 UI 상태 및 공개 API 연결 구조가 자연스럽습니다
isVolumeBarOpen/toggleVolumeBar를 훅 내부 상태로 두고, 반환 객체와TimerPageLogics타입에 같이 노출한 구조가 일관되고 사용하기 좋아 보입니다. 현재 규모에서는toggleVolumeBar를 별도useCallback으로 감싸지 않아도 충분히 단순한 수준이라고 판단됩니다.Also applies to: 285-288, 314-317
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/VolumeBar/VolumeBar.tsx (1)
23-34: 외부에서volumeprop이 변경될 때lastVolume동기화가 누락되었습니다.현재 구현은 이전 리뷰의 제안을 부분적으로 반영했습니다:
- ✅ props에서 초기화 (line 24)
- ✅ 음소거 전 현재 볼륨 저장 (line 31)
- ❌ 외부 볼륨 변경 시 동기화 useEffect 누락
이전 세션에서 볼륨이 8로 저장되고 컴포넌트가 마운트된 후, 슬라이더를 건드리지 않고 음소거 버튼만 누르면 예상대로 8로 복원됩니다. 하지만 외부 로직이나 다른 UI에서
volumeprop을 변경하면lastVolume이 동기화되지 않아, 음소거 해제 시 잘못된 값으로 복원될 수 있습니다.이전 리뷰에서 제안된 useEffect 추가를 고려하세요:
const [lastVolume, setLastVolume] = useState(volume > 0 ? volume : 5); + + // volume이 외부에서 변경될 때 lastVolume 동기화 + useEffect(() => { + if (volume > 0) { + setLastVolume(volume); + } + }, [volume]);
🧹 Nitpick comments (1)
src/page/TimerPage/hooks/useBellSound.ts (1)
13-24: localStorage에서 로드한 볼륨 값의 범위 검증을 추가하세요.현재 구현은
Number.isFinite로 NaN만 검증하지만, Audio API의volume속성은 0-1.0 범위를 요구합니다. localStorage에 1.0을 초과하는 값(예: 5, 100)이 저장되어 있으면 브라우저가 자동으로 1.0으로 클램핑하지만, 명시적으로 범위를 검증하는 것이 더 안전합니다.다음과 같이 범위 검증을 추가할 수 있습니다:
const saved = localStorage.getItem(STORAGE_KEY); if (saved !== null) { // NaN 등의 손상된 값 검증 const parsed = Number(saved); - return Number.isFinite(parsed) ? parsed : 1.0; + return Number.isFinite(parsed) ? Math.max(0, Math.min(1.0, parsed)) : 1.0; } return 1.0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/CustomRangeSlider/CustomRangeSlider.tsx(1 hunks)src/components/VolumeBar/VolumeBar.stories.tsx(1 hunks)src/components/VolumeBar/VolumeBar.tsx(1 hunks)src/page/TimerPage/hooks/useBellSound.ts(3 hunks)src/page/TimerPage/hooks/useTimerPageState.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/CustomRangeSlider/CustomRangeSlider.tsx
- src/page/TimerPage/hooks/useTimerPageState.ts
- src/components/VolumeBar/VolumeBar.stories.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
📚 Learning: 2025-07-29T13:58:40.950Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 334
File: src/page/TimerPage/hooks/useTimeBasedTimer.ts:0-0
Timestamp: 2025-07-29T13:58:40.950Z
Learning: In React hooks using useState, when you need to use an updated state value immediately after setting it, you should return the new value from the function rather than relying on the state variable, since useState setters are asynchronous. The pattern where resetTimerForNextPhase returns the calculated value and resetAndStartTimer uses that returned value directly is a valid solution to handle React's asynchronous state updates, and apparent code duplication in such cases serves a functional purpose.
Applied to files:
src/page/TimerPage/hooks/useBellSound.ts
🧬 Code graph analysis (1)
src/components/VolumeBar/VolumeBar.tsx (2)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
CustomRangeSlider(14-72)
🔇 Additional comments (5)
src/page/TimerPage/hooks/useBellSound.ts (3)
25-29: LGTM!
volumeRef를 사용하여 클로저 내에서 최신 볼륨 값에 접근할 수 있도록 한 패턴이 적절합니다.playBell함수가 항상 최신 볼륨을 참조할 수 있습니다.
40-48: LGTM!500ms 디바운스를 적용하여 슬라이더 드래그 중 localStorage에 반복적으로 쓰는 것을 방지한 것이 좋습니다. cleanup 함수도 올바르게 구현되어 있습니다.
1-76: Volume range conversion is correctly implemented and does not require changes.The volume scaling between
VolumeBar(0-10) anduseBellSound(0-1.0) is properly handled inuseTimerPageState.ts. ThesetVolumefunction validates the input range and converts values usingVOLUME_SCALE = 10before passing tosetRawVolume. The data flow is correct:VolumeBarreceives 0-10 values →setVolumevalidates and converts →useBellSoundreceives 0-1.0 values for the Audio API.src/components/VolumeBar/VolumeBar.tsx (2)
105-117: LGTM!음소거 버튼의 접근성 속성(
title,aria-label)과 조건부 스타일링이 적절하게 구현되어 있습니다.
118-131: LGTM!슬라이더의
onValueChange핸들러가 올바르게 구현되어 있으며,value > 0일 때만lastVolume을 업데이트하여 음소거 해제 시 0으로 복원되는 버그를 방지한 것이 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/VolumeBar/VolumeBar.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 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/components/VolumeBar/VolumeBar.tsx
🧬 Code graph analysis (1)
src/components/VolumeBar/VolumeBar.tsx (2)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
CustomRangeSlider(14-72)
🔇 Additional comments (2)
src/components/VolumeBar/VolumeBar.tsx (2)
24-24: 이전 리뷰 이슈가 완전히 해결되었습니다!
lastVolume초기화 및 동기화 로직이 올바르게 구현되었습니다:
- Line 24: 초기
volume값을 활용하여lastVolume을 초기화- Line 31: 음소거 전에 현재 볼륨을
lastVolume에 저장- Lines 40-44: 외부에서
volume이 변경될 때lastVolume을 동기화하는useEffect추가이제 사용자가 슬라이더를 건드리지 않고 음소거/해제를 해도 올바른 볼륨으로 복원됩니다.
Based on learnings, 이전 PR #403의 리뷰 코멘트에서 제기된 문제가 해결되었습니다.
Also applies to: 31-31, 40-44
109-142: 접근성과 UI 구현이 잘 되어 있습니다!컨텐츠 레이어 구현이 우수합니다:
- 음소거 버튼에
title과aria-label이 상태에 따라 동적으로 설정되어 접근성이 확보되었습니다 (lines 115-116)clsx를 활용한 조건부 스타일링이 깔끔합니다 (lines 119-122)CustomRangeSlider에 올바른 props가 전달되었습니다 (lines 125-138)- Lines 131-133의
lastVolume업데이트와 lines 40-44의useEffect업데이트는 중복처럼 보일 수 있지만, 각각 슬라이더 변경과 외부 변경을 처리하기 위해 필요합니다
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/VolumeBar/VolumeBar.tsx (1)
18-22: 선택사항: volume prop 검증을 추가하면 더 견고해집니다.현재
volumeprop이 유효 범위(0-10)를 벗어난 값을 받을 경우 처리 로직이 없습니다. 상위 컴포넌트에서 이미 보장하고 있다면 문제없지만, 방어적 프로그래밍을 위해 컴포넌트 내부에서 클램핑을 고려해볼 수 있습니다.예시:
export default function VolumeBar({ volume, onVolumeChange, className = '', }: VolumeBarProps) { + // volume 값을 유효 범위로 클램핑 + const clampedVolume = Math.max(MIN_VOLUME, Math.min(MAX_VOLUME, volume)); + // 음소거 해제 시 가장 마지막의 볼륨 값을 복원하기 위함 - const [lastVolume, setLastVolume] = useState(volume > 0 ? volume : 5); + const [lastVolume, setLastVolume] = useState(clampedVolume > 0 ? clampedVolume : 5);그리고 이후
volume대신clampedVolume을 사용하면 됩니다. 다만 상위에서 이미 검증하고 있다면 중복 로직이 될 수 있으니 확인 후 판단하시면 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/VolumeBar/VolumeBar.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 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/components/VolumeBar/VolumeBar.tsx
🧬 Code graph analysis (1)
src/components/VolumeBar/VolumeBar.tsx (2)
src/components/icons/Volume.tsx (1)
DTVolume(3-22)src/components/CustomRangeSlider/CustomRangeSlider.tsx (1)
CustomRangeSlider(14-72)
🔇 Additional comments (2)
src/components/VolumeBar/VolumeBar.tsx (2)
24-44: 음소거/해제 로직이 잘 구현되었습니다.이전 리뷰에서 지적된
lastVolume초기화 및 동기화 문제가 모두 해결되었습니다:
lastVolume이volumeprop을 기반으로 올바르게 초기화됩니다.- 음소거 시 현재 볼륨을 저장한 후 0으로 설정합니다.
useEffect를 통해 외부에서 볼륨이 변경될 때lastVolume을 동기화합니다.로직이 정확하고 모든 엣지 케이스를 적절히 처리하고 있습니다.
112-124: 접근성이 잘 구현되었습니다.버튼에
title과aria-label을 제공하고 음소거 상태에 따라 동적으로 변경하여 스크린 리더 사용자 경험을 고려했습니다. 시각적 피드백도 명확합니다.
jaeml06
left a comment
jaeml06
left a comment
There was a problem hiding this comment.
넵 숀 확인했습니다. 기능은 정상적으로 작동되는 것을 확인했습니다. 하나 버그를 발견해서 확인 요청드립니다.
| import { useTimerBackground } from './useTimerBackground'; | ||
| import useFullscreen from '../../../hooks/useFullscreen'; | ||
|
|
||
| const VOLUME_SCALE = 10; |
There was a problem hiding this comment.
나중에 한번 이런 설정들을 별도의 파일로 분리하여 관리가 필요해보이는 것 같아요. 나중에 해당 설정을 찾을 때 찾기 힘들 수도 있을 것 같네요
| const { children } = props; | ||
|
|
||
| return ( | ||
| <header className="sticky top-0 h-[80px] flex-shrink-0 border-b-[3px] border-default-disabled/hover"> |
There was a problem hiding this comment.
header에 z index를 추가해서 설명문구가 header밑줄에 덮혀지는 문제가 있습니다. 다른 방식의 해결 방법이 필요해보입니다.
There was a problem hiding this comment.
이 부분 모달 Wrapper에 z-50 부여하여 해결했습니다.
일단 헤더에 z-index 부여한 배경에 대해 설명을 드려야 할 것 같습니다. 찾아보니, z-index가 지정되지 않았거나 동일한 경우, HTML 코드상 나중에 작성된 요소가 위에 올라오는 게 원인이었던 것 같습니다. 우리 코드에서 <header>와 <main>에는 둘 다 z-index가 없었고, 순서상 <main>이 <header>의 아래에 있었기 때문에, <header> 공간을 넘어가서 <main> 공간을 일부 침범해 있는 볼륨 바가 클릭이 불가능한 상황이 발생했던 거였죠. 그래서 <header>의 z-index를 명시적으로 <main>보다 높게 두어야 이 문제를 해결할 수 있었습니다.
알려주신 문제는 이처럼 헤더의 z-index가 50으로 지정되었지만, 모달 Wrapper는 z-index가 부여되지 않아, 헤더의 흰색 테두리가 모달보다 위에 올라와서 발생한 것으로 생각됩니다. 따라서 이번에는 모달과 헤더 간 경합을 조정하기 위해, 2가지 수정 사항을 적용했음을 알립니다:
- 헤더의 z-index는 50에서 30으로 감소
- 모달 Wrapper의 z-index는 없었지만 50으로 신규 부여
설명을 최대한 풀어서 쓰려고 노력했는데, 그럼에도 제 설명이 좀 미숙했던 부분이 있으시다면 이따가 통합 회의 때 말씀 남겨주세요. 이 부분 섬세하게 짚어주셔서 감사합니당 bb
There was a problem hiding this comment.
오호 이해가 너무 잘됐습니다!!! 아직 헤더랑 이렇게 겹치는 것이 모달 밖에 없는 것 같은데, 나중에 더 많아지면 z-index 순서를 명시적으로 관리하도록 하는 것도 좋은 방법일 것 같아요!
export const Z_INDEX = {
header: 100,
dropdown: 200,
toast: 500,
modal: 1000,
};
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 !! 구현한 내용 댓글과 PR에 잘 설명해 주셔서 감사해요! 몇 가지 리뷰 달았으니 편하게 확인해 주세요! 그리고 혹시 볼륨 인디케이터 이외 영역 클릭시에 볼륨 인디케이터가 닫히는 기능에 대해 어떻게 생각하시나욥?! 시간 괜찮으시면 이번 PR에 반영해 주시고 아니면 추후에 다시 논의해도 괜찮습니다 ^~^ !!!
| const { children } = props; | ||
|
|
||
| return ( | ||
| <header className="sticky top-0 h-[80px] flex-shrink-0 border-b-[3px] border-default-disabled/hover"> |
There was a problem hiding this comment.
오호 이해가 너무 잘됐습니다!!! 아직 헤더랑 이렇게 겹치는 것이 모달 밖에 없는 것 같은데, 나중에 더 많아지면 z-index 순서를 명시적으로 관리하도록 하는 것도 좋은 방법일 것 같아요!
export const Z_INDEX = {
header: 100,
dropdown: 200,
toast: 500,
modal: 1000,
};
jaeml06
left a comment
jaeml06
left a comment
There was a problem hiding this comment.
넵 확인했습니다. 후에 z-index 설정에서 헷갈릴 수 있다고 생각해서 이 부분은 나중에 분리하는 것이 좋아보이네요. 일단 나머지 의견은 썬데이가 잘 남겨주어서 이만 approve 남기겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/page/TimerPage/hooks/useTimerPageState.ts (1)
79-83:toggleVolumeBar를useCallback으로 감싸는 것을 권장합니다
toggleVolumeBar가useEffect의존성 배열(line 191)에 포함되어 있지만useCallback으로 메모이제이션되지 않았습니다. 현재 로직상 큰 문제는 없지만, 컴포넌트가 리렌더될 때마다 새 함수 참조가 생성되어useEffect가 불필요하게 재실행될 수 있습니다.🔎 useCallback 적용 제안
+ const toggleVolumeBar = useCallback(() => { + setIsVolumeBarOpen((prev) => !prev); + }, []); - const toggleVolumeBar = () => { - setIsVolumeBarOpen((prev) => !prev); - };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/page/TimerPage/TimerPage.tsxsrc/page/TimerPage/hooks/useBellSound.tssrc/page/TimerPage/hooks/useTimerPageState.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
📚 Learning: 2025-09-03T14:25:28.457Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.457Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
Applied to files:
src/page/TimerPage/hooks/useBellSound.tssrc/page/TimerPage/hooks/useTimerPageState.ts
📚 Learning: 2025-07-29T13:58:40.950Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 334
File: src/page/TimerPage/hooks/useTimeBasedTimer.ts:0-0
Timestamp: 2025-07-29T13:58:40.950Z
Learning: In React hooks using useState, when you need to use an updated state value immediately after setting it, you should return the new value from the function rather than relying on the state variable, since useState setters are asynchronous. The pattern where resetTimerForNextPhase returns the calculated value and resetAndStartTimer uses that returned value directly is a valid solution to handle React's asynchronous state updates, and apparent code duplication in such cases serves a functional purpose.
Applied to files:
src/page/TimerPage/hooks/useBellSound.ts
📚 Learning: 2025-08-08T14:21:17.745Z
Learnt from: i-meant-to-be
Repo: debate-timer/debate-timer-fe PR: 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/useTimerPageState.ts
🔇 Additional comments (11)
src/page/TimerPage/TimerPage.tsx (3)
18-19: LGTM!새로운 볼륨 관련 컴포넌트와 아이콘 import가 적절하게 추가되었습니다.
44-51: LGTM!
useTimerPageState훅에서 볼륨 관련 상태와 함수들이 명확하게 destructuring되어 사용됩니다.
113-132: LGTM!볼륨 조절 UI 구현이 적절합니다.
volumeRef를 통한 외부 클릭 감지와 조건부 렌더링이 잘 구성되어 있습니다.src/page/TimerPage/hooks/useBellSound.ts (3)
10-11: LGTM!로컬 스토리지 키를 상수로 분리한 것은 좋은 패턴입니다.
47-49: LGTM!
volumeRef를 통해 최신 볼륨 값을 Audio 인스턴스에 동기화하는 방식이 적절합니다. useEffect로 ref를 갱신하고, 새 Audio 생성 시 ref 값을 사용하여 stale closure 문제를 방지합니다.Also applies to: 54-54
60-68: LGTM!500ms 디바운싱으로 로컬 스토리지 쓰기 빈도를 제한한 구현이 적절합니다. cleanup 함수로 타이머를 정리하여 메모리 누수를 방지합니다.
src/page/TimerPage/hooks/useTimerPageState.ts (5)
22-23: LGTM!볼륨 스케일 상수를 분리한 것은 좋은 패턴입니다. 추후 팀원이 언급한 대로 설정 파일로 분리하는 것도 고려해볼 수 있습니다.
61-64: LGTM!
data?.table[index]?.bell로 optional chaining이 추가되어index범위 이탈 시 런타임 에러를 방지합니다.
66-77: LGTM!UI 스케일(0-10)과 내부 로직 스케일(0.0-1.0) 간의 변환 로직이 명확합니다. 범위 검증과 주석이 잘 작성되어 있습니다.
174-191: LGTM!외부 클릭 감지 로직이 적절하게 구현되어 있습니다. cleanup 함수로 이벤트 리스너를 제거하여 메모리 누수를 방지합니다.
306-310: LGTM!새로운 볼륨 관련 속성들이 반환 객체와 TypeScript 인터페이스에 적절하게 추가되었습니다.
Also applies to: 336-340
🚩 연관 이슈
closed #376
📝 작업 내용
종소리를 조절할 수 있는 볼륨 바를 타이머 페이지에만 한정하여 헤더에 추가했어요.
세부 구현 내용
CustomRangeSlider추가종소리 조절을 포함하여 여러 목적으로 사용할 수 있는 슬라이더를 추가했어요. 아래 매개변수로 세부 사양을 조정할 수 있어요:
종소리 훅에 볼륨 관련 개선 적용
종소리 훅
useBellSound에 볼륨 조절 관련 몇 가지 개선 사항을 적용하고 신규 기능을 추가했어요:localStorage에 저장되도록 구현특히, 슬라이더는 마우스로 클릭한 후 이리저리 끌다 보면, 로컬 저장소로 새 값을 쓰는 작업이 여러 번 반복되어 성능 저하를 유발할 수 있어요. 이를 방지하기 위해 500 ms의 디바운싱을 적용하여, 사용자가 볼륨 조절을 완전히 끝낸 후에 볼륨 쓰기 작업이 실행되도록 구현했습니다.
타이머 페이지에 볼륨 바 추가
타이머 페이지에 볼륨 바를 추가했어요. 이 볼륨 바는 종소리가 울리는 타이머 페이지에만 필요하기 때문에, 다른 페이지에서는 보이지 않도록 TimerPage.tsx에 구현했어요. 기능의 세부 사항은 아래와 같아요:
lastVolume변수에 저장하여 기억해 둠lastVolume값을 복원예를 들어, 사용자가 볼륨을 10에서 6으로 줄였다고 하면, 이 경우
lastVolume이 6으로 갱신돼요. 그리고 음소거를 할 경우 볼륨은 0이 되겠죠? 다시 음소거를 해제할 경우,lastVolume에 저장된 값이었던 6으로 복원되는 그런 시나리오입니다.🏞️ 스크린샷 (선택)
마우스 호버 시 표시되는 실제 볼륨 값
음소거 여부에 따른 음소거 버튼
음소거 X
음소거 O
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
새로운 기능
기타 변경
✏️ Tip: You can customize this high-level summary in your review settings.