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

[김찬기] Sprint7 #250

Conversation

cksrlcks
Copy link
Collaborator

@cksrlcks cksrlcks commented Dec 4, 2024

요구사항

기본

상품 상세

  • 상품 상세 페이지 주소는 "/items/{productId}" 입니다.
  • response 로 받은 아래의 데이터로 화면을 구현합니다.
  • 목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 "/items" 으로 이동합니다

상품 문의 댓글

  • 문의하기에 내용을 입력하면 등록 버튼의 색상은 "3692FF"로 변합니다.
  • response 로 받은 아래의 데이터로 화면을 구현합니다

심화

  • 모든 버튼에 자유롭게 Hover효과를 적용하세요.

주요 변경사항

  • 배포 링크 : https://sprint-mission-chanki-react.netlify.app/
  • axios interceptor로 요청시 access token을 헤더에 삽입해서 요청을 보내도록 수정했습니다.
  • service폴더내 api 호출 함수들을 interceptor를 설정한 axios instance로 교체했습니다.
  • react router의 loader를 사용해서 상품데이터를 미리 받고 상품 상세 페이지를 렌더링 하도록 했습니다.
  • (추가작업) 상품 수정 페이지 작업 : 상품 등록 폼 컴포넌트를 개선해서 재사용하도록 했습니다.
  • (추가작업) 상품 문의도 '작성/수정/삭제' 까지 api를 활용하여 구현해보았습니다. (상품문의의 작성, 수정도 문의등록 폼을 재사용하도록 했습니다.)
  • (추가작업) 좋아요 버튼을 api를 활용하여 구현해보았습니다. (react router의 revalidate를 활용하여 페이지 새로고침없이 업데이트 되도록했습니다.)
  • (추가작업) 멘토링시간에 배운 페이지네이션 반응형을 보고 현재 프로젝트에도 도입해보았습니다. (페이지네이션 갯수 자체도 반응형이 되도록)
  • (추가작업) 상품 상세 페이지에서 이미지를 클릭시 풀스크린으로 contain사이즈로 보는 기능도 추가해보았습니다. (멘토링시간에 보여주신 cloneElement를 활용해서, 기존 썸네일 컴포넌트를 Fullscreen이라는 컴포넌트로 감싸면 작동하도록 추가했습니다.)

스크린샷

sprint7 sprint7-2

멘토에게

  • react router의 loader를 사용시 loading ui를 나타내는 방법에서 겪은 점을 readme에 정리했습니다.
  • axios interceptor의 response에서 토큰 재발급을 받는 과정과 interceptor에서 재요청시 abort시키는 과정을 합치다보니 axios 인스턴스가 좀 복잡해졌습니다. (두 과정을 따로 구글링으로 알게 되어서 혼합 했을때 스스로 생각하기에 순서가 이렇게 되어야 할 것 같은 순서로 재조합 했습니다.)
  • interceptor로 모든 중복요청을 abort시키려고하니, 프로젝트내 모든 에러 핸들링 코드에서 canceled case를 예외처리 해줘야하는 점이 있이서, 필요한곳에서 signal을 보내서 실행하는 방식으로 변경했습니다.
  • 사용자 인증처리 과정을 담당하는 auth context내부로 요청 및 응답에 interceptor로 인증관련 추가작업하도록 이동했습니다.
  • 페이지컴포넌트, 일반 컴포넌트에서 바로 서비스폴더내 api요청함수를 꺼내쓰는 부분을 전부 수정했습니다. (커스텀훅을 통해서 사용하도록)
  • 그래서 로더 함수도 파일도 따로 분리하였습니다. (로더를 페이지컴포넌트에서 작성하면 서비스함수를 임포트하게 되어서)
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

