Skip to content

Conversation

@SeokChan-Lee
Copy link
Collaborator

요구사항

기본

  • 스프린트 미션 1 ~ 7에 대해 typescript를 적용해주세요

심화

  • any타입을 최소한으로 써주세요

주요 변경사항

  • 기존 리액트로 진행했던 스프린트 미션 타입스크립트 적용
  • 모듈 css로 통일

스크린샷

멘토에게

  • 스프린트 미션 8이 늦어져 미션 8,9 동시에 제출하려 했으나 nextjs 미션9에서 막혀서 딜레이 되고 있습니다.
    최대한 차주 멘토링 시간 전까지 미션 9 완성해서 제출하도록 하겠습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@SeokChan-Lee SeokChan-Lee requested a review from Lanace January 12, 2025 07:40
@SeokChan-Lee SeokChan-Lee self-assigned this Jan 12, 2025
@SeokChan-Lee SeokChan-Lee added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jan 12, 2025
Copy link
Collaborator

@Lanace Lanace 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 +11 to +12
const [imagePreviewUrl, setImagePreviewUrl] = useState<string>("");
const fileInputRef = useRef<HTMLInputElement>(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

잘 만드신것같아요 AddImgForm 컴포넌트

조금만 더 나아가서... 아마 이미지는 서버에다가 업로드를 하고, 응답값으로 업로드한 이미지의 url을 받을꺼에요

https://panda-market-api.vercel.app/docs/#/Image/ImageUpload

그럼 여기서 이 컴포넌트를 사용했을때 서버에 업로드하고 url을 위로 올리는 작업이 필요하게 될거에요~

그래서

const imageUrl = URL.createObjectURL(file);
      setImagePreviewUrl(imageUrl);

위 코드처럼 이미지를 미리보기로 만든건 좋은데, 좀더 발전하면 서버에 이미지를 업로드하고 url을 받아와서 해당 url로 이미지를 보여주는게 좀더 좋을것같아요ㅎㅎㅎ!

Comment on lines +4 to +17
interface AddItemFormProps {
label: string;
type?: string;
placeholder: string;
id: string;
value: string;
onChange: (
event: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>
) => void;
isTextArea?: boolean;
onKeyDown?: (
event: React.KeyboardEvent<HTMLInputElement | HTMLTextAreaElement>
) => void;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 공통 컴포넌트를 설계해보신것같네요ㅎㅎ

저라면 기존에 사용하고 있는 속성들을 그대로 가져와서 정의했을것같아요...!

interface AddItemFormProps
  extends React.TextareaHTMLAttributes<HTMLTextAreaElement | HTMLInputElement> {
  label: string;
}

이런식으로 하면, 깜빡하고 누락한 속성도 처리할 수 있거든요ㅎㅎ

이건 조금 어려우니 멘토링떄 같이 보면 좋을것같아요~

Comment on lines +6 to +10
interface AddTagFormProps {
tags: string[];
onAddTag: (tag: string) => void;
onRemoveTag: (tag: string) => void;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

필요한거만 잘 만들어주신것같네요ㅎㅎ!

조금더 개선한다면 중복된 tag는 걸러주는 로직을 넣으면 좀더 좋겠네요ㅎㅎ!

왜냐하면 tag를 key로 사용하는데, 중복된 값이 들어오면 문제가 될꺼거든요ㅠ

};
}

const isButtonEnabled = inputValue.trim().length > 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 값은 물론 이렇게 해도 되지만 좀더 성능을 끌어올리고 싶다면 useMemo를 활용하는것도 좋아요ㅎㅎ!

const isButtonEnabled = useMemo(() => {
    return inputValue.trim().length > 0;
  }, [inputValue]);


function ItemCard({ item, imgSizeClass = "" }: ItemCardProps) {
const imageUrl =
item.images && item.images.length > 0 ? item.images[0] : defaultImage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

lodash 라는 라이브러리가 있는데, 만약 그거를 사용하면 아래처럼 간단하게 만들 수 있었을거에요ㅎㅎ!

const imageUrl = first(item.images) || defaulImage;

이것도 이번 멘토링떄 한번 같이 보시져ㅎㅎ!
자주 사용되고 편한 라이브러리긴 해서... 알아두시면 좋을것같아서요!

Comment on lines +18 to +20
const sortedItems = items
.sort((a, b) => b.favoriteCount - a.favoriteCount)
.slice(0, 4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

아마 이부분도 서버에서 잘 정렬해서 내려줄꺼라 괜찮을거에요ㅎㅎ!

근데 코드는 깔끔하게 잘 작성해주신것같아요~

Comment on lines +32 to +38
if (id === "name") {
setName(value);
} else if (id === "description") {
setDescription(value);
} else if (id === "price") {
setPrice(value);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

react에서는 가급적 id는 사용하지 않는게 좋아요ㅠ

DOM에 id가 여러개 나올 수도 있고, 컴포넌트를 재사용하기 어려워지거든요...
꼭 써야하는 경우엔 차라리 name을 쓰는게 좀더 좋아요ㅎㅎ!

const [loading, setLoading] = useState(true); // 로딩 상태
const ItemDetail: React.FC = () => {
const { productId } = useParams<{ productId: string }>();
const [item, setItem] = useState<any>(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

물론 이거 any써도 잘 동작될텐데, 서버에서 내려오는 상품 데이터의 타입을 정의하면 좀더 좋아요ㅎㅎ!

image

스웨거를 보면 위와같이 오니까 아마 정의하면 다음과 같겠죠?

type Product = {
  createdAt: string;
  favoriteCount: number;
  ownerNickname: string;
  ownerId: number;
  images: string[];
  tags: string[];
  price: number;
  description: string;
  name: string;
  id: number;
};
  const [item, setItem] = useState<Product[]>([]);

setItem(data);
} catch (error) {
console.error("그만...", error);
console.error("Error fetching item details:", error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 이런 에러처리 너무 좋네요ㅎㅎ!

Comment on lines 35 to 37
} finally {
setLoading(false);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

심지어 final 로 마무리까지 잘 처리하셨네여ㅎㅎ!

@Lanace Lanace merged commit bf1d349 into codeit-bootcamp-frontend:React-이석찬 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants