Skip to content
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

Shadow Panda Dialog 사용하기 #236

Merged
merged 45 commits into from
Nov 25, 2024
Merged

Conversation

sumi-0011
Copy link
Member

@sumi-0011 sumi-0011 commented Nov 21, 2024

💡 기능

ui packages에 있던 Modal 컴포넌트들을 Shadow Panda의 Dialog로 변경하였습니다.
사이즈 조정 및 확인 완료하였습니다

Summary by CodeRabbit

  • 새로운 기능

    • 사용자가 포인트가 부족할 때 피드백을 제공하는 메시지 추가: "포인트가 부족합니다."
    • 제품 관리 UI 개선을 위한 "제품 수정" 메시지 추가.
    • 사용자 포인트를 확인하는 새로운 훅 useCheckEnoughMoney 추가.
  • 버그 수정

    • 모달 구현을 Dialog 컴포넌트로 변경하여 사용자 인터페이스 개선.
    • 포인트 부족 시 오류 메시지 표시 및 요청 중단 기능 추가.
  • 문서화

  • 리팩토링

    • 여러 컴포넌트에서 스타일링 및 구조 단순화.

sumi-0011 and others added 30 commits November 4, 2024 00:42
# Conflicts:
#	apps/web/src/app/[locale]/mypage/PersonaList.tsx
#	apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx
#	apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePreview.tsx
#	apps/web/src/app/[locale]/mypage/my-pet/page.tsx
# Conflicts:
#	apps/web/src/app/[locale]/mypage/(github-custom)/FarmPersonaSelect.tsx
#	apps/web/src/app/[locale]/mypage/(github-custom)/LinePersonaSelect.tsx
#	apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx
#	apps/web/src/app/[locale]/shop/PetGotcha/TenPet.tsx
#	packages/ui/panda/src/components/Dialog/Dialog.styles.ts.tsx
#	packages/ui/panda/src/components/Dialog/Dialog.tsx
#	packages/ui/panda/src/components/Dialog/index.ts
#	packages/ui/panda/src/components/Modal/Modal.tsx
#	packages/ui/panda/src/components/Modal/ScreenModal.tsx
#	packages/ui/panda/src/components/Modal/index.ts
@sumi-0011 sumi-0011 self-assigned this Nov 21, 2024
@sumi-0011 sumi-0011 requested a review from hyesungoh as a code owner November 21, 2024 03:20
Copy link

coderabbitai bot commented Nov 21, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

이 풀 리퀘스트는 애플리케이션의 JSON 로컬라이제이션 파일과 여러 컴포넌트에서의 수정 사항을 포함합니다. 새로운 사용자 피드백 메시지와 제품 관리 인터페이스 개선을 위한 항목이 추가되었으며, 다이얼로그 컴포넌트의 스타일링 방식을 간소화하는 변경이 이루어졌습니다. 또한, useCheckEnoughMoney라는 새로운 훅이 추가되어 사용자가 포인트를 충분히 보유하고 있는지 확인하는 기능을 제공합니다. 여러 모달 관련 컴포넌트가 제거되고, 새로운 Dialog 컴포넌트로 대체되었습니다.

Changes

파일 경로 변경 요약
apps/web/messages/en_US.json "Gotcha" 섹션에 "not-enough-points" 추가, "Shop" 섹션에 "edit-product" 추가, "click-card-to-flip" 항목의 형식 수정.
apps/web/messages/ko_KR.json "Gotcha" 섹션에 "not-enough-points" 추가, "Shop" 섹션에 "edit-product" 추가, "click-card-to-flip" 항목 유지.
apps/web/src/app/[locale]/mypage/(github-custom)/FarmPersonaSelect.tsx Dialog의 사용 방식 변경, size prop을 문자열로 직접 전달.
apps/web/src/app/[locale]/mypage/(github-custom)/LinePersonaSelect.tsx Dialog의 사용 방식 변경, size prop을 문자열로 직접 전달.
apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx Dialog의 사용 방식 변경, onSuccess 콜백 수정, SelectPersonaList에 오류 경계 및 서스펜스 처리 추가.
apps/web/src/app/[locale]/shop/PetGotcha/OnePet.tsx ScreenModalBaseDialog로 변경, 포인트 확인 로직 추가.
apps/web/src/app/[locale]/shop/PetGotcha/TenPet.tsx ScreenModalBaseDialog로 변경, 포인트 확인 로직 추가.
apps/web/src/app/[locale]/shop/PetGotcha/useCheckEnoughMoney.ts 새로운 훅 useCheckEnoughMoney 추가, 포인트 확인 기능 구현.
apps/web/src/app/[locale]/shop/SellListSection/EditModal.tsx ModalDialog로 변경, 국제화 지원 추가.
apps/web/src/constants/outlink.ts 새로운 상수 GITHUB_ISSUE_URL 추가.
packages/ui/panda/src/components/Dialog/Dialog.styles.tsx 다이얼로그 스타일 정의 수정, 새로운 변형 추가.
packages/ui/panda/src/components/Dialog/Dialog.tsx Content 컴포넌트의 props 확장 및 스타일링 로직 수정.
packages/ui/panda/src/components/Dialog/index.ts Dialog.styles.ts의 export 제거.
packages/ui/panda/src/components/Modal/Modal.tsx Modal 컴포넌트 삭제.
packages/ui/panda/src/components/Modal/ScreenModal.tsx ScreenModal 및 관련 컴포넌트 삭제.
packages/ui/panda/src/components/Modal/index.ts ModalScreenModal의 export 제거.
packages/ui/panda/src/components/Modal/useDialog.ts useDialog 훅 삭제.
packages/ui/panda/src/components/index.ts Modal의 export 제거.
apps/web/src/app/[locale]/error.tsx useRouter import 위치 변경.

