-
Notifications
You must be signed in to change notification settings - Fork 39
[명지우] sprint5 #184
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 #184
The head ref may contain hidden characters: "React-\uBA85\uC9C0\uC6B0-sprint5"
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.
지우님 5주차 미션 수고하셨습니다!
이미 6,7주차 미션을 제출하신 것을 알아서 다음 미션도 화이팅하시라고 말할 수가 없네요 😁
다음 Next 미션도 화이팅입니다!
-
미션 요구사항 중에 미디어 쿼리를 사용하여 반응형 view 마다 물품 개수를 다르게 보여줍니다 (서버로 요청하는 값은 동일)와 같은 항목이 있습니다. 서버로 요청하는 값은 동일하나 화면에 보여지는 개수만 처리한다는 부분이, 결국엔 사용하지 않을 데이터를 서버에 요청한다는 점에서 자원 낭비라고 생각했습니다. 그래서 저는 반응형 view마다 보여질 상품의 개수(pageSize)를 조절하여 서버에 요청하도록 구현하였습니다. (해당 코드는 AllProductions.jsx와 BestProductions.jsx에서 확인하실 수 있습니다!) :
이번 주차 요구사항중 상충하는 것들이 있어 원하시는 방식으로 구현하시면 됩니다~ 참고 다만 말씀하신 것중사용하지 않을 데이터를 서버에 요청한다는 것은 지금의 방식입니다. 늘 같은 개수, 주로 최대치의 개수를 불러오고 미디어 쿼리문을 이용해 보여지는 개수만 조절하는 방식은 전자보다 요청수가 많을 가능성이 매우 높습니다! -
PC, 태블릿 view와 모바일 view는 전체 상품의 header(정렬 드롭다운 버튼, 검색창 등) 디자인이 다릅니다. 그래서 현재 코드는 모바일 화면인지를 계산하는 isMobile이라는 변수를 통해 반응형 view마다 조건부 렌더링을 하고 있습니다. 하지만 비슷한 코드가 반복되고 조건이 중첩되면서 가독성이 떨어진다는 느낌을 받았습니다. 그래서 UI를 자유롭게 변경할 수 있는 합성 컴포넌트 패턴을 고민했지만, 결국엔 하나의 코드를 화면 크기마다 다른 레이아웃으로 보여줘야 한다는 점에서 합성 컴포넌트 패턴 내부에서도 isMobile과 같은 조건부 렌더링은 불가피하다는 생각이 들었습니다. 이런 경우 코드의 가독성과 중복을 줄이면서 구현하는 좋은 방법엔 무엇이 있는지 여쭤보고 싶습니다.:
우선 이번 디자인의 경우 제 생각에는 css로 처리하는 것이 가능한 디자인이라 미디어 쿼리를 이용해 작업하시는 것이 가장 좋았을 것이라고 생각합니다. 다만 불가피하게 화면 사이즈별로 달라야하는경우, 겹치는 영역이 있다면 해당 영역은 재사용하고 필요한 부분만 조건부 렌더링을 하고 조건부 렌더링 되는 컴포넌트 내부에서도 동일한 디자인이 있다면 컴포넌트로 선언해 재사용하는 것이 좋다고 생각합니다.
// case1 조건부 렌더링 되는 영역이 큽니다.
{isMobile && <MobileHeader />}
{!isMobile && <Header/>}
// case2 조건부 렌더링 되는 영역이 작고 어떤 부분이 화면별로 다른지 파악하기 쉽습니다.
<Header>
<SearchBar/>
<Button>Create</Button>
{!isMobile && <Dropdown/>}
{isMobile && <IconDropdown/>}
</Header>
페이지네이션을 라이브러리 없이 자체 구현해보았습니다. 양 끝의 <과 > 아이콘은 페이지의 맨 처음과 끝으로 이동합니다. 처음으로 자체 구현하다보니 놓친 부분이 있거나 기능상 부족한 부분이 있다면 피드백 부탁드립니다!:
코드에도 피드백을 남겨드렸으니 자세한 사항은 피드백 확인해주세요~ 첨언하자면 페이지네이션은 생각보다 다양한 방식으로 동작합니다. 지우님은 < 버튼을 첫페이지로 이동으로 작업하셨지만 어떤 페이지네이션은 << 버튼이 첫페이지로 이동하고, < 버튼은 전 페이지 혹은 전 단위의 첫 아이템으로 이동하는 등 다양하게 동작하는 페이지네이션이 존재하니 큰 사이트들에서 참고할만한 동작을 찾아보시면 더 좋을 것 같아요!
현재 작업하신 페이지네이션도 잘 하셨습니다~
| @@ -0,0 +1,23 @@ | |||
| <!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"> |
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.
💊 제안
vite로고는 사용되지 않는 것 같아요. 이런 이미지들은 지우시는 것을 추천드려요!
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.
💊 제안
styled 선언문의 위치를 상단, 하단 중 하나로 정하셔서 지켜주시면 가독성 측면에서 더 좋을 것 같아요~
| { | ||
| path: "", | ||
| element: <LandingPage />, | ||
| }, |
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.
💊 제안
index 속성을 이용해 시작페이지라는 것을 더 명확하게 알 수 있게 하시는 것을 추천드립니다~
| { | |
| path: "", | |
| element: <LandingPage />, | |
| }, | |
| { | |
| index: true, | |
| element: <LandingPage />, | |
| }, |
| {bestItemsData?.map((item) => ( | ||
| <ProductCard | ||
| key={item.id} | ||
| id={item.id} | ||
| src={item.images[0]} | ||
| title={item.name} | ||
| price={item.price} | ||
| like={item.favoriteCount} | ||
| ></ProductCard> | ||
| ))} |
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.
💬 여담
객체를 구조분해할당해서 아래처럼 작성하실 수 있습니다~
저는 아래 코드가 더 가독성이 좋은 것 같아요.
| {bestItemsData?.map((item) => ( | |
| <ProductCard | |
| key={item.id} | |
| id={item.id} | |
| src={item.images[0]} | |
| title={item.name} | |
| price={item.price} | |
| like={item.favoriteCount} | |
| ></ProductCard> | |
| ))} | |
| {bestItemsData?.map(({ id, images, name, price, favoriteCount }) => ( | |
| <ProductCard | |
| key={id} | |
| id={id} | |
| src={images[0]} | |
| title={name} | |
| price={price} | |
| like={favoriteCount} | |
| /> | |
| ))} |
| } | ||
|
|
||
| setPageSize(newSize); | ||
| if (onChange) onChange(newSize); |
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.
💬 여담
옵셔널 체이닝을 통해 아래처럼도 작성 가능합니다!
| if (onChange) onChange(newSize); | |
| onChange?.(newSize); |
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Operators/Optional_chaining
| <PageItem key="first" onClick={() => onChange(1)} active={false}> | ||
| <LeftArrowIcon /> | ||
| </PageItem> |
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.
❗️ 수정요청
이런 클릭 가능한 요소의 경우 버튼을 사용해주세요. 더 적절한 태그가 있는데 이미지 태그에 onClick 을 주게되면 button에게 기대하는 동작을 제대로 수행하지 못합니다!
| <PageItem key="first" onClick={() => onChange(1)} active={false}> | |
| <LeftArrowIcon /> | |
| </PageItem> | |
| <PageItem onClick={() => onChange(1)} active={false}> // button태그로 바꿔주세요. key 속성 불필요하니 지워주세요 | |
| <LeftArrowIcon /> | |
| </PageItem> |
추가로 현재 첫 페이지와 마지막 페이지로 이동하는 버튼은 항상 활성화되어 있습니다! 사용자가 이미 첫 페이지나 마지막 페이지에 있을 때 해당 버튼을 disabled 처리하시는 것을 추천드려요.
| const tagList = []; | ||
| for (let i = startPage; i <= endPage; i++) { | ||
| tagList.push( | ||
| <PageItem key={i} onClick={() => onChange(i)} active={i === page}> | ||
| {i} | ||
| </PageItem> | ||
| ); | ||
| } |
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 tagList = []; | |
| for (let i = startPage; i <= endPage; i++) { | |
| tagList.push( | |
| <PageItem key={i} onClick={() => onChange(i)} active={i === page}> | |
| {i} | |
| </PageItem> | |
| ); | |
| } | |
| const tagList = Array.from( | |
| { length: endPage - startPage + 1 }, | |
| (_, i) => { | |
| const pageNumber = startPage + i; | |
| return ( | |
| <PageItem | |
| key={pageNumber} | |
| onClick={() => handlePageClick(pageNumber)} | |
| active={pageNumber === page} | |
| > | |
| {pageNumber} | |
| </PageItem> | |
| ); | |
| } | |
| ); |
저는 개인적으로 jsx 문에서 바로 작성하시는 것이 가장 가독성에 좋을 것 같습니다.
return (
<PaginationList>
<PageItem onClick={() => onChange(1)} />
{ Array.from({ length: endPage - startPage + 1 }).map(el => <PageItem .../>))}
<PageItem onClick={() => onChange(totalPage)} />
</PaginationList>
)| const useDeviceSize = () => { | ||
| const isDesktop = useMediaQuery("(min-width: 1024px)"); | ||
| const isTablet = useMediaQuery("(min-width:768px) and (max-width:1023px)"); | ||
| const isMobile = useMediaQuery("(max-width: 767px)"); | ||
|
|
||
| return { isDesktop, isTablet, isMobile }; | ||
| }; |
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.
💊 제안
각각의 화면에 대응하는지를 알려주는 isDesktop, isTablet, isMobile 대신 'desktop' | 'tablet' | 'mobile'를 반환하시면 더 명확하고 이해하기 쉬울 것 같습니다.
| const ProductContext = createContext({ | ||
| id: null, | ||
| src: "", | ||
| title: "", | ||
| price: 0, | ||
| like: 0, | ||
| }); |
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.
💬 여담
contextAPI를 사용하실 때는 주의해야할 점이 있습니다.
context를 통해 값을 전달할때는 해당 값이 변경되면 구독하는 컴포넌트에서 리렌더링이 발생할 수 있다는 것을 염두에 두고 작업을 해야합니다. (아래 코드의 경우 객체를 value로 공유하고 있어, 렌더링시 새로운 객체가 생성되어 참조가 변경되므로 object.is로 단순 비교시 변경으로 판단되어 리렌더링을 유발합니다. )
이를 해결하실 수 있는 다양한 방법이 있으나 가장 중요한 것은 동작에 대해 이해하는 것이므로 아래의 글들을 읽어보시는 것을 추천드립니다. 추후에는 상태관리 라이브러리를 사용해서 이러한 기능을 구현하실 가능성이 큰데 그때도 context 동작에 대해 이해하고 계시는 것이 도움이 되실거에요!
https://ko.react.dev/reference/react/useContext#optimizing-re-renders-when-passing-objects-and-functions
https://velog.io/@velopert/react-context-tutorial
판다마켓 5
🌐 배포 url: https://myungjiwoo-pandamarket.netlify.app/items
기본 요구사항
체크 리스트 (기본)
데이터 불러오기
상품 정렬
반응형 디자인
체크 리스트 (심화)
스크린샷
멘토에게
미디어 쿼리를 사용하여 반응형 view 마다 물품 개수를 다르게 보여줍니다 (서버로 요청하는 값은 동일)와 같은 항목이 있습니다. 서버로 요청하는 값은 동일하나 화면에 보여지는 개수만 처리한다는 부분이, 결국엔 사용하지 않을 데이터를 서버에 요청한다는 점에서 자원 낭비라고 생각했습니다. 그래서 저는 반응형 view마다 보여질 상품의 개수(pageSize)를 조절하여 서버에 요청하도록 구현하였습니다. (해당 코드는AllProductions.jsx와BestProductions.jsx에서 확인하실 수 있습니다!)isMobile이라는 변수를 통해 반응형 view마다 조건부 렌더링을 하고 있습니다. 하지만 비슷한 코드가 반복되고 조건이 중첩되면서 가독성이 떨어진다는 느낌을 받았습니다. 그래서 UI를 자유롭게 변경할 수 있는 합성 컴포넌트 패턴을 고민했지만, 결국엔 하나의 코드를 화면 크기마다 다른 레이아웃으로 보여줘야 한다는 점에서 합성 컴포넌트 패턴 내부에서도isMobile과 같은 조건부 렌더링은 불가피하다는 생각이 들었습니다. 이런 경우 코드의 가독성과 중복을 줄이면서 구현하는 좋은 방법엔 무엇이 있는지 여쭤보고 싶습니다.<과>아이콘은 페이지의 맨 처음과 끝으로 이동합니다. 처음으로 자체 구현하다보니 놓친 부분이 있거나 기능상 부족한 부분이 있다면 피드백 부탁드립니다!