[박원현] sprint5 #147
[박원현] sprint5 #147
Conversation
[Fix] delete merged branch github action
This reverts commit 6603927.
There was a problem hiding this comment.
원현님..!
프로젝트가 끝난 후에도 열심히 달려주셨군요!! 👍 👍
리엑트는 역시나 많이 익숙하신 거 같고, 재사용과 확장성에 조금만 더 신경 써주시면 좋을 거 같습니다 :)
개인적으로 케밥케이스의 네이밍 컨벤션이 가독성에 조금 더 도움이 되는 것 같긴 한데.. 리액트 컴포넌트의 대문자 네이밍 컨벤션과 헷갈릴수 있을 것 같습니다.
-> 괜찮습니다 :) 이건 회사, 프로젝트, 사람마다 다른데 케밥케이스 네이밍은 많이 쓰이는 방법 중 하나입니다 ㅎ
페이지네이션을 따로 뽑아서 구현했는데 한 번 간단하게 살펴봐주시면 감사하겠습니다!
-> 리뷰 참고해주세요~! :)
반응형으로 현재 pageSize를 useResponsivePageSize 훅으로 관리를 하려고 합니다 그런데 이게 모바일 사이즈에서 새로고침을 할 때 제대로 동작하지 않는 문제가 있는 것 같아서 훅이나 사용된 곳에서 무언가 문제가 있는 건지 아니면 useEffect의 실행순서와 연관이 있는 건지 구별하기가 조금 어려운 것 같습니다. useMediaQuery훅의 경우 window 전역객체를 사용해서 현재 창 크기를 불러와서 비교를 하게 되는데 window전역객체가 아직 준비되지 않은 경우가 있어서 그런 걸까요? 보통은 어떻게 반응형을 감지하나요?
-> 리뷰 참고해주세요~! :)
아 그리고 커밋 메시지도 조금 상세하게 남겼는데 추가로 보완할 게 있을까요?
-> 좋습니다~! 의미있게 기능 단위로 잘 남겨주셨습니다 👍 👍
| return context; | ||
| }; | ||
|
|
||
| function DropDownContainer({ |
| }; | ||
|
|
||
| function DropDownContainer({ | ||
| orderBy = "recent", |
There was a problem hiding this comment.
다만, 컴파운드 패턴을 사용하는 이유 중 하나는 하위 컴포넌트를 유연하게 사용하여 기능을 쉽게 조합하고 재사용할 수 있는 것에 있죠! 근데 지금의 경우 oderBy 같은 도메인 로직이 들어오면서 패턴 사용의 의미가 크게 떨어지는 거 같습니다 🤔
| value: string; | ||
| type: inputType; | ||
| error: boolean; | ||
| handleChange: (e: React.ChangeEvent<HTMLInputElement>) => void; |
There was a problem hiding this comment.
React.ChangeEventHandler 타입도 사용 가능합니다 :)
There was a problem hiding this comment.
아니면 대부분 본래 input이 사용하는 attribute를 프랍으로 그대로 받을 때는 아래처럼 사용도 많이 합니다!
interface Props extends React.HTMLAttributes<HTMLInputElement> {
// .. 기본 속성 이외 커스텀 속성 타입 추가
}| ); | ||
| } | ||
|
|
||
| const Input = { |
There was a problem hiding this comment.
현재 상태만 봤을 때는 약간은 복잡한 느낌이 있습니다 🤔
사실상 기능을 하고, 상태에 반응하는 게 input 하나라 context는 프랍스 전달 용도로만 사용되고 있고, label은 props로 간단하게 처리 가능할 거 같습니다.
function Input({label, ....}) {
return
<div className={styles["input__group"]}>
<label className={`${styles["input__label"]} font-2lg font-bold`}>
{label}
</label>
<input
className={`${styles["input__field"]} ${
error ? styles["input__field--error"] : ""
}`}
type={inputType}
onChange={handleChange}
placeholder={placeholder}
value={value}
onBlur={handleBlur}
/>
</div>
}다만, 제가 드린 거는 그냥 의견입니다 :) 그룹/라벨/필드의 조합이 복잡해질 거를 생각하신 거라면 괜찮은 방향 같습니다.
그래도, 그런 경우에도 위에
export type inputType = "email" | "username" | "password";
const inputType = type === "username" ? "text" : type;같은 도메인 로직은 빠지는 것이 좋겠죠!
| }: PaginationProps) { | ||
| let pages; | ||
| if (currentPage <= 3) { | ||
| pages = Array.from({ length: 5 }, (_, i) => i + 1); |
There was a problem hiding this comment.
데이터 수가 적을 때도 페이지 5개는 무조건 생기게 될 거 같네요..! 🤔
| export interface PaginationProps { | ||
| currentPage: number; | ||
| totalPage: number; | ||
| setPage: React.Dispatch<SetStateAction<number>>; |
There was a problem hiding this comment.
setter를 받는 거 보다 onChangePage 같은 콜백을 받으면 확장성에 도움이 될 수 있습니다!
페이지가 변했을 때 무엇을 할 것인지는 밖에서 결정하는 것이죠.
예를 들면 요구 사항이 수정되어 페이지네이션이 쿼리스트링으로 관리되어야 한다면 현재 컴포넌트는 다시 수정되어야겠죠 :)
There was a problem hiding this comment.
말 나온 김에 더 첨언드리면, 현재 페이지네이션이나 정렬, 검색 모두 상태로 관리되고 있는데, 이거를 쿼리스트링으로 관리해 보셔도 좋아요 :)
지금은 새로고침하면 유저가 보고 있던 페이지, 정렬이 모두 날라가게 되죠!
| <> | ||
| <ItemsHeader /> | ||
| <main style={styles}> | ||
| <BestProducts /> |
| ); | ||
|
|
||
| useEffect(() => { | ||
| setIsHydrated(true); // 클라이언트에서 실행되었음을 표시 |
There was a problem hiding this comment.
불필요한 코드 같네요! 리엑트가 실행되기 전에도 이미 윈도우의 크기는 정해져 있습니다. 렌더링이 끝나고나서 무언가를 할 이유는 없는 거 같아요 🤔 현재 버그의 원인이기도 합니다!
There was a problem hiding this comment.
또한 현재는 모든 렌더링이 클라이언트에서 진행되므로 하이드레이션 용어 사용도 잘못 됐습니다~!
| const [isHydrated, setIsHydrated] = useState(false); | ||
|
|
||
| const [pageSize, setPageSize] = useState<number>( | ||
| pageSizeConfig[defaultSizeKey] |
There was a problem hiding this comment.
이 코드 때문에 서버 요청이 두 번 가고 있습니다~!
렌더링 ->setIsHydrated -> 윈도우가 어떤 크기가 됐던 defaultSizeKey에 맞는 pageSize로 요청 한 번 보내고,
동시에 setPageSize가 실행 -> 실제 윈도우 크기에 맞게 요청을 다시 보내게 되죠!
때문에 모바일에서 새로고침하면 UI가 올바르게 될 때도 있고 안 될 때도 있을텐데, 비동기라서 그렇습니다.
데스크탑 데이터, 모바일 데이터 요청을 둘 다 했을 때, 데스크탑 데이터 요청을 먼저 보냈다고 하더라도 순서대로 도착하고 실행될 보장이 없는거죠..!
There was a problem hiding this comment.
개발자 도구에 네트워크탭을 보시면 좀 더 확인하기 쉬우실 거에요!
4 participants
요구사항
Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
피그마 디자인에 맞게 페이지를 만들어 주세요.
React를 사용합니다
기본
중고마켓 페이지 주소는 “/items” 입니다.
페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
'상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.
중고마켓 반응형
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
Desktop : 10개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
주요 변경사항
스크린샷
멘토에게