[김영현] Sprint5 #172
[김영현] Sprint5 #172
Conversation
|
안녕하세요 주강사님. 이번에 리액트를 활용하여 화면을 구현하는데 많은 어려움이 있었습니다. props를 어떻게 내려야할 지, 재사용성, 컴포넌트 분리 방식 등 많은 생각을 하면서 구현한 것 같습니다. [궁금한 점]
감사합니다!!! |
There was a problem hiding this comment.
영현님 React 미션 고생하셨습니다!
- 배포 사이트 기준으로 가로 스크롤이 생기네요! 확인 후 수정해보세요~
- vite로 설정해주신 것 좋습니다.
- reset.css 처럼 브라우저 기본 스타일을 초기화해주시면 디자인 구현에 더 좋을 것 같아요.
스타일 클래스 네이밍은 BEM 방식에 맞춰 작성했습니다. 혹시 구조적으로나 네이밍 측면에서 더 나은 방식이 있다면 피드백 부탁드립니다.: 클래스 네이밍 방식은 어떤 것을 쓰시던 호불호의 영역일뿐 큰 차이가 없다고 생각합니다. 구조적으로는 페이지가 늘거나 작업을 하시면서 스스로 필요성을 느끼신 후 수정하시는 것이 좋다고 생각합니다~화면 크기를 직접 조절(Resize) 하거나, 개발자 도구에서 모바일 디바이스 외에는 반응형 스타일이 잘 적용되는데, "모바일 디바이스"를 선택하면 반응형 스타일이 적용되지 않습니다. 이런상황에서는 어떤 점을 고려하거나 확인해보면 좋을까요?: 제가 확인해봤을 때는 직접 화면 크기를 줄여도 화면이 줄어들때 각 페이지에서 반응형이 제대로 작업된 것 같지 않습니다~ 해당 페이지를 작업하셨으니 의심가는 컴포넌트가 있으면 코드로 확인해보셔도 되고 검사탭에서 어떤 컴포넌트가 overflow 되서 가로 스크롤을 만드는지 확인해보시고 수정하시면 됩니다!ProductCard, Pagination, SearchInput은 재사용 가능성을 고려하여 컴포넌트화 했는데, 컴포넌트 분리 방식이나 props 전달 구조에 개선할 점이 있을지 궁금합니다.: 해당 프로젝트에서 실제로 재사용하는지는 다른 페이지의 디자인을 봐야알 것 같습니다. 또한 재사용성되지 않아도 컴포넌트로 분리하시는 것이 가독성 및 관리 측면에서 유리합니다.전체적인 컴포넌트 구조, props 흐름, 상태 관리 방식 등에서 개선할 부분이 있다면 조언 부탁드리겠습니다.: 지엽적인 부분들에 대해서는 코멘트 드리고 있습니다. 전체적인 부분들중 작업을 진행하시면서 시행착오를 통해 고치시면 좋거나 취향의 문제라고 생각되는 경우 코멘트 드리지 않으니 양해부탁드립니다!현재 components 폴더 안에 모든 파일이 모여 있어, components/컴포넌트명/컴포넌트.jsx + .css 구조로 리팩토링 하려고 하는데 이 구조에 대해 어떻게 생각하시는지 궁금합니다.: 저는 전자보다는 후자가 더 좋다고 생각합니다~
There was a problem hiding this comment.
💊 제안
3번째 미션에서 추가하셨던 메타 태그도 추가하시면 더 좋을 것 같아요!
| <header className="header"> | ||
| <div className="header__logo"> | ||
| <div> | ||
| <NavLink to="/items"> | ||
| <img src={logo} alt="logo" className="header__logo-img" /> | ||
| </NavLink> | ||
| </div> | ||
|
|
||
| <div className="header__nav-container"> | ||
| <nav className="header__nav"> | ||
| <NavLink to="/board">자유게시판</NavLink> | ||
| <NavLink to="items">중고마켓</NavLink> | ||
| </nav> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="header__right"> | ||
| <img src={userImage} alt="user" className="header__user-img" /> | ||
| </div> | ||
| </header> |
There was a problem hiding this comment.
💊 제안
지금처럼 components 폴더를 만들어 컴포넌트를 분리해주신 것처럼, header도 별도의 컴포넌트로 분리해보는 것을 추천드려요!
앞으로 페이지가 늘어나고, 로그인 여부에 따라 헤더의 내용이 달라질 수도 있기 때문에, 미리 분리해두면 유지보수나 기능 추가 시 훨씬 효율적으로 작업하실 수 있습니다.
| <img src={userImage} alt="user" className="header__user-img" /> | ||
| </div> | ||
| </header> | ||
| <hr style={{ border: "solid 1px #DFDFDF" }} /> |
There was a problem hiding this comment.
❗️ 수정요청
해당 hr 요소를 디자인 구현을 위해 추가해주신 것 같아요. 다만 디자인상에서 보더는 header에 borderBottom 속성을 통해서 구현하시는 것이 더 적절할 것 같습니다.
| <hr style={{ border: "solid 1px #DFDFDF" }} /> |
There was a problem hiding this comment.
❗️ 수정요청
브라우저 스타일이 초기화되어 있지 않아서 body 태그에 margin: 8px가 존재하네요.
그래서 디자인대로 구현이 되지 않으니 확인 후 수정해보세요!
There was a problem hiding this comment.
❗️수정요청
파일이 현재 비어있는 상태로 커밋되어 있습니다.
작업 중인 파일이라면 간단한 주석이라도 남겨 현재 상태를 설명해주시는 것이 좋고, 그렇지 않다면 파일은 실제 구현할 때 추가해주시는 것을 추천드립니다!
There was a problem hiding this comment.
아! 작업이 되고 있을 때 커밋을 올리도록 참고하겠습니다. 좋은 말씀 감사합니다 ㅎㅎ!!
| useEffect(() => { | ||
| const updateVisibleCount = () => { | ||
| const width = window.innerWidth; | ||
|
|
||
| if (width <= 767) { | ||
| // mobile | ||
| setVisibleCount(1); | ||
| } else if (width <= 1199) { | ||
| // tablet | ||
| setVisibleCount(2); | ||
| } else { | ||
| setVisibleCount(4); | ||
| } | ||
| }; | ||
|
|
||
| updateVisibleCount(); | ||
| window.addEventListener("resize", updateVisibleCount); // resize 시 eventListener | ||
|
|
||
| return () => window.removeEventListener("resize", updateVisibleCount); | ||
| }, []); |
There was a problem hiding this comment.
💬 여담
useEffect 내부에 선언된 updateVisibleCount 함수는 컴포넌트 바깥으로 분리하셔도 됩니다.
외부로 updateVisibleCount 함수를 분리하게 되면 useEffect 의존배열에 updateVisibleCount가 들어가게 되니
해당 함수가 재정의되지 않도록 useCallback으로 감싸셔야합니다~
| useEffect(() => { | |
| const updateVisibleCount = () => { | |
| const width = window.innerWidth; | |
| if (width <= 767) { | |
| // mobile | |
| setVisibleCount(1); | |
| } else if (width <= 1199) { | |
| // tablet | |
| setVisibleCount(2); | |
| } else { | |
| setVisibleCount(4); | |
| } | |
| }; | |
| updateVisibleCount(); | |
| window.addEventListener("resize", updateVisibleCount); // resize 시 eventListener | |
| return () => window.removeEventListener("resize", updateVisibleCount); | |
| }, []); | |
| const updateVisibleCount = useCallback(() => { | |
| const width = window.innerWidth; | |
| if (width <= 767) { | |
| setVisibleCount(1); | |
| } else if (width <= 1199) { | |
| setVisibleCount(2); | |
| } else { | |
| setVisibleCount(4); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| updateVisibleCount(); | |
| window.addEventListener("resize", updateVisibleCount); | |
| return () => window.removeEventListener("resize", updateVisibleCount); | |
| }, [updateVisibleCount]); |
There was a problem hiding this comment.
아하 제가 useCallback을 잘몰라서 좀 더 사용예제를 찾아보고 적용시키도록 하겠습니다!!! 감사합니다 ㅎㅎㅎ
|
|
||
| const nav = useNavigate(); // 페이지 이동을 위한 hook 사용 | ||
|
|
||
| const pageSize = 10; // 한 페이지에 보여줄 상품 수 |
There was a problem hiding this comment.
❗️ 수정요청
사용하지 않는 변수는 지우시는 것을 추천드려요~
| return new Date(b.createdAt) - new Date(a.createdAt); | ||
| }); | ||
|
|
||
| const filteredProducts = sortedProducts.filter((product) => product.name.includes(searchKeyword)); |
There was a problem hiding this comment.
❗️ 수정요청
프론트에서 검색 인풋의 값에 따라 상품요소를 필터링하도록 작성하셨는데, 이렇게 되면 전체 상품 목록에서 필터링 되는 것이 아니라, 불러온 데이터 내에서 필터링하는 것이라 의도한 바와 다르게 동작할거에요!
이런 경우 Products api의 Param을 통해 구현하시면 됩니다.
검색 키워드는 search 값으로 넘겨서 서버에서 응답온 값을 이용해주세요.
이와 동일하게 상품 리스트 정렬은 orderBy 값을 이용해주세요!
| <select value={orderBy} onChange={handleOrderChange} className="product-list__select"> | ||
| <option value="createdAt">최신순</option> | ||
| <option value="favorite">좋아요순</option> | ||
| </select> |
There was a problem hiding this comment.
💊 제안
피그마에서 디자인을 확인해보시면 select 태그와 다른 것을 확인할 수 있습니다~
이런 디자인을 구현하기 위해서는 select 태그를 사용하시는 것이 아니라 만드셔야해요.
여유가 되시면 구글에 react dropdown 만들기 같은 식으로 검색하셔서 디자인대로 구현해보시면 좋겠습니다!
There was a problem hiding this comment.
아하 맞아요.. 이것도 좀 생각을 했는데, 우선 기능을 만들자는 생각으로만 접근한 것 같습니다. 알려주신 react dropdown, 다양한 인사이트 정보들을 찾으면서 다음 미션 때 적용할 수 있도록 하겠습니다!!!
There was a problem hiding this comment.
💊 제안
정적 요소와 동적 요소가 혼합되어 작성되면, 추후 유지보수 시 어디서 무엇이 생성되는지 파악하기 어려울 수 있습니다.
가급적 전체 페이지 버튼들을 한 번에 생성하는 방식-Array.map, Array.from-으로 작성하거나, 최소한 이전 버튼과 나머지 페이지 버튼의 초기화를 일관된 스타일로 통일하면 코드의 가독성을 높일 수 있습니다.
가장 추천드리는 방식은 아래처럼 jsx에서 바로 작성하시는 거에요~
const Pagination = ({ currentPage, totalPage, onPageChange }) => {
const pageGroupSize = 5;
const currentGroup = Math.floor((currentPage - 1) / pageGroupSize);
const startPage = currentGroup * pageGroupSize + 1;
const endPage = Math.min(startPage + pageGroupSize - 1, totalPage);
return (
<div className="pagination">
<button
onClick={() => onPageChange(Math.max(1, startPage - pageGroupSize))}
className="pagination__button"
>
<
</button>
{Array.from({ length: endPage - startPage + 1 }, (_, index) => (
<button
key={startPage + index}
onClick={() => onPageChange(startPage + index)}
className={`pagination__button ${
currentPage === startPage + index ? "active" : ""
}`}
>
{startPage + index}
</button>
))}
<button
onClick={() => onPageChange(endPage + 1)}
className="pagination__button"
>
>
</button>
</div>
);
}There was a problem hiding this comment.
아 너무 단편적으로만 생각해서 구현한 것 같습니다. 알려주신 방법(Array.from / Array.map) 찾아보고 적용하겠습니다. 이외에도 좋은 방법이 있다면 찾아보고 적용하겠습니다. 좋은 말씀 감사합니다 ㅎㅎㅎㅎ
2 participants
[기본 요구사항]
베스트 상품 기준
베스트 상품
전체 상품
[심화 요구사항]
주요 변경사항
배포 사이트
https://hyunbara5.netlify.app/
멘토에게