-
Notifications
You must be signed in to change notification settings - Fork 1
[Refactor/#179] 모달 공통 컴포넌트화 #183
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
Conversation
빌드 및 린트 결과모든 검사 통과 🎉
|
…CHA-CALL_FRONT into refactor/#179/common-modal
holdn2
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.
처음에는 그냥 하얀색 배경 컨테이너만 컴포넌트화 하고 컨텐츠는 직접 넣는것이 좋지 않을까 생각했는데 중복되는 부분이 많은 것 같아서 Alert(버튼 하나)와 Confirm(버튼 두개)으로 나누는 것도 좋은 것 같습니다.
개인적으로 Modal.tsx와 CommonModalLayout은 하나로 합쳐도 될 것 같아요! 그냥 Modal.tsx로요. 파일 두개로 나누다보니 추후 수정 및 추가 시 두개를 번갈아가며 봐야할 것 같아서 불편해질 수도 있을 것 같네요. 혹시 이렇게 구현하신 이유가 있을까요?
추가적으로 각 모달은 폴더를 만들어서 넣어주세요!
전체적으로 좋은 코드를 구현하신 것 같아요!
|
추후 팝업으로 다른 형태가 나왔을 때, Modal.Header 등 조립해서 쓰게 하려고 처음 구상이 되었고, Alert와 Confirm으로 분리하는 과정에서 중복 코드가 너무 많아서 분리하다보니 새로운 파일이 생겼습니다. 유지보수를 위해서 CommonModalLayout은 병합하겠습니다! |
Walkthrough여러 페이지에서 개별적으로 쓰이던 모달 컴포넌트들을 공통 Modal/ConfirmModal/AlertModal로 통합하고, 각 페이지에서 해당 공통 컴포넌트를 사용하도록 기존 개별 모달 파일들을 제거하거나 교체했으며 계정 페이지용 AccountModals 래퍼를 추가했습니다. Changes
Sequence Diagram(s)(생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization 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 |
빌드 및 린트 결과일부 검사 실패
|
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: 10
🧹 Nitpick comments (7)
src/shared/components/ui/modal/Modal.tsx (2)
31-45: Body 컴포넌트의 문자열/커스텀 콘텐츠 분리 정책
Body에서 문자열이면 기본 타이포그래피(caption-m-12,whitespace-pre-wrap)를 적용하고, 그 외에는 그대로 children을 감싸는div만 내려주는 설계라, 공통 메시지 VS 완전 커스텀 UI를 명확히 나누는 데 도움이 되는 패턴 같습니다.추가로 고려해볼 수 있는 정도의 포인트는 있습니다만, 필수는 아닙니다.
- 비문자 children에도 기본 여백/레이아웃 클래스만 살짝 공통으로 주고 싶다면
cn을 사용해서 최소한의 공통 스타일을 얹는 방식을 사용할 수도 있습니다.현재도 동작/디자인 상 문제는 없어 보여서, 팀 컨벤션에 맞게 두셔도 될 것 같습니다.
90-93: Modal 정적 서브 컴포넌트 타입 명시 고려아래와 같이
Modal.Header,Modal.Body등을 정적 프로퍼티로 부착하는 패턴 자체는 런타임 상 문제없이 잘 동작합니다만, TypeScript 입장에서는Modal에 해당 프로퍼티 타입이 선언되어 있지 않아 IDE 자동완성/타입 추론에서 손해를 볼 수 있습니다.Modal.Header = Header; Modal.Title = Title; Modal.Body = Body; Modal.Footer = Footer;아래와 같이 컴포넌트 타입을 한 번 정의해 두고
Object.assign으로 합치는 패턴을 쓰면 타입까지 깔끔하게 맞출 수 있습니다.type ModalComponent = (props: ModalProps) => JSX.Element & { Header: typeof Header; Title: typeof Title; Body: typeof Body; Footer: typeof Footer; }; const ModalRoot = ({ isOpen, handleClose, children, className, title, description, footer, }: ModalProps) => { // 기존 Modal 함수 본문 }; const Modal = Object.assign(ModalRoot, { Header, Title, Body, Footer, }) as ModalComponent; export default Modal;지금 코드도 빌드는 되겠지만, 장기적으로 타입 안전성과 DX를 위해 한 번쯤 정리해 두면 좋을 것 같습니다.
TypeScript 설정에 따라 위 패턴이 더 적합한지 한 번 IDE/빌드에서 타입 에러 유무를 확인해 보시면 좋겠습니다.
src/shared/components/ui/modal/AlertModal.tsx (1)
1-37: AlertModal 구조 및 Modal 조합 방식이 명확합니다단일 버튼 alert 용도로
title/description/confirmLabel만 받도록 단순화한 설계가 명확하고, Modal의 공통 레이아웃 모드를 잘 활용하고 있습니다.
스타일을 위해 native<button>을 직접 쓰신 것도 이해되며, 추후 Button 컴포넌트에서 타이포/클래스 머지가 잘 지원되면 ConfirmModal과 함께 공용 Button 기반으로 정리해 두 컴포넌트 스타일/동작을 더 쉽게 일관성 있게 맞출 수 있을 것 같습니다.src/pages/profile-setting/ProfileSetting.tsx (1)
12-61: 회원탈퇴 ConfirmModal 적용 및 핸들러 구성이 자연스럽습니다공통
ConfirmModal로 회원탈퇴 플로우를 옮기면서, 열기/닫기 상태와handleDeleteAccount확정 액션이 잘 분리되어 있고 카피도 명확합니다.
사소한 부분이지만,handleDeleteAccount내navigate('/')는 상단에서 사용하는ROUTES.HOME와 동일한 경로로 보이니, 추후 일관성을 위해 상수를 사용하는 방식으로 맞춰 두셔도 관리 측면에서 조금 더 편할 것 같습니다.src/pages/@owner/estimate/@section/basic-info-section/NeedElectricity.tsx (1)
22-35: NEED_ELECTRICITY 순회 시 타입 캐스팅을 줄일 여지가 있습니다 (선택 사항).
Object.keys(NEED_ELECTRICITY)결과를const key = item as NeedElectricityKey;로 캐스팅하는 패턴은 동작 자체는 문제 없지만, 나중에 상수 키가 바뀌면 타입과 런타임이 어긋날 여지가 있습니다. 추후 리팩터링 여유가 생기면, 예를 들어const ELECTRICITY_OPTIONS = Object.entries(NEED_ELECTRICITY) satisfies Record<NeedElectricityKey, string>[];같은 식으로 배열 상수로 정의해 두고 그 배열을 순회하면 별도의as NeedElectricityKey캐스팅 없이도 동일한 UI를 구성할 수 있을 것 같습니다.src/pages/@owner/estimate/@section/time-section/ActiveTime.tsx (1)
21-31: TimePicker의handleChange는 바로 함수 레퍼런스를 넘겨도 될 것 같습니다 (리팩터링 제안).
handleChange={startActiveTime => updateStartActiveTime(startActiveTime)}/handleChange={endActiveTime => updateEndActiveTime(endActiveTime)}는 실제로는 동일 인자를 그대로 전달하므로, 아래처럼 함수 레퍼런스를 직접 넘기면 코드가 조금 더 단순해집니다.- <TimePicker - value={activeTime.startActiveTime} - handleChange={startActiveTime => updateStartActiveTime(startActiveTime)} - timeTitle='운영 시작' - /> - <TimePicker - value={activeTime.endActiveTime} - handleChange={endActiveTime => updateEndActiveTime(endActiveTime)} - timeTitle='운영 종료' - /> + <TimePicker + value={activeTime.startActiveTime} + handleChange={updateStartActiveTime} + timeTitle='운영 시작' + /> + <TimePicker + value={activeTime.endActiveTime} + handleChange={updateEndActiveTime} + timeTitle='운영 종료' + />src/shared/components/ui/modal/Modal.stories.tsx (1)
49-58: 향후 공통 Button 컴포넌트 사용을 고려하세요.PR 설명에서 언급하신 것처럼, 현재는 font 관련 className 병합 이슈로 인해 네이티브 button을 사용하고 계십니다. 추후 merge 설정을 추가하여 공통 Button 컴포넌트로 리팩토링하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
src/pages/@owner/account/@modal/(.)confirm-delete-modal/ConfirmDeleteModal.tsx(0 hunks)src/pages/@owner/account/@modal/(.)confirm-exit-modal/ConfirmExitModal.tsx(0 hunks)src/pages/@owner/account/@modal/(.)save-account-modal/SaveAccountModal.tsx(0 hunks)src/pages/@owner/account/AccountForm.tsx(2 hunks)src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx(1 hunks)src/pages/@owner/estimate/@section/basic-info-section/Menu.tsx(1 hunks)src/pages/@owner/estimate/@section/basic-info-section/NeedElectricity.tsx(1 hunks)src/pages/@owner/estimate/@section/basic-info-section/Price.tsx(1 hunks)src/pages/@owner/estimate/@section/basic-info-section/RegionSection.tsx(1 hunks)src/pages/@owner/estimate/@section/basic-info-section/index.tsx(1 hunks)src/pages/@owner/estimate/@section/index.ts(1 hunks)src/pages/@owner/estimate/@section/time-section/ActiveDate.tsx(1 hunks)src/pages/@owner/estimate/@section/time-section/ActiveTime.tsx(1 hunks)src/pages/@owner/estimate/@section/time-section/index.tsx(1 hunks)src/pages/@owner/estimate/Estimate.tsx(1 hunks)src/pages/@owner/estimate/constants/estimate.ts(1 hunks)src/pages/@owner/estimate/hooks/use-estimate.ts(1 hunks)src/pages/@owner/estimate/hooks/use-time.ts(1 hunks)src/pages/@owner/estimate/utils/estimate.schema.ts(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckDescription.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckName.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckOperatingInfo.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckOption.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckPhoneNumber.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckPhoto.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/category-section/AvailableQuantity.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/category-section/MenuCategory.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/category-section/NeedElectricity.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/category-section/PaymentMethod.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/menu-section/MenuInfo.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/region-section/RegionSection.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/time-section/ActiveDate.tsx(1 hunks)src/pages/@owner/food-truck-form/@section/time-section/ActiveTime.tsx(1 hunks)src/pages/@owner/food-truck-form/hooks/use-time.ts(1 hunks)src/pages/@owner/food-truck-form/utils/is-date-over-lapping.ts(1 hunks)src/pages/@owner/food-truck-form/utils/use-food-truck-form.ts(1 hunks)src/pages/@owner/food-truck-management/@modal/(.)delete-food-truck-confirm-modal/DeleteFoodTruckConfirmModal.tsx(0 hunks)src/pages/@owner/food-truck-management/FoodTruckManagement.tsx(2 hunks)src/pages/@owner/message-list/@modal/(.)confirm-delete-modal/ConfirmExitModal.tsx(0 hunks)src/pages/@owner/message-list/@modal/(.)confirm-modal/ConfirmModal.tsx(0 hunks)src/pages/@owner/message-list/MessageList.tsx(2 hunks)src/pages/@owner/message-list/components/MessageForm.tsx(2 hunks)src/pages/chat-list/ChatList.tsx(2 hunks)src/pages/home/Home.tsx(2 hunks)src/pages/profile-setting/@modal/(.)delete-account-modal/DeleteAccountModal.tsx(0 hunks)src/pages/profile-setting/ProfileSetting.tsx(2 hunks)src/router/constant/routes.ts(1 hunks)src/router/router.tsx(2 hunks)src/shared/components/ui/modal/AlertModal.stories.tsx(1 hunks)src/shared/components/ui/modal/AlertModal.tsx(1 hunks)src/shared/components/ui/modal/ConfirmModal.stories.tsx(1 hunks)src/shared/components/ui/modal/ConfirmModal.tsx(1 hunks)src/shared/components/ui/modal/Modal.stories.tsx(1 hunks)src/shared/components/ui/modal/Modal.tsx(1 hunks)
💤 Files with no reviewable changes (7)
- src/pages/@owner/message-list/@modal/(.)confirm-delete-modal/ConfirmExitModal.tsx
- src/pages/@owner/account/@modal/(.)confirm-exit-modal/ConfirmExitModal.tsx
- src/pages/profile-setting/@modal/(.)delete-account-modal/DeleteAccountModal.tsx
- src/pages/@owner/account/@modal/(.)save-account-modal/SaveAccountModal.tsx
- src/pages/@owner/account/@modal/(.)confirm-delete-modal/ConfirmDeleteModal.tsx
- src/pages/@owner/food-truck-management/@modal/(.)delete-food-truck-confirm-modal/DeleteFoodTruckConfirmModal.tsx
- src/pages/@owner/message-list/@modal/(.)confirm-modal/ConfirmModal.tsx
🧰 Additional context used
🧬 Code graph analysis (24)
src/pages/@owner/estimate/hooks/use-estimate.ts (2)
src/pages/@owner/estimate/utils/estimate.schema.ts (2)
EstimateFormData(51-51)estimateSchema(13-49)src/pages/@owner/estimate/hooks/use-time.ts (1)
useEstimateTime(15-179)
src/pages/home/Home.tsx (1)
src/shared/components/ui/modal/AlertModal.tsx (1)
AlertModal(11-37)
src/pages/@owner/estimate/@section/basic-info-section/Price.tsx (4)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/input/Input.tsx (1)
Input(10-52)src/shared/utils/price-formatter.ts (1)
formatPrice(1-5)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/shared/components/ui/modal/AlertModal.stories.tsx (2)
src/shared/components/ui/modal/AlertModal.tsx (1)
AlertModal(11-37)src/shared/components/ui/modal/ConfirmModal.stories.tsx (1)
Default(34-41)
src/router/router.tsx (2)
src/router/constant/routes.ts (1)
ROUTES(1-28)src/pages/@owner/estimate/Estimate.tsx (1)
Estimate(17-97)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
src/shared/components/ui/modal/Modal.tsx (1)
Modal(51-88)
src/pages/chat-list/ChatList.tsx (1)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/shared/components/ui/modal/Modal.stories.tsx (1)
src/shared/components/ui/modal/Modal.tsx (1)
Modal(51-88)
src/pages/@owner/estimate/@section/basic-info-section/RegionSection.tsx (4)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/input/Input.tsx (1)
Input(10-52)src/pages/@owner/estimate/constants/estimate.ts (1)
ESTIMATE_MAX_LENGTH(1-17)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/pages/@owner/estimate/@section/basic-info-section/Menu.tsx (4)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/text-area/Textarea.tsx (1)
Textarea(12-40)src/pages/@owner/estimate/constants/estimate.ts (1)
ESTIMATE_MAX_LENGTH(1-17)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/pages/@owner/estimate/utils/estimate.schema.ts (3)
src/pages/@owner/estimate/constants/estimate.ts (2)
ESTIMATE_MAX_LENGTH(1-17)ESTIMATE_ERROR_MESSAGE(19-44)src/shared/types/available-date.ts (1)
AvailableDate(1-5)src/shared/constant/need-electricity.ts (1)
NEED_ELECTRICITY(1-5)
src/pages/@owner/food-truck-management/FoodTruckManagement.tsx (1)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/shared/components/ui/modal/AlertModal.tsx (1)
src/shared/components/ui/modal/Modal.tsx (1)
Modal(51-88)
src/pages/@owner/estimate/hooks/use-time.ts (3)
src/pages/@owner/estimate/utils/estimate.schema.ts (1)
EstimateFormData(51-51)src/pages/@owner/food-truck-form/utils/is-date-over-lapping.ts (1)
isDateOverlapping(3-32)src/pages/@owner/estimate/constants/estimate.ts (2)
ESTIMATE_ERROR_MESSAGE(19-44)ESTIMATE_MAX_LENGTH(1-17)
src/pages/@owner/estimate/@section/time-section/ActiveDate.tsx (8)
src/shared/types/available-date.ts (1)
AvailableDate(1-5)src/shared/types/calendar-types.ts (1)
SelectedDate(1-4)src/shared/utils/date-formatter.ts (1)
dateFormatter(8-14)src/shared/components/calendar/Calendar.tsx (1)
Calendar(15-129)src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/button-text/ButtonText.tsx (1)
ButtonText(11-31)src/shared/components/ui/button-date/ButtonDate.tsx (1)
ButtonDate(16-75)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/shared/components/ui/modal/ConfirmModal.stories.tsx (2)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)src/shared/components/ui/modal/AlertModal.stories.tsx (1)
Default(31-36)
src/pages/@owner/account/AccountForm.tsx (1)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/estimate/Estimate.tsx (7)
src/pages/@owner/estimate/hooks/use-estimate.ts (1)
useEstimateForm(10-121)src/shared/components/layout/navigation/Navigation.tsx (1)
Navigation(13-47)src/shared/components/icon/Icon.tsx (1)
Icon(58-93)src/pages/@owner/estimate/@section/basic-info-section/Menu.tsx (1)
Menu(12-25)src/pages/@owner/estimate/@section/basic-info-section/Price.tsx (1)
Price(11-23)src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx (1)
Etc(12-25)src/shared/components/ui/button/Button.tsx (1)
Button(47-72)
src/pages/profile-setting/ProfileSetting.tsx (1)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/estimate/@section/time-section/ActiveTime.tsx (3)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/time-picker/TimePicker.tsx (1)
TimePicker(35-134)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/shared/components/ui/modal/Modal.tsx (2)
src/shared/utils/cn.tsx (1)
cn(4-6)src/shared/components/layout/overlay/Overlay.tsx (1)
Overlay(12-70)
src/pages/@owner/estimate/@section/basic-info-section/NeedElectricity.tsx (4)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/constant/need-electricity.ts (1)
NEED_ELECTRICITY(1-5)src/shared/components/ui/button/Button.tsx (1)
Button(47-72)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx (4)
src/shared/components/layout/form-layout/FormLayout.tsx (1)
FormLayout(14-51)src/shared/components/ui/text-area/Textarea.tsx (1)
Textarea(12-40)src/pages/@owner/estimate/constants/estimate.ts (1)
ESTIMATE_MAX_LENGTH(1-17)src/shared/components/form/error-text/ErrorText.tsx (1)
ErrorText(7-19)
src/pages/@owner/message-list/MessageList.tsx (1)
src/shared/components/ui/modal/ConfirmModal.tsx (1)
ConfirmModal(15-53)
🪛 ESLint
src/pages/@owner/estimate/@section/basic-info-section/Price.tsx
[error] 8-8: 'price' is defined but never used. Allowed unused args must match /^_/u.
(no-unused-vars)
src/pages/@owner/estimate/@section/basic-info-section/Menu.tsx
[error] 8-8: 'menu' is defined but never used. Allowed unused args must match /^_/u.
(no-unused-vars)
src/pages/@owner/estimate/hooks/use-time.ts
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 12-12: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx
[error] 8-8: 'etc' is defined but never used. Allowed unused args must match /^_/u.
(no-unused-vars)
🪛 GitHub Actions: ci
src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx
[error] 8-8: ESLint: 'etc' is defined but never used. Allowed unused args must match /^_/ (no-unused-vars).
🪛 GitHub Check: build
src/pages/@owner/estimate/@section/basic-info-section/Price.tsx
[failure] 8-8:
'price' is defined but never used. Allowed unused args must match /^_/u
src/pages/@owner/estimate/@section/basic-info-section/Menu.tsx
[failure] 8-8:
'menu' is defined but never used. Allowed unused args must match /^_/u
src/pages/@owner/estimate/hooks/use-time.ts
[failure] 12-12:
Unexpected any. Specify a different type
[failure] 11-11:
Unexpected any. Specify a different type
src/pages/@owner/estimate/@section/basic-info-section/Etc.tsx
[failure] 8-8:
'etc' is defined but never used. Allowed unused args must match /^_/u
🔇 Additional comments (38)
src/pages/@owner/food-truck-form/utils/is-date-over-lapping.ts (1)
1-1:AvailableDate타입 경로 통합 잘 되어 있습니다.
@type/available-date로 타입을 모으는 방향이 다른 푸드트럭 폼 유틸들과도 일관되고, 타입 재사용/이동에도 유리해 보입니다. 이 파일에서는 추가로 손볼 부분 없어 보입니다.src/pages/@owner/food-truck-form/utils/use-food-truck-form.ts (1)
11-11: 공통 타입 경로로의 변경이 폼 스키마와 잘 맞습니다.
AvailableDate를@type/available-date에서 가져오도록 통합해서,use-time,is-date-over-lapping등과 타입 소스를 일치시킨 점이 좋습니다. 현재 스키마 정의와도 정확히 맞아서 별도 수정 필요 없어 보입니다.src/pages/@owner/food-truck-form/hooks/use-time.ts (1)
8-8:AvailableDate타입 경로 일원화로 훅과 폼 타입이 잘 정리되었습니다.
availableDatesWithId: AvailableDate[]에서 사용하는 타입을 공통 경로로 옮겨서, 폼 유틸/훅 간 정의가 한 곳으로 모인 점이 좋습니다. 이 변경 자체로는 동작 변화 없이 타입 구조만 정리된 상태라 그대로 머지해도 무방해 보입니다.src/pages/@owner/food-truck-form/@section/category-section/MenuCategory.tsx (1)
1-1: FormLayout 경로 통합 잘 되어 있습니다.페이지 로컬 컴포넌트 대신 공통
@components/layout/form-layout/FormLayout을 사용하도록 변경해 다른 섹션들과 경로가 일관되어 유지보수에 도움이 될 것 같습니다.src/pages/@owner/food-truck-form/@section/category-section/PaymentMethod.tsx (1)
2-2: FormLayout 중앙 경로 사용으로 일관성 확보
@components/layout/form-layout/FormLayout으로 경로를 통일해서 폼 섹션들 간 구조가 더 명확해졌습니다. 동작 변화 없이 리팩터링만 잘 반영된 것 같습니다.src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckDescription.tsx (1)
2-2: 폼 레이아웃 공통 컴포넌트 사용 👍설명 섹션도 다른 섹션들과 동일하게 공통
FormLayout을 사용하도록 정리되어, 차후 레이아웃 수정 시 한 곳에서 관리하기 쉬워질 것 같습니다.src/shared/components/ui/modal/Modal.tsx (1)
51-87: Modal 베이스 구조 및 인터랙션 설계가 잘 정리되어 있습니다.
Overlay위에 최소 넓이/패딩/라운드가 잡힌 프레임을 올리고, 내부 클릭에stopPropagation을 걸어 바깥 영역 클릭으로만 닫히게 한 구조가 직관적입니다.title유무로 공통 레이아웃 모드(제목/설명/푸터)와 완전 커스텀 모드를 분리해,ConfirmModal같은 단순 케이스와 복잡한 모달 모두 커버할 수 있어 보입니다.description/footer가 있을 때만 조건부 렌더링하는 것도 불필요한 마크업을 줄여줘 좋습니다.현재 요구사항 기준으로는 충분히 깔끔하게 구현된 것 같아서 그대로 사용하셔도 무방해 보입니다.
src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckOperatingInfo.tsx (1)
2-2: 운영정보 섹션도 FormLayout 공통 경로로 정리 완료기존 페이지 전용 경로 대신 공통
FormLayout으로 통일해 다른 기본정보 섹션들과 구조가 맞춰졌습니다. 동작 변화 없이 리팩터링 목적을 잘 달성하신 것 같습니다.src/pages/@owner/food-truck-form/@section/category-section/NeedElectricity.tsx (1)
1-1: 전기 사용 여부 섹션의 레이아웃 공통화
NeedElectricity도 다른 카테고리 섹션들과 동일하게 공통FormLayout을 사용하도록 정리되어, 폼 전체의 레이아웃 일관성이 좋아졌습니다. 나머지 버튼 선택 로직도 기존 패턴과 잘 맞습니다.src/pages/@owner/food-truck-form/@section/category-section/AvailableQuantity.tsx (1)
1-1: 제조 가능 수량 섹션도 공통 FormLayout 사용으로 통일카테고리 관련 섹션들이 모두
@components/layout/form-layout/FormLayout을 바라보도록 맞춰져 있어서, 이후 레이아웃 스펙 변경 시 한 곳에서 관리하기 좋겠습니다. 변경 자체도 사이드 이펙트 없이 잘 반영된 것 같습니다.src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckName.tsx (1)
3-3: 푸드트럭 이름 섹션 FormLayout 경로 정리이름 입력 섹션도 공통
FormLayout경로를 사용하도록 맞춰져서, 다른 기본정보 섹션들과 구조적으로 일관성이 확보되었습니다. 기능 변화 없이 깔끔한 경로 리팩터링으로 보입니다.src/pages/@owner/food-truck-form/@section/menu-section/MenuInfo.tsx (1)
4-4: FormLayout 임포트 경로 중앙화 - 좋습니다!FormLayout을 중앙 집중식 컴포넌트 경로로 이동한 것은 코드베이스 전체의 일관성을 개선합니다.
src/pages/@owner/food-truck-form/@section/time-section/ActiveDate.tsx (1)
6-6: LGTM!FormLayout 임포트 경로를 중앙 집중식으로 변경한 것이 일관성 있게 적용되었습니다.
src/router/constant/routes.ts (1)
27-27: 새로운 라우트 추가 - 좋습니다!OWNER_ESTIMATE 라우트가 기존 패턴을 따르며 올바르게 추가되었습니다.
src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckOption.tsx (1)
3-3: LGTM!FormLayout 임포트 경로 변경이 올바르게 적용되었습니다.
src/pages/@owner/message-list/MessageList.tsx (1)
18-18: ConfirmModal로의 마이그레이션 - 좋습니다!레거시 ConfirmDeleteModal에서 새로운 공통 ConfirmModal로 성공적으로 마이그레이션되었습니다. 새로운 API(
title,description,handleConfirm,handleCancel)가 올바르게 적용되었습니다.Also applies to: 81-88
src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckPhoneNumber.tsx (1)
2-2: LGTM!FormLayout 임포트 경로 변경이 일관되게 적용되었습니다.
src/pages/@owner/food-truck-form/@section/basic-info-section/FoodTruckPhoto.tsx (1)
1-1: LGTM!FormLayout 임포트 경로 변경이 올바르게 적용되었습니다.
src/pages/@owner/food-truck-form/@section/region-section/RegionSection.tsx (1)
3-32: FormLayout 공통 경로 변경, 동작 상 문제 없어 보입니다FormLayout을 공용 컴포넌트 경로로만 옮긴 것으로 보여 기존
useFormContext,navigate흐름과 지역 표시 로직 모두 이전 패턴과 일관적입니다. 이 변경 자체로 인한 리스크는 없어 보입니다.src/pages/@owner/food-truck-form/@section/time-section/ActiveTime.tsx (1)
1-45: FormLayout / TimePicker 공통 컴포넌트 경로 정리 잘 되었습니다FormLayout과 TimePicker를 공용 경로에서 가져오도록만 정리되어 있고,
useTime훅 사용 방식과 시간 조율 스위치/에러 표시 등 기존 동작에는 변화가 없어 보입니다.src/router/router.tsx (1)
29-135: OWNER_ESTIMATE 라우트 추가가 기존 패턴과 일관적입니다
ROUTES.OWNER_ESTIMATE에Estimate페이지를 매핑하는 방식이 다른 owner 관련 라우트들과 동일한 구조라, 네이게이션/레이아웃 측면에서 특별한 문제는 없어 보입니다.src/pages/@owner/estimate/@section/index.ts (1)
1-2: estimate 섹션 barrel export 구성 적절합니다
basic-info-section과time-section을 상위 index에서 재수출하는 구조라 섹션 단위로 import 경로를 단순하게 유지할 수 있어 보입니다. 추후 섹션이 늘어나도 확장하기 쉬운 형태입니다.src/pages/@owner/estimate/@section/time-section/index.tsx (1)
1-2: time-section barrel export로 공개 인터페이스가 명확해졌습니다
ActiveTime,ActiveDate를 명시적으로 이름 붙여 export 해서 섹션 외부에서 사용할 때도 역할이 분리되어 잘 드러납니다. 현재 사용 패턴과도 잘 맞습니다.src/pages/@owner/food-truck-management/FoodTruckManagement.tsx (1)
17-74: 푸드트럭 삭제 ConfirmModal 치환은 적절하나, 확인 시 모달 종료 여부만 한 번 체크 부탁드립니다공통
ConfirmModal로 교체하면서 타이틀/설명/라벨 구성과handleConfirm·handleCancel매핑이 자연스럽습니다.
다만ConfirmModal구현 상 확인 버튼은handleConfirm만 호출하고handleClose는 따로 호출하지 않기 때문에,handleDeleteFoodTrucks내부에서 삭제 후handleConfirmModal을 호출하거나isDeleteConfirmModalOpen을false로 만드는 로직이 있는지 한 번만 확인해 주세요. 그렇지 않으면 삭제 처리 후에도 모달 오버레이가 남아 있을 수 있습니다.src/pages/@owner/estimate/@section/basic-info-section/index.tsx (1)
1-5: basic-info-section 배럴 export 구성이 잘 되어 있습니다.섹션 컴포넌트들을 한 곳에서 export 해 둬서 상위 코드에서 import 경로 관리가 훨씬 단순해질 것 같습니다. 현재 형태 그대로 괜찮아 보입니다.
src/pages/@owner/estimate/constants/estimate.ts (1)
1-44: ESTIMATE_MAX_LENGTH / ESTIMATE_ERROR_MESSAGE 상수 분리가 잘 되어 있습니다.필드별 길이 제약과 에러 메시지를 한 파일에서 관리하고, 메시지에서 길이 상수를 재사용하는 구조라 폼 검증 로직과 카피 수정이 필요할 때 변경 지점을 최소화할 수 있어 보입니다. 현재 코드 상으로는 별다른 문제점은 보이지 않습니다.
src/pages/@owner/estimate/utils/estimate.schema.ts (1)
13-48: ESTIMATE 상수/에러 메시지와 잘 정렬된 스키마 구성
ESTIMATE_MAX_LENGTH,ESTIMATE_ERROR_MESSAGE,NEED_ELECTRICITY와 스키마가 잘 매핑되어 있어서 필드 제약과 에러 메시지가 일관되게 유지될 것 같습니다. 현재 구조 기준으로는 별다른 수정 포인트는 없어 보입니다.src/pages/@owner/message-list/components/MessageForm.tsx (1)
69-76: ConfirmModal 신규 공통 API로의 마이그레이션이 자연스럽게 적용됨공통 ConfirmModal 컴포넌트로의 전환(
handleConfirm/handleCancel,title/description추가)이 기존 뒤로가기 플로우와 잘 맞게 적용되었습니다. 모달 열림 조건과 닫힘 핸들러도 그대로 유지되어 동작 측면에서 문제 없어 보입니다.src/shared/components/ui/modal/ConfirmModal.stories.tsx (1)
4-50: ConfirmModal 스토리 구성이 용도(일반/파괴적 액션)에 맞게 잘 분리됨Meta 설정, docs 설명, 기본 args, 그리고 일반/파괴적 액션 스토리 분리가 명확해서 디자인/QA에서 공통 모달을 이해하고 재사용하기 좋을 것 같습니다. 특별히 수정해야 할 부분은 없어 보입니다.
src/pages/@owner/estimate/Estimate.tsx (1)
17-95: 섹션 단위 분리와 훅 연동이 깔끔하게 정리된 Estimate 페이지
useEstimateForm에서 가져온 상태와 에러, 업데이트 핸들러들이 각 섹션 컴포넌트에 자연스럽게 분배되어 있고, 하단 CTA 버튼의isValid연동도 직관적입니다. 전체 플로우 관점에서 구조적으로 잘 정리된 페이지 같습니다.src/shared/components/ui/modal/ConfirmModal.tsx (1)
4-52: 공통 ConfirmModal API와 Modal 합성이 직관적으로 잘 설계됨
title/description+confirmLabel/cancelLabel+handleConfirm/handleCancel조합이 명확하고, Modal의footer슬롯으로 버튼 영역을 분리한 구조도 재사용성 측면에서 좋습니다. PR 설명에서 언급하신 대로 Button 공용 컴포넌트로의 치환은 나중에 진행해도 될 것 같고, 현재 구현만으로도 충분히 일관된 UX를 제공할 수 있어 보입니다.src/shared/components/ui/modal/AlertModal.stories.tsx (1)
4-45: AlertModal 스토리북 설정이 기존 피드백까지 반영되어 일관성 있게 구성됨스토리북 title을 포함한 Meta 설정이 기존 리뷰 피드백대로
'Components/UI/Modal/AlertModal'로 정리되어 있고, 기본/긴 콘텐츠 두 스토리로 주요 케이스를 커버하고 있어서 컴포넌트 사용 예시로 충분해 보입니다.src/shared/components/ui/modal/Modal.stories.tsx (2)
4-33: Storybook 메타 설정이 잘 구성되어 있습니다.컴포넌트 문서화가 명확하고, argTypes가 적절하게 정의되어 있습니다. Common Mode와 Primitive Mode의 차이점을 잘 설명하고 있습니다.
66-100: 복합 구성 예시가 잘 작성되어 있습니다.Modal의 서브컴포넌트를 활용한 커스텀 구성 방법을 명확하게 보여주고 있으며, 향후 복잡한 UI가 필요할 때 유용한 참고 자료가 될 것입니다.
src/pages/@owner/estimate/@section/time-section/ActiveDate.tsx (1)
95-114: 날짜 렌더링 로직이 잘 구현되어 있습니다.빈 상태와 데이터가 있는 상태를 모두 적절하게 처리하고 있으며, 날짜 문자열을 Date 객체로 변환하는 로직도 올바릅니다.
src/pages/@owner/account/AccountForm.tsx (3)
50-58: ConfirmModal을 사용한 이탈 방지 모달이 올바르게 구현되었습니다.새로운 공통 컴포넌트로 성공적으로 마이그레이션되었으며, 사용법이 명확하고 간결합니다.
94-102: 계좌 삭제 모달이 올바르게 구현되었습니다.ConfirmModal을 사용한 삭제 확인 플로우가 명확하고 사용자에게 적절한 경고를 제공합니다.
60-92: 검증 완료: description prop이 올바르게 지원됩니다.ConfirmModal의
descriptionprop 타입 정의에서string | ReactNode로 명시되어 있으므로, AccountForm.tsx의 복잡한 JSX 구조를 전달하는 사용법은 완전히 올바릅니다. 타입 호환성 문제는 없습니다.
hamxxn
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.
왜 제 브랜치 변경 사항이 여기 있는거죠,,,?
모달 모든 페이지에 적용해주세요~
0a0a6a4 to
ad4963c
Compare
빌드 및 린트 결과일부 검사 실패
|
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
♻️ Duplicate comments (1)
src/pages/chat-list/ChatList.tsx (1)
43-50: 삭제 로직이 실행되지 않는 치명적인 문제가 있습니다.현재
handleConfirm={handleCloseModal}로 설정되어 있어 사용자가 확인 버튼을 눌러도 실제 삭제가 실행되지 않고 모달만 닫힙니다.handleDeleteChat함수가 호출되도록 수정해야 합니다.다음과 같이 수정하세요:
<ConfirmModal isOpen={isDeleteModalOpen} handleClose={handleCloseModal} title={'선택한 대화를 삭제할까요?'} description={`${selectChatList.size}건이 삭제되며 되돌릴 수 없습니다.`} - handleConfirm={handleCloseModal} + handleConfirm={() => { + handleDeleteChat(); + handleCloseModal(); + }} handleCancel={handleCloseModal} />
🧹 Nitpick comments (2)
src/shared/components/ui/modal/Modal.tsx (1)
63-84: title과 children 속성의 상호 배타적 동작을 문서화하면 좋겠습니다.Line 63의
isCommonLayoutMode로직에 따라title이 제공되면children이 무시됩니다. 이는 의도된 설계로 보이지만, 두 속성을 동시에 전달하는 실수를 방지하기 위해 다음을 고려해보세요:
- JSDoc 주석으로 이 동작을 명시
- 또는 개발 환경에서 두 속성이 모두 제공되면 console.warn 추가
다음과 같이 JSDoc을 추가할 수 있습니다:
+/** + * Modal component with two usage modes: + * 1. Common Layout Mode: Pass `title`, `description`, and `footer` props + * 2. Composition Mode: Pass `children` with Modal.Header, Modal.Title, etc. + * + * Note: When `title` is provided, `children` will be ignored. + */ export default function Modal({src/pages/@owner/account/AccountForm.tsx (1)
50-58: 문자열 리터럴에 불필요한 중괄호가 있습니다.Line 53의
title={'정말 나가시겠어요?'}는 문자열 리터럴이므로 중괄호가 불필요합니다. Line 55의confirmLabel='나가기'처럼 작성하는 것이 일관성 있습니다.- title={'정말 나가시겠어요?'} + title='정말 나가시겠어요?'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/pages/@owner/account/AccountForm.tsx(2 hunks)src/pages/@owner/food-truck-management/FoodTruckManagement.tsx(2 hunks)src/pages/@owner/message-list/MessageList.tsx(2 hunks)src/pages/@owner/message-list/components/MessageForm.tsx(2 hunks)src/pages/chat-list/ChatList.tsx(2 hunks)src/pages/home/Home.tsx(2 hunks)src/pages/profile-setting/ProfileSetting.tsx(2 hunks)src/shared/components/ui/modal-alert/AlertModal.stories.tsx(1 hunks)src/shared/components/ui/modal-alert/AlertModal.tsx(1 hunks)src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx(1 hunks)src/shared/components/ui/modal-confirm/ConfirmModal.tsx(1 hunks)src/shared/components/ui/modal/Modal.stories.tsx(1 hunks)src/shared/components/ui/modal/Modal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/shared/components/ui/modal/Modal.stories.tsx
- src/pages/@owner/message-list/MessageList.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
src/pages/@owner/food-truck-management/FoodTruckManagement.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/profile-setting/ProfileSetting.tsx (2)
src/router/constant/routes.ts (1)
ROUTES(1-27)src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/chat-list/ChatList.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/shared/components/ui/modal-alert/AlertModal.stories.tsx (2)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
AlertModal(11-37)src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx (1)
Default(34-41)
src/pages/@owner/account/AccountForm.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/shared/components/ui/modal/Modal.tsx (2)
src/shared/utils/cn.tsx (1)
cn(4-6)src/shared/components/layout/overlay/Overlay.tsx (1)
Overlay(12-70)
src/pages/home/Home.tsx (1)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
AlertModal(11-37)
🪛 GitHub Actions: ci
src/pages/home/Home.tsx
[error] 3-3: 'Overlay' is defined but never used
🪛 GitHub Check: build
src/pages/home/Home.tsx
[failure] 3-3:
'Overlay' is declared but its value is never read.
[failure] 3-3:
'Overlay' is defined but never used
[failure] 3-3:
'Overlay' is defined but never used
🔇 Additional comments (13)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
11-37: 구현이 깔끔합니다!AlertModal 컴포넌트가 단일 버튼 모달의 용도에 맞게 잘 구현되었습니다. 기본 Modal 컴포넌트를 활용하여 footer를 전달하는 방식이 명확합니다.
src/pages/profile-setting/ProfileSetting.tsx (1)
54-61: ConfirmModal 통합이 올바르게 구현되었습니다.회원탈퇴 모달이 새로운 ConfirmModal 패턴으로 잘 전환되었습니다. handleConfirm과 handleCancel이 적절히 연결되어 있습니다.
src/pages/home/Home.tsx (1)
46-51: AlertModal로의 전환이 올바르게 완료되었습니다.기존 Overlay 기반 모달이 새로운 AlertModal 컴포넌트로 성공적으로 교체되었습니다.
src/shared/components/ui/modal-alert/AlertModal.stories.tsx (1)
1-45: Storybook 스토리가 잘 작성되었습니다.AlertModal의 기본 사용 사례와 긴 컨텐츠 케이스를 모두 다루는 스토리가 적절히 구성되었습니다.
src/pages/@owner/food-truck-management/FoodTruckManagement.tsx (1)
66-74: ConfirmModal 통합이 올바르게 구현되었습니다.삭제 확인 모달이 새로운 ConfirmModal로 잘 전환되었으며,
handleConfirm과handleCancel이 적절한 핸들러에 올바르게 연결되어 있습니다.src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
15-53: ConfirmModal 구현이 우수합니다.양방향 확인 모달의 핵심 로직이 명확하게 구현되었습니다.
descriptionprop이string | ReactNode를 지원하여 유연성을 제공하는 점이 좋습니다.src/pages/@owner/message-list/components/MessageForm.tsx (1)
69-76: ConfirmModal로의 전환이 올바르게 완료되었습니다.모달의
handleConfirm과handleCancel이 각각 적절한 동작(나가기 vs 취소)에 올바르게 연결되어 있습니다.src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx (1)
1-50: Storybook 구성이 잘 되어 있습니다.두 가지 시나리오(일반 확인과 삭제 등 위험한 액션)를 명확하게 보여주고 있으며, 핸들러 함수들을 controls에서 적절히 숨겨놨습니다. 스토리 구성이 깔끔합니다.
src/shared/components/ui/modal/Modal.tsx (3)
60-62: 이벤트 버블링 차단 처리가 적절합니다.모달 내부 클릭 시 오버레이로 이벤트가 전파되어 모달이 닫히는 것을 방지하는 올바른 구현입니다.
31-45: Body 컴포넌트의 타입별 렌더링이 잘 구현되어 있습니다.문자열인 경우
whitespace-pre-wrap으로 줄바꿈(\n)을 처리하고, ReactNode인 경우 JSX를 렌더링하는 유연한 구조입니다. AccountForm.tsx의 저장 모달처럼 복잡한 description을 지원할 수 있어 좋습니다.
90-93: 컴포지션 API 패턴이 잘 적용되어 있습니다.서브컴포넌트를 static 속성으로 노출하는 방식은 React에서 널리 사용되는 패턴이며, Modal.Header, Modal.Title 등의 형태로 유연한 조합이 가능해집니다.
src/pages/@owner/account/AccountForm.tsx (2)
60-92: JSX description을 활용한 저장 정보 확인 모달이 잘 구현되었습니다.ConfirmModal의
descriptionprop이 ReactNode를 지원하는 점을 잘 활용하여, 은행/예금주/계좌번호를 구조화된 레이아웃으로 보여주고 있습니다. 사용자가 저장할 내용을 명확히 확인할 수 있어 좋은 UX입니다.
94-102: 삭제 확인 모달이 적절하게 구성되어 있습니다.되돌릴 수 없는 액션임을 명확히 알리는 문구와 함께, 새로운 ConfirmModal을 올바르게 사용하고 있습니다. 문자열 속성도 불필요한 중괄호 없이 깔끔하게 작성되었습니다.
빌드 및 린트 결과일부 검사 실패
|
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 (1)
src/pages/@owner/message-list/MessageList.tsx (1)
81-88: 삭제 확인 모달에서 버튼 라벨을 명시적으로 지정하면 UX가 더 명확해질 수 있습니다현재
ConfirmModal의 기본 라벨(확인/취소)에 의존하고 있는데, 타이틀/설명이 “삭제” 동작인 만큼 확인 버튼을삭제하기처럼 명시적으로 바꾸면 사용자가 파괴적 액션임을 더 쉽게 인지할 수 있습니다.예시:
<ConfirmModal isOpen={isConfirmDeleteModalOpen} handleClose={handleCloseConfirmModal} title='이 메시지를 삭제할까요?' description='삭제 후에는 되돌릴 수 없습니다.' + confirmLabel='삭제하기' + cancelLabel='취소' handleConfirm={handleFinalDelete} handleCancel={handleCloseConfirmModal} />필수는 아니지만, 전체 서비스의 다른 삭제 모달들과 톤을 맞출 때도 도움이 될 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/@owner/account/AccountForm.tsx(2 hunks)src/pages/@owner/message-list/MessageList.tsx(2 hunks)src/pages/home/Home.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/@owner/account/AccountForm.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/home/Home.tsx (1)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
AlertModal(11-37)
src/pages/@owner/message-list/MessageList.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
🪛 GitHub Actions: ci
src/pages/@owner/message-list/MessageList.tsx
[error] 8-8: Cannot find module '@pages/@owner/message-list/@modal/DeleteMessageBottomSheet' or its corresponding type declarations.
🪛 GitHub Check: build
src/pages/@owner/message-list/MessageList.tsx
[failure] 8-8:
Cannot find module '@pages/@owner/message-list/@modal/DeleteMessageBottomSheet' or its corresponding type declarations.
🔇 Additional comments (5)
src/pages/home/Home.tsx (2)
1-7: import 문이 올바르게 수정되었습니다.이전 리뷰에서 지적된 사용하지 않는 Overlay import 문제가 해결되었고, 새로운 AlertModal과 기타 필요한 컴포넌트들이 정확하게 import되어 코드 내에서 모두 사용되고 있습니다.
45-50: 이전 중복 모달 문제가 해결되었고 새로운 AlertModal이 올바르게 사용되고 있습니다.레거시 Overlay 모달이 제거되고 새로운 AlertModal 컴포넌트만 사용하도록 성공적으로 리팩토링되었습니다. Props가 AlertModal의 인터페이스와 정확히 일치하며, 상태 관리도 적절하게 구현되어 있습니다.
src/pages/@owner/message-list/MessageList.tsx (3)
3-5: 새 공통 컴포넌트/훅 import 사용이 적절합니다
Information,ConfirmModal,useToast모두 아래에서 정상적으로 사용되고 있고, alias 구조도 일관되어 보여 별도 수정 필요 없어 보입니다.
12-12: 메시지 템플릿 조회 훅 import 변경 문제 없습니다
useOwnerChatTemplates는 아래에서 데이터 조회에 사용되고 있고, 같은 파일에서useDeleteOwnerChatTemplates도 함께 import 되어 있어 역할이 명확합니다. 네이밍과 위치 모두 일관적이라 유지보수 측면에서도 괜찮아 보입니다.
14-18: 라우팅/버튼 및 라우터 훅 import 구성이 자연스럽습니다
ROUTES,ButtonFloating,Button,useState,useNavigate모두 본문에서 적절히 사용되고 있고, 각 역할(라우팅, FAB, 하단 버튼, 상태, 네비게이션)이 명확하게 분리되어 있어서 추가 조정 없이도 읽기 좋습니다.
빌드 및 린트 결과모든 검사 통과 🎉
|
junghyungo
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.
ConfirmModal과 AlertModal에서 Button 공컴대신, button 태그로 버튼을 만드신 이유가 있나요?
…ALL/CHA-CALL_FRONT into refactor/#179/common-modal
빌드 및 린트 결과일부 검사 실패
|
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 (4)
src/pages/@owner/message-list/MessageForm.tsx (1)
60-71: 중복된 핸들러 함수를 통합하는 것을 고려해보세요.
handleClickCancel과handleCloseModal함수가 동일한 로직(setIsOpen(false))을 수행하고 있습니다. 하나의 함수로 통합하거나,handleClickCancel을handleClose로 이름을 변경하여handleCloseModal을 제거할 수 있습니다.🔎 제안하는 리팩토링
- const handleClickCancel = () => { + const handleCloseModal = () => { setIsOpen(false); }; - const handleClickConfirm = () => { - setIsOpen(false); - navigate(-1); - }; - - const handleCloseModal = () => { + const handleClickConfirm = () => { setIsOpen(false); + navigate(-1); };그리고 ConfirmModal의 props를 업데이트:
<ConfirmModal isOpen={isOpen} handleClose={handleCloseModal} title='저장하지 않고 나가시겠습니까?' description={`작성 중인 내용은 저장되지 않으며, \n나가면 모두 삭제됩니다.`} handleConfirm={handleClickConfirm} - handleCancel={handleClickCancel} + handleCancel={handleCloseModal} />src/pages/profile-setting/ProfileSetting.tsx (1)
19-24: 에러 핸들링 추가를 권장합니다.현재 로딩 상태는 처리되고 있지만,
useGetUserInfo훅의 에러 상태에 대한 처리가 없습니다. 네트워크 오류나 API 실패 시 사용자 경험을 개선하기 위해 에러 핸들링을 추가하는 것이 좋습니다.🔎 제안된 수정
- const { data: userData, isPending: isUserDataPending } = useGetUserInfo(); + const { data: userData, isPending: isUserDataPending, isError } = useGetUserInfo(); const [isModalOpen, setIsModalOpen] = useState(false); if (isUserDataPending) { return <Loading />; } + if (isError) { + return <div>사용자 정보를 불러오는데 실패했습니다.</div>; + }src/pages/home/Home.tsx (1)
54-59: AlertModal 통합이 성공적으로 완료되었습니다.기존 Overlay 기반 모달을 새로운 AlertModal 컴포넌트로 올바르게 교체했습니다. 이전 리뷰에서 지적된 중복 모달 렌더링 문제가 완전히 해결되었고, props 연결도 정확합니다. 데모 페이지의 특성상 하드코딩된 문자열도 적절합니다.
선택적 개선사항: 문자열 리터럴 간소화
JSX에서 정적 문자열 props는 중괄호 없이 직접 전달할 수 있습니다:
<AlertModal isOpen={isModalOpen} handleClose={handleCloseModal} - title={'모달 제목'} - description={'모달 내용입니다.'} + title="모달 제목" + description="모달 내용입니다." />둘 다 동일하게 작동하지만, 중괄호 없이 사용하는 것이 더 간결합니다.
src/pages/@owner/account/AccountForm.tsx (1)
65-89: 복잡한 JSX를 별도 컴포넌트로 추출하는 것을 고려해보세요.저장 모달의
description에 인라인으로 작성된 JSX가 상당히 복잡합니다(25줄). 가독성과 유지보수성 향상을 위해 이를 별도의 컴포넌트(예:AccountSummary)로 추출하는 것을 고려해볼 수 있습니다.리팩토링 예시
별도 파일 생성:
AccountSummary.tsxinterface AccountSummaryProps { bankName: string; accountHolderName: string; accountNumber: string; } export function AccountSummary({ bankName, accountHolderName, accountNumber }: AccountSummaryProps) { return ( <> <div className='bg-grayscale-100 my-[1rem] h-[0.1rem] w-full' /> <div className='flex flex-col gap-[1rem]'> <div className='flex items-center justify-between'> <p className='title-sb-12 text-grayscale-500'>은행</p> <p className='caption-m-12 text-grayscale-900'>{bankName}</p> </div> <div className='flex items-center justify-between'> <p className='title-sb-12 text-grayscale-500'>예금주</p> <p className='caption-m-12 text-grayscale-900'>{accountHolderName}</p> </div> <div className='flex items-center justify-between'> <p className='title-sb-12 text-grayscale-500'>계좌번호</p> <p className='caption-m-12 text-grayscale-900'>{accountNumber}</p> </div> </div> </> ); }사용 예시:
<ConfirmModal isOpen={saveModal.isOpen} handleClose={saveModal.handleClose} title='저장정보 확인' - description={ - <> - <div className='bg-grayscale-100 my-[1rem] h-[0.1rem] w-full' /> - <div className='flex flex-col gap-[1rem]'> - ... - </div> - </> - } + description={ + <AccountSummary + bankName={formData.bankName} + accountHolderName={formData.accountHolderName} + accountNumber={formData.accountNumber} + /> + } confirmLabel='저장' handleConfirm={saveModal.handleConfirm} handleCancel={saveModal.handleCancel} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/pages/@owner/account/@bottom-sheet/SelectBankBottomSheet.tsxsrc/pages/@owner/account/AccountForm.tsxsrc/pages/@owner/food-truck-management/FoodTruckManagement.tsxsrc/pages/@owner/message-list/MessageForm.tsxsrc/pages/@owner/message-list/MessageList.tsxsrc/pages/chat-list/ChatList.tsxsrc/pages/home/Home.tsxsrc/pages/profile-setting/ProfileSetting.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/@owner/food-truck-management/FoodTruckManagement.tsx
- src/pages/chat-list/ChatList.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/pages/@owner/message-list/MessageList.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/account/AccountForm.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/home/Home.tsx (1)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
AlertModal(11-37)
src/pages/profile-setting/ProfileSetting.tsx (2)
src/router/constant/routes.ts (1)
ROUTES(1-33)src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
🔇 Additional comments (12)
src/pages/@owner/message-list/MessageForm.tsx (1)
81-88: 새로운 ConfirmModal 컴포넌트가 올바르게 적용되었습니다!통합된 ConfirmModal API를 정확하게 사용하고 있으며, 사용자에게 명확한 경고 메시지를 제공하고 있습니다.
src/pages/@owner/message-list/MessageList.tsx (3)
8-8: 이전 리뷰의 치명적인 빌드 오류가 수정되었습니다!
DeleteMessageBottomSheet의 import 경로가@modal에서@bottom-sheet로 올바르게 수정되어 빌드 실패 문제가 해결되었습니다.
48-71: 삭제 플로우 로직이 올바르게 구현되었습니다.2단계 확인 프로세스(DeleteMessageBottomSheet → ConfirmModal → 실제 삭제)가 명확하게 구현되어 있으며, 상태 관리 및 에러 처리가 적절합니다.
81-88: 통합된 ConfirmModal이 정확하게 적용되었습니다!새로운 ConfirmModal API를 올바르게 사용하고 있으며, 삭제 작업의 불가역성에 대한 명확한 경고 메시지를 제공하고 있습니다.
src/pages/profile-setting/ProfileSetting.tsx (3)
2-14: LGTM! 올바른 임포트 구조입니다.새로운 공통 모달 컴포넌트와 필요한 의존성들이 올바르게 임포트되었습니다.
56-63: LGTM! ConfirmModal이 올바르게 사용되었습니다.모달 컴포넌트가 적절한 props와 함께 올바르게 렌더링되고 있으며, Navigation 컴포넌트보다 먼저 렌더링되어 오버레이가 정상적으로 작동합니다.
85-101: LGTM! 푸터 UI가 올바르게 구현되었습니다.로그아웃과 회원탈퇴 버튼이 적절하게 배치되었고, 이벤트 핸들러가 올바르게 연결되었습니다.
src/pages/home/Home.tsx (1)
1-9: Import 구성이 올바르게 정리되었습니다.새로운 AlertModal import가 추가되었고, 기존 import들이 적절히 재정렬되었습니다. 모든 import가 실제로 사용되고 있으며, 이전 리뷰에서 지적된 미사용 Overlay import 문제도 해결되었습니다.
src/pages/@owner/account/AccountForm.tsx (4)
2-12: LGTM! 임포트가 올바르게 수정되었습니다.새로운 공통
ConfirmModal컴포넌트로의 임포트 변경이 정확하게 적용되었습니다.
94-103: LGTM! 삭제 확인 모달이 올바르게 구현되었습니다.계좌 삭제 확인 모달이 새로운
ConfirmModal컴포넌트를 사용하여 정확하게 구현되었습니다. 명확한 경고 메시지와 적절한 라벨이 사용되었습니다.
50-59: 줄바꿈 처리는 올바르게 동작합니다. Modal 컴포넌트의 Body 컴포넌트(Modal.tsx 36번 줄)에서whitespace-pre-wrapCSS 클래스가 description에 적용되므로,\n문자가 실제 줄바꿈으로 올바르게 렌더링됩니다.
61-93: 타입 안전성 확인 완료 - 문제 없음
ConfirmModal의descriptionprop은string | ReactNode타입을 지원하고, 내부의Modal컴포넌트도 동일하게description?: string | ReactNode타입을 정의하고 있습니다. AccountForm.tsx의 복잡한 JSX 구조는 올바르게 처리되며 타입 안전성에 문제가 없습니다.
빌드 및 린트 결과모든 검사 통과 🎉
|
빌드 및 린트 결과모든 검사 통과 🎉
|
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 (1)
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx (1)
35-46: 코드가 명확하고 동작합니다.각 prop을 개별적으로 전달하는 방식이 명시적이고 이해하기 쉽습니다.
선택적 제안: 객체 구조 분해를 활용한 코드 간소화
modalConfig의 프로퍼티명을ConfirmModal의 prop 이름과 일치시키면 더 간결하게 작성할 수 있습니다:const modalConfig = isOnboarding ? { title: '신청하시겠습니까?', description: '최종 승인까지 영업일 기준 3-5일이 소요돼요.', confirmLabel: '확인', cancelLabel: '취소', - onConfirm: handleConfirm, - onCancel: handleCloseModal, + handleConfirm: handleConfirm, + handleCancel: handleCloseModal, } : { title: '아직 완료되지 않았어요!', description: '페이지를 나가면 작성 중인 내용이 사라집니다.', confirmLabel: '나가기', cancelLabel: '취소', - onConfirm: handleConfirm, - onCancel: handleCloseModal, + handleConfirm: handleConfirm, + handleCancel: handleCloseModal, }; return ( <ConfirmModal isOpen={isModalOpen} handleClose={handleCloseModal} - title={modalConfig.title} - description={modalConfig.description} - confirmLabel={modalConfig.confirmLabel} - cancelLabel={modalConfig.cancelLabel} - handleConfirm={modalConfig.onConfirm} - handleCancel={modalConfig.onCancel} + {...modalConfig} /> );다만 현재 방식도 충분히 명확하므로 선택 사항입니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsxsrc/pages/@owner/menu/MenuEdit.tsxsrc/pages/@owner/menu/components/MenuDeleteModal.tsxsrc/pages/@owner/menu/components/index.ts
💤 Files with no reviewable changes (2)
- src/pages/@owner/menu/components/MenuDeleteModal.tsx
- src/pages/@owner/menu/components/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/menu/MenuEdit.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
🔇 Additional comments (3)
src/pages/@owner/menu/MenuEdit.tsx (2)
11-11: 공통 모달 컴포넌트 import 확인 완료
ConfirmModal공통 컴포넌트로의 import가 올바르게 추가되었습니다. 기존MenuDeleteModal을 제거하고 공통 컴포넌트로 대체하는 리팩토링 목표에 부합합니다.
79-88: ConfirmModal 적용이 올바르게 구현되었습니다기존
MenuDeleteModal을 공통ConfirmModal로 성공적으로 대체했습니다:
- 모든 필수 props가 올바르게 전달되었습니다
handleClose와handleCancel모두handleCloseModal을 사용하는 것이 의도된 설계입니다 (handleClose는 오버레이/닫기 버튼용,handleCancel은 취소 버튼용)- 한국어 UI 텍스트가 삭제 확인 맥락에 적합합니다
- 기존 기능을 유지하면서 코드 재사용성이 개선되었습니다
선택적으로, 다음 사항을 수동으로 확인해 주세요:
- 삭제 버튼 클릭 시 모달이 올바르게 열리는지
- 취소 버튼 클릭 시 모달이 닫히는지
- 삭제 버튼 클릭 시 메뉴 삭제가 정상적으로 수행되는지
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx (1)
1-8: 리팩토링 방향이 좋습니다.공통
ConfirmModal컴포넌트를 사용하여 중복을 제거하고, 인터페이스 정의도 명확합니다.
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx
Outdated
Show resolved
Hide resolved
holdn2
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.
고생하셨습니다! 코드래빗 리뷰도 확인해주시고 제가 드린 제안도 한번 생각해보시면 좋을 것 같아요!
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx
Outdated
Show resolved
Hide resolved
빌드 및 린트 결과모든 검사 통과 🎉
|
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: 4
♻️ Duplicate comments (1)
src/pages/chat-list/ChatList.tsx (1)
43-50: 삭제 확인 버튼이 실제 삭제를 수행하지 않습니다.
handleClickRight와handleClickLeft모두handleCloseModal만 호출하고 있어, 사용자가 확인 버튼을 눌러도 실제 삭제가 일어나지 않고 모달만 닫힙니다.handleDeleteChat함수가 존재하지만(Line 22) 연결되지 않았습니다.🔎 제안된 수정
<ConfirmModal isOpen={isDeleteModalOpen} handleClose={handleCloseModal} title={'선택한 대화를 삭제할까요?'} description={`${selectChatList.size}건이 삭제되며 되돌릴 수 없습니다.`} - handleClickRight={handleCloseModal} + handleClickRight={() => { + handleDeleteChat(); + handleCloseModal(); + }} handleClickLeft={handleCloseModal} />
🧹 Nitpick comments (2)
src/shared/components/ui/modal-alert/AlertModal.stories.tsx (1)
4-26: LGTM! Storybook 설정이 적절합니다.Meta 설정이 잘 구성되어 있습니다:
- 컴포넌트 계층 구조가 명확하고
- 기본 args와 argTypes가 적절히 설정되어 있습니다
- 한국어 설명이 일관되게 사용되었습니다
🔎 선택적 개선 사항: 중복된 기본값
Line 19의
confirmLabel: '확인'은AlertModal.tsx에서 이미 기본값으로 설정되어 있어 기술적으로는 중복입니다. 하지만 Storybook 문서화 목적으로는 명시적으로 표시하는 것이 유용할 수 있으므로 현재 구현도 괜찮습니다.src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
15-42: 파괴적 액션을 위한 변형 또는 가이드라인 추가를 고려해주세요.현재
ConfirmModal은 오른쪽 버튼에 항상bg-primary-700(강조 스타일)을 적용합니다. 이는 일반적인 확인 동작에는 적합하지만, 삭제와 같은 파괴적 액션에는 부적절할 수 있습니다.다음 개선을 권장합니다:
variantprop 추가 (예:variant='destructive')를 통해 파괴적 액션 시 오른쪽 버튼에 경고 색상 적용- 또는 컴포넌트 문서/Storybook에 파괴적 액션 사용 시 버튼 순서 가이드라인 명시
🔎 variant prop 추가 예시
interface ConfirmModalProps { isOpen: boolean; handleClose: () => void; title: string; description: string | ReactNode; leftLabel?: string; rightLabel?: string; handleClickRight: () => void; handleClickLeft: () => void; + variant?: 'default' | 'destructive'; } export default function ConfirmModal({ isOpen, handleClose, title, description, rightLabel = '확인', leftLabel = '취소', handleClickRight, handleClickLeft, + variant = 'default', }: ConfirmModalProps) { + const rightButtonClass = variant === 'destructive' + ? 'title-sb-14 bg-error-500 w-full rounded-[0.8rem] py-[0.85rem] text-white' + : 'title-sb-14 bg-primary-700 w-full rounded-[0.8rem] py-[0.85rem] text-white'; + const footer = ( <> <button type='button' className='title-sb-14 text-grayscale-700 border-grayscale-200 w-full rounded-[0.8rem] border bg-white py-[0.85rem] text-center' onClick={handleClickLeft} > {leftLabel} </button> <button type='button' - className='title-sb-14 bg-primary-700 w-full rounded-[0.8rem] py-[0.85rem] text-white' + className={rightButtonClass} onClick={handleClickRight} > {rightLabel} </button> </> );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/pages/@owner/account/@account-modals/AccountModals.tsxsrc/pages/@owner/account/AccountForm.tsxsrc/pages/@owner/food-truck-management/FoodTruckManagement.tsxsrc/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsxsrc/pages/@owner/menu/MenuEdit.tsxsrc/pages/@owner/message-list/MessageForm.tsxsrc/pages/@owner/message-list/MessageList.tsxsrc/pages/chat-list/ChatList.tsxsrc/pages/profile-setting/ProfileSetting.tsxsrc/shared/components/ui/modal-alert/AlertModal.stories.tsxsrc/shared/components/ui/modal-confirm/ConfirmModal.stories.tsxsrc/shared/components/ui/modal-confirm/ConfirmModal.tsxsrc/shared/components/ui/modal/Modal.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/shared/components/ui/modal/Modal.stories.tsx
- src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx
- src/pages/@owner/menu/MenuEdit.tsx
- src/pages/@owner/message-list/MessageForm.tsx
- src/pages/profile-setting/ProfileSetting.tsx
- src/pages/@owner/account/AccountForm.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
src/shared/components/ui/modal-alert/AlertModal.stories.tsx (2)
src/shared/components/ui/modal-alert/AlertModal.tsx (1)
AlertModal(11-37)src/shared/components/ui/modal-confirm/ConfirmModal.stories.tsx (1)
Default(36-43)
src/pages/chat-list/ChatList.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/message-list/MessageList.tsx (4)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)src/shared/components/layout/navigation/Navigation.tsx (1)
Navigation(13-61)src/shared/components/icon/Icon.tsx (1)
Icon(58-93)src/shared/components/ui/button/Button.tsx (1)
Button(47-72)
src/pages/@owner/account/@account-modals/AccountModals.tsx (2)
src/pages/@owner/account/schemas/account.schema.ts (1)
AccountFormData(19-19)src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
src/pages/@owner/food-truck-management/FoodTruckManagement.tsx (1)
src/shared/components/ui/modal-confirm/ConfirmModal.tsx (1)
ConfirmModal(15-53)
🔇 Additional comments (4)
src/shared/components/ui/modal-alert/AlertModal.stories.tsx (2)
1-2: LGTM!임포트 구조가 올바르며, Storybook 타입도 적절히 가져오고 있습니다.
31-45: LGTM! 스토리 구성이 우수합니다.두 스토리가 각각 다른 목적을 명확히 보여줍니다:
Default: 일반적인 사용 사례를 실제 비즈니스 시나리오로 시연LongContent: 긴 콘텐츠 처리 및 레이아웃 동작을 검증하는 엣지 케이스
ConfirmModal.stories.tsx와 일관된 패턴을 따르고 있어 유지보수성이 좋습니다.src/pages/@owner/food-truck-onboarding/@modal/(.)onboarding-modal/OnboardingModal.tsx (1)
16-32: 파괴적 액션 배치가 적절하게 수정되었습니다.과거 리뷰에서 지적된 파괴적 액션('나가기') 배치 문제가 해결되었습니다. 현재 구현에서 '나가기'는 덜 강조된 왼쪽 버튼에, '취소'는 primary 스타일의 오른쪽 버튼에 배치되어 사용자가 실수로 나가기를 클릭할 위험이 감소했습니다.
src/pages/@owner/account/@account-modals/AccountModals.tsx (1)
37-69: 저장 모달의 구현이 적절합니다.저장 확인 모달은 안전한 액션('저장')을 primary 버튼 위치에 배치하여 적절한 UX를 제공합니다. 계좌 정보를 구조화된 형태로 표시하여 사용자가 저장 전 내용을 확인할 수 있도록 구현되었습니다.
빌드 및 린트 결과모든 검사 통과 🎉
|
빌드 및 린트 결과모든 검사 통과 🎉
|
📌 Related Issues
✅ 체크 리스트
📄 Tasks
@modal등 기존 modal 대체 후 사용하지 않게 된 컴포넌트와 폴더 제거⭐ PR Point (To Reviewer)
AlertModal, ConfirmModal의 버튼에서, title-sb-14 등 폰트 관련 className은 merge되지 않아서 일반 button 태그를 사용함.
📷 Screenshot
기존 모달

수정된 모달

단일 버튼 모달

🔔 ETC
Summary by CodeRabbit
릴리스 노트
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.