-
Notifications
You must be signed in to change notification settings - Fork 37
[김도균] Sprint12 #329
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
[김도균] Sprint12 #329
The head ref may contain hidden characters: "Next-\uAE40\uB3C4\uADE0-sprint12"
Conversation
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.
작업하시느라 고생 많으셨습니다!!ㅎㅎ
<ItemCard | ||
key={item.id} | ||
item={{ ...item, imageUrl: item.images[0] }} | ||
className='w-[282px] object-cover h-[282px]' |
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.
약간 고정값들이 너무 많이 나오는것같아요
보통 magic number라고 하는데, 가급적이면 이런건 따로 상수로 뽑아서 만들어두시는게 좋아요ㅎㅎ!
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.
매직넘버를 줄이는 게 좋다고 생각합니다! 좋은 의견 감사합니다.
${index === 2 ? 'hidden md:flex' : ''} | ||
${index === 3 ? 'hidden lg:flex' : ''} `} |
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 default function ItemCard({ item, className, display }: ItmeCardProps) { | ||
return ( | ||
<div className={`flex flex-col gap-4 ${display}`}> |
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.
display 만 따로 props로 받는것보단 차라리 className 을 받도록 처리하는건 어떨까요?ㅎ
추후 확장성을 위해서요~!!
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.
className으로 일괄 처리해주는 방식이 더 깔끔해 보입니다, 참고하겠습니다!
description: string; | ||
price: number; | ||
tags: string[]; | ||
images: string | null[]; |
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.
앗앗... 이거 괄호가 없으면 string 또는 null[] 이 타입으로 정의될꺼에요
그래서 (string | null)[] 로 해야할꺼에요ㅎㅎ~
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.
헉... 깜빡했습니다. 좋은 지적 감사합니다.
요구사항
기본
중고 마켓
상품 상세
상품 등록
심화
주요 변경사항
멘토에게