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 #102

Conversation

heonq
Copy link
Collaborator

@heonq heonq commented Nov 3, 2024

기본 요구사항

랜딩 페이지

  • HTML과 CSS로 구현한 랜딩페이지를 React로 마이그레이션하세요.
  • 랜딩 페이지 url path는 "/"로 설정하세요.

중고마켓 페이지

  • 중고마켓 페이지 url path를 "/items"으로 설정하세요.
  • 페이지 주소가 "/items" 일 때 상단내비게이션바의 "중고마켓" 버튼의 색상은 "3692FF"입니다.
  • 중고마켓 페이지 판매 중인 상품은 본인이 만든 GET 메서드를 사용해 주세요.
  • 다만 좋아요 순 정렬 기능은 제외해 주세요.
  • 사진은 디폴트 이미지로 프론트엔드에서 처리해주세요.
  • 베스트 상품 목록 조회는 구현하지 않습니다.
  • '상품 등록하기' 버튼을 누르면 "/registration" 로 이동합니다. ( 빈 페이지 )

상품 등록 페이지

  • PC, Tablet, Mobile 디자인에 해당하는 상품 등록 페이지를 만들어 주세요.
  • 상품 등록 url path는 "/registration"입니다.
  • 상품 등록은 본인이 만든 POST 메서드를 사용해 주세요.
  • 등록 성공 시, 해당 상품 상세 페이지로 이동합니다. (빈페이지)

심화 요구사항

상품 등록 페이지

  • 모든 입력 input box에 빈 값이 있을 경우, 등록 버튼이 비활성화됩니다.

  • 태그를 입력한 후 엔터키를 누르면, 그 태그가 칩 형태로 쌓입니다.

  • 상품명, 상품 소개, 판매 가격, 태그에 대한 유효성 검사 Custom Hook을 만들어주세요. 유효성 검사를 통과하지 않을 경우, 각 input에 빨간색 테두리와, 각각의 Input 아래에 빨간색 에러 메시지를 보여주세요.

  • 유효한 조건

  • 상품명: 1자 이상, 10자 이내

  • 상품 소개: 10자 이상, 100자 이내

  • 판매 가격: 1자 이상, 숫자

  • 태그: 5글자 이내

주요 변경사항

  • 상품 등록 페이지 구현

스크린샷

배포 : https://adorable-phoenix-7e2244.netlify.app/

스크린샷 2024-11-03 오전 10 03 04
localhost_5173_registration_
localhost_5173_registration_ (1)

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@heonq heonq changed the base branch from main to react-함헌규 November 3, 2024 01:07
@heonq heonq requested a review from kimjong95 November 3, 2024 01:07
@heonq heonq self-assigned this Nov 3, 2024
@heonq heonq added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 최종 제출 스프린트미션 최종 제출본입니다. labels Nov 3, 2024
Copy link
Collaborator

@kimjong95 kimjong95 left a comment

Choose a reason for hiding this comment

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

상수값들을 적절히 사용중이시고, 구조도 적절히 나눠주신것 같아요.

디테일하게보자면 컴포넌트의 리렌더링에 고민을 가져보셔야 할 것 같습니다. 실제로 내가 사용하고있지 않은 부분에서 리렌더링이 발생한다는 뜻은 성능에 문제가 발생할 수도 있다는 뜻 입니다. 현대에와서 하드웨어의 성능이 워낙 좋다보니 성능에대한 고려를 하지않는 경우가 많습니다. 하지만 컨트롤하지 않는 곳에서 리렌더링이 발생할 수 있다는것은 성능뿐만아니라 의도하지않은 사이드이펙트가 발생할 수 있다는 점이 있어요. 이부분에 대해서는 깊게 고민이 필요합니다

import { ICreateProductParams, ISearchProductsParams } from "../types/parameter";
import api from "./axiosInstance";

