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 #250

Conversation

leesh7048
Copy link
Collaborator

@leesh7048 leesh7048 commented Aug 2, 2024

요구사항

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

멘토에게

  • 타입스크립트로 마이그레이션중에 시간이 부족해서 미완성입니다.. 일단 pr올리고 좀더 작업해서 커밋하겠습니다!

@leesh7048 leesh7048 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 2, 2024
@leesh7048 leesh7048 requested a review from 201steve August 2, 2024 14:24
Copy link
Collaborator

@201steve 201steve 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 +7 to +13
interface ProductInfo {
images: string[];
name: string;
price: number;
favoriteCount: number;
id: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

BestProducts에서 선언한 interface랑 같아보여요.
선언한 interface를 export해서 재사용도 가능해요
혹은, Products, BestProducts에서 같이 쓰이는 type.ts 파일을 만들어서 여기서 공통으로 쓰이는 타입을 관리할 수 도 있어요.

//type.ts
export interface ProductType {
  id: string;
  name: string;
  price: number;
  favoriteCount: number;
  images: string[];
}


//BestProducts.tsx
import {ProcustType} from "./type.ts"
...

//Product.tsx
import {ProcustType} from "./type.ts"

Comment on lines +11 to +17
interface ProductType {
id: string;
name: string;
price: number;
favoriteCount: number;
images: string[];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 interface도 위에 리뷰랑 동일해요 :-)

const selectedValue = e.target.innerText;
const onClickSortSelect = (e: React.MouseEvent<HTMLDivElement>) => {
const selectedValue = e.currentTarget.innerText;
console.log(selectedValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

commit 하기전에 vscode의 찾기 기능으로 남겨진 console.log는 없는지 찾아보시고 삭제 해 보세요!
commit 하시면 이런 log를 출력하는 코드는 깔끔하게 정리하고 commit 하실 수 있을꺼에요! :-)

favoriteCount: number;
images: string[];
}

function BestProducts() {
const [products, setProducts] = useState([]);
const [pageSize, setPageSize] = useState(4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [pageSize, setPageSize] = useState(4);
const [pageSize, setPageSize] = useState(4);

4가 어떤의미인지 예상하기 조금 어려워요
상수라면 4라는 값을 변수에 담아서 표현하면 예상하기 좋을것같아요.
제가 예상하기로는 모바일,테블릿,데스크탑 인것같아요.

const MOBILE_SIZE = 1;
const TABLET_SIZE = 2;
const DESKTOP_SIZE = 4;
const [pageSize, setPageSize] = useState(DESKTOP_SIZE);

이렇게 표기해주면 어떨까요?

favoriteCount: number;
images: string[];
}

function BestProducts() {
const [products, setProducts] = useState([]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [products, setProducts] = useState([]);
const [products, setProducts] = useState<여기에>([]);

배열타입일때는 useState에 미리 타입을 적어주는게 좋습니다.
products에 배열에 어떤 타입의 값이 들어올지 타입추론이 가능해요.

id: string;
}

function Product({ product }: { product: ProductInfo }) {
const { images, name, price, favoriteCount, id } = product;
console.log(`${id}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
console.log(`${id}`);
console.log(`${id}`);

여기도 로컬에서 확인이 끝났다면 지워주시는게 좋습니다 :-)

@@ -24,12 +33,17 @@ function SalesProducts() {
const [orderBy, setOrderBy] = useState("recent");
const [pageSize, setPageSize] = useState(getHtmlSize());
const [page, setPage] = useState(1);
const [totalPageNum, setTotalPageNum] = useState();
const [totalPageNum, setTotalPageNum] = useState<number>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [totalPageNum, setTotalPageNum] = useState<number>();
const [totalPageNum, setTotalPageNum] = useState<number>(여기에);

useState(여기에) number type의 초깃값을 넣어주세요!

@201steve 201steve merged commit 6836722 into codeit-bootcamp-frontend:React-이승현 Aug 4, 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