-
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
[실험실] 같은 레벨, 타입의 펫 한번에 팔기 기능 #248
Conversation
Walkthrough이 풀 리퀘스트는 애플리케이션의 지역화에 사용되는 JSON 구조를 수정하여 새로운 기능인 "실험실"과 관련된 내용을 추가합니다. "Mypage" 섹션에 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 11
🧹 Outside diff range and nitpick comments (9)
apps/web/src/app/[locale]/laboratory/property-pet-sell/page.tsx (3)
152-154
: Memoization 함수에서 비교할 props를 확인해주세요
MemoizedPersonaItem
의memo
함수에서prev
와next
의type
,level
만 비교하고 있습니다. 하지만count
나onClick
도 변경될 수 있으므로 필요한 경우 비교 대상에 포함시켜야 합니다.
76-77
: 'level' 값을 숫자형으로 변환해주세요
level
변수가 문자열일 경우 숫자형으로 변환하여 이후 로직에서 일관되게 숫자형으로 사용하는 것이 좋습니다.다음과 같이 코드를 수정해 보세요:
const level = persona.level; + const levelNumber = Number(level);
또는 기존 변수 자체를 숫자형으로 변경하세요:
- const level = persona.level; + const level = Number(persona.level);
134-134
: 'level'의 타입을 number로 변경해주세요
PersonaItemProps
인터페이스에서level
의 타입을string
에서number
로 변경하여 타입 일관성을 유지하고 불필요한 형 변환을 줄일 수 있습니다.다음과 같이 코드를 수정해 보세요:
interface PersonaItemProps { type: string; - level: string; + level: number; onClick: () => void; count: number; }apps/web/src/app/[locale]/laboratory/layout.tsx (1)
9-9
: 테마 색상 변수를 사용하여 일관성을 유지해주세요
backgroundColor
에 하드코딩된 색상(#019C5A
) 대신 프로젝트의 테마 색상 변수를 사용하면 유지 보수성과 스타일 일관성이 향상됩니다.예를 들어, 테마 색상 변수를 사용하여 다음과 같이 수정할 수 있습니다:
- backgroundColor: '#019C5A', + backgroundColor: 'theme.colors.green.500',(실제 사용 가능한 테마 색상 변수 이름은 프로젝트에 따라 다를 수 있습니다.)
apps/web/src/app/[locale]/laboratory/_component/AlertDialog.tsx (1)
19-19
: 버튼 라벨에 대한 국제화 적용 필요
'Close'
버튼의 라벨이 하드코딩되어 있습니다. 국제화를 위해useTranslations
를 사용하여 라벨을 처리해주세요.다음과 같이 코드를 수정해 보세요:
+import { useTranslations } from 'next-intl'; export function AlertDialog(props: { isOpen: boolean; onClose: () => void; title: string; description: React.ReactNode; }) { + const t = useTranslations('Common'); return ( <Dialog open={props.isOpen} onOpenChange={props.onClose}> <Dialog.Content> <Dialog.Title className={titleStyle}>{props.title}</Dialog.Title> <Dialog.Description className={descriptionStyle}>{props.description}</Dialog.Description> <Flex gap="8px" justifyContent="flex-end" width="100%"> <Button onClick={props.onClose} variant="primary" size="m"> - Close + {t('close')} </Button> </Flex> </Dialog.Content> </Dialog> ); }
Common
메시지 파일에'close'
키에 해당하는 번역 문자열을 추가하는 것도 잊지 마세요.apps/web/src/app/[locale]/laboratory/_component/ConfirmDialog.tsx (1)
5-11
: Props 인터페이스 개선이 필요합니다props 타입을 별도로 정의하고, 버튼 텍스트를 커스터마이즈할 수 있도록 하면 좋을 것 같습니다.
+interface ConfirmDialogProps { + isOpen: boolean; + onClose: () => void; + onConfirm: () => void; + title: string; + description: string; + confirmText?: string; + cancelText?: string; +} -export function ConfirmDialog(props: { - isOpen: boolean; - onClose: () => void; - onConfirm: () => void; - title: string; - description: string; -}) +export function ConfirmDialog({ + isOpen, + onClose, + onConfirm, + title, + description, + confirmText = '확인', + cancelText = '취소' +}: ConfirmDialogProps)apps/web/src/app/[locale]/laboratory/page.tsx (1)
17-23
: Card 컴포넌트 타입 및 접근성 개선이 필요합니다Card 컴포넌트에 타입 정의와 접근성 속성을 추가하면 좋을 것 같습니다.
-function Card({ children, href }: { children: React.ReactNode; href: string }) { +interface CardProps { + children: React.ReactNode; + href: string; + 'aria-label'?: string; +} + +function Card({ children, href, 'aria-label': ariaLabel }: CardProps) { return ( - <Link href={href} className={cardStyle}> + <Link + href={href} + className={cardStyle} + aria-label={ariaLabel} + > {children} </Link> ); }apps/web/src/app/[locale]/mypage/layout.tsx (1)
36-58
: 반응형 디자인 개선이 필요합니다.하드코딩된 픽셀값들은 다양한 화면 크기에서 문제를 일으킬 수 있습니다.
다음과 같이 수정하는 것을 고려해보세요:
const laboButtonStyle = css({ position: 'absolute', background: 'white.white_10', backdropFilter: 'blur(7px)', borderRadius: '8px', p: '10px 20px', display: 'flex', alignItems: 'center', gap: '10px', textStyle: 'glyph16.regular', color: 'white.white_100', - top: '64px', - right: '200px', + top: 'calc(64px)', + right: 'clamp(100px, 15vw, 200px)', - '@media (max-width: 1400px)': { - right: '100px', - }, _mobile: { display: 'none', }, });apps/web/messages/en_US.json (1)
132-137
: 메시지 텍스트를 더 명확하게 개선해주세요.다음과 같은 개선사항을 제안드립니다:
"Laboratory": { "property-pet-sell": { - "title": "Property Pet Sell", - "description": "Sell your pet to other users!", - "count": "ea" + "title": "Bulk Pet Sale", + "description": "Sell multiple pets of the same level and type at once!", + "count": "pets" } }개선 이유:
- "Property Pet Sell"이라는 제목보다 "Bulk Pet Sale"이 기능을 더 잘 설명합니다.
- 설명문에 동일한 레벨과 타입의 펫을 한 번에 판매할 수 있다는 핵심 기능을 명시했습니다.
- "ea"보다 "pets"가 더 직관적입니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
apps/web/messages/en_US.json
(2 hunks)apps/web/messages/ko_KR.json
(2 hunks)apps/web/src/app/[locale]/laboratory/_component/AlertDialog.tsx
(1 hunks)apps/web/src/app/[locale]/laboratory/_component/ConfirmDialog.tsx
(1 hunks)apps/web/src/app/[locale]/laboratory/layout.tsx
(1 hunks)apps/web/src/app/[locale]/laboratory/page.tsx
(1 hunks)apps/web/src/app/[locale]/laboratory/property-pet-sell/page.tsx
(1 hunks)apps/web/src/app/[locale]/mypage/layout.tsx
(4 hunks)apps/web/src/components/GlobalComponent/GlobalComponent.tsx
(1 hunks)apps/web/src/components/GlobalComponent/useDialog.tsx
(1 hunks)packages/api/src/shop/dropPet.ts
(1 hunks)
🔇 Additional comments (2)
apps/web/src/components/GlobalComponent/GlobalComponent.tsx (1)
7-7
: DialogComponent 추가 확인 완료
GlobalComponent
에 DialogComponent
를 추가하여 전역에서 다이얼로그를 사용할 수 있도록 한 변경 사항을 확인했습니다.
Also applies to: 13-13
apps/web/messages/en_US.json (1)
110-111
: 변경사항이 적절합니다!
Mypage 섹션에 실험실 기능을 위한 메뉴 항목이 올바르게 추가되었습니다.
title: '펫 판매', | ||
description: `펫을 판매하시겠습니까? ${ids.length}마리를 판매합니다.`, | ||
onConfirm: () => onSell(ids), | ||
}); |
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
하드코딩된 문자열을 국제화 처리해주세요
현재 showDialog
에서 사용되는 '펫 판매'
, '펫 판매 완료'
등의 문자열이 하드코딩되어 있습니다. 국제화를 위해 useTranslations
를 사용하여 메시지를 처리하는 것이 좋습니다.
Also applies to: 58-68
const onSell = async (ids: string[]) => { | ||
const res = await dropPets({ personaIds: ids }); | ||
|
||
const totalPrice = res.success.reduce((acc, curr) => acc + curr.givenPoint, 0); | ||
|
||
trackEvent('laboratory', { | ||
type: '레벨, 타입 같은 펫 한번에 팔기', | ||
}); | ||
|
||
showDialog({ | ||
title: '펫 판매 완료', | ||
description: ( | ||
<div> | ||
<p> | ||
{res.success.length}마리 판매 완료, {res.errors.length}마리 판매 실패 | ||
</p> | ||
<p>총 판매 금액: {totalPrice}P</p> | ||
</div> | ||
), | ||
}); | ||
}; |
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
API 호출 오류 처리를 추가해주세요
dropPets
API 호출 시 예기치 않은 오류에 대비하여 try-catch
문을 사용하여 오류를 처리하는 것이 좋습니다. 오류 발생 시 사용자에게 적절한 메시지를 표시해주세요.
다음과 같이 코드를 수정해 보세요:
const onSell = async (ids: string[]) => {
+ try {
const res = await dropPets({ personaIds: ids });
// ... 기존 코드 유지
+ } catch (error) {
+ // 오류 메시지 표시
+ showDialog({
+ title: '오류 발생',
+ description: '펫 판매 중 오류가 발생했습니다. 다시 시도해주세요.',
+ });
+ console.error(error);
+ }
};
Committable suggestion skipped: line range outside the PR's diff.
export const dropPets = async ({ personaIds }: { personaIds: string[] }) => { | ||
const results = await Promise.allSettled(personaIds.map((id) => dropPet({ personaId: id }))); | ||
|
||
const successResults: DropPetResponse[] = []; | ||
const errorResults: any[] = []; | ||
|
||
results.forEach((result) => { | ||
if (result.status === 'fulfilled') { | ||
successResults.push(result.value); | ||
} else { | ||
errorResults.push(result.reason); | ||
} | ||
}); | ||
|
||
return { | ||
success: successResults, | ||
errors: errorResults, | ||
}; | ||
}; |
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
에러 처리와 타입 안정성 개선이 필요합니다
다음 사항들을 개선하면 좋을 것 같습니다:
errorResults
타입이any[]
로 되어있어 타입 안정성이 떨어집니다- 빈 배열 검증이 없습니다
- 에러 응답 구조가 명확하지 않습니다
다음과 같이 개선해보세요:
+type DropPetsError = {
+ personaId: string;
+ error: unknown;
+};
+
+type DropPetsResponse = {
+ success: DropPetResponse[];
+ errors: DropPetsError[];
+};
-export const dropPets = async ({ personaIds }: { personaIds: string[] }) => {
+export const dropPets = async ({ personaIds }: { personaIds: string[] }): Promise<DropPetsResponse> => {
+ if (personaIds.length === 0) {
+ throw new Error('personaIds 배열이 비어있습니다');
+ }
const results = await Promise.allSettled(personaIds.map((id) => dropPet({ personaId: id })));
const successResults: DropPetResponse[] = [];
- const errorResults: any[] = [];
+ const errorResults: DropPetsError[] = [];
results.forEach((result, index) => {
if (result.status === 'fulfilled') {
successResults.push(result.value);
} else {
- errorResults.push(result.reason);
+ errorResults.push({
+ personaId: personaIds[index],
+ error: result.reason
+ });
}
});
return {
success: successResults,
errors: errorResults,
};
};
📝 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 dropPets = async ({ personaIds }: { personaIds: string[] }) => { | |
const results = await Promise.allSettled(personaIds.map((id) => dropPet({ personaId: id }))); | |
const successResults: DropPetResponse[] = []; | |
const errorResults: any[] = []; | |
results.forEach((result) => { | |
if (result.status === 'fulfilled') { | |
successResults.push(result.value); | |
} else { | |
errorResults.push(result.reason); | |
} | |
}); | |
return { | |
success: successResults, | |
errors: errorResults, | |
}; | |
}; | |
type DropPetsError = { | |
personaId: string; | |
error: unknown; | |
}; | |
type DropPetsResponse = { | |
success: DropPetResponse[]; | |
errors: DropPetsError[]; | |
}; | |
export const dropPets = async ({ personaIds }: { personaIds: string[] }): Promise<DropPetsResponse> => { | |
if (personaIds.length === 0) { | |
throw new Error('personaIds 배열이 비어있습니다'); | |
} | |
const results = await Promise.allSettled(personaIds.map((id) => dropPet({ personaId: id }))); | |
const successResults: DropPetResponse[] = []; | |
const errorResults: DropPetsError[] = []; | |
results.forEach((result, index) => { | |
if (result.status === 'fulfilled') { | |
successResults.push(result.value); | |
} else { | |
errorResults.push({ | |
personaId: personaIds[index], | |
error: result.reason | |
}); | |
} | |
}); | |
return { | |
success: successResults, | |
errors: errorResults, | |
}; | |
}; |
<Button onClick={props.onConfirm} variant="secondary" size="m"> | ||
Ok | ||
</Button> | ||
<Button onClick={props.onClose} variant="primary" size="m"> | ||
Close | ||
</Button> |
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
버튼 텍스트 현지화 및 UX 개선이 필요합니다
- "Ok"와 "Close" 텍스트가 하드코딩되어 있습니다
- 확인/취소 버튼의 순서가 일반적인 UX 패턴과 다릅니다
- variant 설정이 직관적이지 않습니다 (취소는 secondary, 확인은 primary가 일반적)
- <Button onClick={props.onConfirm} variant="secondary" size="m">
- Ok
- </Button>
- <Button onClick={props.onClose} variant="primary" size="m">
- Close
+ <Button onClick={props.onClose} variant="secondary" size="m">
+ 취소
+ </Button>
+ <Button onClick={props.onConfirm} variant="primary" size="m">
+ 확인
</Button>
Committable suggestion skipped: line range outside the PR's diff.
<h2>레벨, 타입 같은 펫 한번에 팔기</h2> | ||
<p>펫 레벨, 타입 등 펫 속성을 선택하여 한번에 팔 수 있어요.</p> | ||
</Card> | ||
<Card href="#">Card</Card> |
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.
미완성된 카드 컴포넌트가 있습니다
개발 중인 카드 컴포넌트가 있습니다. 프로덕션 배포 전에 완성하거나 제거해야 합니다.
- <Card href="#">Card</Card>
async function LaboButton() { | ||
const t = await getTranslations('Mypage'); | ||
return ( | ||
<Link href="/laboratory" className={laboButtonStyle}> | ||
<FlaskConical /> | ||
{t('laboratory')} | ||
</Link> | ||
); | ||
} |
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
LaboButton 컴포넌트의 접근성 개선이 필요합니다.
버튼에 스크린 리더를 위한 aria-label이 누락되었습니다.
다음과 같이 수정해주세요:
return (
- <Link href="/laboratory" className={laboButtonStyle}>
+ <Link href="/laboratory" className={laboButtonStyle} aria-label={t('laboratory')}>
<FlaskConical />
{t('laboratory')}
</Link>
);
📝 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.
async function LaboButton() { | |
const t = await getTranslations('Mypage'); | |
return ( | |
<Link href="/laboratory" className={laboButtonStyle}> | |
<FlaskConical /> | |
{t('laboratory')} | |
</Link> | |
); | |
} | |
async function LaboButton() { | |
const t = await getTranslations('Mypage'); | |
return ( | |
<Link href="/laboratory" className={laboButtonStyle} aria-label={t('laboratory')}> | |
<FlaskConical /> | |
{t('laboratory')} | |
</Link> | |
); | |
} |
const confirmDialog = async () => { | ||
if (isLoading) return; | ||
|
||
if (dialog.onConfirm) { | ||
setIsLoading(true); | ||
await dialog.onConfirm(); | ||
setIsLoading(false); | ||
} | ||
// closeDialog(); | ||
}; |
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.
confirmDialog 함수의 개선이 필요합니다.
- 주석 처리된
closeDialog()
는 의도가 불분명합니다. - 에러 처리가 누락되었습니다.
다음과 같이 수정해주세요:
const confirmDialog = async () => {
if (isLoading) return;
if (dialog.onConfirm) {
setIsLoading(true);
+ try {
await dialog.onConfirm();
+ closeDialog();
+ } catch (error) {
+ console.error('Dialog confirmation failed:', error);
+ } finally {
setIsLoading(false);
+ }
}
- // closeDialog();
};
📝 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 confirmDialog = async () => { | |
if (isLoading) return; | |
if (dialog.onConfirm) { | |
setIsLoading(true); | |
await dialog.onConfirm(); | |
setIsLoading(false); | |
} | |
// closeDialog(); | |
}; | |
const confirmDialog = async () => { | |
if (isLoading) return; | |
if (dialog.onConfirm) { | |
setIsLoading(true); | |
try { | |
await dialog.onConfirm(); | |
closeDialog(); | |
} catch (error) { | |
console.error('Dialog confirmation failed:', error); | |
} finally { | |
setIsLoading(false); | |
} | |
} | |
}; |
<Button onClick={confirmDialog} variant="secondary" size="m" disabled={isLoading}> | ||
{isLoading ? '처리중...' : '확인'} | ||
</Button> |
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
버튼 텍스트의 현지화가 필요합니다.
하드코딩된 한국어 텍스트는 국제화에 문제가 될 수 있습니다.
getTranslations
를 사용하여 텍스트를 현지화해주세요:
- {isLoading ? '처리중...' : '확인'}
+ {isLoading ? t('Common.processing') : t('Common.confirm')}
Committable suggestion skipped: line range outside the PR's diff.
<Button onClick={closeDialog} variant="primary" size="m"> | ||
닫기 | ||
</Button> |
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
닫기 버튼 텍스트의 현지화가 필요합니다.
하드코딩된 한국어 텍스트는 국제화에 문제가 될 수 있습니다.
getTranslations
를 사용하여 텍스트를 현지화해주세요:
- 닫기
+ {t('Common.close')}
Committable suggestion skipped: line range outside the PR's diff.
"title": "Property Pet Sell", | ||
"description": "Sell your pet to other users!", | ||
"count": "개" |
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.
영문 텍스트의 한글화가 필요합니다.
Laboratory 섹션의 텍스트가 한글로 번역되지 않았습니다.
다음과 같이 수정해주세요:
"Laboratory": {
"property-pet-sell": {
- "title": "Property Pet Sell",
- "description": "Sell your pet to other users!",
+ "title": "펫 일괄 판매",
+ "description": "다른 사용자에게 펫을 판매하세요!",
"count": "개"
}
}
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.
👍 👍 👍
function PersonaItem({ type, level, onClick, count }: PersonaItemProps) { | ||
const t = useTranslations('Laboratory.property-pet-sell'); | ||
return ( | ||
<button onClick={onClick} className={css({ outline: 'none', bg: 'transparent' })}> | ||
<div className={levelTagStyle}> | ||
{count} | ||
{t('count')} | ||
</div> | ||
<LevelBanner image={getPersonaImage(type)} level={Number(level)} size="small" /> | ||
</button> | ||
); | ||
} | ||
|
||
const MemoizedPersonaItem = memo(PersonaItem, (prev, next) => { | ||
return prev.type === next.type && prev.level === next.level; | ||
}); |
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.
메모를 나눠돈 이유를 말하시는걸까요?!
종종 memo 할떄랑 안할때 나눠서 테스ㅌ하기도 하고, 메모된 컴포넌트랑 기존 컴포넌트를 구분 & 따로 두고 싶었습니다.
여기선 큰 상관없지만, 이전에 마이페이지 개발할 때 작업했던것을 따왔어요
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.
일단 어푸
💡 기능
🔎 기타
Summary by CodeRabbit
새로운 기능
버그 수정
문서화