-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] 종소리 관련 오류 수정 #360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] 종소리 관련 오류 수정 #360
Conversation
WalkthroughTimerCreationContent.tsx updates bell configuration handling: initializes from initData or sessionStorage defaults, validates and sanitizes bell inputs (minutes/seconds and count), persists bell configs to sessionStorage on NORMAL timer submission, and replaces number inputs with numeric text inputs. Minor logging removed and constants added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Component as TimerCreationContent
participant Storage as sessionStorage
Note over Component: Initialization
Component->>Storage: getItem('savedBellInputConfigs')
alt Saved configs exist
Storage-->>Component: Parsed bell configs
Component->>Component: Set bells from storage
else No saved configs
alt initData.bell exists
Component->>Component: Set bells from initData.bell
else
Component->>Component: Set bells from defaultBellConfig
end
end
Note over User,Component: Editing inputs
User->>Component: Change min/sec fields
Component->>Component: getValidateTimeValue (0–59)
User->>Component: Change count field
Component->>Component: handleBellCountChange (1–3)
Note over Component,Storage: Submit (NORMAL)
User->>Component: Submit timer (NORMAL)
Component->>Storage: setItem('savedBellInputConfigs', bells)
Component->>Component: onSubmit(...)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 없음) 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (7)
71-71: 세션 스토리지 키에 네임스페이스/버전 태깅을 권장합니다.다른 기능과의 키 충돌 방지 및 향후 마이그레이션(스키마 변경)을 쉽게 하려면 키에 제품 네임스페이스와 버전을 포함하세요.
-const SAVED_BELL_CONFIGS_KEY = 'savedBellInputConfigs'; +const SAVED_BELL_CONFIGS_KEY = 'debate-timer:bells:v1';
332-332: 세션 스토리지 저장 시 예외 처리와 빈 배열 처리 정책을 명확히 해주세요.Quota/프라이빗 모드 등에서 setItem이 throw될 수 있습니다. 또한 bells가 빈 배열일 때 기존 백업을 덮어쓸지(빈으로 유지) 제거할지(디폴트로 복귀) 정책 결정을 권장합니다.
- sessionStorage.setItem(SAVED_BELL_CONFIGS_KEY, JSON.stringify(bells)); + try { + if (bells.length > 0) { + sessionStorage.setItem(SAVED_BELL_CONFIGS_KEY, JSON.stringify(bells)); + } else { + // 정책 1) 빈 배열 저장을 원치 않으면 제거하여 디폴트로 폴백 + // sessionStorage.removeItem(SAVED_BELL_CONFIGS_KEY); + // 정책 2) '빈 설정 유지'가 의도라면 아래 한 줄만 두고 remove는 제거 + sessionStorage.setItem(SAVED_BELL_CONFIGS_KEY, JSON.stringify(bells)); + } + } catch { + // 저장 실패는 무시(UX 영향 최소화) + }정책(빈 배열 유지 vs 디폴트 복귀) 방향 알려주시면 코드 제안 반영해 드리겠습니다.
434-467: 횟수 입력 처리 단순화/일관화(숫자 클램프 1–3).마지막 문자만 취하는 대신 시간 입력과 동일한 전략(숫자만 취득 → 정수화 → 범위 클램프)을 사용하면 간결하고 예측 가능한 UX가 됩니다.
- const handleBellCountChange = useCallback( - (e: React.ChangeEvent<HTMLInputElement>) => { - const value = e.target.value; - - // Backspace 대응 - if (value === '') { - setBellInput((prev) => ({ - ...prev, - count: 1, - })); - return; - } - - // 마지막 입력 문자를 가져옴 - const lastInput = value.slice(-1); - let newCount; - - if (['1', '2', '3'].includes(lastInput)) { - // 유효한 입력(1, 2, 3)이면 해당 값으로 설정 - newCount = lastInput; - } else { - // 유효하지 않은 문자(알파벳, 1~3 이외 숫자 등)가 마지막에 입력된 경우 무시 - return; - } - - setBellInput((prev) => ({ - ...prev, - count: Number(newCount), - })); - }, - [], - ); + const handleBellCountChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { + const digits = e.target.value.replace(/[^0-9]/g, ''); + const next = digits === '' ? 1 : Math.max(1, Math.min(3, parseInt(digits, 10))); + setBellInput((prev) => ({ ...prev, count: next })); + }, []);
468-477: getValidateTimeValue 범용화(문자/숫자 입력 모두 허용).타입을 확장해 재사용성을 조금 높이고 Number.isFinite로 안전성을 확보하세요.
- const getValidateTimeValue = (value: string) => { - let num = parseInt(value, 10); - if (isNaN(num)) { - num = 0; - } - - return Math.max(0, Math.min(59, num)); - }; + const getValidateTimeValue = (value: string | number) => { + const n = typeof value === 'number' ? value : parseInt(value, 10); + const num = Number.isFinite(n) ? n : 0; + return Math.max(0, Math.min(59, num)); + };
724-739: 분 입력 UX/접근성 소폭 개선 제안.maxLength로 과입력을 사전 차단하고, 스크린리더를 위해 aria-label을 추가하세요.
- <input + <input type="text" inputMode="numeric" pattern="[0-9]*" className="w-[60px] rounded-[4px] border border-default-border p-[8px]" + maxLength={2} + aria-label="종소리 분" value={bellInput.min} onChange={(e: React.ChangeEvent<HTMLInputElement>) => { const safeValue = e.target.value.replace( /[^0-9]/g, '', );
745-760: 초 입력도 동일하게 UX/접근성 개선.- <input + <input type="text" inputMode="numeric" pattern="[0-9]*" className="w-[60px] rounded-[4px] border border-default-border p-[8px]" + maxLength={2} + aria-label="종소리 초" value={bellInput.sec} onChange={(e: React.ChangeEvent<HTMLInputElement>) => { const safeValue = e.target.value.replace( /[^0-9]/g, '', );
769-774: 횟수 입력 패턴 지정 및 접근성 보강.숫자 키패드 유도는 이미 잘 하셨고, 패턴/최대 길이를 추가하면 더 견고해집니다.
- <input + <input type="text" inputMode="numeric" + pattern="[1-3]" + maxLength={1} + aria-label="종소리 횟수" className="w-[60px] rounded-[4px] border border-default-border p-[8px]" value={bellInput.count} onChange={handleBellCountChange} placeholder="횟수" />
📜 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 (1)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe#359
File: src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx:745-758
Timestamp: 2025-09-03T14:25:28.427Z
Learning: 사용자 i-meant-to-be는 PR #359에서 종소리 입력들(분, 초, 횟수 등)에 관한 리뷰를 생략하고 다른 PR에서 정리할 예정이라고 명시했다. 이런 입력 처리 로직 개선은 별도 PR로 분리하는 것을 선호한다.
🔇 Additional comments (1)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (1)
200-206: 프로퍼티 타입 및getInitialBells로직을 직접 확인해보고 싶습니다. 확인 후 다시 코멘트 드리겠습니다.
| // 이전 종소리 설정 | ||
| const rawBellConfigData = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY); | ||
| const defaultBellConfig: BellInputConfig[] = [ | ||
| { type: 'BEFORE_END', min: 0, sec: 30, count: 1 }, | ||
| { type: 'BEFORE_END', min: 0, sec: 0, count: 2 }, | ||
| ]; | ||
| const savedBellOptions: BellInputConfig[] = | ||
| rawBellConfigData === null | ||
| ? defaultBellConfig | ||
| : JSON.parse(rawBellConfigData); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON.parse 예외 미처리로 최초 렌더 크래시 위험이 있습니다.
세션 스토리지가 비정상 값(손상/수작업 편집)을 담고 있으면 JSON.parse에서 throw되어 컴포넌트가 초기 렌더 단계에서 터집니다. try/catch로 디폴트 폴백하세요.
- const rawBellConfigData = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY);
- const defaultBellConfig: BellInputConfig[] = [
+ const defaultBellConfig: BellInputConfig[] = [
{ type: 'BEFORE_END', min: 0, sec: 30, count: 1 },
{ type: 'BEFORE_END', min: 0, sec: 0, count: 2 },
];
- const savedBellOptions: BellInputConfig[] =
- rawBellConfigData === null
- ? defaultBellConfig
- : JSON.parse(rawBellConfigData);
+ let savedBellOptions: BellInputConfig[] = defaultBellConfig;
+ try {
+ const raw = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY);
+ if (raw) {
+ const parsed = JSON.parse(raw);
+ if (Array.isArray(parsed)) {
+ savedBellOptions = parsed as BellInputConfig[];
+ }
+ }
+ } catch {
+ // 손상된 값이면 안전하게 디폴트로 폴백
+ savedBellOptions = defaultBellConfig;
+ }📝 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 rawBellConfigData = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY); | |
| const defaultBellConfig: BellInputConfig[] = [ | |
| { type: 'BEFORE_END', min: 0, sec: 30, count: 1 }, | |
| { type: 'BEFORE_END', min: 0, sec: 0, count: 2 }, | |
| ]; | |
| const savedBellOptions: BellInputConfig[] = | |
| rawBellConfigData === null | |
| ? defaultBellConfig | |
| : JSON.parse(rawBellConfigData); | |
| // 이전 종소리 설정 | |
| const defaultBellConfig: BellInputConfig[] = [ | |
| { type: 'BEFORE_END', min: 0, sec: 30, count: 1 }, | |
| { type: 'BEFORE_END', min: 0, sec: 0, count: 2 }, | |
| ]; | |
| let savedBellOptions: BellInputConfig[] = defaultBellConfig; | |
| try { | |
| const raw = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY); | |
| if (raw) { | |
| const parsed = JSON.parse(raw); | |
| if (Array.isArray(parsed)) { | |
| savedBellOptions = parsed as BellInputConfig[]; | |
| } | |
| } | |
| } catch { | |
| // 손상된 값이면 안전하게 디폴트로 폴백 | |
| savedBellOptions = defaultBellConfig; | |
| } |
🤖 Prompt for AI Agents
In
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx
around lines 185 to 195, JSON.parse on sessionStorage data can throw on
corrupted values causing initial render crashes; wrap the parse in a try/catch,
and on any error fall back to defaultBellConfig (optionally remove the bad
sessionStorage item) so savedBellOptions is always a valid BellInputConfig[].
jaeml06
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| : 'time-based-timer'; | ||
|
|
||
| // 이전 종소리 설정 | ||
| const rawBellConfigData = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분을 통해서 session에 데이터를 저장해서 이전 벨 설정을 따라가는데, useState나 ref가 아니라 session스토리지를 선택하신 이유가 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 숀의 이유가 궁금합니다 !! 새로고침 후에도 유지하기 위해서라면 세션도 좋은 선택지가 될 수 있겠네요 ㅎㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 정확히는 이렇다라고 확정지어 말씀드리긴 애매하긴 한데, 만약 이 코드에서 useState나 ref를 사용하게 되면 타임박스 추가 모달이 닫혔을 때 정보가 다 사라져버릴 거라고 생각했어요. 그런데 우리는 모달 닫아도 이전 타임박스의 종소리 정보가 기억되기를 원하잖아요? 그럼 세션 스토리지에 저장하는게 현재로서는 가장 나은 선택지라고 생각했습니다.
useon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정상 작동하는 것 확인했습니다 !! 001 이렇게 입력되거나 .이 입력되는 문제가 사라지고 이제 잘 입력되네요 !!! PR도 간결하고 잘 설명해 주셔서 이해하기 편했습니다 !! 디테일 챙겨주셔서 감사해요 숀 !!! 일단 동작에는 문제 없고 질문 하나만 남겼습니다 ㅎㅎ 어프루브 ~!
| : 'time-based-timer'; | ||
|
|
||
| // 이전 종소리 설정 | ||
| const rawBellConfigData = sessionStorage.getItem(SAVED_BELL_CONFIGS_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 숀의 이유가 궁금합니다 !! 새로고침 후에도 유지하기 위해서라면 세션도 좋은 선택지가 될 수 있겠네요 ㅎㅎㅎ


🚩 연관 이슈
closed #357
📝 작업 내용
리딩 제로 문제 수정
타종 입력 중 아래 3가지에 대한 리딩 제로 문제를 수정했습니다:
분과 초의 경우
위 함수를 분과 초의
onChange로직에 포함했습니다. 일단 조건문을 통해 NaN을 검증하여 값을 확실하게 숫자로 만들고, 숫자로 정해진 값을min함수와max함수를 통해 [0, 59]에 포함되도록 강제하였습니다.타종 횟수의 경우
사용자가 입력하는 가장 최신 문자는 무조건 문자열 끝에 배치된다는 점에 착안하여, 모든 입력이 무조건 문자열의 마지막 문자 1개로 치환되도록 개선하였습니다.
글만 봐서는 이해가 어려울 수 있어 예시를 첨부하겠습니다.
e.target.value === "1"e.target.value === "12"가 됨또한, 1, 2, 3 외 다른 입력에도 대응할 수 있게 다음 검증 조건도 추가했습니다:
타종 설정 저장 버그 수정
문제
기존에는 일반 타이머 생성 후 시간 총량제 타이머를 곧바로 생성하면, 일반 타이머의 타종 설정이 사라져서 새로 처음부터 작성해야 하는 문제가 있었습니다.
원인
이는 타이머 생성 시 가장 마지막으로 생성한 타이머 의 값을 그대로 가져오기 때문입니다. 문제가 발생하는 과정은 아래와 같습니다:
해결책
세션 스토리지에 가장 마지막으로 추가한 일반 타이머의 타종 설정이 저장되도록 구현했습니다. 결론적으론 아래와 같이 동작합니다:
여담
가능한 기존의 백업-복원 로직을 최대한 활용해보려고 했으나, 그렇게 하려면 너무 수정 사항이 많아져서 세션 스토리지를 사용하는 쪽으로 가닥을 잡았습니다. 다른 제안이 있다면 언제든 편하게 의견 제시해주세요!
🏞️ 스크린샷 (선택)
없음
🗣️ 리뷰 요구사항 (선택)
없음
Summary by CodeRabbit
New Features
Bug Fixes