Skip to content
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

[정성현] sprint6 #93

Conversation

jsh1147
Copy link
Collaborator

@jsh1147 jsh1147 commented Sep 14, 2024

지난 주 수행하지 못한 5주차 미션을 진행하였습니다.

6주차 미션은 최대한 다음 주에 7주차와 함께 진행해보겠습니다.

배포주소

요구사항

기본

중고마켓

  • 중고마켓 페이지 주소는 “/items” 입니다.
  • 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
  • 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
  • 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
  • '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
  • 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

중고마켓 반응형

  • 베스트 상품
    • Desktop : 4개 보이기
    • Tablet : 2개 보이기
    • Mobile : 1개 보이기
  • 전체 상품
    • Desktop : 12개 보이기
    • Tablet : 6개 보이기
    • Mobile : 4개 보이기

심화

  • 페이지 네이션 기능을 구현합니다.

주요 변경사항

  • 중고마켓 페이지 구현
  • 디렉터리 구조 변경 및 파일 명칭 변경

스크린샷

image

image

image

멘토에게

  • img 태그를 이용한 이미지 표현과 backgorund-image CSS를 이용한 이미지 표현이 혼재되어, 다음 주에 기준(이미지가 태그로 표현될만큼 의미를 가지는지)을 잡아 정리할 계획입니다.
  • Dropdown이 아직 수동으로 넣어준 옵션값으로 동작하여, 이를 수정해 진짜 공통 컴포넌트로 쓰이도록 고칠 계획입니다.
  • 멘토링 때 논의되긴 하였지만, 일단 window.innerWidth를 통한 미디어 판단은 필요하다고 판단되어 사용 중인데, 현재 커스텀 훅의 형태에서 컨텍스트의 형태로 바꾸고자 합니다.
  • 가능하면, 레이아웃 푸터와 메인 홈페이지, 로그인/회원가입 페이지까지 옮겨올 수 있도록 계획중입니다ㅠㅠ

- React Router 라이브러리로 페이지 라우팅 구현
- 잘못된 경로로 접근할 경우 에러 표시
- 헤더를 갖고 컨텐츠를 main에 담는 레이아웃 구현
- 피그마를 참고하여 레이아웃 CSS 작성
- 반응형 디자인에 따라 이비지가 바뀌는 경우
  aria-label 속성으로 대체 텍스트 제공
- 중고마켓 제품을 조회하는 getProducts 함수 구현
- 컴포넌트에서의 API 로딩/에러 제어를 위한 useApi 훅 구현
- CSS 미디어쿼리와 유사하게 화면 폭으로 미디어 종류를
  파악하는 useMediaQuery 커스텀 훅 구현
- 현재 페이지에 해당하는 nav 아이템에 색상 적용
- 추가로, main 요소를 페이지 컴포넌트에서 다루도록 제거
- 라우터의 items 경로에 페이지 컴포넌트 연결
- 페이지{최고{상품}, 전체{상품,페이지네이션바}} 구조
- 구현에 필요한 유틸 함수(금액 형식 변환) 구현
- 구현에 필요한 이미지 저장
- 변수명 변경
- 매개변수/반환값 형태 변경
- background-image CSS속성 대신 img 요소 사용
- 미디어 변경 시 API 데이터 갱신되도록 수정
- 전체 풀품 헤드와 페이지네이션 바를 제외한 CSS 작성
- 검색, 정렬, 페이지네이션 구현
- api 관련 오류 수정
- 피그마 디자인에 맞춰  페이지네이션 CSS 구현
- 전체 상품 헤더를 ItemListHead로 분리
- 피그마 디자인에 맞춰 CSS 구현
- CSS 제약에서 벗어나기 위해 별토 컴포넌트로 구현
- 피그마 디자인에 맞춰 CSS 구현
- page와 section의 경우 명칭에 명시
- 특정 페이지 구성 컴포넌트를 page 하위 폴더로 이동
- 페이지의 page & section을 제외한 하위 컴포넌트를
  페이지별 components 폴더 내에 구성
- 공통 컴포넌트를 components 내에 두고,
  layout 컴포넌트의 경우 별도로 하위 폴더 구성
- convertToMoney 대신 toLocaleString 사용
- 불필요한 App.js 제거
@jsh1147 jsh1147 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Sep 14, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

성현님 이번주차 스프린트 미션하시느라 수고하셨습니다!
매우 깔끔하고 고민하신 흔적이 보이는 코드라 리뷰하기 수월했습니다.
스프린트 미션은 학습을 위한 도구로 가능하신 범위까지 해서 제출하시면 되니
너무 부담가지지 마시고, 하신 범위까지 올려주세요~

  • 멘토에게 섹션을 보니 하시고자하는 리팩토링 방향도 명확하신 것 같습니다. 기존에 작업하셨던 페이지를 옮기시는건 좋은 계획이지만 양이 꽤 되니 다른것부터 하시는것을 추천드릴께요~

