펫 drop 확인창 추가 #255
펫 drop 확인창 추가 #255
Conversation
Walkthrough이 풀 리퀘스트는 웹 애플리케이션의 사용자 인터페이스와 현지화를 개선하기 위한 여러 변경 사항을 포함하고 있습니다. 주요 변경 사항은 영어와 한국어 메시지 파일에 새로운 번역 항목 추가, 펫 판매 확인 대화 상자 구현, UI 컴포넌트 라이브러리 확장 등을 포함합니다. 이러한 변경은 사용자 경험을 향상시키고 애플리케이션의 국제화 지원을 강화하는 데 초점을 맞추고 있습니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant SelectedPetTable
participant SellConfirmDialog
participant Backend
User->>SelectedPetTable: 펫 선택
User->>SelectedPetTable: 판매 버튼 클릭
SelectedPetTable->>SellConfirmDialog: 판매 확인 대화상자 표시
SellConfirmDialog->>User: 판매 확인 요청
User->>SellConfirmDialog: 판매 확인
SellConfirmDialog->>SelectedPetTable: 판매 승인
SelectedPetTable->>Backend: 펫 판매 요청
Backend-->>SelectedPetTable: 판매 완료
SelectedPetTable->>User: UI 업데이트
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🔇 Additional comments (6)apps/web/src/constants/storage.ts (1)
apps/web/src/app/[locale]/mypage/my-pet/SelectedPetTable.tsx (5)
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/src/app/[locale]/mypage/my-pet/SelectedPetTable.tsx (3)
33-33: 로컬 스토리지 상태 연동 주의
useDoNotShowAgain()훅에서 가져온isChecked를 직접 사용하지 않고, 별도isDoNotShowAgain라는 변수로 쓰고 있습니다. 변수가 중복되어 관리가 혼동되지 않도록 주의가 필요합니다.
35-46: 에러 처리 및 성공 처리 분기
실패 시 에러 토스트, 성공 시 재조회하고 토스트 노출 과정을 명확히 구분해서 보여줘서 사용자가 결과를 빠르게 인지할 수 있습니다.
잘 동작하지만, 사용자 경험 측면에서 에러 메시지를 좀 더 구체적으로 노출할 수 있으면 좋겠습니다(예: 서버에서 제공되는 에러 메시지 활용).
75-93: null 체크 및 UI 표시
{currentPersona && (...)}구문을 통해 안전하게 null 체크하고 있습니다. currentPersona가 없는 경우 UI를 노출하지 않는 방식이 직관적입니다.
다음에 확장 기능을 계획한다면, '펫 선택을 먼저 해달라'는 안내 문구 등을 배치해서 사용자 경험을 보완할 수도 있겠습니다.packages/ui/panda/src/components/CheckBox/CheckBox.tsx (1)
1-26: 구현이 깔끔하고 잘 되어있습니다!Radix UI를 활용한 접근성 높은 체크박스 구현이 인상적입니다. 몇 가지 제안사항이 있습니다:
BaseCheckbox.displayName에 더 명확한 이름을 사용하면 좋을 것 같습니다.- 컴포넌트 props 타입에 대한 문서화를 추가하면 좋을 것 같습니다.
다음과 같이 개선하는 것을 고려해보세요:
const BaseCheckbox = React.forwardRef< React.ElementRef<typeof CheckboxPrimitive.Root>, React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root> >(({ className, ...props }, ref) => { // ... }); -BaseCheckbox.displayName = CheckboxPrimitive.Root.displayName; +BaseCheckbox.displayName = 'BaseCheckbox';apps/web/src/components/Global/useDialog.tsx (1)
54-54: 국제화 구현이 잘 되었습니다만, 로딩 상태 처리를 개선할 수 있습니다.다음과 같은 개선 사항을 제안드립니다:
- 로딩 상태를 전역 상태로 관리하는 것이 좋을 것 같습니다.
- 버튼 비활성화 시 시각적 피드백을 추가하면 좋을 것 같습니다.
다음과 같은 리팩토링을 제안합니다:
- const [isLoading, setIsLoading] = useState(false); + const [dialogState, setDialogState] = useAtom( + atom({ + isLoading: false, + error: null, + }) + ); const confirmDialog = async () => { - if (isLoading) return; + if (dialogState.isLoading) return; if (dialog.onConfirm) { - setIsLoading(true); + setDialogState({ isLoading: true, error: null }); try { await dialog.onConfirm(); + closeDialog(); + } catch (error) { + setDialogState((prev) => ({ ...prev, error })); } finally { - setIsLoading(false); + setDialogState((prev) => ({ ...prev, isLoading: false })); } } - closeDialog(); };Also applies to: 83-83, 87-87
apps/web/messages/ko_KR.json (1)
74-76: 판매 확인 메시지가 잘 구성되어 있습니다만, 작은 개선사항이 있습니다.메시지가 명확하고 한국어 사용자에게 친숙한 형식을 따르고 있습니다. 다만 다음과 같은 작은 개선을 제안드립니다:
- "sell-confirm": "정말로 판매하시겠습니까?", + "sell-confirm": "정말 판매하시겠습니까?","정말로"에서 "로"를 제거하면 더 자연스러운 한국어 표현이 될 것 같습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/web/messages/en_US.json(2 hunks)apps/web/messages/ko_KR.json(3 hunks)apps/web/src/app/[locale]/mypage/my-pet/SelectedPetTable.tsx(1 hunks)apps/web/src/app/[locale]/mypage/my-pet/page.tsx(1 hunks)apps/web/src/components/Global/useDialog.tsx(4 hunks)packages/ui/panda/package.json(1 hunks)packages/ui/panda/src/components/CheckBox/CheckBox.tsx(1 hunks)packages/ui/panda/src/components/CheckBox/index.ts(1 hunks)packages/ui/panda/src/components/Label/Label.tsx(1 hunks)packages/ui/panda/src/components/Label/index.tsx(1 hunks)packages/ui/panda/src/components/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/ui/panda/src/components/CheckBox/index.ts
- packages/ui/panda/src/components/Label/index.tsx
🔇 Additional comments (10)
apps/web/src/app/[locale]/mypage/my-pet/page.tsx (2)
7-7: 타입 명시 사용에 대한 가독성 향상
해당 라인에서 type Persona를 명시적으로 가져오는 것은 타입 정보를 즉시 파악할 수 있어 좋습니다. 팀 컨벤션에 맞춰 일관성 있게 사용 중인지 확인해 주세요.
13-13: 컴포넌트 분리로 인한 코드 구조 개선
기존에 한 파일에서 모든 로직을 처리하던 방식을 개선하여, UI 및 로직을 SelectedPetTable 컴포넌트로 분리해 유지보수가 용이해졌습니다. 추후에 재사용 가능 여부를 고려해 볼 수 있을 것 같습니다.
apps/web/src/app/[locale]/mypage/my-pet/SelectedPetTable.tsx (2)
48-55: 판매 클릭 로직의 분기
이미 '다시 묻지 않기'가 활성화된 경우 확인창을 뛰어넘도록 설계하셨네요. 사용자 경험에 이점을 줄 수 있으므로 적절히 구현된 것으로 보입니다.
188-238: 판매 확인 다이얼로그 로직
사용자가 '다시 묻지 않기' 옵션을 설정할 수 있고, 실제 판매를 진행하기 전 문자열을 한 번 더 확인케 하는 최종 구조가 안정적입니다.
다만, 로딩 상태(isLoading)가 완료된 뒤에는 setIsDoNotShowAgain(false) 등 내부 상태 초기화 여부도 점검해 보세요.
packages/ui/panda/src/components/Label/Label.tsx (1)
1-5: Label 컴포넌트 도입
styled('label', label)을 통해 라벨 스타일을 재사용 가능하게 잘 분리하셨습니다. 프로젝트 전반에서 동일한 스타일의 라벨을 일관성 있게 사용할 수 있어, 유지보수에 유리합니다.
packages/ui/panda/src/components/index.ts (1)
9-10: Checkbox와 Label 내보내기
새로운 UI 요소를 index로부터 내보내어 재사용성을 높인 점이 좋습니다. 앞으로도 공용 UI 컴포넌트는 이와 같은 방식으로 정리해 주시면 됩니다.
packages/ui/panda/package.json (1)
14-14: 의존성 추가가 적절합니다.
Radix UI 컴포넌트 라이브러리의 의존성이 올바르게 추가되었습니다.
보안 취약점 확인을 위해 다음 스크립트를 실행합니다:
Also applies to: 16-16
✅ Verification successful
Radix UI 패키지들의 보안 취약점이 발견되지 않았습니다
GitHub Security Advisory 데이터베이스에서 확인한 결과, @radix-ui/react-checkbox와 @radix-ui/react-label 패키지에서 알려진 보안 취약점이 없습니다. 의존성 추가가 안전하게 진행될 수 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Radix UI 패키지들의 보안 취약점 확인
echo "Checking @radix-ui/react-checkbox vulnerabilities..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@radix-ui/react-checkbox") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
echo "Checking @radix-ui/react-label vulnerabilities..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@radix-ui/react-label") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 825
apps/web/messages/ko_KR.json (1)
23-26: 공통 번역이 자연스럽게 추가되었습니다!
번역이 간결하고 한국어 사용자에게 익숙한 용어로 잘 구성되어 있습니다.
apps/web/messages/en_US.json (2)
30-33: 영문 공통 번역이 적절하게 구성되었습니다!
UI에서 일반적으로 사용되는 영문 표현들이 올바르게 사용되었습니다.
73-75: 영문 판매 확인 메시지가 한글 버전과 일관성 있게 구현되었습니다.
영문 메시지가 한글 버전의 의미를 정확하게 전달하면서도 자연스러운 영어 표현을 사용하고 있습니다.
| @@ -66,7 +67,7 @@ export function DialogComponent() { | |||
| await dialog.onConfirm(); | |||
| setIsLoading(false); | |||
| } | |||
| // closeDialog(); | |||
| closeDialog(); | |||
There was a problem hiding this comment.
에러 처리가 누락되었습니다.
onConfirm 함수 실행 중 발생할 수 있는 에러를 적절히 처리해야 합니다.
다음과 같은 에러 처리를 추가하는 것을 권장합니다:
const confirmDialog = async () => {
if (isLoading) return;
if (dialog.onConfirm) {
setIsLoading(true);
- await dialog.onConfirm();
- setIsLoading(false);
+ try {
+ await dialog.onConfirm();
+ } catch (error) {
+ console.error('Dialog confirmation failed:', error);
+ // 사용자에게 에러 메시지 표시
+ setDialog((prev) => ({
+ ...prev,
+ description: t('error.confirmation_failed'),
+ }));
+ } finally {
+ setIsLoading(false);
+ }
}
closeDialog();
};Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ui/panda/src/components/CheckBox/CheckBox.tsx (1)
26-26: 타입 정의 추가가 필요합니다스타일이 적용된 컴포넌트에 대한 명시적인 타입 정의가 없습니다.
다음과 같이 수정하는 것을 제안드립니다:
-export const Checkbox = styled(BaseCheckbox); +export const Checkbox: typeof BaseCheckbox = styled(BaseCheckbox);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/panda/src/components/CheckBox/CheckBox.tsx(1 hunks)
🔇 Additional comments (1)
packages/ui/panda/src/components/CheckBox/CheckBox.tsx (1)
1-9: 필요한 모든 의존성이 올바르게 임포트되었습니다!
클라이언트 컴포넌트 지시문과 필수 의존성들이 적절하게 구성되어 있습니다.
| const checkboxLightStyle = css({ | ||
| borderColor: '#fafafa', | ||
|
|
||
| '&[data-state="checked"]': { | ||
| backgroundColor: '#fafafa', | ||
| color: '#000', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
디자인 토큰 사용을 권장드립니다
하드코딩된 색상 값들은 테마 커스터마이징을 어렵게 만듭니다. 디자인 시스템의 토큰을 사용하는 것이 더 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
const checkboxLightStyle = css({
- borderColor: '#fafafa',
+ borderColor: 'token(colors.border.default)',
'&[data-state="checked"]': {
- backgroundColor: '#fafafa',
- color: '#000',
+ backgroundColor: 'token(colors.background.active)',
+ color: 'token(colors.text.primary)',
},
});Committable suggestion skipped: line range outside the PR's diff.
| const BaseCheckbox = React.forwardRef< | ||
| React.ElementRef<typeof CheckboxPrimitive.Root>, | ||
| React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root> | ||
| >(({ className, ...props }, ref) => { | ||
| const styles = checkbox(); | ||
|
|
||
| return ( | ||
| <CheckboxPrimitive.Root ref={ref} className={cx('peer', styles.root, checkboxLightStyle, className)} {...props}> | ||
| <CheckboxPrimitive.Indicator className={styles.indicator}> | ||
| <Check className={icon()} /> | ||
| </CheckboxPrimitive.Indicator> | ||
| </CheckboxPrimitive.Root> | ||
| ); | ||
| }); | ||
| BaseCheckbox.displayName = CheckboxPrimitive.Root.displayName; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
체크박스 컴포넌트에 접근성 속성이 누락되어 있습니다. 스크린 리더 사용자를 위해 aria 속성을 추가하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
<CheckboxPrimitive.Root
ref={ref}
className={cx('peer', styles.root, checkboxLightStyle, className)}
+ aria-label={props['aria-label'] || '체크박스'}
+ role="checkbox"
{...props}
>📝 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.
| const BaseCheckbox = React.forwardRef< | |
| React.ElementRef<typeof CheckboxPrimitive.Root>, | |
| React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root> | |
| >(({ className, ...props }, ref) => { | |
| const styles = checkbox(); | |
| return ( | |
| <CheckboxPrimitive.Root ref={ref} className={cx('peer', styles.root, checkboxLightStyle, className)} {...props}> | |
| <CheckboxPrimitive.Indicator className={styles.indicator}> | |
| <Check className={icon()} /> | |
| </CheckboxPrimitive.Indicator> | |
| </CheckboxPrimitive.Root> | |
| ); | |
| }); | |
| BaseCheckbox.displayName = CheckboxPrimitive.Root.displayName; | |
| const BaseCheckbox = React.forwardRef< | |
| React.ElementRef<typeof CheckboxPrimitive.Root>, | |
| React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root> | |
| >(({ className, ...props }, ref) => { | |
| const styles = checkbox(); | |
| return ( | |
| <CheckboxPrimitive.Root | |
| ref={ref} | |
| className={cx('peer', styles.root, checkboxLightStyle, className)} | |
| aria-label={props['aria-label'] || '체크박스'} | |
| role="checkbox" | |
| {...props} | |
| > | |
| <CheckboxPrimitive.Indicator className={styles.indicator}> | |
| <Check className={icon()} /> | |
| </CheckboxPrimitive.Indicator> | |
| </CheckboxPrimitive.Root> | |
| ); | |
| }); | |
| BaseCheckbox.displayName = CheckboxPrimitive.Root.displayName; |
| const [isLoading, setIsLoading] = useState(false); | ||
| const t = useTranslations(); | ||
| const [isDoNotShowAgain, setIsDoNotShowAgain] = useState(false); | ||
| console.log('isDoNotShowAgain: ', isDoNotShowAgain); |
| }, | ||
| }); | ||
|
|
||
| const DO_NOT_SHOW_AGAIN_KEY = '@gitanimals/do-not-show-again'; |
There was a problem hiding this comment.
저희 스토리지 키 모아둔 상수 객체가 있었던 거 같기도 하고 ㅋㅋㅋ
|
@hyesungoh 리뷰 반영하였습니다! |
💡 기능
관련 issue : git-goods/gitanimals#202
펫을 drop시 정말로 팔것인지 확인하는 확인창을 추가하였습니다.
추가적으로 펫을 빠르게 팔고싶은 사용자를 위해 이후부터는 확인창을 무시할 수 있는 기능도 같이 추가하였습니다.
🔎 기타
Summary by CodeRabbit
새로운 기능
버그 수정
문서화