- product,auth 서비스 함수들 axios로 변경
- comment 서비스 함수 작성
- interceptor로 요청시 header에 토큰 주입 및 응답시 refresh token 사용 로직 추가
- authcontext 수정 (interceptor로 빠진 로직 제거)
- abort case 수정 (axios와 fetch의 error name 차이)
- textarea에 size props 추가
- module css로 변경
- onRemoveItem이 있을경우에만 onclick, removable 전달
- 업로드 컴포넌트에서 string타입으로 이미지주소를 받으면 preview로 바로 넘기도록 개선
- 등록, 수정 페이지에 서비스 함수들을 가져와서 submit 함수를 만들어서 공용등록폼에 전달해서 실행
- comment 컴포넌들을 공용으로 분리
- products, articles 에서 공용으로 쓸 수 있게 개선
- useComment, useComments 훅 생성
- useSearchParams 내부를 보니, SetURLSearchParams가 navigateOpts을 선택적으로 받을수 잇게 되어있음을 보고, 스크롤 리셋 방지 옵션을 true 설정
- 게시물 페이지네이션 카운트수에도 적용
- product페이지에서만 사용하는 훅으로 변경
- variant props 추가
- color css 추가 (secondary, error)
@cksrlcks cksrlcks added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Dec 4, 2024
- 앱 마운트시, 토큰 만료 검증후 재발급 하는 과정 추가
- 토큰이 재발급거나, 토큰이 유효한 경우에 유저정보를 불러옴
- 리프레시토큰으로 재발급요청하는 instance를 분리(기존 instance를 사용하면 다시 검증과 요청과정을 거치게 됨)
@cksrlcks cksrlcks force-pushed the React-김찬기-sprint7 branch from 228e4d7 to f5ceef0 Compare December 5, 2024 11:13
- useref를 활용하여 유저정보조회 훅의 실행을 조절
- 토큰 재발급이 필요한지 체크하는 훅이 끝난후에 useref를 변경하여 유저정보조회가 가능하도록 개선
- 중복된 요청의 abort를 진짜 사용할곳에서만 하도록 변경
- 모든 요청에 중복요청을 못하도록 막으니까, 모든 요청의 에러에 aborterror일경우에는 다르게 핸들링하는 코드를 넣어야됨
- 인증관련 interceptor 설정을 auth context 내부로 들고옴 (state와 local storage의 sync를 위해)
- 맨토링시간에 배운 context api 개선건 적용
Comment on lines +12 to +29
const DropdownStateContext = createContext();
const DropdownDispatchContext = createContext();

const useDropdownState = () => {
const context = useContext(DropdownStateContext);
if (!context) {
throw new Error("Dropdown components must be used within an Dropdown.");
}
return context;
};

