Skip to content

Comments

모달 공통화#383

Merged
HA-SEUNG-JEONG merged 1 commit intodevelopfrom
refactor/modal
Feb 7, 2026
Merged

모달 공통화#383
HA-SEUNG-JEONG merged 1 commit intodevelopfrom
refactor/modal

Conversation

@HA-SEUNG-JEONG
Copy link
Contributor

@HA-SEUNG-JEONG HA-SEUNG-JEONG commented Feb 7, 2026

🌱 연관된 이슈

☘️ 작업 내용

  • 시급한 부분에 대해서만 공통화 진행

🍀 참고사항

스크린샷 (선택)

Summary by CodeRabbit

릴리스 노트

  • 스타일
    • 모든 모달 대화 상자의 시각적 디자인을 표준화했습니다.
    • 일관된 닫기 버튼을 모든 모달에 적용했습니다.
    • 폼과 경고 유형 모달에 맞춤형 스타일링을 추가했습니다.

@HA-SEUNG-JEONG HA-SEUNG-JEONG self-assigned this Feb 7, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 7, 2026

📝 Walkthrough

Walkthrough

Modal 컴포넌트 API를 리팩토링하여 variant prop ('alert' | 'form')을 추가하고 새로운 Modal.CloseButton 컴포넌트를 도입했습니다. 15개 이상의 modal 파일이 업데이트되어 클래스 기반 스타일링을 variant 기반 스타일링으로 변경했습니다.

Changes

Cohort / File(s) Summary
Modal 기본 컴포넌트
src/components/ui/modal/index.tsx
새로운 ModalVariant 타입('alert' | 'form'), ModalCloseButton 컴포넌트, Modal.Header/Body/Footervariant prop 추가. 각 variant별 스타일 매핑 구현.
Form 패턴 modal
src/components/modals/{account-info,add-account,create-evaluation,create-mission,discretionary-evaluation,edit-evaluation,edit-homework,edit-mission,submit-homework}-modal.tsx
Modal.Header/Body/Footervariant="form"으로 업데이트. Modal.CloseModal.CloseButton으로 교체. XIcon import 제거.
Alert 패턴 modal
src/components/modals/{admin-approve-settlement,admin-complete-settlement,admin-forced-cancellation,admin-refund-approval,delete-evaluation,delete-homework,delete-mission,delete-peer-review}-modal.tsx
Modal.Header/Body/Footervariant="alert"으로 업데이트. body 콘텐츠를 span으로 래핑하고 폰트 클래스 추가.
Multi-step modal
src/components/modals/end-group-study.tsx
여러 단계에서 variant prop 적용. Step2에서 Modal.CloseModal.CloseButton으로 변경. 다양한 단계에 variant="alert" 또는 variant="form" 적용.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Modal은 새 옷을 입고,
variant 덕분에 alert, form으로 변신!
CloseButton이 XIcon을 대신하니,
더 깔끔해진 API로 모달 춤을 춘다 ✨
열다섯 곳의 변경도 겁내지 않아,
일관된 패턴이 날개를 달아주거든!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '모달 공통화' (Modal Commonization/Unification) directly and accurately describes the main objective of the changeset, which standardizes modal components across multiple files by introducing variant-based styling and a new CloseButton component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/modal

Comment @coderabbitai help to get the list of available commands and usage tips.

@HA-SEUNG-JEONG HA-SEUNG-JEONG added the refactoring 리팩토링 label Feb 7, 2026
Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/modals/account-info-modal.tsx (1)

42-62: ⚠️ Potential issue | 🟡 Minor

Modal.Bodyvariant="form"이 누락되었습니다.

Modal.HeaderModal.Footer에는 variant="form"이 적용되어 있지만, Modal.Body에는 variant가 없습니다. 다른 form 타입 모달(예: edit-evaluation-modal.tsx)에서는 Body에도 variant를 적용하고 있어 일관성을 위해 확인이 필요합니다.

의도적으로 커스텀 스타일링을 사용한 것이라면 무시해도 됩니다.

🔧 일관성을 위한 수정 제안
-            <Modal.Body>
+            <Modal.Body variant="form">
🤖 Fix all issues with AI agents
In `@src/components/modals/end-group-study.tsx`:
- Around line 135-139: Modal.Body is missing the variant prop (Modal.Header and
Modal.Footer use variant="form"); update the Modal.Body element in
end-group-study.tsx to include variant="form" so the modal sections are
consistent (look for the Modal.Body JSX near Modal.Header and Modal.Footer and
add the variant="form" prop).
🧹 Nitpick comments (2)
src/components/ui/modal/index.tsx (2)

225-235: XIcon에 크기와 접근성 속성 추가를 권장합니다.

XIcon에 명시적인 크기와 접근성을 위한 aria-hidden 속성이 없습니다. 일관된 UI와 접근성을 위해 추가하는 것이 좋습니다.

또한 onClick은 이미 ...props에 포함되어 있으므로 명시적으로 전달할 필요가 없습니다.

♻️ 권장 수정 사항
 function ModalCloseButton({
   className,
-  onClick,
   ...props
 }: React.ComponentProps<typeof DialogPrimitive.Close> & {
   className?: string;
 }) {
   return (
     <DialogClose
       data-slot="modal-close-button"
       className={cn('cursor-pointer', className)}
-      onClick={onClick}
       {...props}
     >
-      <XIcon />
+      <XIcon size={24} aria-hidden="true" />
     </DialogClose>
   );
 }

131-134: variant 스타일 매핑을 상수로 추출하는 것을 고려해 보세요.

variantClass 객체가 ModalHeader, ModalBody, ModalFooter에 각각 정의되어 있습니다. 향후 유지보수성과 일관성을 위해 파일 상단에 공통 상수로 추출할 수 있습니다.

♻️ 선택적 리팩토링 예시
const MODAL_VARIANT_STYLES = {
  header: {
    alert: 'border-border-default flex justify-center border-b py-200',
    form: 'border-border-default flex justify-between border-b',
  },
  body: {
    alert: 'font-designer-14r text-text-default flex justify-center py-250 text-center',
    form: 'flex flex-col gap-400 px-400 py-300',
  },
  footer: {
    alert: 'flex justify-center gap-200 border-t-0 py-250',
    form: 'flex justify-end gap-100',
  },
} as const;

Also applies to: 156-160, 183-186

@HA-SEUNG-JEONG HA-SEUNG-JEONG merged commit 87b01a7 into develop Feb 7, 2026
9 checks passed
@HA-SEUNG-JEONG HA-SEUNG-JEONG deleted the refactor/modal branch February 7, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant