fix: 폼 모달 입력 및 닫힘 동작 개선 - #113
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough공통 텍스트 길이 상수를 견적·리뷰 모달에 적용했습니다. 모달의 Escape 및 오버레이 닫기를 제어하는 Changes입력 및 모달 동작
검사 및 스토리 정리
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant 사용자
participant EstimateModal
participant ModalMain
participant Textarea
사용자->>EstimateModal: 텍스트 입력
EstimateModal->>Textarea: 공통 길이 제한 적용
사용자->>EstimateModal: 제출
EstimateModal->>ModalMain: dismissible=false 설정
사용자->>ModalMain: Escape 키 또는 오버레이 클릭
ModalMain-->>EstimateModal: 닫기 동작 차단
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/constants/validation.ts (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
as const를 적용하세요.경로 규칙은 상수에
as const를 사용해 리터럴 타입을 유지하도록 요구합니다.수정 예시
-export const MIN_TEXT_CONTENT_LENGTH = 10; -export const MAX_TEXT_CONTENT_LENGTH = 1000; +export const MIN_TEXT_CONTENT_LENGTH = 10 as const; +export const MAX_TEXT_CONTENT_LENGTH = 1000 as const;경로 지침에 따라 "
as const를 붙여 리터럴 타입을 유지합니다." 규칙을 적용하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/constants/validation.ts` around lines 2 - 3, Apply `as const` to both MIN_TEXT_CONTENT_LENGTH and MAX_TEXT_CONTENT_LENGTH in the validation constants so their inferred types remain numeric literals.Source: Path instructions
src/components/estimate/RejectEstimateModal.tsx (1)
98-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win최소 글자 수 안내에
MIN_TEXT_CONTENT_LENGTH를 사용하세요.검증은 공통 상수를 사용하지만 안내 문구는
10을 직접 사용합니다. 최소 길이 상수가 변경되면 입력 검증과 사용자 안내가 불일치합니다.
src/components/estimate/RejectEstimateModal.tsx#L98-L98:placeholder를`최소 ${MIN_TEXT_CONTENT_LENGTH}자 이상 입력해 주세요`로 변경하세요.src/components/estimate/SendEstimateModal.tsx#L142-L142:placeholder를`최소 ${MIN_TEXT_CONTENT_LENGTH}자 이상 입력해 주세요`로 변경하세요.경로 지침에 따라 "매직 넘버·매직 스트링을 지양합니다." 규칙을 적용하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/estimate/RejectEstimateModal.tsx` at line 98, Update the placeholder in src/components/estimate/RejectEstimateModal.tsx at lines 98-98 to interpolate the existing MIN_TEXT_CONTENT_LENGTH constant instead of hardcoding 10. Apply the same placeholder change in src/components/estimate/SendEstimateModal.tsx at lines 142-142, preserving the existing Korean message.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/estimate/RejectEstimateModal.tsx`:
- Line 98: Update the placeholder in
src/components/estimate/RejectEstimateModal.tsx at lines 98-98 to interpolate
the existing MIN_TEXT_CONTENT_LENGTH constant instead of hardcoding 10. Apply
the same placeholder change in src/components/estimate/SendEstimateModal.tsx at
lines 142-142, preserving the existing Korean message.
In `@src/lib/constants/validation.ts`:
- Around line 2-3: Apply `as const` to both MIN_TEXT_CONTENT_LENGTH and
MAX_TEXT_CONTENT_LENGTH in the validation constants so their inferred types
remain numeric literals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 84269257-0daa-4503-ba74-3f4b26d0c83c
📒 Files selected for processing (17)
eslint.config.mjssrc/components/chat/ChatRoomModal.tsxsrc/components/common/Input/Input.tsxsrc/components/common/Input/Textarea.tsxsrc/components/common/Modal/ModalMain.tsxsrc/components/estimate/RejectEstimateModal.tsxsrc/components/estimate/SendEstimateModal.tsxsrc/components/review/ReviewWriteModal.tsxsrc/lib/api/profile.tssrc/lib/constants/validation.tssrc/stories/Icons.stories.tsxsrc/stories/Modal.stories.tsxsrc/stories/Rejectestimatemodal.stories.tsxsrc/stories/ReviewWriteModal.stories.tsxsrc/stories/SelectableChip.stories.tsxsrc/stories/Sendestimatemodal.stories.tsxsrc/stories/Skeleton.stories.tsx
💤 Files with no reviewable changes (5)
- src/stories/Rejectestimatemodal.stories.tsx
- src/stories/Icons.stories.tsx
- src/stories/Sendestimatemodal.stories.tsx
- src/stories/ReviewWriteModal.stories.tsx
- src/stories/Skeleton.stories.tsx
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 공통
Modal에dismissible옵션을 추가하고 기본값을true로 유지해 기존 모달 동작에 영향을 최소화한 점이 좋았습니다. dismissible={false}일 때 ESC와 오버레이 클릭만 차단하고, 기존 X 버튼은 그대로 사용할 수 있어 PR 설명과 실제 동작이 일치합니다.- 견적 보내기, 견적 반려, 리뷰 작성, 채팅 모달에
dismissible={false}가 실제로 적용되어 의도치 않은 닫힘 방지 범위가 빠지지 않고 반영되었습니다. - 견적 보내기·반려·리뷰 textarea에서 기존 Enter 제출용
onKeyDown로직을 제거해 Enter가 본래 textarea의 줄바꿈 동작으로만 사용되도록 한 점이 좋았습니다. - 코멘트·반려 사유·리뷰의 최소/최대 길이를
MIN_TEXT_CONTENT_LENGTH,MAX_TEXT_CONTENT_LENGTH공통 상수로 분리해 동일한 10~1000자 정책을 한 곳에서 관리하도록 정리한 점이 좋았습니다. - 각 폼의
maxLength, placeholder, validation error, 글자 수 표시까지 같은 공통 상수를 사용하고 있어 검증과 UI 문구가 서로 어긋날 가능성을 줄였습니다. - 공통
Input,Textarea에서 기존에 전달된aria-describedby를 덮어쓰지 않고 오류 메시지 id를 추가로 병합한 점이 좋았습니다. - 오류가 있을 때만 생성된 id를
aria-describedby에 연결하고 실제 오류 Text에도 동일 id를 부여해 스크린리더가 입력 요소와 오류 설명의 관계를 인식할 수 있도록 한 점이 적절합니다. - ChatRoomModal에도 동일하게
dismissible={false}를 적용해 채팅 도중 ESC나 오버레이 클릭으로 대화창이 실수로 닫히는 문제를 막은 점도 작업 목적과 잘 맞습니다.
🔍 확인 및 제안
1. dismissible={false} 동작 범위
현재 구현은 useFocusTrap의 onEscape와 overlay click handler만 dismissible 조건으로 막고 있습니다.
따라서 PR에 적힌 것처럼 ESC/오버레이 클릭만 차단하고,
Modal.Close를 통한 명시적인 닫기는 그대로 유지됩니다.
의도한 동작대로 잘 구현된 것으로 보입니다.
2. textarea Enter 제출 제거
견적 보내기, 견적 반려, 리뷰 작성 세 모달 모두 기존 onKeyDown 기반 Enter 제출 로직이 제거되었습니다.
각 textarea는 더 이상 Enter를 submit trigger로 사용하지 않기 때문에
여러 줄 입력 중 실수로 제출되는 문제는 잘 막힌 것으로 보입니다.
3. 공통 길이 제한
세 입력 모두 기존 정책이 10~1000자로 동일했고,
이번에 공통 상수로 묶은 뒤 maxLength, validation, 에러 문구, 카운터까지 모두 동일 상수를 사용하고 있어 빠진 부분은 없어 보입니다.
4. aria-describedby
기존 aria-describedby가 전달되는 경우에도
기존 id + 오류 메시지 id
형태로 합쳐주고 있어 기존 접근성 설명을 잃지 않습니다.
Input, Textarea 모두 같은 방식으로 적용되어 있어 공통 컴포넌트 간 처리도 일관적입니다.
💬 To Reviewer
말씀해주신 세 가지 부분을 중점적으로 확인했습니다.
먼저 dismissible={false}는 실제 구현상 ESC와 오버레이 클릭만 차단합니다.
useFocusTrap의 onEscape를 dismissible이 true일 때만 전달하고,
overlay handler에서도 !dismissible이면 바로 return하도록 되어 있습니다.
반면 X 버튼은 기존 Modal.Close와 onClose 흐름을 그대로 사용하므로
명시적인 닫기 동작은 유지됩니다.
Input·Textarea의 aria-describedby 처리도
기존 prop을 대체하지 않고 오류 메시지 id를 함께 합치는 방식으로 구현되어 있습니다.
따라서 기존에 helper text 등 다른 설명 id가 연결되어 있던 경우에도
기존 설명 + 오류 메시지를 모두 스크린리더에서 참조할 수 있습니다.
Storybook 정적 산출물을 ESLint 검사에서 제외하는 변경도
빌드 결과물 자체를 소스 lint 대상으로 두지 않는 목적이라 적절한 방향으로 보입니다.
이번 PR에서 설명한 작업 범위를 기준으로 확인했을 때 빠진 부분은 보이지 않았습니다.
수고하셨습니다! 👍
2 participants
📋 작업 내용
폼·채팅 모달의 의도치 않은 제출 및 닫힘을 방지하고, 텍스트 입력 검증과 오류 메시지 접근성을 개선했습니다.
🔥 변경 사항
Modal의dismissible옵션 추가Input·Textarea가 오류 메시지를aria-describedby로 연결하도록 개선eslint.config.mjs의 전역 ignore 목록에storybook-static/**추가✅ 체크리스트
📷 스크린샷 (선택)
동작 및 접근성 개선으로, 별도 UI 변경 사항은 없습니다.
💬 To Reviewer
dismissible={false}는 ESC와 오버레이 클릭만 막고, X 버튼 닫힘은 유지합니다.Input·Textarea에 기존aria-describedby가 전달된 경우에도 오류 메시지 id를 함께 연결합니다.