export const getProductList = async (params: Partial<ISearchProductsParams> = {}) => {
Copy link
Collaborator

@kimjong95 kimjong95 Nov 5, 2024

Choose a reason for hiding this comment

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

조회조건을 선택적으로 컨트롤하기위해 Partial타입을 써주신게 좋은것 같아요!


export default ProductRegistrationComponent;

ProductRegistrationComponent.Title = RegistrationTitle;
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴파운드 패턴으로 폼을 구성해주신것 좋습니다. 다만 몇가지 확인해�볼 사항이 있어서 각 컴포넌트에서 이어갈게요

return (
<ProductInputContainer>
<RegistrationH2>상품 소개</RegistrationH2>
<TextArea
Copy link
Collaborator

Choose a reason for hiding this comment

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

먼저 첫번째로 컴포넌트를 사용하는 의미에대해서 생각해볼 수 있을것 같아요.

Form으로 구성하셨기때문에, Form에대해서 생각을 해보겠습니다. 우리는 폼에 정보를 입력하고, 그것이 실제로 submit되기전까지 어떠한 일도 하지 않습니다! 좀더 쉽게말하자면 submit되기 전까지는 그냥 유저에게 보여주기만 하면되는거죠. 실제로 HTML에서의 Form은 이렇게동작합니다. 하지만 헌규님이 지금 작성해주신 컴포넌트를 보면 onchange마다 전역상태에 값을 저장하고있습니다. 이는 실제 Form의 동작방식과는 차이가 있을수 있는데요, 이를 통해 발생할 수 있는 문제점은 store를 구성하신 곳에서 추가로 작성할게요

<RegistrationH2>판매가격</RegistrationH2>
<Input
type="number"
min={0}
Copy link
Collaborator

Choose a reason for hiding this comment

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

min값을 정해주신것 좋습니다. 하지만 사용자가 input에 억지로 -10000이라는 값을 넣으면 어떻게 될까요?!

onKeyUp={handleTags}
onChange={(e) => setProductTag(e.target.value)}
/>
<TagContainer tags={productTags} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

태그를 여기서 Input에서 입력하고, 엔터를 누르면 tags에 저장되는 플로우인것 같아요.
그렇다면 (input에 입력하는 기준)tag라는 상태를 전역상태롤 관리할 필요가 있었을까요??

전역상태를 사용하는 이유를 생각해봐야 할 것 같아요


export const productTagsState = atom<string[]>([]);

export const productFormState = atom(
Copy link
Collaborator

Choose a reason for hiding this comment

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

상품을 등록하기위한 정보를 atom으로 구현해주시고, 그걸 get형태로 담아서 FormContainer에서 사용해주시는것 같아요.

문제가 발생하는 사례를 올려드립니다. 어떠한것이 문제점인지 충분히 파악하실 수 있을것 같아요. 해결에 정답은 없습니다.
몇가지 힌트를 드리면

  1. form을 사용하는데 전역상태 혹은 상태를 사용해야하는가? (controlled, unControlled 방식)
  2. 전역상태를 사용하면서도 해결할 방법이 있는가? (메모이제이션)
2024-11-06.07.40.36.mov

@@ -111,25 +89,17 @@ function Products() {
}, [screenWidth, productSortBy, searchKeyword, page]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 요구사항을 제가 잘 모르는것 일수도있는데, 조회조건이 바뀔때 best목록도 조회가 되어야 하는걸까요?

orderBy: PRODUCT_SORT_BY.favorite.parameter,
});
const products = useProducts({
pageSize: pageSize.normal,
pageSize: pageSize?.normal ?? 10,
orderBy: productSortBy,
searchKeyword,
page,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 useEffect에서 의존성배열이 바뀔때 refetch를 해주는 부분이 있어요. 하지만 여기서도 page, productSortBy 와 같은 값들이 변경되면서 useProducts내부 의존성배열에도 같은 값이 있기때문에 중복으로 조회하는 경우가 발생합니다.

bestProducts도 마찬가지입니다.

@kimjong95 kimjong95 merged commit ae52028 into codeit-sprint-fullstack:react-함헌규 Nov 9, 2024
1 check passed
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