Skip to content

[임재은] Sprint6 #233

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

Open
wants to merge 3 commits into
base: React-임재은
Choose a base branch
from

Conversation

Lim-JaeEun
Copy link
Collaborator

요구사항

기본

  • 상품 등록 페이지 주소는 “/additem” 입니다.
  • 페이지 주소가 “/additem” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
  • 상품 이미지는 최대 한개 업로드가 가능합니다.
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • API를 통한 상품 등록은 추후 미션에서 적용합니다.

심화

  • 이미지 안의 X 버튼을 누르면 이미지가 삭제됩니다.
  • 추가된 태그 안의 X 버튼을 누르면 해당 태그는 삭제됩니다.

스크린샷

  • localhost_5173_addItem (2)
  • localhost_5173_addItem (1)
  • localhost_5173_addItem

멘토에게

  • 미션5코드리뷰 참고하여 리팩토링과 미션6를 함께 진행했습니다 !
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@Lim-JaeEun Lim-JaeEun added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jul 17, 2025
@addiescode-sj addiescode-sj self-requested a review July 18, 2025 06:55
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

이전 미션 리팩토링을 통해 불필요한 state를 안쓰는 방법에 대해 이해를 확실히 하신것같아요.
코드 퀄리티도 너무 좋아졌고요 👍 굳굳!

조금만 더 다듬어보면 멋진 결과물 나올것 같네요!

주요 리뷰 포인트

  • 포맷팅 개선 (import 문과 컴포넌트 시작 구문 사이에 공백 넣어주기)
  • 조건식 상수화해서 사용하기 (리턴문 가독성 높이기)
  • lodash 패키지 사용에 대한 피드백, debounce + transition 조합으로 UX 개선하기
  • deps list가 많은 useEffect의 경우, 방어 코드 추가해주기

Comment on lines +7 to +8
import SearchInput from './SearchInput';
function AllProductsSection({
Copy link
Collaborator

Choose a reason for hiding this comment

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

import문과 컴포넌트 시작 구문 사이에 공백 한칸 띄워주세요! :)

Comment on lines +3 to +4
import ProductCard from './ProductCard';
function BestProductsSection({ bestItems }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서도 포맷팅 고쳐야겠네요 :)

Comment on lines +9 to +27
// 정렬 옵션 정의
const [isOpen, setIsOpen] = useState(false);
// 현재 선택된 옵션의 label을 찾기
const option = [
{ value: 'recent', label: '최신순' },
{ value: 'favorite', label: '좋아요순' },
];

const selectedOption = option.find(opt => opt.value === value)?.label;
// 옵션 클릭 시 처리
const handleOptionClick = value => {
setOrder(value);
setIsOpen(false);
};

// 드롭다운 열고 닫기 토글
const toggleDropdown = () => {
setIsOpen(!isOpen);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳! 딱 필요한 state만 쓰셨고, 주석도 꼼꼼히 달아주셨네요 👍

return (
<NavWrap>
<Logo>
<a href='/'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서는 Link 컴포넌트가 아닌 a 태그를 사용한 이유가 있을까요~?

<ItemImgWrapper>
<ItemImage
onError={imageErrLoadHandler}
src={!imgSrc || errorState ? tempImage : imgSrc}
Copy link
Collaborator

Choose a reason for hiding this comment

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

  const isImageError = !imgSrc || errorState;

조건식을 상수화해서 사용하시면 좋을것같아요!
리턴문은 항상 가독성을 중시하고, 깔끔하게 정리해 작성하는 습관을 들여봅시다! :)

Comment on lines +7 to +9
const onSearchItems = debounce(e => {
handlerSearchItems(e.target.value);
}, 500);
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 throttle과 debounce 두가지 함수를 위해 lodash를 쓰고 계신데,
lodash 자체가 매우 무거운 패키지 사이즈를 가지고있기때문에 다방면에서 활용될게 아니라면 되도록 사용하지않는것이 좋고, 필요하더라도 최적화된 버전인 lodash-es를 채택해 쓰시는게 좋습니다.

그리고 디바운스 처리 시 useTransition hook을 같이 활용하면 UI가 더 매끄럽게 반응하 게끔 만들수있어요! :)

lodash import를 제거하고, 커스텀 디바운스 로직을 hook으로 만들어볼까요?

function useDebounce(fn, delay) {
  const timeoutRef = useRef();
  const callback = useCallback(
    (...args) => {
      if (timeoutRef.current) clearTimeout(timeoutRef.current);
      timeoutRef.current = setTimeout(() => {
        fn(...args);
      }, delay);
    },
    [fn, delay],
  );
  return callback;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

그 다음, useTransition hook을 사용해 handlerSearchItems가 트랜지션 내에서 실행되도록 개선해봅시다:

  const [isPending, startTransition] = useTransition();
  const debouncedSearch = useDebounce(value => {
    startTransition(() => {
      handlerSearchItems(value);
    });
  }, 500);

해당 로직에서 useTransition의 역할

일부 업데이트(비동기 작업)를 트랜지션으로 처리할 수 있게 해줍니다.

즉, 사용자의 입력에 즉각적으로 반응해야 하는 업데이트(입력창에 글씨가 바로 보이는 것)와, 조금 느려도 되는 업데이트(검색 결과 목록 갱신)의 우선순위를 재조정하고 작업을 분리하여, 시간이 오래 걸릴 수 있는 작업에 대한 우선순위가 사용자 입력 처리보다 낮아지게되니, 입력창이 버벅이지않고 즉각적으로 반응하게되어 좀더 매끄럽고 효율적으로 처리될 수 있게 만들어줍니다 :)

isPending의 경우에는, 트랜지션이 진행 중일 때 true가 되는 값으로 이 값을 활용해 로딩 스피너를 쓰는 등 사용자에게 UI로 상태를 표시할 수 있습니다 :)

이 값을 활용해 이런식으로 처리해주면 좀더 사용자에게 각 처리 상태에 대한 피드백을 올바로 줄 수 있겠죠?

{isPending && <PendingMessage>검색 중...</PendingMessage>}

import { throttle } from "lodash";
import { useEffect, useState } from 'react';

import { throttle } from 'lodash';
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서도 lodash import를 없애고 커스텀 throttle 로직을 구현해봅시다!

Comment on lines +56 to +71
// 검색어 / 정렬 기준 변경 시 전체 상품 다시 불러오기
useEffect(() => {
if (!allPageSize) return;

const loadAllProducts = async () => {
const { list } = await fetchProducts({
page: currentPage,
pageSize: allPageSize,
orderBy: order,
keyword: searchValue,
});
setAllItems(list);
};

loadAllProducts();
}, [order, searchValue, allPageSize, currentPage]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 useEffect deps list가 조금 많아보이네요!
불필요한 fetch가 발생하지않도록 조건을 추가하는 식으로 방어 코드를 작성해주면 어떨까요?

useEffect(() => {
  if (!allPageSize) return;

  // 이전 파라미터 저장용 ref
  const prevParams = useRef({});

  const params = {
    page: currentPage,
    pageSize: allPageSize,
    orderBy: order,
    keyword: searchValue,
  };

  // 파라미터가 이전과 동일하면 fetch하지 않음
  const isSameParams = JSON.stringify(prevParams.current) === JSON.stringify(params);
  if (isSameParams) return;

  prevParams.current = params;

  const loadAllProducts = async () => {
    const { list } = await fetchProducts(params);
    setAllItems(list);
  };

  loadAllProducts();
}, [order, searchValue, allPageSize, currentPage]);

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