[박광민] sprint5 #179
[박광민] sprint5 #179
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
광민님 5주차 미션 고생하셨습니다~
다음 미션도 화이팅입니다!
-
배포 주소에서 확인해보니 전체 상품 영역에서 10개의 아이템이 보이고 있어요. 확인해보시고 수정하시면 좋겠습니다~
-
페이지네이션 기능은 UX 측면에서 한 페이지씩 이동하는 방식이 좋을지, 아니면 5페이지 단위로 이동하는 방식이 더 적절할지 궁금합니다.: 이렇게 UX 가 궁금하실 경우, 이미 운영중인 서비스들에 가서 만져보시면서 분석해보시고 선택하시면 좋습니다~ 찾아보시면 페이지네이션도 디자인이 다 다르기때문에 그때그때 다를 수 있습니다. -
UX 측면에서 사용자 편의성을 고려해 새로고침 시 세션스토리지를 활용해 현재 페이지를 유지하도록 구현했습니다.하지만 Sort 기준이 변경될 경우에는 1페이지로 이동하는 것이 더 자연스럽다고 판단해서 ... 와 같이 처리했지만, 페이지 번호는 1로 변경되었음에도 불구하고 화면에는 이전 페이지의 데이터가 그대로 출력되는 문제가 발생합니다. 이럴 때는 어떤 방식으로 접근해야 할지 궁금합니다.:
지금 확인해보니 우선적으로 화면 사이즈에 따라 노출되는 아이템의 개수가 달라져야 하는데 이 부분이 제대로 동작하지 않는 것 같습니다. (저는 tablet 사이즈에서 접근시 PC에 해당하는 10개가 노출됩니다) 이 문제를 해결하시고 위에서 언급해주신 문제를 고치시면 더 좋을 것 같습니다. 질문에 적어주신 상황을 제가 확인하지 못해 더 자세한 답변은 어렵습니다~ -
모바일에서 검색창 부분의 구조가 바뀌어야 해서 현재는 window.resize를 이용해 조건에 따라 JSX 구조를 변경하고 있습니다. 원래는 CSS로만 처리하려 했지만 원하는 형태가 나오지 않아 리사이즈 방식으로 구현했는데, 이런 방식이 괜찮은지, 혹시 더 나은 방법이 있을지도 궁금합니다.: 당연히 성능상 CSS 로만 처리하시는 것이 더 좋습니다. 다만 이렇게 하실 수 밖에 없다면 괜찮습니다.
There was a problem hiding this comment.
💊 제안
3번째 미션에서 추가하셨던 메타 태그도 추가하시면 더 좋을 것 같아요!
There was a problem hiding this comment.
💬 여담
React Router를 사용하고 계신데 향후 라우트가 많아지면 Route들을 별도의 파일로 분리하면 App 컴포넌트가 더 깔끔해지고, 라우트 관리도 쉬워집니다.
| <NavLink | ||
| to="/items" | ||
| className={styles.menuLink} | ||
| style={getLinkStyle} | ||
| > |
| <button | ||
| disabled={currentPage === 1} | ||
| onClick={handlePrevPage} | ||
| className={styles.navButton} | ||
| > |
There was a problem hiding this comment.
👍 칭찬
버튼이 동작되지 않는 조건 처리하신 점 좋습니다!
| const handlePrevPage = () => { | ||
| if (currentPage > 1) { | ||
| onPageChange(currentPage - 1); | ||
| } | ||
| }; |
There was a problem hiding this comment.
💊 제안
이미 button에서 currentPage가 1 일때 disabled 처리를 하셔서 조건문을 빼셔도 될 것 같아요~
| const handlePrevPage = () => { | |
| if (currentPage > 1) { | |
| onPageChange(currentPage - 1); | |
| } | |
| }; | |
| const handlePrevPage = () => { | |
| onPageChange(currentPage - 1); | |
| }; |
| {Array.from({ length: endPage - startPage + 1 }, (_, i) => { | ||
| const page = startPage + i; | ||
| return ( | ||
| <button |
There was a problem hiding this comment.
💊 제안
button type 명시해주시면 더 좋겠습니다~
| <button | |
| <button | |
| type="button" |
| const sortedProducts = [...products].sort((a, b) => { | ||
| if (orderBy === "likes") return b.favoriteCount - a.favoriteCount; | ||
| return new Date(b.recent) - new Date(a.recent); | ||
| }); |
There was a problem hiding this comment.
❗️ 수정요청
프론트에서 검색 인풋의 값에 따라 상품요소를 필터링하도록 작성하셨는데, 이렇게 되면 전체 상품 목록에서 필터링 되는 것이 아니라, 불러온 데이터 내에서 필터링하는 것이라 의도한 바와 다르게 동작할거에요!
이런 경우 Products api의 Param을 통해 구현하시면 됩니다.
검색 키워드는 search 값으로 넘겨서 서버에서 응답온 값을 이용해주세요.
이와 동일하게 상품 리스트 정렬은 orderBy 값을 이용해주세요!
There was a problem hiding this comment.
❗️ 수정요청
아래 검색어 입력시도 동일합니다~ 프론트에서 filter 하지 마시고 API를 통해 불러오세요~
| {products.slice(0, visibleCount).map((item) => ( | ||
| <ProductCard | ||
| key={item.id} | ||
| imageUrl={item.images?.[0]} | ||
| title={item.name} | ||
| price={item.price} | ||
| likes={item.favoriteCount} | ||
| variant="best" | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
💬 여담
아래 코드도 동일동작합니다~
| {products.slice(0, visibleCount).map((item) => ( | |
| <ProductCard | |
| key={item.id} | |
| imageUrl={item.images?.[0]} | |
| title={item.name} | |
| price={item.price} | |
| likes={item.favoriteCount} | |
| variant="best" | |
| /> | |
| ))} | |
| {products.slice(0, visibleCount).map(({ id, images, name, price, favoriteCount }) => ( | |
| <ProductCard | |
| key={id} | |
| imageUrl={images?.[0]} | |
| title={name} | |
| price={price} | |
| likes={favoriteCount} | |
| variant="best" | |
| /> | |
| ))} |
| <li onClick={() => handleSelect("최신순")} className={styles.recent}> | ||
| 최신순 | ||
| </li> |
There was a problem hiding this comment.
❗️ 수정요청
이런 클릭 가능한 요소의 경우 버튼을 사용해주세요. 더 적절한 태그가 있는데 이미지 태그에 onClick 을 주게되면 button에게 기대하는 동작을 제대로 수행하지 못합니다!
| <li onClick={() => handleSelect("최신순")} className={styles.recent}> | |
| 최신순 | |
| </li> | |
| <li className={styles.recent}> | |
| <button type="button" value={sortMap["최신순"]} onClick={handleSelect}>최신순</button> | |
| </li> |
위에처럼 button 태그로 바꾸시고 onClick 시 event 객체에서 button 태그의 value를 받아오는 방식으로 구현하시는 것을 추천드려요!
2 participants
요구사항
https://sprint5-minimo.netlify.app/
기본
중고마켓
중고마켓 반응형
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
주요 변경사항
스크린샷
데스크탑
태블릿
모바일
멘토에게
하지만 Sort 기준이 변경될 경우에는 1페이지로 이동하는 것이 더 자연스럽다고 판단해서
useEffect(() => { setPage(1); }, [orderBy]);와 같이 처리했지만, 페이지 번호는 1로 변경되었음에도 불구하고 화면에는 이전 페이지의 데이터가 그대로 출력되는 문제가 발생합니다. 이럴 때는 어떤 방식으로 접근해야 할지 궁금합니다.
모바일에서 검색창 부분의 구조가 바뀌어야 해서 현재는 window.resize를 이용해 조건에 따라 JSX 구조를 변경하고 있습니다.
원래는 CSS로만 처리하려 했지만 원하는 형태가 나오지 않아 리사이즈 방식으로 구현했는데, 이런 방식이 괜찮은지, 혹시 더 나은 방법이 있을지도 궁금합니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.