export default function Item({ data }) {
const { id, name, description, price, images, favoriteCount } = data;
return (
<Link className="item" to={`/item/${id}`}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:

Suggested change
<Link className="item" to={`/item/${id}`}>
<Link className="item" to={`/items/${id}`}>

Comment on lines +14 to +17
const response = await fetch(url);
if (!response.ok) throw Error(`${response.status}: ${response.statusText}`);
const body = await response.json();
return body;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3:
로직을 아주 깔끔하게 잘 작성하셨어요!
다만 이 부분은 다른 API 호출시도 사용가능 할 것 같아서 따로 빼면 더 좋을 것 같습니다.

// 이름은 참고만해주세요. 더 좋은 이름이 있을 것 같습니다.
async function getData(url) {
  const response = await fetch(url);
  if (!response.ok) throw Error(`${response.status}: ${response.statusText}`);

  return await response.json();
}

export async function getProducts({
  page = 1,
  pageSize = 10,
  orderBy = "recent",
  keyword,
}) {
  const url = new URL("products", baseUrl);
  const paramObj = { page, pageSize, orderBy };
  if (keyword) paramObj.keyword = keyword;
  url.search = new URLSearchParams(paramObj);

  return getData(url);
}

Comment on lines +33 to +35
<li className={checkPath("/boards")}>
<Link to="/boards">자유게시판</Link>
</li>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
react-router-dom 라이브러리의 NavLink component를 사용하시면 더 간단하게 구현이 가능합니다.

Suggested change
<li className={checkPath("/boards")}>
<Link to="/boards">자유게시판</Link>
</li>
<li className={checkPath("/boards")}>
<NavLink to="/boards" className={({ isActive }) => isActive ? "header__navItem--active" : ""}>자유게시판</Link>
</li>

*react-router-dom 문서
https://reactrouter.com/en/main/components/nav-link

const [error, setError] = useState(null);
const [data, setData] = useState(null);

const wrrapedFunc = useCallback(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:

Suggested change
const wrrapedFunc = useCallback(
const wrappedFunc = useCallback(

Comment on lines +10 to +15
let result;
try {
setIsLoading(true);
setError(null);
result = await asyncFunc(paramObj);
setData(result);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
result 변수에 재할당할 일이 없으니 const로 선언하시는 걸 추천드려요.

Suggested change
let result;
try {
setIsLoading(true);
setError(null);
result = await asyncFunc(paramObj);
setData(result);
try {
setIsLoading(true);
setError(null);
const result = await asyncFunc(paramObj);
setData(result);

</div>
<PagenationBar
totalPage={Math.ceil(data.totalCount / pageSizeTable[media])}
useParamObj={[paramObj, setParamObj]}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
paramObj라는 state는 해당 API와 관련된 모든 파람을 가지고 있기 때문에
pagination 컴포넌트에서는 불필요한 정보까지 알게 됩니다.
가능하면 해당 컴포넌트가 필요한 값만 넘기시는 걸 추천드립니다.

Suggested change
useParamObj={[paramObj, setParamObj]}
currentPage={paramObj.page}
onChange={onChangeCurrentPage}

const [isLoading, error, data] = useApi(getProducts, paramObj);

useEffect(() => {
setParamObj({ pageSize: pageSizeTable[media], orderBy: "favorite" });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
orderBy 값이 중복되니 아래처럼 작성하시는 걸 추천드립니다.

Suggested change
setParamObj({ pageSize: pageSizeTable[media], orderBy: "favorite" });
setParamObj((prev) => ({ ...prev, pageSize: pageSizeTable[media]}));

<Link className="itemListHead__addButton" to="/additem">
상품 등록하기
</Link>
<Dropdown setParamObj={setParamObj} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
드롭다운 컴포넌트에서 필요한것은 orderBy값을 바꾸는 것이니 그것과 관련된 함수만 내려주시는 것을 추천드립니다.

Suggested change
<Dropdown setParamObj={setParamObj} />
<Dropdown onChange={(orderBy) => setParamObj((prev) => ({...prev, orderBy}))} />

Comment on lines +15 to +22
const handleRecentClick = () => {
setValue("recent");
setIsActive(false);
};
const handleFavoriteClick = () => {
setValue("favorite");
setIsActive(false);
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3:
하나의 함수로 사용하시면 더 좋을 것 같습니다.

Suggested change
const handleRecentClick = () => {
setValue("recent");
setIsActive(false);
};
const handleFavoriteClick = () => {
setValue("favorite");
setIsActive(false);
};
const handleClick = (e) => {
setValue(e.target.value);
setIsActive(false);
};

);

const checkPage = (page) => {
if (page === paramObj.page) return "pagenationBar__button--current";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:

Suggested change
if (page === paramObj.page) return "pagenationBar__button--current";
if (page === paramObj.page) return "paginationBar__button--current";

@GANGYIKIM GANGYIKIM merged commit 9389899 into codeit-bootcamp-frontend:React-정성현 Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants