-
Notifications
You must be signed in to change notification settings - Fork 21
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 #79
The head ref may contain hidden characters: "React-\uD55C\uC218\uC9C4-sprint5"
[한수진]sprint5 #79
Conversation
[Fix] delete merged branch github action
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.
수진님 첫 react 스프린트 미션 고생하셨습니다!
JS로만 작업하시다가 react로 작업하시기 쉽지 않으셨을텐데
요구사항도 구현하시고, 일정도 지키시느라 고생하셨습니다.
리액트 사용이 미숙하다고 하셨는데, 컴포넌트 분리도 잘하셧고, 로직도 깔끔해서
리뷰드릴 사항이 많이 없었습니다 😁
아직 구현하지 못하신 부분은 수진님 페이스에 맞춰 천천히 작업하셔도 되니
부담갖지 마시고 즐겁게 작업하시면 좋겠습니다.
- 참고로 코드리뷰 제출 마감일인 토요일이후에는 수정 및 추가 제출이 안됩니당
@@ -1,5 +1,5 @@ | |||
<!DOCTYPE html> | |||
<html lang="en"> | |||
<html lang="kr"> |
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.
P2:
해당 페이지가 한국어로 되어 있다고 알려주시려면 ko
로 적어주세요~
https://www.w3schools.com/tags/ref_language_codes.asp
<html lang="kr"> | |
<html lang="ko"> |
@@ -0,0 +1,17 @@ | |||
export async function getProducts(params = {}) { | |||
const query = new URLSearchParams(params).toString(); |
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.
P3:
어떤 의도로 위와 같이 작성하셨는지 이해가 갑니다.
다만 이렇게 되면 잘못된 값을 넘겨도 알 수 없습니다. (ex. orderBy대신 order로 넘김)
함수를 각 API별로 만들어 주실 거라면 param도 각 API에 해당하는 애들만 받을 수 있게 작업하시는 것을 추천드립니다.
만약 편리를 선택하실거라면 이 방법도 좋습니다~
useEffect(() => { | ||
if (products.length > 0) { | ||
const top4 = products | ||
.sort((a, b) => b.favoriteCount - a.favoriteCount) | ||
.slice(0, 4); | ||
setBestProducts(top4); | ||
} | ||
}, [products]); |
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.
P1:
Products API 를 보시면 orderBy
라는 param이 있습니다.
이를 통해 특정 기준으로 정렬된 데이터를 받을 수 있으니
이를 활용하시면 좋겠습니다.
https://panda-market-api.vercel.app/docs/#/Product/ListProducts
useEffect(() => { | |
if (products.length > 0) { | |
const top4 = products | |
.sort((a, b) => b.favoriteCount - a.favoriteCount) | |
.slice(0, 4); | |
setBestProducts(top4); | |
} | |
}, [products]); |
|
||
return ( | ||
<div className="allProducts"> | ||
<BestProducts products={products} /> |
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.
P1:
좋아요순으로 데이터를 받아서 바로 내려주시거나, 해당 데이터를 BestProducts 안에서 불러오시는 것을 추천드립니다.
const [products, setProducts] = useState([]);
const [bestProducts, setBestProducts] = useState([]);
useEffect(() => {
getProducts().then((data) => setProducts(data.list));
}, []);
useEffect(() => {
getProducts({ orderBy: 'favorite', pageSize: 4 }).then((data) => setBestProducts(data.list));
}, []);
return (
<div className="allProducts">
<BestProducts products={bestProducts} />
{...}
</div>
)
<Link | ||
to="/items" | ||
className={`nav-link ${currentPath === "/items" ? "active" : ""}`} | ||
> |
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.
P2:
react-router-dom에서 쓰실만한 컴포넌트가 있을 것 같아 추천드립니다.
NavLink라는 친구로 공식문서에서의 설명은 아래와 같습니다.
A <NavLink> is a special kind of <Link> that knows whether or not it is "active", "pending", or "transitioning".
따라서 지금과 같이 active된 메뉴를 알고싶을때 사용하면 좋습니다.
https://reactrouter.com/en/main/components/nav-link
<Link | |
to="/items" | |
className={`nav-link ${currentPath === "/items" ? "active" : ""}`} | |
> | |
<Link | |
to="/items" | |
className={({ isActive }) => `nav-link ${isActive ? 'active' : '' }`} | |
> |
기본 요구사항
체크리스트 [기본]
중고마켓
중고마켓 반응형
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
체크리스트 [심화]
멘토에게
아직 리액트 이용이 많이 미숙한 것 같습니다. 나머지 버튼과 페이지네이션 기능을 주말동안 더 공부하여 코드 리뷰 전까지 수정해보도록 하겠습니다