-
Notifications
You must be signed in to change notification settings - Fork 39
[김치영] sprint6 #162
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 #162
The head ref may contain hidden characters: "React-\uAE40\uCE58\uC601-sprint6"
Conversation
… mount 초기에 setstate가 실행되도록 수정
…ld에서만 처리하는 Logic은 유지)
addiescode-sj
left a comment
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.
수고하셨습니다!
이번에도 한번에 읽어야하는 코드양이 많아서 리뷰가 조금 힘들었네요 ㅎㅎ
열심히하셔서 좋지만, 나중에 팀 프로젝트로 협업하실땐 리뷰어 입장에서 퀄리티있는 리뷰가 가능하도록 PR을 쪼개서 올려보는 연습을 해봅시다! ㅎㅎ
주요 리뷰 포인트
- 코드 스타일
- 가독성, 관리 포인트 개선
- 스켈레톤 (합성 컴포넌트 패턴) 개선 제안
- 공용으로 취급되는 폴더 구분 제안
|
|
||
| const CurrentItemsSection = ({ pageSize }) => { | ||
| //prettier-ignore | ||
| const isLogin = useIsLogin(); |
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.
LoginStateContext 내부를 들여다보니 이렇게 반환값마다 따로 훅을 만드셨던데,
반환값마다 훅을 만들어 사용하게되면, 오히려 Context를 사용해 관련있는 값끼리 응집도를 가지고 모여져있어 유지보수에 용이해지는 장점도 사라지고, 개발자가 어떤 훅을 사용해야 어떤 반환값을 사용할수있다는 사실을 인지해야하므로 불필요한 관리가 추가되게됩니다.
LoginContext를 만들때, 로그인 여부와 간단한 유저정보 등 로그인에 관련된 모든 정보를 한꺼번에 관리하는 형태로 개선해주세요! :)
- BEFORE
export const useIsLogin = () => {
const context = useContext(LoginStateContext);
if (!context) {
throw new Error('반드시 LoginStateProvider 안에서 사용해야 합니다');
}
return context.isLogin;
};
export const useSetIsLogin = () => {
const context = useContext(LoginStateContext);
if (!context) {
throw new Error('반드시 LoginStateProvider 안에서 사용해야 합니다');
}
return context.setIsLogin;
};- AS-IS
export const LoginProvider = ({ children }) => {
// 로그인 식별 정보뿐만 아니라, 로그인에 관련된 다른 정보 (유저 닉네임, 연락 처 등등)도 같이 한 Context로 관리
const [isLogin, setIsLogin] = useState(false);
return (
<LoginContext.Provider value={{ isLogin, setIsLogin }}>
{children}
</LoginContext.Provider>
);
};
export const useLoginContext = () => {
const context = useContext(LoginContext);
if (!context) {
throw new Error('반드시 LoginProvider 안에서 사용해야 합니다');
}
return context;
};| <div className={`${styles["items-container"]} ${styles[listName]}`}> | ||
| {itemList.length === 0 | ||
| ? Array.from({ length: pageSize }, (_, i) => { | ||
| return <ItemCardSkeleton key={i} />; |
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.
스켈레톤 적용하셨군요! 굳굳 👍
다만, 해당 스켈레톤은 여러 버전의 UI를 가질 수 있지만 재사용 가능한 부분 (예를 들면 background color나 사이즈에 따라 스켈레톤 UI가 생성되는 등의 반복되는 로직들)을 분리해 합성 컴포넌트 패턴으로 Skeleton 컴포넌트로 만들어보는건 어떨까해요.
조금 생소하겠지만 이 아티클 참고해보시고, 제가 예시로 작성한 코드를 같이 보면서 읽어만보셔도됩니다! :)
- 기본 스켈레톤 컴포넌트 만들기
import styles from "./Skeleton.module.css";
const Skeleton = ({ children }) => {
return children;
};
// 기본 스켈레톤 컴포넌트
Skeleton.Base = ({
width = "100%",
height = "100%",
borderRadius = "4px",
className,
...props
}) => {
return (
<div
className={`${styles["skeleton-base"]} ${className || ""}`}
style={{
width,
height,
borderRadius,
}}
{...props}
/>
);
};- 기본 스켈레톤을 합성한 상품 목록 스켈레톤 만들기
// 상품 목록 스켈레톤
Skeleton.ProductList = ({ count, className }) => {
return (
<div className={`${styles["product-list-skeleton"]} ${className || ""}`}>
{Array.from({ length: count }, (_, i) => (
<Skeleton.Base
key={i}
width="280px"
height="360px"
className={styles["product-card-skeleton"]}
/>
))}
</div>
);
};- 사용 예시
// 상품 목록 스켈레톤
<Skeleton.ProductList count={5} />
// 커스텀 크기의 카드 스켈레톤
<Skeleton.Card width="320px" height="240px" />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.
이처럼, 리액트는 상속보다는 합성에 특화되어있습니다.
예전 버전이긴하지만, 공식문서에도 나와있는 내용입니다.
질문에 대한 답변
컨텐츠의 특성에 따라 다를것같네요. 스켈레톤의 경우 어느정도의 레이아웃이 픽스된 상태에서 CLS를 방지하기 유리하고, 로딩 스피너는 레이아웃이 픽스되어있지않다거나, 한꺼번에 처리하는 데이터의 양이 매우 많고, 과정이 복잡할 경우 사용하기 적합합니다.
Items 컴포넌트는 usePagination, Pagination을 사용하는 컴포넌트인거죠? 내부 로직을 작성할때 이런 점들을 좀 더 신경써주시면 좋을것같아요.
스켈레톤을 예시로 합성 컴포넌트 패턴을 알려드렸는데, 참고해보시면 도움이 되실것같네요 :) |
요구사항
기본
상품 등록
심화
상품 등록
주요 변경사항
상품 등록하기버튼이 비활성화 되도록 수정배포
https://sprint-mission-kcy.netlify.app/
스크린샷
미션5 변경사항 - skeleton UI
미션5 변경사항 - 쿼리스트링
반응형 디자인(PC)
반응형 디자인(TABLET, MOBILE)
이미지 업로드
이미지 업로드(이미지가 있는 상태에서 이미지 등록 버튼을 한번 더 누를 경우)
이미지 삭제
가격 입력(자동 콤마 적용)
태그 입력(텍스트 입력 후 Enter)
태그 삭제(x 버튼)
등록 버튼 활성화
멘토에게
Loading Spinner와 Skeleton UI 중 어떤 상황에서 어떤 것이 더 적합한지, 어떤 것을 더 많이 사용하는 추세인지 등을 알고 싶습니다.
pagination 기능을 커스텀 훅으로 분리하면서 아래와 같이 기능 분리를 정해 놓고 작업했는데, 더 좋은 방식이 있는지 의견을 여쭤보고 싶습니다.
미션6에서 Field를 컴포넌트로 만들어서 넣고 싶었는데, input의 형태가 다르다보니 어떻게 하는게 좋을 지 고민이 돼서 일단 Flat한 형태로 마무리 했습니다. 모양이 다른 Field들 마다 컴포넌트를 별도로 만드는게 좋을까요?
셀프 코드 리뷰를 통해 질문 이어가겠습니다.