Skip to content

Conversation

@heewls
Copy link
Collaborator

@heewls heewls commented Jan 21, 2025

요구사항

기본

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

심화

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

주요 변경사항

스크린샷

멘토에게

heewls added 12 commits January 17, 2025 21:32
- tag delete handler
- submit handler(prevent submit using enter)
- when submitting successfully, navigate '/items'
- addItem validate
- submit button disabled
- tag IME problem fix
- tag list responsive css
- equal tag enterEvent error fix
- image onError
- itemsPage :  separation of responsibility
@heewls heewls requested a review from kiJu2 January 21, 2025 11:35
@heewls heewls added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jan 21, 2025
@kiJu2
Copy link
Collaborator

kiJu2 commented Jan 22, 2025

스프리트 미션 하시느라 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다. 😊

Comment on lines +23 to +31
const updateBestItems = useCallback(() => {
if (window.innerWidth <= 767) {
setShowItems(1);
} else if (window.innerWidth >= 768 && window.innerWidth <= 1199) {
setShowItems(2);
} else {
setShowItems(4);
}
}, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(제안/의견) showItems가 재사용이 많이 되는 것 같아요.

커스텀 훅을 만들어보는건 어떨까요?:

export function useResponsiveItems({ mobile = 767, tablet = 1199, items = { mobile: 1, tablet: 2, desktop: 4} }) {
  const [showItems, setShowItems] = useState(getItemsCount(window.innerWidth));

  const updateItemsCount = useCallback(() => {
    const newCount = getItemsCount(window.innerWidth);
    setShowItems(newCount);
  }, []);

  function getItemsCount(width) {
    if (width <= breakpoints.mobile) {
      return breakpoints.items.mobile;
    } else if (width <= breakpoints.tablet) {
      return breakpoints.items.tablet;
    } else {
      return breakpoints.items.desktop;
    }
  }

  useEffect(() => {
    updateItemsCount(); // 초기값 설정
    window.addEventListener("resize", updateItemsCount);

    return () => {
      window.removeEventListener("resize", updateItemsCount); // 이벤트 클린업
    };
  }, [updateItemsCount]);

  return showItems;
}

그리고 다음과 같이 사용해볼 수 있어요:

  const showItems = useResponsiveItems({
    mobile: 767,
    tablet: 1199,
    items: {
      mobile: 1,
      tablet: 2,
      desktop: 4,
    },
  });

Copy link
Collaborator

Choose a reason for hiding this comment

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

디바운싱/쓰로틀링도 적용해볼 수 있어요:

  useEffect(() => {
    updateItemsCount(); // 초기값 설정
    window.addEventListener("resize", updateItemsCount);

    return () => {
      window.removeEventListener("resize", updateItemsCount); // 이벤트 클린업
    };
  }, [updateItemsCount]);

해당 이벤트에 console.log를 호출해보면 리사이징이 될 때마다 정말정말 많은 호출하는 것을 볼 수 있을거예요 !
그만큼 성능에 좋지 못하다는 이야기겠지요?
따라서, 프론트엔드 개발자들은 이렇게 잦은 이벤트가 발생할 때(리사이징, 스크롤, 타이핑 등) 디바운싱/쓰로틀링을 통하여 최적화를 시키곤 합니다.

쓰로틀링(Throttling): 일정 시간 동안 하나의 함수만 호출되도록 하는 기법입니다. 예를 들어, 사용자가 스크롤을 할 때, 매번 이벤트를 처리하지 않고 일정 간격으로 한 번만 처리하게 합니다. 이를 통해 성능을 향상시킬 수 있습니다.

디바운싱(Debouncing): 여러 번 발생하는 이벤트 중 마지막 이벤트가 발생한 후 일정 시간이 지난 다음에 한 번만 실행되도록 하는 기법입니다. 예를 들어, 사용자가 검색어를 입력할 때, 입력이 끝난 후 일정 시간 동안 추가 입력이 없으면 검색 요청을 보냅니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

예를 들어서 다음과 같이 최적화를 시킬 수 있습니다 !:

function debounce(func, wait) {
  let timeout;
  return function executedFunction(...args) {
    const later = () => {
      clearTimeout(timeout);
      func(...args);
    };
    clearTimeout(timeout);
    timeout = setTimeout(later, wait);
  };
}

function Container() {
  const handleResize = () => {
    console.log('Window resized');
  };

  useEffect(() => {
    const debouncedHandleResize = debounce(handleResize, 300);
    window.addEventListener('resize', debouncedHandleResize);
    return () => {
      window.removeEventListener('resize', debouncedHandleResize);
    };
  }, []);

  // ... Some code
}

이렇게 하면 연속된 이벤트가 끝난 후 0.3초마다 호출하게 되어 기존보다 훨씬 최적화된 기능이 될 수 있습니다 !

Comment on lines +1 to +5
export const isValidAddItem = (values) => {
const isValid = values.name.trim() && values.description.trim() && values.price > 0 && values.tags.length > 0;

return isValid;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

크으 유효성 검사 관심사 분리까지 👍

import plus from "../../assets/icons/plus.svg";
import { useState, useRef } from "react";

export default function FileInput({ lable, images, setValues }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오타 발견 ! 🔎

Suggested change
export default function FileInput({ lable, images, setValues }) {
export default function FileInput({ label, images, setValues }) {

Comment on lines +78 to +114
const INPUT = [
{
label: "상품명",
name: "name",
type: "text",
placeholder: "상품명을 입력해주세요",
value: values.name,
onChange: handleInputChange,
},
{
label: "상품 소개",
name: "description",
type: "text",
placeholder: "상품 소개를 입력해주세요",
style: { height: "282px" },
value: values.description,
isTextarea: true,
onChange: handleInputChange,
},
{
label: "판매가격",
name: "price",
type: "number",
placeholder: "판매 가격을 입력해주세요",
value: values.price ? values.price : "",
onChange: handleInputChange,
},
{
label: "태그",
name: "tags",
type: "text",
placeholder: "태그를 입력해주세요",
value: tag,
onChange: (e) => setTag(e.target.value),
onKeyDown: handleTagChange,
},
];
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 +145 to +147
{values.tags.map((t, idx) => (
<Tag key={idx} tag={t} onClick={() => handleTagDelete(t)} />
))}
Copy link
Collaborator

Choose a reason for hiding this comment

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

태그 배열의 index로 처리를 하게 되면 예기치 못한 오류가 발생할 수도 있지 않을까 사료됩니다.

Suggested change
{values.tags.map((t, idx) => (
<Tag key={idx} tag={t} onClick={() => handleTagDelete(t)} />
))}
{values.tags.map((t, idx) => (
<Tag key={t.name} tag={t} onClick={() => handleTagDelete(t)} />
))}

key는 "고유한 값"으로 지정하는게 좋습니다 ! delete로 인해 배열 내 요소가 바뀌게 되는 경우 과거 데이터를 보여주는 등의 예기치 못한 상황이 발생할 수도 있지 않을까 추측해봅니다.(직접 실행해본 것은 아니기에 아닐 수도 있습니다 😉)

가장 권장드리고 싶은건 태그의 고유 값인 name을 지정하는게 어떨까 싶어서 제안드려요 ! 같은 name의 태그가 중복으로 지정되는 것은 태그의 일반적인 사용자 경험이 아니니까요 !

관련 자료

@kiJu2
Copy link
Collaborator

kiJu2 commented Jan 22, 2025

수고하셨습니다 희진님 !
스프린트 미션 6을 가장 먼저 완료하셨네요 ㅎㅎㅎ
희진님의 역량은 아마 기초프로젝트에서도 팀원분들께 큰 힘이 되시지 않을까 싶어요.
나중에 스프린트 미션 수행하실 때 제가 제안드린 커스텀 훅 만드시는 것도 한 번 고려해보세요 !
전반적으로 리액트를 리액트답게 잘 작성하셔서 리뷰하는 데에 정말 수월했습니다 😊

스프린트 미션 수행하시느라 정말 수고 많으셨습니다 🙇‍♂️🙇‍♂️

@kiJu2 kiJu2 merged commit ba2bf71 into codeit-bootcamp-frontend:React-김희진 Jan 22, 2025
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