Skip to content

Comments

Feat: card edit modal#68

Merged
jjangminii merged 16 commits intodevelopfrom
feat/#63/card-Edit-modal
Sep 11, 2025
Merged

Feat: card edit modal#68
jjangminii merged 16 commits intodevelopfrom
feat/#63/card-Edit-modal

Conversation

@jjangminii
Copy link
Collaborator

@jjangminii jjangminii commented Sep 10, 2025

📌 Related Issues

관련된 Issue를 태그해주세요. (e.g. - close #25)

📄 Tasks

  • 버튼클릭시 수정 모달 열림+ 오버레이
  • 공컴 합체 작업

⭐ PR Point (To Reviewer)

현재 저장버튼 누르면 저장 실패 토스트 팝업이 나타납니다-! 위치 확인용이고 추후 api연결하면서 수정하고자합니다

밑 영상속 로고는 바꾸었습니다

다들 잘 알고 계실테지만 그래도 좀 더 사용하기 편하게 사용방법을 남깁니다

const [isOpen, setIsOpen] = useState(false);

  return (
    <div>
      <div>
        <button onClick={() => setIsOpen(true)}>카테고리</button>
        {/* …여기에 리스트/카드 등 배경 콘텐츠 */}
      </div>
         {/* 추가 오버레이*/}
      {isOpen && (
        <div className="fixed inset-0 z-50">
          <div
            className="absolute inset-0 bg-black/60 backdrop-blur-[2px]"
            onClick={() => setIsOpen(false)}
          />
          <div className="absolute inset-0 flex items-center justify-center p-4">
            <CardEditModal onClose={() => setIsOpen(false)} />
          </div>
        </div>
      )}
    </div>

📷 Screenshot

2025-09-11.4.45.54.mov

Summary by CodeRabbit

  • 새 기능
    • 카드 편집 모달 추가: 제목/출처, 카테고리 선택·추가 팝업, 메모 입력, 리마인드 스위치, 날짜·시간 입력 및 저장 토스트 알림 제공.
  • 리팩터링
    • 레이아웃 구조 변경으로 본문 영역의 크기 및 스크롤 동작이 일부 변경됨.
  • 스타일
    • 인포박스가 고정 너비에서 가로 폭에 맞춰 확장되도록 조정되어 반응형 가독성 개선.
  • 개선
    • 날짜·시간 입력에 대한 검증과 오류 메시지 표시가 추가되어 입력 신뢰성 향상.

@jjangminii jjangminii linked an issue Sep 10, 2025 that may be closed by this pull request
@coderabbitai
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

레이아웃에서 Outlet을 감싸던 main 래퍼를 제거하고 Outlet을 외부 flex 컨테이너의 직접 자식으로 옮겼다. 카드 편집 모달 컴포넌트와 날짜/시간 유효성 검사 유틸이 신규 추가되었고, 디자인 시스템의 InfoBox 루트 폭이 고정폭에서 w-full로 변경됐다.

Changes

Cohort / File(s) Summary
레이아웃 구조 변경
apps/client/src/layout/Layout.tsx
기존에 Outlet을 감싸던 <main class="flex-1 overflow-y-auto"> 래퍼를 제거하고 Outlet을 외부 div.flex의 직접 자식으로 이동. 사이드바와 Outlet의 DOM 계층/스크롤 제어 위치 변경.
카드 편집 모달 추가
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx
신규 CardEditModal 컴포넌트 추가 (CardEditModalProps { onClose: () => void }). 헤더, InfoBox, 카테고리 드롭다운 및 추가 팝업, 메모 입력, 리마인드 스위치, 날짜/시간 입력, 유효성 검사, 실패 토스트(자동 닫힘) 등 UI 및 로컬 상태/이벤트 처리 구현.
유효성 검사 유틸 추가
apps/client/src/shared/utils/ValidateData.ts
validateDate(value: string): string 및 `validateTime(value: string
디자인 시스템: InfoBox 폭 변경
packages/design-system/src/components/infobox/InfoBox.tsx
루트 컨테이너 클래스 w-[24.8rem]w-full로 변경. 내부 API/props 변경 없음.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as 사용자
  participant M as CardEditModal
  participant D as Dropdown/Popup
  participant V as ValidateData
  participant T as Toast

  rect rgba(220,235,255,0.35)
  U->>M: 모달 열기 / 입력 상호작용
  U->>M: 카테고리 선택
  M->>D: 드롭다운 표시
  U->>D: "카테고리 추가" 선택
  D-->>M: 팝업 열기/닫기
  end

  rect rgba(220,255,220,0.35)
  U->>M: 날짜/시간 입력
  M->>V: validateDate(value)
  V-->>M: '' 또는 오류문자열
  M->>V: validateTime(value)
  V-->>M: '' 또는 오류문자열
  M-->>U: 오류 표시(있을 경우)
  end

  rect rgba(255,235,220,0.35)
  U->>M: 저장 클릭
  M-->>T: 실패 토스트 오픈
  T-->>T: 자동 종료(타이머)
  end
Loading
sequenceDiagram
  autonumber
  participant App as App
  participant L as Layout.tsx
  participant S as Sidebar
  participant O as Outlet

  App->>L: 렌더
  L->>S: Sidebar 렌더
  L->>O: Outlet을 외부 flex 컨테이너의 직접 자식으로 렌더 (기존 main 래퍼 제거)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

🛠️ Feature, frontend

Suggested reviewers

  • jllee000
  • constantly-dev

Pre-merge checks (2 passed, 3 warnings)

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning 이 PR은 linked issues 중 [#63]의 주요 목표(카테고리/카드 편집 UI)를 충족합니다; apps/client/src/shared/components/cardEditModal/CardEditModal.tsx 추가와 ValidateData 유틸은 의도된 UI와 입력 검증을 구현하고 있습니다. 그러나 [#25] (design-system의 Progress 컴포넌트 구현)는 이 PR에 포함되어 있지 않으며 Progress 컴포넌트, variant(profile/tree) 지원, 값 클램프 및 접근성 속성, Storybook 스토리·인터랙션 테스트와 런타임 의존성(@radix-ui/react-progress) 추가가 누락되어 해당 이슈의 요구사항을 만족하지 않습니다. 따라서 linked issues 전체 관점에서는 일부 요구사항이 미완료하여 불합격입니다. 해결 방안: 이 PR에 [#25]까지 포함하려면 Progress 컴포넌트 구현(variant 'profile'/'tree' 제공, value 0–100 클램프, 접근성 속성 포함), Storybook 스토리 및 인터랙션 테스트 추가, 그리고 @radix-ui/react-progress 의존성 등록을 포함해 코드를 보완하거나, 아니면 [#25]를 linked issues에서 제거하여 PR 범위를 [#63]으로 명확히 분리해 주세요.
Out of Scope Changes Check ⚠️ Warning apps/client/src/layout/Layout.tsx의 변경은 메인 콘텐츠 래퍼를 제거해 전역 레이아웃과 스크롤 동작에 영향을 주며 PR의 주된 목표([#63] 카드 편집 모달)와 직접적인 관련이 없어 보입니다. packages/design-system/src/components/infobox/InfoBox.tsx의 너비 변경은 CardEditModal에서 InfoBox 사용을 고려한 조정일 수 있으나 디자인 시스템 패키지의 변경이 별도 설명 없이 포함된 점은 검토가 필요합니다. 반면 CardEditModal과 ValidateData 추가는 PR 목적에 부합합니다. 해결 방안: Layout 관련 전역 수정은 별도의 PR로 분리하거나 PR 본문에 변경 이유와 영향 범위(리그레션 리스크, 스크린샷/테스트 결과)를 추가해 주세요. 디자인 시스템 변경도 의도와 하위 컴포넌트 영향(호환성)을 설명하고 필요 시 스토리/테스트를 추가하거나 변경을 축소해 주세요. 이렇게 하면 리뷰와 릴리즈 리스크를 줄일 수 있습니다.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed PR 제목 'Feat: card edit modal'은 이 PR의 핵심 변경사항인 카드 편집 모달 추가를 명확하고 간결하게 표현하고 있어 관련성이 높습니다. 제목은 한 문장으로 구성되어 있고 불필요한 파일 목록이나 이모지가 없어 스캔하기도 쉽습니다. 다만 팀 컨벤션에 따라 scope 접두사(예: Feat(client): card edit modal)를 추가하면 변경 이력에서 더 빠르게 구분할 수 있습니다.
Description Check ✅ Passed PR 설명은 저장소의 템플릿 구조를 충족하며 Related Issues에 'close #63'이 명시되어 있고 Tasks, PR Point(사용 예제 포함) 및 스크린샷이 포함되어 있어 리뷰에 필요한 정보가 대부분 기재되어 있습니다. 다만 Tasks에 상세 체크리스트나 API 연동 예정 등의 간단한 추적 정보를 추가하면 후속 작업 추적과 검증에 더 도움이 됩니다.

Poem

"당근 한 입, 코드 한 줄, 모달이 폴짝 열리네 🥕
날짜는 반듯히, 시간은 딱! ⏰
사이드바는 왼쪽에, Outlet은 곧장 나와,
토스트가 삐약—실패해도 다시 뛰지요.
InfoBox는 가로로 쭉—배불러라!"

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#63/card-Edit-modal

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.

❤️ Share

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

@github-actions github-actions bot added the feat 기능 개발하라 개발 달려라 달려 label Sep 10, 2025
@github-actions github-actions bot requested a review from jllee000 September 10, 2025 20:15
@github-actions
Copy link

github-actions bot commented Sep 10, 2025

✅ Storybook chromatic 배포 확인:
🐿️ storybook

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: 4

🧹 Nitpick comments (8)
apps/client/src/layout/Layout.tsx (1)

6-11: 메인 영역 폭 오버플로 방지 및 모바일 뷰포트 대응

  • 스크롤 가능한 메인 영역에 min-w-0이 없으면 내부 요소의 truncate가 깨지거나 가로 스크롤이 생길 수 있습니다.
  • 모바일(iOS Safari 등)에서 h-screen 대신 h-dvh 사용을 권장합니다.
-    <div className="flex h-screen">
+    <div className="flex h-dvh">
       <Sidebar />
-      <main className="flex-1 overflow-y-auto">
+      <main className="flex-1 min-w-0 overflow-y-auto overflow-x-hidden">
         <Outlet />
       </main>
     </div>
packages/design-system/src/components/infobox/InfoBox.tsx (1)

9-9: 이미지 대체 텍스트 추가

시각장애인 사용자 지원 및 접근성 향상을 위해 alt를 추가하세요. 장식용이라면 빈 문자열로 처리합니다.

-      <img className="h-[4.4rem] w-[4.4rem] rounded-[0.4rem]" src={imgUrl} />
+      <img
+        className="h-[4.4rem] w-[4.4rem] rounded-[0.4rem]"
+        src={imgUrl}
+        alt=""
+      />
apps/client/src/shared/utils/ValidateData.ts (2)

22-25: 미래 날짜 검증 로직 보완 제안

현재는 날짜만 미래인지 확인합니다. 카드 편집 모달처럼 날짜+시간을 함께 사용하는 경우, “오늘”을 선택했을 때는 시간까지 현재 이후인지 함께 검증하는 통합 검증이 필요할 수 있습니다(컴포넌트 레벨에서 처리해도 무방).


2-6: 날짜 입력 포맷과 디자인 시스템 placeholder 불일치
ValidateData.ts(apps/client/src/shared/utils/ValidateData.ts)에서 YYYY.MM.DD(4자리 연도)만 검증하는 반면, Design System DateTime 컴포넌트(packages/design-system/src/components/dateTime/DateTime.tsx:163)의 placeholder는 YY.MM.DD(2자리 연도)를 사용하고 있습니다. UX 혼선을 방지하려면 placeholder를 YYYY.MM.DD로 통일하거나, regex를 2자리 연도까지 허용하도록 확장하세요.

apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (4)

56-61: 모달 레이블 제공으로 접근성 강화

role="dialog"에 레이블이 없어 스크린 리더에서 용도를 파악하기 어렵습니다. 간단히 aria-label을 추가하세요(또는 aria-labelledby로 제목 요소와 연결).

-        role="dialog"
+        role="dialog"
+        aria-label="카드 수정"
         aria-modal="true"

49-51: 리마인드 OFF 시 에러 상태 초기화

스위치를 끄면 입력이 비활성화되지만 기존 에러 메시지는 그대로 남습니다. OFF 시 에러를 초기화하고, ON으로 전환 시 기존 값이 있다면 재검증하세요.

   const handleSwitchChange = (checked: boolean) => {
     setIsRemindOn(checked);
+    if (!checked) {
+      setDateError('');
+      setTimeError('');
+    } else {
+      if (date) setDateError(validateDate(date));
+      if (time) setTimeError(validateTime(time));
+    }
   };

134-135: 비활성 상태에서의 불필요한 에러 메시지 노출 방지

리마인드가 꺼져도 에러 문구가 그대로 렌더링됩니다. isRemindOn 조건을 추가하세요.

-          {dateError && <p className="body3-r text-error">{dateError}</p>}
-          {timeError && <p className="body3-r text-error">{timeError}</p>}
+          {isRemindOn && dateError && (
+            <p className="body3-r text-error">{dateError}</p>
+          )}
+          {isRemindOn && timeError && (
+            <p className="body3-r text-error">{timeError}</p>
+          )}

120-131: 시간 포맷(12h)과 검증(24h 범위 체크)의 정책 정합성 확인

DateTime오전/오후 HH:MM(12시간제) 포맷을 사용하지만, validateTimeHH:MM(24시간 범위)로만 범위를 확인합니다. 현재는 단순 범위 체크라 실사용엔 문제 없을 수 있으나, 저장 시점에 24h로 변환/정규화가 필요하다면 정책을 명확히 해두는 것이 좋습니다.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7e4e83 and 98caeb1.

📒 Files selected for processing (4)
  • apps/client/src/layout/Layout.tsx (1 hunks)
  • apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (1 hunks)
  • apps/client/src/shared/utils/ValidateData.ts (1 hunks)
  • packages/design-system/src/components/infobox/InfoBox.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T20:00:13.756Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#80
File: apps/client/src/shared/components/ui/modalPop/ModalPop.tsx:36-41
Timestamp: 2025-07-15T20:00:13.756Z
Learning: In apps/client/src/shared/components/ui/modalPop/ModalPop.tsx, the InfoBox component uses hardcoded values for title, location, and icon URL as temporary test data. These should be replaced with dynamic data from props when implementing actual functionality and should be marked with TODO comments for future changes.

Applied to files:

  • packages/design-system/src/components/infobox/InfoBox.tsx
  • apps/client/src/shared/components/cardEditModal/CardEditModal.tsx
🧬 Code graph analysis (2)
apps/client/src/layout/Layout.tsx (1)
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
  • Sidebar (27-150)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (5)
apps/client/src/shared/utils/ValidateData.ts (2)
  • validateDate (1-27)
  • validateTime (30-46)
packages/design-system/src/lib/utils.ts (1)
  • cn (108-110)
packages/design-system/src/components/dateTime/DateTime.tsx (1)
  • DateTime (56-176)
packages/design-system/src/components/toast/hooks/uesFadeOut.tsx (1)
  • AutoDismissToast (12-49)
packages/design-system/src/components/toast/Toast.tsx (1)
  • Toast (7-21)
🪛 GitHub Actions: ci
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx

[error] 28-28: TypeScript error TS6133: 'setTitle' is declared but its value is never read. (Command: tsc -b && vite build)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: storybook

Comment on lines +1 to +16
import { Icon } from '@pinback/design-system/icons';
import {
AutoDismissToast,
Button,
DateTime,
Dropdown,
InfoBox,
PopupContainer,
Switch,
Textarea,
Toast,
validateDate,
validateTime,
} from '@pinback/design-system/ui';
import { cn } from '@pinback/design-system/utils';
import { useState } from 'react';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

유효성 함수 import 경로 오류 가능성

validateDate, validateTime를 Design System에서 import하고 있는데, 이번 PR에서 추가된 파일은 apps/client/src/shared/utils/ValidateData.ts입니다. 중복 정의나 빌드 실패의 원인이 될 수 있으니 실제 소스에서 가져오도록 경로를 정정하세요.

 import {
   AutoDismissToast,
   Button,
   DateTime,
   Dropdown,
   InfoBox,
   PopupContainer,
   Switch,
   Textarea,
   Toast,
-  validateDate,
-  validateTime,
 } from '@pinback/design-system/ui';
 import { cn } from '@pinback/design-system/utils';
 import { useState } from 'react';
+import { validateDate, validateTime } from '../../utils/ValidateData';
📝 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.

Suggested change
import { Icon } from '@pinback/design-system/icons';
import {
AutoDismissToast,
Button,
DateTime,
Dropdown,
InfoBox,
PopupContainer,
Switch,
Textarea,
Toast,
validateDate,
validateTime,
} from '@pinback/design-system/ui';
import { cn } from '@pinback/design-system/utils';
import { useState } from 'react';
import { Icon } from '@pinback/design-system/icons';
import {
AutoDismissToast,
Button,
DateTime,
Dropdown,
InfoBox,
PopupContainer,
Switch,
Textarea,
Toast,
} from '@pinback/design-system/ui';
import { cn } from '@pinback/design-system/utils';
import { useState } from 'react';
import { validateDate, validateTime } from '../../utils/ValidateData';
🤖 Prompt for AI Agents
In apps/client/src/shared/components/cardEditModal/CardEditModal.tsx around
lines 1-16, the file currently imports validateDate and validateTime from the
design system but the project defines these utilities in
apps/client/src/shared/utils/ValidateData.ts; update the import to use the local
module instead of '@pinback/design-system/ui' (remove validateDate and
validateTime from that import list and import them from the correct relative
path, e.g. '../../utils/ValidateData' adjusted to the correct relative path),
ensuring there are no duplicate definitions and the build uses the local
implementations.

const InfoBox = ({ title, source, imgUrl }: InfoBoxProps) => {
return (
<div className="border-main400 flex h-[6.8rem] w-[24.8rem] items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
<div className="border-main400 flex h-[6.8rem] w-[full] items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Tailwind 클래스 오타: w-[full]는 유효하지 않습니다

w-[full]는 임의 값 구문이지만 full은 길이 값이 아니므로 스타일이 적용되지 않습니다. 의도는 w-full로 보입니다.

-    <div className="border-main400 flex h-[6.8rem] w-[full] items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
+    <div className="border-main400 flex h-[6.8rem] w-full items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
📝 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.

Suggested change
<div className="border-main400 flex h-[6.8rem] w-[full] items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
<div className="border-main400 flex h-[6.8rem] w-full items-center justify-between gap-[0.8rem] rounded-[4px] border bg-white px-[0.8rem] py-[1.2rem]">
🤖 Prompt for AI Agents
In packages/design-system/src/components/infobox/InfoBox.tsx around line 8, the
Tailwind class contains a typo: `w-[full]` is an invalid arbitrary value usage
for width; replace it with the standard `w-full` class to restore the intended
full-width styling.

@constantly-dev constantly-dev self-requested a review September 11, 2025 00:53
Copy link
Member

@constantly-dev constantly-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다! 토스트랑 이외 몇 개 코멘트 달았는데 생각보다 중요한 문제라 꼭 확인해주세요!! 👍

Comment on lines 6 to 11
<div className="flex h-screen">
<Sidebar />
<Outlet />
</>
<main className="flex-1 overflow-y-auto">
<Outlet />
</main>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 부분 제 레이아웃 PR에서 수정해서 없어도 괜찮습니다!

Comment on lines +12 to +13
validateDate,
validateTime,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요게 UI안쪽으로 들어가있나요??

}

export default function CardEditModal({ onClose }: CardEditModalProps) {
const [isRemindOn, setIsRemindOn] = useState<boolean>(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

초기 값 작성하면 타입 추론되니 제네릭 필요 없을 것 같습니다.

Comment on lines 140 to 150
{toastIsOpen && (
<div className="flex justify-center pt-[1.6rem]">
<AutoDismissToast
duration={1000}
fadeMs={1000}
onClose={() => setToastIsOpen(false)}
>
<Toast text={`저장에 실패했어요.\n다시 시도해주세요`} />
</AutoDismissToast>
</div>
)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로직을 보다보니 AutoDismissToasttoastopen/close되는 상태를 담당하고 Toast가 UI를 담당하는 로직 같네요. 두 개의 역할을 분리해주신 부분은 너무 좋지만, 사용하는 곳마다 toastIsOpen state를 정의해야하는 것은 조금은 비효율적인 설계같다고 생각이 들어요.

AutoDismissToastopen/close되는 상태를 내부에 가지고 있던데 이를 이용해서 toastIsOpen를 대체할 수 있지 않을까요? 혹은 useToast같은 훅으로 로직을 담당해서 open/close관련된 값들을 return하도록 해서 사용할 수도 있을 것 같아요.

참고할만한 민하님 라이브러리 첨부할게요!
Toast UI
Toast useToast

혹시나 스프린트 기간 내에 조금 많은 리소스가 들 것 같다면 이후에 꼭 리팩토링을 진행해봐요!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 생각치도 못한 부분이었는데 좋은 의견 감사합니다-!! 시간 들여서 꼭 공부해보고싶은 부분이네요

Comment on lines 140 to 150
{toastIsOpen && (
<div className="flex justify-center pt-[1.6rem]">
<AutoDismissToast
duration={1000}
fadeMs={1000}
onClose={() => setToastIsOpen(false)}
>
<Toast text={`저장에 실패했어요.\n다시 시도해주세요`} />
</AutoDismissToast>
</div>
)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가로 지금 올려주신 영상 보니 토스트가 모달과 형제 관계라 토스트가 열릴 때 모달이 위로 밀리는 현상이 발생하던데, 제 생각엔 이렇게 되면 안될 것 같은데 어떻게 생각하시나요?!

fixed를 통해 전체 화면 bottom 기준으로 설정하게 해도 좋을 것 같아요! 이 부분은 가능하다면 디자인 분들과 이야기를 해야할 것 같습니다!

이를 나중에는 위치 조정도 가능하게 toast interface에 추가해도 좋겠네요 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed로 수정했습니다-!

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

♻️ Duplicate comments (1)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (1)

12-14: 유효성 함수 import 경로를 로컬 utils로 교체하세요

현재 Design System에서 validateDate, validateTime를 가져오고 있으나, 본 리포지토리의 apps/client/src/shared/utils/ValidateData.ts가 단일 소스로 보입니다. 중복/불일치 방지 및 향후 유지보수를 위해 로컬 utils에서 import하도록 수정하세요.

 import {
   AutoDismissToast,
   Button,
   DateTime,
   Dropdown,
   InfoBox,
   PopupContainer,
   Switch,
   Textarea,
   Toast,
-  validateDate,
-  validateTime,
 } from '@pinback/design-system/ui';
 import { cn } from '@pinback/design-system/utils';
 import { useState } from 'react';
+import { validateDate, validateTime } from '../../utils/ValidateData';
🧹 Nitpick comments (6)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (6)

150-152: 리마인드 OFF일 때 에러 메시지 비노출

비활성화 상태에서 에러 메시지가 보이면 UX가 어색합니다. isRemindOn에 따라 에러 문구를 가드하세요.

-          {dateError && <p className="body3-r text-error">{dateError}</p>}
-          {timeError && <p className="body3-r text-error">{timeError}</p>}
+          {isRemindOn && dateError && (
+            <p className="body3-r text-error">{dateError}</p>
+          )}
+          {isRemindOn && timeError && (
+            <p className="body3-r text-error">{timeError}</p>
+          )}

61-63: 리마인드 토글 시 값/에러 상태 초기화

OFF로 전환 시 입력값과 에러를 초기화하면 재활성화 시 혼동이 줄어듭니다.

-  const handleSwitchChange = (checked: boolean) => {
-    setIsRemindOn(checked);
-  };
+  const handleSwitchChange = (checked: boolean) => {
+    setIsRemindOn(checked);
+    if (!checked) {
+      setDate('');
+      setTime('');
+      setDateError('');
+      setTimeError('');
+    }
+  };

31-33: 카테고리 추가 로직 보완: trim/빈값 검사 + 리스트 반영 + 상태 리셋

현재 saveCategory가 길이만 검사하고 리스트 반영/선택/상태 리셋이 없습니다. 또한 팝업을 열고 닫을 때 에러/입력값이 유지됩니다. 아래와 같이 보완을 제안합니다.

-  const [categories] = useState<string[]>([]);
+  const [categories, setCategories] = useState<string[]>([]);
@@
   const saveCategory = () => {
-    if (categoryTitle.length > 20) {
+    const trimmed = categoryTitle.trim();
+    if (!trimmed) {
+      setIsPopError(true);
+      setErrorTxt('제목을 입력해주세요');
+      return;
+    }
+    if (trimmed.length > 20) {
       setIsPopError(true);
       setErrorTxt('20자 이내로 작성해주세요');
-    } else {
-      setIsPopupOpen(false);
-    }
+      return;
+    }
+    setCategories((prev) =>
+      prev.includes(trimmed) ? prev : [...prev, trimmed]
+    );
+    setSelected(trimmed);
+    setIsPopError(false);
+    setErrorTxt('');
+    setCategoryTitle('');
+    setIsPopupOpen(false);
   };
@@
-            onLeftClick={() => setIsPopupOpen(false)}
+            onLeftClick={() => {
+              setIsPopError(false);
+              setErrorTxt('');
+              setCategoryTitle('');
+              setIsPopupOpen(false);
+            }}
             onRightClick={saveCategory}
@@
-            onAddItem={() => setIsPopupOpen(true)}
+            onAddItem={() => {
+              setIsPopError(false);
+              setErrorTxt('');
+              setCategoryTitle('');
+              setIsPopupOpen(true);
+            }}

Also applies to: 42-49, 88-90, 113-115


68-74: dialog 접근성: 레이블 연결(aria-labelledby) 추가

role="dialog"에는 레이블이 필요합니다. 스크린리더용 제목을 추가하고 aria-labelledby로 연결하세요.

-      <div
+      <div
         role="dialog"
         aria-modal="true"
+        aria-labelledby="card-edit-title"
         className={cn(
           'w-[31.2rem] rounded-[1.2rem] bg-white px-[3.2rem] py-[2.4rem] shadow-xl',
           'flex flex-col gap-[1.6rem]'
         )}
       >
@@
-        <header className="flex items-center justify-between">
+        <header className="flex items-center justify-between">
+          <h2 id="card-edit-title" className="sr-only">
+            카드 수정
+          </h2>
           <Icon name="logo" width={72} height={20} />
           <button
             type="button"
             aria-label="닫기"
             onClick={onClose}
             className="rounded-[0.6rem] p-[0.4rem] hover:bg-gray-100"
           >
             <Icon name="ic_close" size={20} />
           </button>
         </header>

Also applies to: 92-102


153-155: 저장 버튼 비활성화 조건 추가

검증 실패 또는 필수값 미입력 시 저장 버튼을 비활성화하면 오동작/불필요한 토스트 노출을 줄일 수 있습니다.

-        <Button onClick={() => setToastIsOpen(true)}>저장하기</Button>
+        <Button
+          disabled={
+            isRemindOn &&
+            (!!dateError || !!timeError || !date.trim() || !time.trim())
+          }
+          onClick={() => setToastIsOpen(true)}
+        >
+          저장하기
+        </Button>

156-166: Toast 포지셔닝: fixed + z-index로 오버레이/레이아웃 간섭 방지

현재 absolute는 컨테이너의 position 문맥에 따라 의도치 않은 위치/겹침이 발생할 수 있습니다. 오버레이/모달과 독립적으로 떠 있도록 fixedz-index를 사용하세요.

-      {toastIsOpen && (
-        <div className="absolute bottom-[2.4rem] left-1/2 -translate-x-1/2">
+      {toastIsOpen && (
+        <div className="fixed bottom-[2.4rem] left-1/2 -translate-x-1/2 z-50">
           <AutoDismissToast
             duration={1000}
             fadeMs={1000}
             onClose={() => setToastIsOpen(false)}
           >
             <Toast text={`저장에 실패했어요.\n다시 시도해주세요`} />
           </AutoDismissToast>
         </div>
       )}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98caeb1 and 1cff62d.

📒 Files selected for processing (2)
  • apps/client/src/layout/Layout.tsx (1 hunks)
  • apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/client/src/layout/Layout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T20:00:13.756Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#80
File: apps/client/src/shared/components/ui/modalPop/ModalPop.tsx:36-41
Timestamp: 2025-07-15T20:00:13.756Z
Learning: In apps/client/src/shared/components/ui/modalPop/ModalPop.tsx, the InfoBox component uses hardcoded values for title, location, and icon URL as temporary test data. These should be replaced with dynamic data from props when implementing actual functionality and should be marked with TODO comments for future changes.

Applied to files:

  • apps/client/src/shared/components/cardEditModal/CardEditModal.tsx
🧬 Code graph analysis (1)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (5)
apps/client/src/shared/utils/ValidateData.ts (2)
  • validateDate (1-27)
  • validateTime (30-46)
packages/design-system/src/lib/utils.ts (1)
  • cn (108-110)
packages/design-system/src/components/dateTime/DateTime.tsx (1)
  • DateTime (56-176)
packages/design-system/src/components/toast/hooks/uesFadeOut.tsx (1)
  • AutoDismissToast (12-49)
packages/design-system/src/components/toast/Toast.tsx (1)
  • Toast (7-21)

Comment on lines +56 to +59
const handleTimeChange = (value: string) => {
setTime(value);
setTimeError(validateTime(value));
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

time 입력에 대해 부모 onChange가 호출되지 않아 검증/상태가 동작하지 않습니다

Design System의 DateTime 구현(제공된 스니펫 기준)에서 type="time"은 내부 입력 이벤트를 모두 onBeforeInput/onKeyDown으로 처리하고, props.onChange는 호출하지 않습니다. 결과적으로:

  • handleTimeChange가 결코 호출되지 않아 time/timeError 상태가 갱신되지 않습니다.
  • state={timeError ? 'error' : ...}와 에러 메시지 렌더링이 무의미해집니다.

해결 방안(권장): DS의 DateTime(time)에서 값 변경 시 부모로 표준화된 문자열(HH:MM 24h 등)을 전달하도록 수정하세요. 최소 수정 예시는 아래와 같습니다.

// packages/design-system/src/components/dateTime/DateTime.tsx (요지)
-      const next = (timeDigits + add).slice(0, 4);
+      const next = (timeDigits + add).slice(0, 4);
       setTimeDigits(next);
       setInput(formatTime12(next));
+      props.onChange?.(digitsToHHMM(next)); // NEW: 부모로 24h "HH:MM" 전달
       nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
       e.preventDefault();

... (delete/paste 처리 지점에도 동일하게 호출 추가)

+function digitsToHHMM(digits: string) {
+  const h = digits.slice(0, 2).padStart(2, '0');
+  const m = digits.slice(2, 4).padStart(2, '0');
+  return `${h}:${m}`;
+}

DS 반영 전 임시 대응을 원하시면 말씀 주세요. UI에서 time 검증/표시를 비활성화하는 가드 적용안도 드릴 수 있습니다.

Also applies to: 142-147

@jjangminii jjangminii merged commit ab04579 into develop Sep 11, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat 기능 개발하라 개발 달려라 달려

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] 카테고리 수정 ui

2 participants