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

Conversation

yeon0036
Copy link
Collaborator

@yeon0036 yeon0036 commented Dec 1, 2024

요구사항

기본

상품 등록

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

심화

상품 등록

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

주요 변경사항

  • 미션 5를 하지 못해서, 답 코드 내려받아 사용했습니다.

스크린샷

상품 등록

멘토에게

-� 제작한 부분까지만 올리겠습니다..!

  • 가격 인풋 필드에 placeholder가 아니라 0이 뜨는데 이 부분을 어떻게 해야하는지 모르겠습니다

@yeon0036 yeon0036 requested a review from 1005hoon December 1, 2024 15:48
@yeon0036 yeon0036 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Dec 1, 2024
Copy link
Collaborator

@1005hoon 1005hoon left a comment

Choose a reason for hiding this comment

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

혜연님!
정말 코드 깔끔하게 잘 작성해주고 계시네요 ㅎㅎ
이제 기능들 조금 어려운 내용이 생기면서 조금 막힐법도 한데
콤포넌트 크기도 상당히 적절하게 잘 나누어 가면서 개발하고 계십니다.

가격을 입력하는 부분에서 placeholder가 아니라 0이 뜬다고 하셨잖아요
그건 우리 기본값이 0으로 설정되어있고, input type number가 생각할때 숫자 0은 값이 이미 입력되어있다고 판단해서 placeholder를 안보여주는거에요.
이걸 어떻게 처리하면 좋을지 한번 고민해보는것도 좋겠습니다!

고생 너무 많으셨고 자세한 내용은 코드리뷰에 남겨두었으니 참고 부탁드릴게요.

Comment on lines +4 to +5
// URLSearchParams을 이용하면 파라미터 값을 자동으로 쉽게 인코딩할 수 있어요.
const query = new URLSearchParams(params).toString();
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 +11
if (!response.ok) {
throw new Error(`HTTP error: ${response.status}`);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

fetch의 Response.ok에 대해 한번 살펴보고 더 적절한 에러 핸들링을 해보면 좋겠어요!
https://developer.mozilla.org/en-US/docs/Web/API/Response/ok

Comment on lines +23 to +24
method: "POST",
body: formData,
Copy link
Collaborator

Choose a reason for hiding this comment

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

음 json형태로 전송해야하는거 같은데 formdata형태로 전송해도 괜찮은가요?
image

const body = await response.json();
return body;
} catch (error) {
console.error("Failed to fetch products:", error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

fetch가 아니라 생성에 실패했다는 로깅이 필요해요

function DropdownList({ onSortSelection }) {
return (
<div className="dropdownList">
<div className="dropdownItem" onClick={() => onSortSelection("recent")}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

클릭할 수 있는 요소인만큼 버튼으로 두면 어떨까요?

setIsSubmitting(true);
result = await createProduct(formData);
} catch (error) {
setSubmittingError(error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

submittingError를 문자열로 변경하고, 에러 메시지를 더 사용자 친화적으로 표시하는 것이 좋아요

setSubmittingError(error.message || "상품 등록 중 오류가 발생했습니다.");

Comment on lines +10 to +22
const getPageSize = () => {
const width = window.innerWidth;
if (width < 768) {
// Mobile viewport
return 4;
} else if (width < 1280) {
// Tablet viewport
return 6;
} else {
// Desktop viewport
return 10;
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 너무 좋은데요,
다만 조금 더 욕심 내어보면, getPageSize는 전달받은 width값에 따라 달라지도록 처리해주는게 이후 코드 관리차원에서 더 수월할거에요

function getPageSize(viewportWidth) {
 ...
}

Comment on lines +32 to +36
const fetchSortedData = async ({ orderBy, page, pageSize }) => {
const products = await getProducts({ orderBy, page, pageSize });
setItemList(products.list);
setTotalPageNum(Math.ceil(products.totalCount / pageSize));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

orderBy, page, pageSize 변경에 따라 getProducts 함수가 호출이 되어야 하죠?
그렇다면 이렇게 fetchSortedData를 한번 래핑해서 만들기 보다는, 바로 useEffect에서 이 로직들이 실행되도록 처리해보면 좀 더 직관적으로 보일 수 있지 않을까 생각을 해요


const handleSortSelection = (sortOption) => {
setOrderBy(sortOption);
setIsDropdownVisible(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

dropdown이 보이고 안보이고에 대한 판단 여부는 DropdownList가 지니고 있어야 할 것 같아요.
즉, DropdownList콤포넌트가 다음과 같이 만들어져 관리되면 어떨까 싶어요

function SortOptions({ value, onValueChange }) {
  const [open, setOpen] = useState(false)
  function handleOptionClick(option) { onValueChange(option) }
  return <div>
    <button onClick={() => setOpen(prev => !prev)}>{value}</button>
    { open && ... }
  </div>
}

Comment on lines +44 to +55
const handleResize = () => {
setPageSize(getPageSize());
};

// 화면 크기 변경할 때마다 pageSize를 다시 계산해 넣음
window.addEventListener("resize", handleResize);
fetchSortedData({ orderBy, page, pageSize });

// Cleanup function
return () => {
window.removeEventListener("resize", handleResize);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이걸 관리하는 커스텀 훅을 만들어보면 어떨까요 ㅎㅎ

@1005hoon 1005hoon merged commit c20238f into codeit-bootcamp-frontend:React-정혜연 Dec 4, 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