[FIX] 타이머 화면의 음소거 아이콘 미반영 버그 및 NormalTimer 레이아웃 문제를 수정#442
[FIX] 타이머 화면의 음소거 아이콘 미반영 버그 및 NormalTimer 레이아웃 문제를 수정#442
Conversation
spec, plan, research, data-model, test-contracts, tasks, checklists 포함 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 팀명/토론자 각각 독립 DOM 요소 렌더링 검증 - 팀명 요소 truncate, 토론자 요소 truncate 없음 검증 - h1 text-center 클래스 존재 검증 (한글/영어 순서명) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DTDebate 아이콘 제거 - 제목 텍스트 중앙 정렬(text-center) 적용 - 팀명과 발언자를 각 줄로 분리, 중간 구분선 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- volume === 0 일 때 RiVolumeMuteFill 아이콘 표시 - 볼륨 있을 때 기존 DTVolume 아이콘 유지 - 각 아이콘에 data-testid 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 볼륨 > 0일 때 일반 아이콘 표시 확인 - 볼륨 = 0일 때 음소거 아이콘 표시 확인 - 음소거 버튼 클릭 시 아이콘 즉시 변경 확인 - 음소거 해제 시 아이콘 복원 확인 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Walkthrough고정(spec) 문서 여러 건과 TimerPage/NormalTimer의 UI 변경(헤더 음소거 아이콘 조건부 렌더링, NormalTimer의 팀명·토론자 두 줄 레이아웃), 관련 테스트 추가 및 bash 스크립트의 피처 디렉터리 탐색 로직 확장(refactor)이 포함된 변경입니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🚀 Preview 배포 완료!
|
jaeml06
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a mute icon in the header that reflects the volume state and refactors the NormalTimer component to a two-line centered layout for improved readability. It also updates the repository's bash scripts to support categorized specifications (e.g., fix, feat) and adds comprehensive documentation and TDD-based tests for the changes. Review feedback suggests improving the robustness of specification lookups for legacy branch names and notes the undocumented removal of the DTDebate icon during the UI refactor.
| local type_prefix="feat" # default for legacy branches | ||
| if [[ "$branch_name" =~ ^([a-z]+)/#[0-9]+ ]]; then | ||
| type_prefix="${BASH_REMATCH[1]}" | ||
| fi |
There was a problem hiding this comment.
The type_prefix defaults to "feat" for legacy branch names (those not following the type/#number pattern). This means if a legacy branch (e.g., 441-mute-timer-layout-fix) refers to a specification that has been moved to specs/fix/, this script will fail to locate it. To improve robustness, consider iterating through all subdirectories of specs/ to find a matching issue number if the branch name doesn't explicitly include a type prefix, similar to the logic implemented in get_current_branch.
| {item.speaker && | ||
| t(' | {{speaker}} 토론자', { speaker: t(item.speaker) })} | ||
| </p> | ||
| <span className="flex w-full max-w-[600px] flex-col items-center justify-center gap-y-[8px] xl:gap-y-[12px]"> |
There was a problem hiding this comment.
The DTDebate icon (previously located at line 84 in the old version) has been removed in this refactor. While the transition to a two-line layout improves readability for long team names, the removal of this visual element isn't explicitly mentioned in the PR description or the research document. If the icon was removed intentionally to simplify the UI, please update the documentation to confirm this. Otherwise, consider restoring it (e.g., centered above the team name) to maintain visual consistency.
![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 (2)
src/page/TimerPage/components/NormalTimer.test.tsx (1)
60-69: 두 요소 동시 표시 케이스에 구분선 검증도 추가하면 더 견고합니다.현재는 팀/토론자 존재만 확인하므로, separator(
<hr>) 존재까지 함께 검증하면 레이아웃 계약 회귀를 더 잘 잡아낼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/TimerPage/components/NormalTimer.test.tsx` around lines 60 - 69, The test case in NormalTimer.test.tsx should also assert that the visual separator is rendered when both team and speaker are present: update the test that calls renderNormalTimer('찬성', '발언자 1') to additionally query for the separator element (e.g., getByRole('separator') or getByTestId if the component uses data-testid) and expect it to be in the document; keep the existing assertions for teamEl and speakerEl and the identity check so the test verifies team/speaker DOM nodes and the presence of the <hr> separator produced by the NormalTimer rendering path.src/page/TimerPage/components/NormalTimer.tsx (1)
94-96: 발언자 줄의 긴 무공백 문자열 대비 줄바꿈 유틸 클래스 추가를 고려해 주세요.현재는 truncate를 제거한 의도가 맞지만, 공백 없는 긴 문자열에서 가로 overflow 가능성이 있어
break-words(또는break-all)를 추가하면 UI 안정성이 더 좋아집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/TimerPage/components/NormalTimer.tsx` around lines 94 - 96, In the NormalTimer component, update the speaker <p> element (the one with className "w-full min-w-0 text-center text-[20px] xl:text-[28px]") to include a word-breaking utility such as "break-words" (or "break-all" if you prefer more aggressive breaking) so long no-space speaker strings cannot overflow; just add the chosen class to that className list to ensure safe wrapping for very long tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/fix/441-mute-timer-layout-fix/test-contracts/NormalTimer.md`:
- Around line 28-30: The example inputs use teamName values that include the
suffix "팀" (e.g., teamName = '찬성 팀'), which will double-render when the
component outputs "{{team}} 팀" and produce "찬성 팀 팀"; update the examples to pass
the raw team identifier (e.g., teamName = '찬성') so that the component's
rendering rule ({{team}} 팀) produces the correct display, and ensure all rows
that reference teamName and speaker in this spec use the raw team name format
consistently.
In `@src/page/TimerPage/TimerPage.test.tsx`:
- Around line 48-81: Add a new test in TimerPage.test.tsx that covers the
missing path of lowering the VolumeBar's range input to 0: render the page with
renderTimerPage(), open the VolumeBar by clicking the volume button
(getByRole('button', { name: '볼륨 조절' })), query the range input (e.g., by role
'slider' or by label used in VolumeBar), fire a change/input event to set its
value to '0', then assert the header icon updates to muted (expect
getByTestId('volume-icon-muted') to be present) and localStorage or component
state reflects 0; reference existing helpers and queries used in the file
(renderTimerPage, volume button selectors, and test ids
'volume-icon-muted'/'volume-icon-normal') when adding the test.
---
Nitpick comments:
In `@src/page/TimerPage/components/NormalTimer.test.tsx`:
- Around line 60-69: The test case in NormalTimer.test.tsx should also assert
that the visual separator is rendered when both team and speaker are present:
update the test that calls renderNormalTimer('찬성', '발언자 1') to additionally
query for the separator element (e.g., getByRole('separator') or getByTestId if
the component uses data-testid) and expect it to be in the document; keep the
existing assertions for teamEl and speakerEl and the identity check so the test
verifies team/speaker DOM nodes and the presence of the <hr> separator produced
by the NormalTimer rendering path.
In `@src/page/TimerPage/components/NormalTimer.tsx`:
- Around line 94-96: In the NormalTimer component, update the speaker <p>
element (the one with className "w-full min-w-0 text-center text-[20px]
xl:text-[28px]") to include a word-breaking utility such as "break-words" (or
"break-all" if you prefer more aggressive breaking) so long no-space speaker
strings cannot overflow; just add the chosen class to that className list to
ensure safe wrapping for very long tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c88f9ce0-2b42-4473-bf75-5a11c7b3dee5
📒 Files selected for processing (13)
.specify/scripts/bash/common.shspecs/fix/441-mute-timer-layout-fix/checklists/requirements.mdspecs/fix/441-mute-timer-layout-fix/data-model.mdspecs/fix/441-mute-timer-layout-fix/plan.mdspecs/fix/441-mute-timer-layout-fix/research.mdspecs/fix/441-mute-timer-layout-fix/spec.mdspecs/fix/441-mute-timer-layout-fix/tasks.mdspecs/fix/441-mute-timer-layout-fix/test-contracts/NormalTimer.mdspecs/fix/441-mute-timer-layout-fix/test-contracts/TimerPage-mute-icon.mdsrc/page/TimerPage/TimerPage.test.tsxsrc/page/TimerPage/TimerPage.tsxsrc/page/TimerPage/components/NormalTimer.test.tsxsrc/page/TimerPage/components/NormalTimer.tsx
specs/fix/441-mute-timer-layout-fix/test-contracts/NormalTimer.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.specify/scripts/bash/common.sh (2)
43-43: SC2155: 선언과 할당을 분리하면 에러 마스킹을 방지할 수 있습니다.Line 40의
type_name처리와 일관성을 위해, 명령 실패 시 반환값이 마스킹되지 않도록 분리하는 것이 좋습니다.♻️ 제안된 수정
- local dirname=$(basename "$dir") + local dirname + dirname=$(basename "$dir")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specify/scripts/bash/common.sh at line 43, The assignment local dirname=$(basename "$dir") masks command failures per SC2155; change it to split declaration and assignment so errors aren't hidden by using separate steps: first declare the local variable (local dirname) then assign it (dirname=$(basename "$dir")), and apply the same pattern used for type_name to keep behavior consistent and ensure any failure in basename is not masked.
138-138: SC2155: 선언과 할당 분리 권장
extract_issue_number나printf실패 시 에러 코드가local선언의 성공(0)으로 마스킹될 수 있습니다. 중요한 경로이므로 분리를 고려해 주세요.♻️ 제안된 수정
# Extract issue number from branch name - local issue_num=$(extract_issue_number "$branch_name") + local issue_num + issue_num=$(extract_issue_number "$branch_name") if [[ -z "$issue_num" ]]; then # If no issue number found, fall back to exact match under specs/{type}/ echo "$specs_dir/$branch_name" return fi # Zero-pad to 3 digits for matching - local padded=$(printf "%03d" "$((10#$issue_num))") + local padded + padded=$(printf "%03d" "$((10#$issue_num))")Also applies to: 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specify/scripts/bash/common.sh at line 138, Split the declaration and assignment to avoid masking failures: replace lines like local issue_num=$(extract_issue_number "$branch_name") with a two-step form (declare local issue_num; issue_num="$(extract_issue_number "$branch_name")" or local issue_num; issue_num="$(printf ... )" where printf is used) so that a non-zero exit from extract_issue_number or printf is not hidden by the local declaration; apply the same change for the other occurrence referenced (line 147) and ensure you quote command substitutions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.specify/scripts/bash/common.sh:
- Line 43: The assignment local dirname=$(basename "$dir") masks command
failures per SC2155; change it to split declaration and assignment so errors
aren't hidden by using separate steps: first declare the local variable (local
dirname) then assign it (dirname=$(basename "$dir")), and apply the same pattern
used for type_name to keep behavior consistent and ensure any failure in
basename is not masked.
- Line 138: Split the declaration and assignment to avoid masking failures:
replace lines like local issue_num=$(extract_issue_number "$branch_name") with a
two-step form (declare local issue_num; issue_num="$(extract_issue_number
"$branch_name")" or local issue_num; issue_num="$(printf ... )" where printf is
used) so that a non-zero exit from extract_issue_number or printf is not hidden
by the local declaration; apply the same change for the other occurrence
referenced (line 147) and ensure you quote command substitutions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8bdfd077-dbf0-4b87-9126-cfdea0a6b10c
📒 Files selected for processing (3)
.specify/scripts/bash/common.shspecs/fix/441-mute-timer-layout-fix/test-contracts/NormalTimer.mdsrc/page/TimerPage/TimerPage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/page/TimerPage/TimerPage.test.tsx
- specs/fix/441-mute-timer-layout-fix/test-contracts/NormalTimer.md
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
오잉 왜 치코 이 pr 프리뷰가 안열리는걸까요??? 그리고 이렇게 정의한 문서들이 많이 남는데 남겨두는 것이 좋을지 고민되네용 .. 다들 어떻게 생각하시는지도 궁금합니다!
PR 프리뷰가 제거 없을 떄, 만들어진것 같은데 어떻게 이용하는 것이가요? 제 화경에서는 local에서는 열리긴해요. |
2 participants
🚩 연관 이슈
closed #441
📝 작업 내용
이 PR은 타이머 화면의 음소거 아이콘 미반영 버그 및 NormalTimer 레이아웃 문제를 수정합니다.
🔇 US1: 음소거 아이콘 미반영 수정
음소거 상태가 변경되어도 헤더의 볼륨 아이콘이 업데이트되지 않던 버그를 수정합니다.
TimerPage헤더 볼륨 아이콘이 음소거 상태(isMuted)를 반영하도록 변경VolumeX, OFF 시Volume2아이콘으로 전환📐 US2·US3: NormalTimer 발언자/팀 정보 레이아웃 개선
최대 글자수 확대 이후 발언자·팀 정보 한 줄 표시 시 발생하던 정렬 불일치와 잘림 문제를 개선합니다.
🧪 테스트 추가
TimerPage음소거 아이콘 동작 테스트 (US1)NormalTimer두 줄 레이아웃 및 순서명 정렬 테스트 (US2·US3)📄 문서
specs/fix/441-mute-timer-layout-fix/스펙 및 설계 문서 추가🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
버그 수정
문서
테스트