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

Conversation

kimsayhi
Copy link
Collaborator

@kimsayhi kimsayhi commented Jun 28, 2024

요구사항

기본 요구사항

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

심화 요구사항

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

멘토에게

  • 실수로 이미지를 여러개 올릴 수 있도록 구현했습니다.
  • 여전히 상위 컴포넌트에 state와 함수들이 몰리고있습니다.

미리보기 페이지 입니다

- 마이그레이션 미구현
- 검색에 onChange때문에 랜더링 많이 되는 문제
- 뷰포트 크기 잘 인식하지 못하는 문제
@kimsayhi kimsayhi requested a review from lisarnjs June 28, 2024 16:09
@kimsayhi kimsayhi added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Jun 28, 2024
@kimsayhi kimsayhi removed the 미완성🫠 죄송합니다.. label Jul 1, 2024
Copy link
Collaborator

@lisarnjs lisarnjs left a comment

Choose a reason for hiding this comment

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

세환님 좀 늦어서 미안해요 ㅜㅜ
아직 파트2인데 이렇게 잘해도 되는건가!! 너무 잘해요 세환님 👍
곧 프로젝트 시작하니까 그때도 실력을 뽐내주세용 ㅎㅎ
이번주도 화이팅!

Copy link
Collaborator

Choose a reason for hiding this comment

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

앜ㅋㅋㅋㅋ 이거 뭐야 너무 귀엽잖아요 ㅜㅜㅜㅜㅋㅋㅋㅋㅋ !!

name="images"
value=""
/>
{!images.length ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

!image.length 보다는 image?.length > 0 가 더 직관적이고 옵셔널 체이닝으로 undefined 혹은 null 체크까지 할 수 있어서 더 좋을 것 같아용!

<div
className="preview-wrapper"
key={Date.now() + i * 13}
onClick={onDelete}
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 button에 적용된 클릭 이벤트가 onDelete로 이 div까지 onDelete이벤트를 걸어줄 필요는 없어보이는데 혹시 여기에도 걸어준 이유가 있으실까용?

const josa = `${label.을를}`;
return (
<>
{name !== "description" ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 삼항연산자를 && 연산자로 업그레이드 시킬 수 있을까요!? (할 수 있어요!!)


function AddItemMain() {
const [tagValue, setTagValue] = useState("");
const [values, setValues] = useState({
Copy link
Collaborator

Choose a reason for hiding this comment

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

위에서 초기값들을 상수로 만들어주셨으니 여기서도 useState(INIT_VALUE) 로 사용하실 수 있을 것 같아요 👍

const handleChange = (e) => {
const { name, value } = e.target;
switch (name) {
case "tags":
Copy link
Collaborator

Choose a reason for hiding this comment

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

switch 문도 좋지만 케이스별로 함수를 따로 만들어주는 것도 좋은 방법일 것 같아요!

setPageSize(4);
}
}, [sizeName]);
useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

useEffect(() => {
    loadItems({ page: pageNum, pageSize, orderBy, keyWord: keyWord });
    setPageNum(1);
  }, [orderBy, pageSize, keyWord]);

useState에 각각 초기값을 넣어줬다면 밑에 코드도 한번 더 실행될거라 빈배열로 한번더 useEffect를 만들어줄 필요가 없을 것 같긴합니다! 한번 지워보시는 것도 테스트해보세요!

setPageNum(1);
}, [orderBy, pageSize, keyWord]);
useEffect(() => {
if (!didMount) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오잉 요 didMount라는 state는 무엇을 위한 건지 잘 모르겠어용..! 굳이 필요하지 않다면 없애는 쪽으로 !

/>
<PageArrowButton
imgSrc={ArrowRight}
direction="right"
Copy link
Collaborator

Choose a reason for hiding this comment

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

direction으로 만들어주는 것도 좋지만
함수 자체를 나눠주시는 것도 좋아여
예를 들면 onClickLeft onClickRight 요런식으로!?

@lisarnjs lisarnjs merged commit c91abd0 into codeit-bootcamp-frontend:React-김세환 Jul 2, 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