[전지윤] Sprint 5 #199
[전지윤] Sprint 5 #199
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
지윤님 5번째 미션 제출 고생하셨습니다~
올려주신 PR 상에서는 반응형 처리가 잘 된것으로 보이네요.
다만 제가 올려주신 배포 주소에서는 잘 동작하지 않아
이를 기준으로 피드백 드렸으니 참고해주세요~
이번에 React 미션 하시느라 고생하셨습니다!
| <> | ||
| <Nav /> | ||
| <Routes> | ||
| <Route path="/" element={<AddItem />} /> |
There was a problem hiding this comment.
💬 여담
index 속성을 이용해 시작페이지라는 것을 더 명확하게 알 수 있게 하실수도 있습니다~
| <Route path="/" element={<AddItem />} /> | |
| <Route index element={<AddItem />} /> |
| <span | ||
| onClick={() => navigate("/items")} | ||
| style={ | ||
| location === "/items" || "/additems" ? { color: "#3692FF" } : {} | ||
| } | ||
| > | ||
| 중고마켓 | ||
| </span> |
There was a problem hiding this comment.
❗️ 수정요청
사용하고 계신 라우팅 라이브러리에서는 네비게이션에서 사용되는 wrapping된 link 컴포넌트를 제공합니다~
이를 통해 해당 컴포넌트들에서 일반적인 요구사항을 더 쉽게 구현할 수 있고, 컴포넌트의 역할도 더 명확해지니
아래 문서를 읽고 수정해보시면 더 좋겠습니다.
| return ( | ||
| <nav> | ||
| <div className={styles["nav--left"]}> | ||
| <div className={styles.logo} onClick={() => navigate("/")}> |
There was a problem hiding this comment.
💊 제안
react router를 사용하시니 이런경우 Link 컴포넌트를 쓰세요~
There was a problem hiding this comment.
❗️ 수정요청
배포 사이트에서 확인해보면 가로 스크롤이 생기고 반응형 처리가 안 된 것 같아요~
나중에 화면 사이즈가 작아지면 보여지는 상품 개수가 줄고 디자인대로 보일 수 있도록 변경하시면 좋겠네요.
There was a problem hiding this comment.
❗️ 수정요청
components 폴더에 페이지 컴포넌트들도 들어있는데 이는 올바르지 않은 위계인 것 같아요.
pages라는 폴더를 따로 만드시는 것을 추천드려요!
| const [pagesArr, setPagesArr] = useState([]); | ||
| useEffect(() => { | ||
| const start = pages[0]; | ||
| const end = pages[1]; | ||
| const arr = Array.from({ length: end - start + 1 }, (_, i) => start + i); | ||
| setPagesArr(arr); | ||
| }, [pages]); |
There was a problem hiding this comment.
💊 제안
정적 요소와 동적 요소가 혼합되어 작성되면, 추후 유지보수 시 어디서 무엇이 생성되는지 파악하기 어려울 수 있습니다.
가급적 전체 페이지 버튼들을 한 번에 생성하는 방식-Array.map, Array.from-으로 작성하거나, 최소한 이전 버튼과 나머지 페이지 버튼의 초기화를 일관된 스타일로 통일하면 코드의 가독성을 높일 수 있습니다.
가장 추천드리는 방식은 아래처럼 jsx에서 바로 작성하시는 거에요~
export default function Pagination({
pages,
page: selectedPage,
setPage,
totalPages,
setPages,
}) {
function pagesUpdate(prev, direction) { ... }
return (
<div className={styles.pages}>
<button/>
{Array.from({ length: end - start + 1 }, (_, i) => start + i).map( page => (
<button
key={page}
onClick={() => setPage(page)}
className={`${page === selectedPage ? styles.isCurrentPage : ""}`}
>
{pageNum}
</button>
))}
<button/>
</div>
);
}| <div className={styles["allItems__header"]}> | ||
| <h3 className={styles.title}>전체 상품</h3> | ||
| <div className={styles["allItems__controllers"]}> | ||
| <input |
There was a problem hiding this comment.
💊 제안
요구사항에는 없었지만 상품 검색기능도 구현하시면 더 좋을 것 같아요!
| if (sortIn === "recent" && sort !== "recent") { | ||
| setSort("recent"); | ||
| setPage(1); | ||
| } else if (sortIn === "favorite" && sort !== "favorite") { | ||
| setSort("favorite"); | ||
| setPage(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
💊 제안
로직이 불필요하게 복잡한 것 같아요~ sortIn 즉 선택된 정렬값은 "recent" | "favorite" 이니 아래처럼 작성하셔도 충분합니다.
| if (sortIn === "recent" && sort !== "recent") { | |
| setSort("recent"); | |
| setPage(1); | |
| } else if (sortIn === "favorite" && sort !== "favorite") { | |
| setSort("favorite"); | |
| setPage(1); | |
| } | |
| } | |
| if(sortIn === sort) return; // 기존과 같은 값이 선택될 경우 1페이지로 가는 것을 막기위해 | |
| setSort(sortIn); | |
| setPage(1); | |
| } |
| {/* // <ImageWithValidation | ||
| // styleClass={styleClass} | ||
| // imageURL={item.images} | ||
| // alt="프로필 이미지" | ||
| // /> */} |
There was a problem hiding this comment.
❗️ 수정요청
사용하지 않는 코드는 공유될 필요가 없으니 지우고 올려주세요~
| {/* // <ImageWithValidation | |
| // styleClass={styleClass} | |
| // imageURL={item.images} | |
| // alt="프로필 이미지" | |
| // /> */} |
|
|
||
| <div className={styles.item__description}> | ||
| <h3 className={styles["item__name"]}>{item.name}</h3> | ||
| <p className={styles["item__price"]}>{item.price}</p> |
There was a problem hiding this comment.
💊 제안
금액의 경우 (,)를 통해 단위가 구분되도록 아래처럼 작성해주세요!
| <p className={styles["item__price"]}>{item.price}</p> | |
| <p className={styles["item__price"]}>{item.price.toLocaleString()}</p> |
2 participants
요구사항
배포 링크
https://react-15-7.netlify.app/
기본
베스트 상품
전체 상품
심화
주요 변경사항
스크린샷
데스크탑
태블릿
모바일
멘토에게