-
Notifications
You must be signed in to change notification settings - Fork 39
[조인성] sprint5 #213
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
[조인성] sprint5 #213
The head ref may contain hidden characters: "React-\uC870\uC778\uC131-sprint5"
Conversation
[Fix] delete merged branch github action
…인성-sprint1 [조인성] Sprint1
- 바벨 추가로 인한 JSX 주석 제거
GANGYIKIM
left a comment
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.
인성님 5번째 미션 작업 수고하셨습니다!
환경 변수 설정도 해주시고 custom hook으로 로직 분리해주신 것도 좋습니다!
시간이 되실때 페이지네이션도 만드시면 더 좋을 것 같아요!
| @@ -0,0 +1,17 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
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.
❗️ 수정요청
| <html lang="en"> | |
| <html lang="ko"> |
| href="https://cdn.jsdelivr.net/gh/orioncactus/pretendard/dist/web/static/pretendard.css" | ||
| /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Vite + React</title> |
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.
💊 제안
유저가 알 수 있게 타이틀 태그를 수정해주시고 미션에서 추가하셨던 메타 태그도 추가하시면 더 좋을 것 같아요!
| <title>Vite + React</title> | |
| <title>판다마켓</title> |
| <Route element={<Header />}> | ||
| <Route index element={<MainPage />} /> |
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.
👍 칭찬
Layout 컴포넌트를 만드신 것 좋아요! 다만 저희 main 페이지의 경우 item 페이지와 디자인이 다른 것으로 압니다!
한번 확인해보세요!
| const query = `page=${page}&pageSize=${pageSize}&orderBy=${orderBy}${ | ||
| keyword ? `&keyword=${keyword}` : "" | ||
| }`; | ||
| const res = await axios.get( | ||
| `${API}products?${query}` | ||
| ); |
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.
💬 여담
아래처럼 간단하게도 작성 가능합니다~
| const query = `page=${page}&pageSize=${pageSize}&orderBy=${orderBy}${ | |
| keyword ? `&keyword=${keyword}` : "" | |
| }`; | |
| const res = await axios.get( | |
| `${API}products?${query}` | |
| ); | |
| const query = { page, pageSize, orderBy, ...(keyword && { keyword }) }; | |
| const res = await axios.get(`${API}products`, { params: query }); |
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.
❗️ 수정요청
이름은 Button이고 태그는 Link라서 너무 헷갈릴 것 같아요!
어떤 컴포넌트인지 알 수 있게 다른 이름이면 더 좋을 것 같아요.
|
|
||
| window.addEventListener("resize", updateSize); | ||
| return () => window.removeEventListener("resize", updateSize); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
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.
💊 제안
린트 룰은 이유가 있어서 존재하니 getSize를 useCallback으로 감싸시고 의존성 배열에 getSize를 추가하시면 더 좋겠네요~
const getSize = useCallback(() => { ... }, [defaultSize, smallSize, mediumSize]);
useEffect(() => { }, [getSize])| function ItemList({ item, variant }) { | ||
| const { name, price, images, favoriteCount } = item; |
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.
💊 제안
ItemList 라는 객체로 props를 받기보다 images, name... 이렇게 받으시는게 더 좋아보여요.
지금 해당 컴포넌트에서 variant 와 item이 동일한 위계로 보여서 객체로 받으실 필요가 없어보입니다~
| function ItemList({ item, variant }) { | |
| const { name, price, images, favoriteCount } = item; | |
| function ItemList({ name, price, images, favoriteCount, variant }) { |
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.
❗️ 수정요청
boolean 값임을 알 수 있게 variant가 아닌 다른 이름이면 좋겠습니다~
| /> | ||
| <p css={ItemNameStyle}>{name}</p> | ||
| <p css={ItemPriceStyle}> | ||
| {price.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") + "원"} |
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.
💊 제안
간단하게 아래처럼도 작성 가능합니다!
| {price.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") + "원"} | |
| {`${price.toLocaleString()} 원`} |
|
|
||
| function Item({ items, variant }) { | ||
| return ( | ||
| <> |
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.
❗️ 수정요청
ul 하나만 반환하고 있어서 Fragment로 감쌀 필요가 없을 것 같아요.
| <> |
| @@ -0,0 +1,23 @@ | |||
| import { titleStyle } from "../../styles/common"; | |||
| import Button from "../Button/Button"; | |||
| import * as S from './TooBar.styles'; | |||
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.
💊 제안
각 파일에서 styles 을 가지고 오는 방법이 통일되면 좋겠어요~
요구사항
기본
중고마켓 페이지 주소는 “/items” 입니다.
페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
'상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.
중고마켓 반응형
심화
주요 변경사항
스크린샷
멘토에게