Possibly related issues

Possibly related PRs

  • 다국어 지원을 위한 next-intl 세팅 #178: 이 PR은 en_US.jsonko_KR.json 파일의 로컬라이제이션 변경 사항을 도입하며, 본 PR과 직접적으로 관련이 있습니다.
  • Auction 섹션 리뉴얼 #191: 이 PR은 en_US.jsonko_KR.json 파일에 새로운 키를 추가하여 본 PR의 변경 사항과 연결됩니다.
  • 배경 구매 기능 추가 #217: 이 PR은 en_US.jsonko_KR.json 파일에 배경 구매 기능에 대한 새로운 로컬라이제이션 항목을 추가하여 본 PR과 관련이 있습니다.
  • 마이페이지 펫 선택 -> 모달 #222: 이 PR은 en_US.jsonko_KR.json 파일에 펫 선택에 대한 새로운 로컬라이제이션 항목을 추가하여 본 PR과 직접적으로 관련이 있습니다.
  • 펫 Merge 기능 추가  #230: 이 PR은 en_US.jsonko_KR.json 파일에 펫 병합 기능에 대한 새로운 로컬라이제이션 항목을 도입하여 본 PR의 변경 사항과 연결됩니다.

Suggested labels

diff: XL, area: Lib

Suggested reviewers

  • hyesungoh

🐰 변화의 바람이 불어와
다이얼로그가 새로워졌네,
포인트 확인, 사용자 피드백,
더 나은 경험을 위해 뛰어가네!
귀여운 펫과 함께하는 길,
즐거움과 함께 달려가자! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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

🧹 Outside diff range and nitpick comments (14)
apps/web/src/app/[locale]/shop/PetGotcha/useCheckEnoughMoney.ts (2)

1-6: 상수 정의 방식 개선 제안

상수 관리를 더욱 효율적으로 하기 위한 개선사항을 제안드립니다:

-const TEN_PET_POINT = 10000;
-const ONE_PET_POINT = 1000;
+const GOTCHA_POINTS = {
+  TEN: 10000,
+  ONE: 1000,
+} as const;

이렇게 수정하면:

  • 관련 상수들을 하나의 객체로 그룹화하여 관리가 용이
  • as const를 통해 타입 안정성 향상
  • 향후 포인트 값 변경 시 한 곳에서 관리 가능

7-7: 타입 정의 개선 제안

타입 안정성과 유지보수성을 높이기 위한 개선사항입니다:

-type CheckType = 'ten-pet-gotcha' | 'one-pet-gotcha';
+const GOTCHA_TYPES = {
+  TEN: 'ten-pet-gotcha',
+  ONE: 'one-pet-gotcha',
+} as const;
+type CheckType = typeof GOTCHA_TYPES[keyof typeof GOTCHA_TYPES];

이렇게 수정하면:

  • 타입과 값이 항상 동기화됨
  • 문자열 리터럴 사용 시 오타 방지
  • IDE의 자동완성 지원 개선
packages/ui/panda/src/components/Dialog/Dialog.styles.ts.tsx (1)

14-27: default 사이즈 변형의 반응형 스타일 보완 필요

default 사이즈에 대한 모바일 대응이 누락되어 있습니다. large나 screen 변형처럼 반응형 스타일을 추가하는 것이 좋겠습니다.

  default: {
    display: 'flex',
    width: '100%',
    flexDirection: 'column',
    justifyContent: 'center',
    alignItems: 'center',
    gap: '28px',
    color: 'white',
    '& .dialog-title': {
      textStyle: 'glyph20.regular',
      textAlign: 'left',
      width: '100%',
    },
+   '@media (max-width: 768px)': {
+     gap: '20px',
+     '& .dialog-title': {
+       textStyle: 'glyph18.regular',
+     },
+   },
  },
apps/web/src/app/[locale]/shop/SellListSection/EditModal.tsx (1)

55-74: Dialog 구현에 대한 접근성 개선 제안