const useDropdownDispatch = () => {
const context = useContext(DropdownDispatchContext);
if (!context) {
throw new Error("Dropdown components must be used within an Dropdown.");
}
return context;
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

금요일 멘토링 시간에 보여주신 context api를 state와 dispatch로 구분해서 제공하는 방법 적용했습니다.

- 멘토링시간에 배운 cloneElement활용
- 썸네일 컴포넌트를 Fullscreen으로 감싸면 작동하도록 작업
Comment on lines +5 to +27
export function Fullscreen({ children }) {
const [imgSrc, setImgSrc] = useState(null);

function handleOpenScreen(src) {
setImgSrc(src);
}

function handleCloseScreen() {
setImgSrc(null);
}

return (
<>
{cloneElement(children, { onOpenScreen: handleOpenScreen })}
{imgSrc &&
createPortal(
<ImgViewer src={imgSrc} onCloseScreen={handleCloseScreen} />,
document.querySelector("#root")
)}
</>
);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

썸네일 컴포넌트를 감싸게될 Fullscreen 컴포넌트가 자식요소인 썸네일 컴포넌트에게 props를 주도록 작업했습니다.
확대보기컴포넌트는 포탈을 이용해서 dom 렌더링 위치를 root바로 아래로 옮겼습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

이후에 Modal 컴포넌트를 만들일이 있을 때,
createPortal 하는 기능을 분리해서 Modal 컴포넌트를 만들고,
FullScreen은 Modal에 ImgViewer를 얹는 방향으로 구현해도 좋을 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

다음 미션때, 모달컴포넌트를 분리해보겠습니다. 감사합니다.

Copy link
Contributor

@withyj-codeit withyj-codeit left a comment

Choose a reason for hiding this comment

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

스프린트 미션 하느라 수고 많으셨어요~👏🏻

#### react router의 loader를 사용할시 추가로 설정해줘야했던 설정들

- React의 Suspense와 fallback, React router의 loader, hydrateFallback을 조사해보고 정리해보았습니다.
- https://heavy-bear.tistory.com/13
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

<Author avatar={image} nickname={nickname} updatedAt={updatedAt} />
</div>
</div>
<More
Copy link
Contributor

Choose a reason for hiding this comment

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

More 기능이 댓글 작성자에게만 보여지는게 자연스럽다고 생각해서
스프린트 미션에 디테일한 요구사항이 없긴 하지만, hasEditPermission 같은 데이터로 렌더링 여부를 결정하는게 더 좋지 않을까 생각해요.

조금 덧붙이면 comment데이터를 넘겨주는 useComments 레벨에서 useAuth를 호출해서 권한에 대한 데이터도 comments에 함께 담아 넘겨줘도 좋을 것 같아요.

Copy link
Collaborator Author

@cksrlcks cksrlcks Dec 10, 2024

Choose a reason for hiding this comment

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

image
image

오 제가 처음에 생각했던 방법과 완전히 동일합니다. 다음 미션때 다시 수정해두겠습니다.ㅎㅎ
(useCommets에서 useAuth의 id와 동일한 comment에 isOwner를 전달하는 방법으로 했었습니다.)

white-space: pre-wrap;
}

.footer {
Copy link
Contributor

Choose a reason for hiding this comment

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

module.scss를 사용하고, 컴포넌트 안에서 클래스 이름이 중복되지 않는 경우가 일반적이라
클래스 이름을 부모 자녀 관계가 드러나지 않고 flat하게 작성해도 좋아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다. nesting한 css를 한번 flat하게 변경해보겠습니다.

Comment on lines +5 to +27
export function Fullscreen({ children }) {
const [imgSrc, setImgSrc] = useState(null);

function handleOpenScreen(src) {
setImgSrc(src);
}

function handleCloseScreen() {
setImgSrc(null);
}

return (
<>
{cloneElement(children, { onOpenScreen: handleOpenScreen })}
{imgSrc &&
createPortal(
<ImgViewer src={imgSrc} onCloseScreen={handleCloseScreen} />,
document.querySelector("#root")
)}
</>
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

이후에 Modal 컴포넌트를 만들일이 있을 때,
createPortal 하는 기능을 분리해서 Modal 컴포넌트를 만들고,
FullScreen은 Modal에 ImgViewer를 얹는 방향으로 구현해도 좋을 것 같아요.

@@ -2,20 +2,20 @@ import { useEffect, useState } from "react";
import { getDeviceType } from "@util/breakpoints";
import { debounce } from "@util/debounce";

export default function usePageSize(
initialPageSize = {
export default function useReponsive(
Copy link
Contributor

Choose a reason for hiding this comment

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

useResponsive 라는 이름이 반응형에 따라 일반적으로 사용하려는 걸로 보여서
나머지 pageSize, setPageSize도 같이 이름을 변경하면 좋을 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

훅의 용도를 일반적으로 바꾸면서 내부 state변수명을 생각을 못했네요;;
감사합니다. ㅎㅎ

// 두 요청을 거의 동시에 보내도록 우선 await없이 promise를 각 변수에 할당함
// 여기서 미리 await을 작성하면 그 요청을 기다린후 다음줄의 코드가 실행되어서 동시에 요청이 안됨
const detail = getProduct(id);
const comments = getComments("products", { productId: id, limit: 5 });
Copy link
Contributor

Choose a reason for hiding this comment

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

getComments를 먼저 실행하고
await getProduct 실행해도
return 에서 await 하는 거랑 결과가 같지 않나요?

return 에서 await 하는걸 수정하면 좋겠다는 의도에서 하는 얘기는 아니고 그냥 궁금해서 남겨요ㅎㅎ

Copy link
Collaborator Author

@cksrlcks cksrlcks Dec 11, 2024

Choose a reason for hiding this comment

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

image
image

결과가 같게 나옵니다. 😱

  • 왜 코드작성할때는 자꾸 코멘트를 가져오는 구문을 꼭 상품상세 밑쪽으로 적으려했는지 ㅠㅠ
  • 리뷰해주신내용처럼 코멘트 promise를 실행시켜버리고, 상품데이터 promise를 await하는 방식이 더좋은것같아요;;;

참고로 젤 처음에 생각한 방식은 아래 이미지였습니다.
image

만약에 리턴에서 깨끗하게 await을 안쓰고 동일한 방식으로 동작하려면 이렇게 작성해야하더라구요.
(promise를 실행시키는건 거의 동시적으로 일어나면서, 한쪽은 다시 await하는 방식)

코드 한줄이 더늘어나는것 같아서 리턴문에서 기다리자하고 수정했었는데, 멘토님이 말해주신방법이 더 좋은것 같습니다ㅎㅎ

@withyj-codeit withyj-codeit merged commit 2f11aa1 into codeit-bootcamp-frontend:React-김찬기 Dec 10, 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