-
Notifications
You must be signed in to change notification settings - Fork 7
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
Conversation
…ods/git-animal-client into feat/setting-shadow-panda
# 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
Caution Review failedThe pull request is closed. Walkthrough이 풀 리퀘스트는 애플리케이션의 JSON 로컬라이제이션 파일과 여러 컴포넌트에서의 수정 사항을 포함합니다. 새로운 사용자 피드백 메시지와 제품 관리 인터페이스 개선을 위한 항목이 추가되었으며, 다이얼로그 컴포넌트의 스타일링 방식을 간소화하는 변경이 이루어졌습니다. 또한, Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
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
🧹 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 컴포넌트가 잘 구현되었습니다만, 다음과 같은 접근성 개선을 고려해보시는 것은 어떨까요?
- input 요소에 aria-label 추가
- 저장/삭제 버튼에 대한 키보드 네비게이션 순서 최적화
<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
callIt is not necessary to use
Boolean
call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBoolean
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
: 포인트 체크 로직에 대한 개선 제안포인트 부족 상황에 대한 기본적인 처리가 잘 구현되어 있습니다. 다만 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- 사용자에게 현재 보유 포인트와 필요한 포인트를 함께 표시
- 포인트 충전 페이지로 이동할 수 있는 옵션 제공
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
: 상태 관리 로직의 분리를 제안합니다.현재 컴포넌트 내부에 머지 관련 상태 관리 로직이 복잡하게 구현되어 있습니다. 다음과 같은 개선을 제안드립니다:
- 머지 관련 로직을 커스텀 훅으로 분리
- 상태 업데이트 로직을 단순화
예시 구현:
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
📒 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
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 }; | ||
}; |
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.
훅 구현의 안정성 개선 필요
다음과 같은 중요한 개선사항이 필요합니다:
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
+ };
};
개선사항:
- 로딩 상태 처리 추가
- 에러 상태 처리 추가
Number()
대신BigInt
사용하여 큰 숫자 처리 안정성 향상- 상태 정보를 반환하여 컴포넌트에서 더 나은 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.
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 | |
}; | |
}; |
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', | ||
}, | ||
}, |
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.
🛠️ 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.
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', | |
}, | |
}, |
value={Boolean(price) ? price : ''} | ||
onChange={(e) => setPrice(Number(e.target.value))} |
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.
🛠️ 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.
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)
>(({ children, ...props }, ref) => { | ||
console.log('props: ', props); | ||
return ( | ||
<DialogPrimitive.Title ref={ref} {...props} className={cx('dialog-title', titleStyle, props.className)}> | ||
{children} | ||
</DialogPrimitive.Title> | ||
); | ||
}); |
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.
🛠️ 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.
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.
마이너한 고민들 좀 적어봤어용
고생하셨어요 최고 👍
if (!checkEnoughMoney()) { | ||
toast.error(t('not-enough-points')); | ||
return; | ||
} |
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.
try문 안에 있으면 throw 하는 것도 방법일 거 같네요 🤔
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.
error를 throw 하고 catch나 에러바운더리에서 error를 캐치해서 토스트를 띄우면 좋겠다 라고 이해했는데 맞을까요?
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.
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.
뫄자용 👍 exception을 띄워서 핸들링해도 될 거 같긴 한데, 규모도 커지고 블랙박스처럼 느껴질 수도 있을 거 같아서
더 생각해보니 원래 작성해두신 방향도 괜찮을 거 같기도 하네요 ㅋㅋㅋ
수미님은 어떤게 더 나은 방법이라고 생각하시나용
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.
저는 지금처럼 에러 throw하고, catch에서 잡아서 핸들링하는게 더 좋은 것 같아요!
블랙박스 처럼 느껴질 수도 있을것 같다 <- 이 부분에도 동의하지만
만약 이런 예와사항등이 여러개가 생겨서 if,if, if 이렇게 반복되게 되는 상황에 제안해주시는 방법이 더 효과적이고 "에러를 토스트로 보여준다." 라는것을 한번에 볼 수 있어서 좋을 것 같아요
const TEN_PET_POINT = 10000; | ||
const ONE_PET_POINT = 1000; |
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.
요 포인트 수치에 대한 의존성이 여기 있는것이 판단하기 쉬울지 고민돼요 🤔
각 뽑기를 하는 곳에 있는 것이 일반적인 생각처럼 느껴지기도 해서요 🤔
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.
좋아요~
💡 기능
ui packages에 있던 Modal 컴포넌트들을 Shadow Panda의 Dialog로 변경하였습니다.
사이즈 조정 및 확인 완료하였습니다
Summary by CodeRabbit
새로운 기능
useCheckEnoughMoney
추가.버그 수정
Dialog
컴포넌트로 변경하여 사용자 인터페이스 개선.문서화
리팩토링