Dialog 컴포넌트가 잘 구현되었습니다만, 다음과 같은 접근성 개선을 고려해보시는 것은 어떨까요?

  1. input 요소에 aria-label 추가
  2. 저장/삭제 버튼에 대한 키보드 네비게이션 순서 최적화
 <input
   className={inputStyle}
   placeholder="Type price..."
   type="number"
   value={Boolean(price) ? price : ''}
   onChange={(e) => setPrice(Number(e.target.value))}
+  aria-label={t('edit-product-price-label')}
 />
🧰 Tools
🪛 Biome

[error] 62-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

packages/ui/panda/src/components/Dialog/Dialog.tsx (2)

9-10: 불필요한 console.log 제거 필요

디버깅을 위한 console.log가 프로덕션 코드에 남아있습니다. 제거하는 것이 좋겠습니다.

- console.log('props: ', props);

20-20: 다이얼로그 크기 설정 로직 개선 제안

size prop의 기본값 설정 로직을 컴포넌트 외부로 분리하면 가독성이 개선될 것 같습니다.

- className={cx(dialogContentCva({ size: props.size ?? 'default' }), props.className)}
+ const contentSize = props.size ?? 'default';
+ className={cx(dialogContentCva({ size: contentSize }), props.className)}

Also applies to: 24-28

apps/web/src/app/[locale]/shop/PetGotcha/TenPet.tsx (2)

89-94: 포인트 체크 로직이 적절히 구현되었습니다.

