[FIX] 발언자 길이 사전검증하기 #405
[FIX] 발언자 길이 사전검증하기 #405
Conversation
Walkthrough발언자 최대 길이 상수 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
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: CodeRabbit 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: 0
🧹 Nitpick comments (1)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (1)
310-312: 발언자 5자 검증은 맞지만, 입력이 비활성/미노출일 때는 건너뛰는 편이 좋습니다.
speaker.trim().length > 5체크 자체는 요구사항에 맞고, 공백 제거 후 길이 기준도 합리적입니다. 다만 이 검증이 타이머 종류/발언 유형/팀 상태와 무관하게 항상 실행되어서:
timerType === 'TIME_BASED'인 경우처럼 화면에 발언자 입력이 아예 없을 때,- 혹은
stance === 'NEUTRAL'또는currentSpeechType === 'TIMEOUT'인 경우처럼 입력이 disabled 인 상태에서,과거에 저장돼 있던 값이 5자를 넘으면 사용자가 수정할 수 없는 필드 때문에 경고를 보게 되는 엣지 케이스가 생길 수 있습니다.
실제 API로 전송되는 값은 NORMAL 타이머이면서 비-NEUTRAL, 비-TIMEOUT일 때만 의미가 있으니, 아래처럼 조건을 좁혀주는 걸 추천합니다.
- let speechTypeToSend: string; - let stanceToSend: Stance; - if (speaker.trim().length > 5) { - errors.push('발언자는 최대 5자까지 입력할 수 있습니다.'); - } + let speechTypeToSend: string; + let stanceToSend: Stance; + const trimmedSpeaker = speaker.trim(); + if ( + timerType === 'NORMAL' && + stance !== 'NEUTRAL' && + currentSpeechType !== 'TIMEOUT' && + trimmedSpeaker.length > 5 + ) { + errors.push('발언자는 최대 5자까지 입력할 수 있습니다.'); + }이렇게 하면 실제로 발언자 정보를 사용하는 경우에만 길이 검증이 동작해서, 사용자 입장에서도 더 직관적인 동작이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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로 분리하는 것을 선호한다.
🔇 Additional comments (1)
src/page/TableComposition/components/TimerCreationContent/TimerCreationContent.tsx (1)
602-603: 입력 단계 5자 제한 구현이 요구사항에 부합합니다.
speaker입력에maxLength={5}를 추가하고, 플레이스홀더를 간단한"N번"으로 바꾼 구성은 “최대 5자” 요구사항에 맞게 잘 정리된 것 같습니다. HTMLinput의maxLength속성은 키보드 입력·붙여넣기 모두에서 길이를 제한하므로,ClearableInput이 이 prop을 내부<input>에 그대로 전달하고 있다면:
- 입력 시점에서 5자 초과 입력이 물리적으로 차단되고,
handleSubmit쪽의speaker.trim().length > 5검증과 함께 API 에러도 사전에 방지되는 구조라 일관성 있게 동작할 것으로 보입니다.한 번
ClearableInput구현에서maxLength가 정확히 전달되는지만 확인해 두면 충분해 보입니다.
i-meant-to-be
left a comment
i-meant-to-be
left a comment
There was a problem hiding this comment.
기능은 제 환경에서는 잘 동작하네요. 5글자 이상 입력하려고 하면 뒤가 잘립니다. 구현은 잘 된 것 같아요. 리뷰는 기능상 문제가 있는 건 아니고 제안 정도 수준이라서, 승인 남기고 갑니다. 고생하셨어요!
| onClear={() => setSpeaker('')} | ||
| placeholder="N번 토론자" | ||
| maxLength={5} | ||
| placeholder="N번" |
There was a problem hiding this comment.
대안: 발언자 이름 예시 구체화
발언자 이름 예시로 'N번'은 (사람마다 다를 수 있기는 한데) 다소 모호한 느낌을 유발할 수 있을 것 같아요. 아래 2가지 후보 중 하나는 어떠신가요?
- '1번' | 1번, 2번, 3번 등으로 설정 가능함을 암시
- '1번, 홍길동 등' | 반드시 번호로만 설정해야 될 필요는 없음을 암시
근데 이 부분 사람에 따라 굉장히 주관적으로 받아들일 것 같기도 해요. 그러니 의견 수렴이나 참고 정도로만 인식하고 읽어 주시면 될 것 같아요.
There was a problem hiding this comment.
이 부분은 오늘 회의에서 이야기 해보는 것도 좋을 것 같습니다.
| // SpeechType에 맞게 문자열 매핑 | ||
| let speechTypeToSend: string; | ||
| let stanceToSend: Stance; | ||
| if (speaker.trim().length > 5) { |
There was a problem hiding this comment.
대안: 숫자를 상수화
현재 발언자 최대 길이가 숫자 5로 하드코딩되어 있습니다. 여기에 더해 이 숫자 현재 코드 602번 줄에서도 maxLength={5}의 형태로 동일하게 하드코딩되어 있는데요. 이 경우 나중에 정책이 바뀌어 발언자 제한이 늘어났을 때, 하드코딩된 숫자 전부를 일일이 찾아 바꿔줘야 하는 불편함이 생길 수 있다고 생각해요. 따라서, 발언자 제한을 const val MAX_SPEAKER_LEN = 5 등으로 상수화하여 쓰면 유지 관리에 도움이 될 것 같습니다.
There was a problem hiding this comment.
확인했습니다. 이부분은 해당 파일에 상수로 선언하겠습니다. 다만 이 부분도 추후에 상수는 따라 파일로 분리하는 것이 필요해 보입니다.
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
치코 ~!! 코멘트 하나 달았는데 확인 부탁드려요 !!!! 입력 5자까지만 들어가도록 하는 동작은 확인했습니다. 감사합니다 !!
| <ClearableInput | ||
| id="speaker" | ||
| value={speaker} | ||
| onChange={(e) => setSpeaker(e.target.value)} |
There was a problem hiding this comment.
5자까지만 반영되긴 하는데 6자까지 입력이 되긴해서
setSpeaker(e.target.value.slice(0, MAX_SPEAKER_LEN))
요렇게 수정하면 Input에서도 5자까지만 입력되도록 할 수 있을 것 같아요 !
There was a problem hiding this comment.
넵 확인했습니다. 해당 의견 반영했습니다.
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
5자리에서 막히는 것 확인했습니다 ~!! 반영해 주셔서 감사합니다 ^_^!!!
🚩 연관 이슈
closed #404
📝 작업 내용
발언자 글자수 검증 로직 공통화
기존에는 currentSpeechType === 'CUSTOM' 조건문 내부에서만 speaker.trim().length > 5 검증이 실행되고 있어
커스텀 발언 유형이 아닐 경우 글자수 제한이 전혀 적용되지 않았습니다.
검증 위치를 공통 영역으로 이동하여 모든 타이머 유형에서 5자 초과 입력 시 경고가 표시되도록 수정했습니다.
ClearableInput 입력 단계에서도 글자 수 제한 적용
발언자 입력 필드에서 글자 수 제한이 실시간으로 반영되지 않아 5자 이상 입력이 가능했던 문제를 해결했습니다.
리뷰 요구사항 (선택)
한번씩 글자 수 제한이 제대로 작동하는지 확인해주세요
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.