Skip to content
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

[배영준] sprint8(재업로드) #611

Conversation

dudwns0213
Copy link
Collaborator

요구사항

기본

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

주요 변경사항

  • 메인, 로그인 등은 아직 React로 옮겨오지 못해서(순수 js) 추후에 만들려고 빈 페이지로 두었습니다!
  • 타입스크립트 변환 완료하였습니다!

스크린샷

멘토에게

  • 우여곡절 끝에 제출합니다... 브랜치 관련 문제도 있어서 아예 새로 제출하는 점 정말 죄송합니다 ㅠㅠ
  • 타입스크립트의 타입 코드와, 커밋 세분화를 해봤는데 혹시 제대로 되었는지 확인해주시면(알아보기 쉬운가) 감사하겠습니다!
  • 파트2동안 멘토님 덕분에 항상 멘토링 즐겁고 유익하게 했던 것 같습니다! 그동안 정말 감사했습니당 수료식 때 뵐 수 있었으면 좋겠습니다😍

그동안 정말 감사했습니다!!!

@dudwns0213 dudwns0213 requested a review from airman5573 June 2, 2024 12:53
@dudwns0213 dudwns0213 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 2, 2024
Copy link
Collaborator

@airman5573 airman5573 left a comment

Choose a reason for hiding this comment

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

안녕하세요 영준님, 우선 눈에 띄는 부분들 먼저 코멘트 드렸습니다!
이 외의 부분은 따로 또 코멘트 남기고 디스코드로 말씀드릴께요 :D

--- 추가 수정 ----
image


컴포넌트 작성 방식을 일관되게 하는게 약간 더 예뻐 보입니다.
image


개인적으로. 디렉토리는 하이픈으로 연결하는게 마음이 편하더라구요.
테스트 해본건 아니지만, 특정 OS에서는 디렉토리의 대소문자를 잘 구분 못할것 같기도 해서요.
이를테면 MacOS와 Window, Linux에서 모두 동일하게 대소문자를 잘 구분해 줄지 의문이 드네요.
image

-- 마지막으로
영준님 고마워요 ~~~~ 하핳..

import "./Header.css";

// react-router-dom의 NavLink를 이용하면 활성화된 네비게이션 항목을 하이라이트해줄 수 있어요!
function getLinkStyle({ isActive }) {
interface LinkStyleProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[참고해주세요]

일반적으로 Props는 리액트 컴포넌트에 사용되는 용어입니다.
현재는 getLinkStyle 유틸함수이기 때문에 props보다는 함수 타입을 만드시는게 더 적절할것 같아요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

interface LinkStyleParams {
  isActive: boolean;
}

type GetLinkStyle = (params: LinkStyleParams) => React.CSSProperties;

const getLinkStyle: GetLinkStyle = ({ isActive }) => ({
  color: isActive ? "var(--blue)" : "inherit",  // 기본값을 "inherit"로 명시
});

이렇게 수정하셔도 좋을것 같아요.

return { color: isActive ? "var(--blue)" : undefined };
}

function Header() {
const location = useLocation(); // 현재 경로 정보
const location: Location = useLocation();
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입을 명확히 하셨군요 👍

onSortSelection: (sortType: string) => void;
}

const DropdownMenu: React.FC<DropdownMenuProps> = ({ onSortSelection }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입을 꼼꼼히 작성해주셨네요 👍

const handleImageChange = (event) => {
const file = event.target.files[0];
const handleImageChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const file = event.target.files ? event.target.files[0] : null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

[참고해주세요]

아래처럼 개선해볼 수 있습니다.
const file = event.target.files?.[0] ?? null;

placeholder="개인정보를 공유 및 요청하거나, 명예 훼손, 무단 광고, 불법 정보 유포시 모니터링 후 삭제될 수 있으며, 이에 대한 민형사상 책임은 게시자에게 있습니다."
isTextArea
onKeyDown={undefined}
Copy link
Collaborator

Choose a reason for hiding this comment

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

[참고해주세요]

onKeyDown을 넣지 않으면 자동으로 undefined가 되기 때문에, 빼셔도 괜찮습니다.


const fetchSortedData = async ({ orderBy, page, pageSize }) => {
const fetchSortedData = async ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

[참고해주세요]

type때문에 코드가 길어지고 있어요, 타입을 따로 분리하는걸 추천드립니다.

@@ -1,7 +1,14 @@
import React from "react";
import { ReactComponent as HeartIcon } from "../../../assets/images/icons/ic_heart.svg";

function ItemCard({ item }) {
interface Item {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[수정해주세요]

통일성 있게 ItemCardProps로 명명해주세요!
혹은
Item 타입은 그대로 두고, 아래에 ItemCardProps를 추가해 주세요.

itemTag: string[];
}

export interface ItemType {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ItemType에서 Item을 추출해주세요.

export interface ItemType {
  id: number;
  name: string;
  description: string;
  price: number;
  tags: string[];
  images: string[];
  ownerId: number;
  favoriteCount: number;
  createdAt: Date;
  updatedAt: Date;
  isFavorite: boolean;
}

type Item = Pick<ItemType, 'images' | 'name' | 'favoriteCount' | 'price'>;

이런식으로 사용하실 수 있습니다.

@airman5573 airman5573 merged commit 8f1a6da into codeit-bootcamp-frontend:React-배영준 Jul 26, 2024
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