사용자의 포인트가 부족할 경우 적절한 에러 메시지를 표시하고 조기 반환하는 로직이 잘 구현되어 있습니다. 다만, 아래와 같은 개선사항을 고려해보세요:

 const onAction = async () => {
-  if (!checkEnoughMoney()) {
+  const hasEnoughMoney = checkEnoughMoney();
+  if (!hasEnoughMoney) {
     toast.error(t('not-enough-points'));
     return;
   }
   if (isPending) return;

가독성을 위해 변수로 추출하는 것을 추천드립니다.


107-119: Dialog 컴포넌트가 올바르게 구현되었습니다.

Shadow Panda의 Dialog 컴포넌트로의 마이그레이션이 잘 이루어졌습니다. 모달의 구조와 스타일링이 적절합니다.

접근성 관련 속성들이 잘 설정되어 있으나, 스크린 리더 사용자를 위한 추가적인 고려사항이 있습니다:

-<Dialog.Content size="screen">
+<Dialog.Content size="screen" aria-live="polite">

진행 상태가 변경될 때 스크린 리더가 이를 알릴 수 있도록 aria-live 속성 추가를 고려해보세요.

apps/web/src/app/[locale]/shop/PetGotcha/OnePet.tsx (2)

43-47: 포인트 체크 로직에 대한 개선 제안

포인트 부족 상황에 대한 기본적인 처리가 잘 구현되어 있습니다. 다만 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:

  1. 사용자에게 현재 보유 포인트와 필요한 포인트를 함께 표시
  2. 포인트 충전 페이지로 이동할 수 있는 옵션 제공
 if (!checkEnoughMoney()) {
-  toast.error(t('not-enough-points'));
+  toast.error(t('not-enough-points'), {
+    action: {
+      label: t('charge-points'),
+      onClick: () => router.push('/points')
+    }
+  });
   return;
 }

116-124: Dialog 컴포넌트가 적절하게 구현되었습니다.

Shadow Panda의 Dialog 컴포넌트로의 전환이 잘 이루어졌습니다. 스타일링도 기존의 기능을 유지하면서 잘 적용되었습니다.

한 가지 제안드릴 부분은 Dialog의 크기 조정 시 반응형 디자인을 고려해보시면 좋을 것 같습니다.

-      <Dialog.Content size="screen">
+      <Dialog.Content 
+        size={{
+          base: "full",
+          md: "screen"
+        }}>
apps/web/messages/en_US.json (1)

42-42: 메시지가 명확하게 작성되었습니다!

사용자에게 포인트 부족 상황을 명확하게 전달하는 메시지가 잘 추가되었습니다. 다만, 다른 에러 메시지들과의 일관성을 위해 끝에 마침표를 추가하는 것이 좋을 것 같습니다.

-    "not-enough-points": "You don't have enough points to draw a pet."
+    "not-enough-points": "You don't have enough points to draw a pet.."
apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx (3)

Line range hint 38-52: 상태 관리 로직의 분리를 제안합니다.

현재 컴포넌트 내부에 머지 관련 상태 관리 로직이 복잡하게 구현되어 있습니다. 다음과 같은 개선을 제안드립니다:

  1. 머지 관련 로직을 커스텀 훅으로 분리
  2. 상태 업데이트 로직을 단순화

예시 구현:

const useMergePersonaState = (initTargetPersona: Persona) => {
  const [state, setState] = useState({
    materialPersona: null,
    targetPersona: initTargetPersona,
    resultData: null
  });

  const resetState = (newTargetPersona: Persona) => {
    setState({
      materialPersona: null,
      targetPersona: newTargetPersona,
      resultData: null
    });
  };

  return { state, setState, resetState };
};

Line range hint 108-108: 정렬 관련 TODO 항목 처리가 필요합니다.

정렬 기능 구현이 필요한 것으로 보입니다. 페르소나 목록에 대한 정렬 로직을 추가해주세요.

정렬 로직 구현을 도와드릴까요? 다음과 같은 옵션을 고려할 수 있습니다:

  • 레벨 기준 정렬
  • 획득 시간 기준 정렬
  • 페르소나 타입 기준 정렬

Line range hint 99-102: 에러 바운더리 폴백 UI 개선이 필요합니다.

현재 에러 발생 시 단순히 "error" 텍스트만 표시됩니다. 사용자 경험 향상을 위해 더 자세한 에러 메시지와 재시도 옵션을 제공하는 것이 좋겠습니다.

const ErrorFallback = ({ error, resetErrorBoundary }) => (
  <div className={errorContainerStyle}>
    <p>페르소나 목록을 불러오는  문제가 발생했습니다.</p>
    <Button onClick={resetErrorBoundary}>다시 시도</Button>
  </div>
);
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4e00a2f and f7292cb.

📒 Files selected for processing (18)
  • apps/web/messages/en_US.json (2 hunks)
  • apps/web/messages/ko_KR.json (2 hunks)
  • apps/web/src/app/[locale]/mypage/(github-custom)/FarmPersonaSelect.tsx (2 hunks)
  • apps/web/src/app/[locale]/mypage/(github-custom)/LinePersonaSelect.tsx (2 hunks)
  • apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx (2 hunks)
  • apps/web/src/app/[locale]/shop/PetGotcha/OnePet.tsx (3 hunks)
  • apps/web/src/app/[locale]/shop/PetGotcha/TenPet.tsx (4 hunks)
  • apps/web/src/app/[locale]/shop/PetGotcha/useCheckEnoughMoney.ts (1 hunks)
  • apps/web/src/app/[locale]/shop/SellListSection/EditModal.tsx (3 hunks)
  • apps/web/src/constants/outlink.ts (1 hunks)
  • packages/ui/panda/src/components/Dialog/Dialog.styles.ts.tsx (2 hunks)
  • packages/ui/panda/src/components/Dialog/Dialog.tsx (3 hunks)
  • packages/ui/panda/src/components/Dialog/index.ts (0 hunks)
  • packages/ui/panda/src/components/Modal/Modal.tsx (0 hunks)
  • packages/ui/panda/src/components/Modal/ScreenModal.tsx (0 hunks)
  • packages/ui/panda/src/components/Modal/index.ts (0 hunks)
  • packages/ui/panda/src/components/Modal/useDialog.ts (0 hunks)
  • packages/ui/panda/src/components/index.ts (0 hunks)
💤 Files with no reviewable changes (6)
  • packages/ui/panda/src/components/Dialog/index.ts
  • packages/ui/panda/src/components/Modal/Modal.tsx
  • packages/ui/panda/src/components/Modal/ScreenModal.tsx
  • packages/ui/panda/src/components/Modal/index.ts
  • packages/ui/panda/src/components/Modal/useDialog.ts
  • packages/ui/panda/src/components/index.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/web/src/constants/outlink.ts
🧰 Additional context used
🪛 Biome
apps/web/src/app/[locale]/shop/SellListSection/EditModal.tsx

[error] 62-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

🔇 Additional comments (19)
packages/ui/panda/src/components/Dialog/Dialog.styles.ts.tsx (2)

71-76: 타입 정의 및 기본값 설정이 적절합니다.

defaultVariants 설정과 DialogContentVariants 타입 정의가 잘 되어있습니다. 타입 안전성을 보장하면서도 기본값을 명확하게 지정하고 있습니다.


4-11: 베이스 스타일의 색상 대비 검토 필요

gray.gray_150 배경색과 white.white_100 텍스트 색상의 조합이 접근성 기준을 충족하는지 확인이 필요합니다. WCAG 2.1 기준에 따른 충분한 대비를 보장해야 합니다.

apps/web/src/app/[locale]/mypage/(github-custom)/LinePersonaSelect.tsx (2)

7-7: Dialog 컴포넌트 임포트 간소화 승인

Shadow Panda의 Dialog 컴포넌트로의 전환이 깔끔하게 이루어졌습니다.


39-39: 스타일링 접근 방식 개선 확인 필요

Dialog.Content의 스타일링이 더 간단해졌습니다. 하지만 다른 크기 옵션들도 정상적으로 동작하는지 확인이 필요합니다.

다른 컴포넌트들의 Dialog 사용을 확인하기 위해 다음 스크립트를 실행하세요:

✅ Verification successful

Dialog.Content의 size prop 사용이 일관되게 적용됨

코드베이스 검사 결과, Dialog.Content의 size prop 사용이 모든 컴포넌트에서 일관되게 적용되어 있습니다. 'large'와 'screen' 크기가 각각의 용도에 맞게 적절히 사용되고 있으며, 기본 Dialog 컴포넌트에서도 size prop이 올바르게 지원됩니다.

  • packages/ui/panda/src/components/Dialog/Dialog.tsx에서 size prop이 정상적으로 처리되고 있음
  • 게임 관련 화면(PetGotcha)에서는 'screen' 크기 사용
  • 선택 모달(LinePersonaSelect, FarmPersonaSelect, MergePersona)에서는 'large' 크기 사용
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Dialog 컴포넌트의 size prop 사용 패턴 확인

# Dialog.Content 컴포넌트의 size prop 사용 검색
ast-grep --pattern 'Dialog.Content size="$_"'

# Dialog 관련 스타일링 패턴 검색
rg "Dialog\.Content.*size.*" -t tsx

Length of output: 113


Script:

#!/bin/bash
# Search for Dialog.Content usage patterns
rg "Dialog\.Content.*size" --type-add 'tsx:*.tsx' --type tsx

# Find all Dialog.Content components
rg "<Dialog\.Content" --type-add 'tsx:*.tsx' --type tsx -A 2

# Find any styling related to Dialog.Content
rg "dialogContentCva|Dialog\.Content.*className" --type-add 'tsx:*.tsx' --type tsx

Length of output: 3363

apps/web/src/app/[locale]/shop/SellListSection/EditModal.tsx (3)

4-4: import 변경사항이 적절합니다!

Shadow Panda Dialog로의 마이그레이션과 다국어 지원을 위한 import 문이 잘 추가되었습니다.

Also applies to: 7-7


113-113: 스타일 변경이 적절합니다!

버튼 래퍼의 너비를 100%로 설정한 것이 레이아웃 일관성에 도움이 됩니다.


13-13: 다국어 설정이 추가되었습니다.

Shop 네임스페이스의 번역이 올바르게 구현되었습니다. 다만, 관련 번역 키가 모든 언어에 추가되었는지 확인이 필요합니다.

✅ Verification successful

번역 설정이 올바르게 구현되었습니다.

Shop 네임스페이스의 'edit-product' 키가 한국어와 영어 모두 messages 파일에 정의되어 있으며, EditModal 컴포넌트에서 올바르게 사용되고 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify translation keys exist in all locale files
rg -g "*.json" "edit-product" public/locales/

Length of output: 101


Script:

#!/bin/bash
# First, let's find where the locale files are stored
fd -t f -e json messages

# Then check the content of these files
fd -t f -e json messages -x cat {}

Length of output: 65


Script:

#!/bin/bash
# Let's try to find any translation related files
fd -t f -e json . | grep -i "locale\|translation\|lang"

# Also search for translation related directories
fd -t d . | grep -i "locale\|translation\|lang"

# And check for any imports of translation files in the codebase
rg -g "*.{ts,tsx}" "import.*translation|useTranslation"

Length of output: 6756


Script:

#!/bin/bash
# Let's check the messages directory in the Next.js app
fd -t f -e json . apps/web/messages

# Also check for any translation configuration files
fd -t f -e js -e ts "i18n.config"

# And look for translation namespace definitions
rg -g "*.{ts,tsx}" "Shop.*=.*{" -A 5

Length of output: 252


Script:

#!/bin/bash
# Let's check the content of translation files
cat apps/web/messages/en_US.json
cat apps/web/messages/ko_KR.json

# Also check for Shop namespace usage patterns
rg -g "*.{ts,tsx}" "useTranslations\('Shop'\)" -B 2 -A 2

Length of output: 12711

apps/web/src/app/[locale]/shop/PetGotcha/TenPet.tsx (2)

8-8: import 구문이 적절히 구성되어 있습니다.

Shadow Panda의 Dialog 컴포넌트와 필요한 의존성들이 올바르게 임포트되었습니다.

Also applies to: 13-13, 18-18


33-34: 포인트 체크 로직 검증이 필요합니다.

useCheckEnoughMoney 훅이 추가되었습니다. 10회 뽑기에 필요한 포인트가 올바르게 설정되어 있는지 확인해주세요.

✅ Verification successful

Let me search for the point constants to verify the calculation logic.


포인트 체크 로직이 정상적으로 구현되어 있습니다.

useCheckEnoughMoney 훅의 포인트 체크 로직을 확인한 결과:

  • 10회 뽑기: 10,000 포인트
  • 1회 뽑기: 1,000 포인트
    로 올바르게 설정되어 있습니다.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify point calculation logic in useCheckEnoughMoney hook

# Search for point calculation logic
ast-grep --pattern 'useCheckEnoughMoney' -A 10

Length of output: 4493


Script:

#!/bin/bash
# Search for point constants
ast-grep --pattern 'const TEN_PET_POINT' -A 2
ast-grep --pattern 'const ONE_PET_POINT' -A 2

Length of output: 565

apps/web/src/app/[locale]/shop/PetGotcha/OnePet.tsx (2)

8-8: import 구문이 적절하게 수정되었습니다.

UI 패키지의 Modal 컴포넌트를 Shadow Panda의 Dialog로 교체하는 PR의 목적에 맞게 import 구문이 잘 수정되었습니다.

Also applies to: 14-14, 20-20


39-39: useCheckEnoughMoney 훅의 구현을 확인해주세요.

'one-pet-gotcha' 파라미터가 올바르게 전달되었는지, 그리고 훅이 예상대로 동작하는지 확인이 필요합니다.

✅ Verification successful

useCheckEnoughMoney 훅이 정상적으로 구현되어 있습니다

useCheckEnoughMoney 훅의 구현을 확인한 결과:

  • 'one-pet-gotcha' 파라미터가 올바르게 처리되며 1,000 포인트를 기준으로 검사합니다
  • userQueries를 통해 사용자 데이터를 가져와 포인트를 확인하는 로직이 정상적으로 구현되어 있습니다
  • 반환되는 checkEnoughMoney 함수가 예상대로 동작합니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of useCheckEnoughMoney hook
ast-grep --pattern 'export function useCheckEnoughMoney($_) {
  $$$
}'

Length of output: 72


Script:

#!/bin/bash
# Search for useCheckEnoughMoney hook implementation with different patterns
rg -l "useCheckEnoughMoney"

# Search for files containing 'one-pet-gotcha' string
rg "one-pet-gotcha"

Length of output: 455


Script:

#!/bin/bash
# Get the implementation of useCheckEnoughMoney hook
cat apps/web/src/app/[locale]/shop/PetGotcha/useCheckEnoughMoney.ts

Length of output: 632

apps/web/src/app/[locale]/mypage/(github-custom)/FarmPersonaSelect.tsx (2)

7-7: Shadow Panda Dialog로의 마이그레이션이 잘 진행되었습니다!

기존의 복잡한 스타일링 접근 방식에서 Shadow Panda의 Dialog 컴포넌트로 깔끔하게 전환되었습니다.


83-83: 다른 컴포넌트들과의 일관성 확인이 필요합니다.

Dialog 크기 설정이 size="large"로 직접 전달되도록 변경되었습니다. 이 방식이 다른 컴포넌트들에서도 일관되게 적용되었는지 확인이 필요합니다.

다음 스크립트를 실행하여 Dialog 사용 패턴을 확인해보세요:

✅ Verification successful

Dialog 컴포넌트의 size prop 사용이 일관되게 적용되어 있습니다.

검증 결과, Dialog.Content의 size prop 사용이 다음과 같이 일관되게 적용되어 있음을 확인했습니다:

  • size="large": FarmPersonaSelect, LinePersonaSelect, MergePersona에서 사용
  • size="screen": PetGotcha 관련 컴포넌트(OnePet, TenPet)에서 사용
  • size prop 없음: EditModal에서 기본값 사용

또한 packages/ui/panda/src/components/Dialog/Dialog.tsx에서 dialogContentCva를 통해 size prop이 올바르게 처리되고 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Dialog 컴포넌트의 size prop 사용 패턴 검증

# Dialog.Content 컴포넌트의 사용 패턴 검색
ast-grep --pattern 'Dialog.Content size="$_"'

# 이전 방식(dialogContentCva)이 남아있는지 확인
rg "dialogContentCva"

Length of output: 581


Script:

#!/bin/bash
# Dialog.Content 컴포넌트의 모든 사용 패턴을 검색
rg -A 1 "Dialog\.Content" --type tsx

# Dialog 컴포넌트의 구현 확인
ast-grep --pattern 'const Dialog = $$$'

Length of output: 817


Script:

#!/bin/bash
# Dialog.Content 컴포넌트의 모든 사용 패턴을 검색 (확장자 수정)
rg -A 1 "Dialog\.Content" -g "*.tsx" -g "*.jsx"

# DialogContent 컴포넌트 구현 확인
ast-grep --pattern 'const DialogContent = ({ $$$'

Length of output: 2471

apps/web/messages/ko_KR.json (2)

41-42: 메시지가 명확하고 일관성이 있습니다!

사용자에게 포인트 부족 상황을 명확하게 전달하는 메시지입니다.


69-69: 메시지가 적절하며 기능을 잘 설명합니다!

"펫 경매 수정"이라는 문구가 기능을 명확하게 설명하고 있습니다.

apps/web/messages/en_US.json (2)

41-41: 포맷팅이 올바르게 적용되었습니다!

후행 쉼표를 추가한 것은 향후 새로운 항목을 추가할 때 더 깔끔한 변경 이력을 만들 수 있어 좋은 방식입니다.


67-67: 상품 편집 메시지가 적절하게 추가되었습니다!

Shadow Panda Dialog 컴포넌트로의 전환 과정에서 필요한 상품 편집 관련 메시지가 잘 구현되었습니다.

apps/web/src/app/[locale]/mypage/my-pet/(merge)/MergePersona.tsx (2)

8-8: UI 패키지 의존성이 개선되었습니다.

Shadow Panda의 Dialog 컴포넌트로 전환하면서 import 구문이 단순화되었습니다.


70-70: Dialog 컴포넌트의 size prop 사용을 검증해주세요.

Shadow Panda Dialog로 마이그레이션하면서 size prop을 직접 사용하도록 변경되었습니다. 모든 케이스에서 올바른 크기가 적용되는지 확인이 필요합니다.

✅ Verification successful

Dialog.Content의 size prop 사용이 일관되게 적용되어 있습니다

Dialog.Content 컴포넌트의 size prop 사용을 검토한 결과:

  • 카드 선택/가챠 관련 다이얼로그(OnePet.tsx, TenPet.tsx)는 size="screen" 사용
  • 페르소나 선택/병합 관련 다이얼로그(LinePersonaSelect.tsx, FarmPersonaSelect.tsx, MergePersona.tsx)는 size="large" 사용
  • 기본 편집 다이얼로그(EditModal.tsx)는 size prop 없이 기본 크기 사용

각 다이얼로그의 용도에 맞게 size prop이 적절하게 사용되고 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Dialog 컴포넌트의 size prop 사용 패턴 확인
ast-grep --pattern 'Dialog.Content size="$_"'

Length of output: 47


Script:

#!/bin/bash
# Dialog.Content 컴포넌트의 사용 패턴을 더 넓게 검색
rg -U "Dialog\.Content.*size.*" -A 2

# Dialog.Content 컴포넌트의 전반적인 사용 패턴 확인
rg -U "Dialog\.Content" -A 2

Length of output: 5465

Comment on lines 9 to 18
export const useCheckEnoughMoney = (type: CheckType) => {
const { data: userData } = useQuery(userQueries.userOptions());

const checkEnoughMoney = () => {
const point = type === 'ten-pet-gotcha' ? TEN_PET_POINT : ONE_PET_POINT;
return userData?.points && Number(userData.points) >= point;
};

return { checkEnoughMoney };
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

훅 구현의 안정성 개선 필요

다음과 같은 중요한 개선사항이 필요합니다:

 export const useCheckEnoughMoney = (type: CheckType) => {
-  const { data: userData } = useQuery(userQueries.userOptions());
+  const { data: userData, isLoading, error } = useQuery(userQueries.userOptions());
 
   const checkEnoughMoney = () => {
+    if (isLoading) return false;
+    if (error || !userData?.points) return false;
+
     const point = type === 'ten-pet-gotcha' ? TEN_PET_POINT : ONE_PET_POINT;
-    return userData?.points && Number(userData.points) >= point;
+    return BigInt(userData.points) >= BigInt(point);
   };
 
-  return { checkEnoughMoney };
+  return { 
+    checkEnoughMoney,
+    isLoading,
+    error 
+  };
 };

개선사항:

  1. 로딩 상태 처리 추가
  2. 에러 상태 처리 추가
  3. Number() 대신 BigInt 사용하여 큰 숫자 처리 안정성 향상
  4. 상태 정보를 반환하여 컴포넌트에서 더 나은 UX 구현 가능
📝 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
export const useCheckEnoughMoney = (type: CheckType) => {
const { data: userData } = useQuery(userQueries.userOptions());
const checkEnoughMoney = () => {
const point = type === 'ten-pet-gotcha' ? TEN_PET_POINT : ONE_PET_POINT;
return userData?.points && Number(userData.points) >= point;
};
return { checkEnoughMoney };
};
export const useCheckEnoughMoney = (type: CheckType) => {
const { data: userData, isLoading, error } = useQuery(userQueries.userOptions());
const checkEnoughMoney = () => {
if (isLoading) return false;
if (error || !userData?.points) return false;
const point = type === 'ten-pet-gotcha' ? TEN_PET_POINT : ONE_PET_POINT;
return BigInt(userData.points) >= BigInt(point);
};
return {
checkEnoughMoney,
isLoading,
error
};
};

Comment on lines +49 to +68
screen: {
margin: 'auto',
borderRadius: '0',
backgroundColor: 'gray.gray_150',
padding: '24px',
width: '100vw',
height: '100vh',
maxWidth: '100vw',
maxHeight: '100vh',
display: 'flex',
flexDirection: 'column',
alignItems: 'center',
justifyContent: 'center',

'@media (min-width: 1920px)': {
borderRadius: '16px',
width: '1400px',
height: 'fit-content',
},
},
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

screen 사이즈 변형의 스타일 최적화 제안

screen 변형에서 반복되는 스타일 값들이 있습니다. 또한 큰 화면에서의 반응형 처리가 1920px로 고정되어 있어 다양한 화면 크기에 대한 대응이 제한적일 수 있습니다.

  screen: {
+   // 공통 스타일을 상단으로 이동
+   display: 'flex',
+   flexDirection: 'column',
+   alignItems: 'center',
+   justifyContent: 'center',
+   backgroundColor: 'gray.gray_150',
+   
    margin: 'auto',
    borderRadius: '0',
-   backgroundColor: 'gray.gray_150',
    padding: '24px',
    width: '100vw',
    height: '100vh',
    maxWidth: '100vw',
    maxHeight: '100vh',
-   display: 'flex',
-   flexDirection: 'column',
-   alignItems: 'center',
-   justifyContent: 'center',

-   '@media (min-width: 1920px)': {
+   '@media (min-width: 1200px)': {
      borderRadius: '16px',
-     width: '1400px',
+     width: 'min(1400px, 90vw)',
      height: 'fit-content',
    },
  },
📝 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
screen: {
margin: 'auto',
borderRadius: '0',
backgroundColor: 'gray.gray_150',
padding: '24px',
width: '100vw',
height: '100vh',
maxWidth: '100vw',
maxHeight: '100vh',
display: 'flex',
flexDirection: 'column',
alignItems: 'center',
justifyContent: 'center',
'@media (min-width: 1920px)': {
borderRadius: '16px',
width: '1400px',
height: 'fit-content',
},
},
screen: {
// 공통 스타일을 상단으로 이동
display: 'flex',
flexDirection: 'column',
alignItems: 'center',
justifyContent: 'center',
backgroundColor: 'gray.gray_150',
margin: 'auto',
borderRadius: '0',
padding: '24px',
width: '100vw',
height: '100vh',
maxWidth: '100vw',
maxHeight: '100vh',
'@media (min-width: 1200px)': {
borderRadius: '16px',
width: 'min(1400px, 90vw)',
height: 'fit-content',
},
},

Comment on lines +62 to +63
value={Boolean(price) ? price : ''}
onChange={(e) => setPrice(Number(e.target.value))}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

불필요한 Boolean 변환 제거 필요

value 속성에서 불필요한 Boolean 변환이 발견되었습니다. 다음과 같이 간단화할 수 있습니다:

-value={Boolean(price) ? price : ''}
+value={price || ''}
📝 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
value={Boolean(price) ? price : ''}
onChange={(e) => setPrice(Number(e.target.value))}
value={price || ''}
onChange={(e) => setPrice(Number(e.target.value))}
🧰 Tools
🪛 Biome

[error] 62-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

Comment on lines +46 to +53
>(({ children, ...props }, ref) => {
console.log('props: ', props);
return (
<DialogPrimitive.Title ref={ref} {...props} className={cx('dialog-title', titleStyle, props.className)}>
{children}
</DialogPrimitive.Title>
);
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Title 컴포넌트의 하드코딩된 클래스명 개선 필요

'dialog-title'이 하드코딩되어 있습니다. 상수로 분리하거나 스타일 시스템을 활용하는 것이 좋겠습니다.

+ const DIALOG_TITLE_CLASS = 'dialog-title';
...
- className={cx('dialog-title', titleStyle, props.className)}
+ className={cx(DIALOG_TITLE_CLASS, titleStyle, props.className)}

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Member

@hyesungoh hyesungoh 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 43 to 46
if (!checkEnoughMoney()) {
toast.error(t('not-enough-points'));
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

try문 안에 있으면 throw 하는 것도 방법일 거 같네요 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

error를 throw 하고 catch나 에러바운더리에서 error를 캐치해서 토스트를 띄우면 좋겠다 라고 이해했는데 맞을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

carbon (4)

Choose a reason for hiding this comment

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

뫄자용 👍 exception을 띄워서 핸들링해도 될 거 같긴 한데, 규모도 커지고 블랙박스처럼 느껴질 수도 있을 거 같아서

더 생각해보니 원래 작성해두신 방향도 괜찮을 거 같기도 하네요 ㅋㅋㅋ

수미님은 어떤게 더 나은 방법이라고 생각하시나용

Copy link
Member Author

Choose a reason for hiding this comment

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

저는 지금처럼 에러 throw하고, catch에서 잡아서 핸들링하는게 더 좋은 것 같아요!
블랙박스 처럼 느껴질 수도 있을것 같다 <- 이 부분에도 동의하지만
만약 이런 예와사항등이 여러개가 생겨서 if,if, if 이렇게 반복되게 되는 상황에 제안해주시는 방법이 더 효과적이고 "에러를 토스트로 보여준다." 라는것을 한번에 볼 수 있어서 좋을 것 같아요

Comment on lines 4 to 5
const TEN_PET_POINT = 10000;
const ONE_PET_POINT = 1000;
Copy link
Member

Choose a reason for hiding this comment

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

요 포인트 수치에 대한 의존성이 여기 있는것이 판단하기 쉬울지 고민돼요 🤔

각 뽑기를 하는 곳에 있는 것이 일반적인 생각처럼 느껴지기도 해서요 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

좋아요~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants