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

[이형준]sprint7 #209

Conversation

leehj322
Copy link
Collaborator

@leehj322 leehj322 commented Jul 5, 2024

사이트 배포 링크

요구사항

기본 요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • React를 사용합니다.

체크리스트 (기본)

상품상세

  • 상품 상세 페이지 주소는 "/items/{productId}" 입니다.
  • response로 받은 아래의 데이터로 화면을 구현합니다.
    => favoriteCount : 하트 개수
    => images : 상품 이미지
    => tags : 상품 태그
    => name : 상품 이름
    => description : 상품 설명
  • 목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 "/items"으로 이동합니다.

상품 문의 댓글

  • 문의하기에 내용을 입력하면 등록 버튼의 색상은 "3692FF"로 변합니다.
  • response로 받은 아래의 데이터로 화면을 구현합니다.
    => image : 작성자 이미지
    => nickname : 작성자 닉네임
    => content : 작성자가 남긴 문구
    => description : 상품 설명
    => updatedAt : 문의글 마지막 업데이트 시간

스프린트 미션6 누락 사항

  • 파일을 제외한 Input이 모두 입력된 경우 등록 버튼을 활성화 합니다.

피드백 반영

  • 디렉토리 정리하기 + Items.js 파일 변경
  • pagination을 SalesProductList component 내부로 이동 → styled-components로 전환하면서 할 예정

추가 예정 사항

  • styled component로 리팩토링
  • 로그인, 회원가입 페이지 구현
  • ItemForm.js 파일 컴포넌트 모듈화 하기
  • SalesProductList.js 의 필요없는 return 값 하나로 정리하기 → styled-components로 전환하면서 할 예정

주요 변경사항

  • sprint mission 7 완료
  • 이전 sprint mission 리팩토링 및 마이그레이션 진행 중

멘토에게

  • 변경사항이 많아서 개인레포에서 직접 보시는게 나을 수도 있을거 같아 링크 남깁니다! 개인레포링크
  • 기능 구현까지만 완료했고 아직 더 수정 중입니다! 계속 푸시하겠습니다.
  • pagination 컴포넌트 이동을 아직 못했습니다 시간되는대로 최대한 빠르게 적용 해 보겠습니다.
  • 디렉토리 관련해서 언급해주셔서 최대한 정리하긴 했는데 styled component를 쓰는거랑 안쓰는 것이 섞여서 일단은 좀 뒤죽박죽입니다.
  • styled component를 처음 사용하는데 이런식으로 사용하는게 맞을까요?
  • api 요청을 어디서 실행하는 것이 좋은지 잘 모르겠습니다. 찾아보니까 상위 컴포넌트에서 하는것을 추천하던데 이유가 있나요?
  • 이번에 작성한 부분에 한해서라도 추상화 가능한 부분이나 가독성을 높일 수 있는 부분이 있다면 짚어주시면 감사하겠습니다.. 코드를 잘 작성해서 협업에 신경을 쓰고 싶습니다.

@leehj322 leehj322 requested a review from arthurkimdev July 5, 2024 11:00
@leehj322 leehj322 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Jul 5, 2024
@leehj322 leehj322 removed the 미완성🫠 죄송합니다.. label Jul 6, 2024
Copy link
Collaborator

@arthurkimdev arthurkimdev left a comment

Choose a reason for hiding this comment

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

마지막 과제까지 수고하셨습니다! 🙏

Comment on lines +40 to +56
export async function getProductComments({ productId, limit }) {
const query = `limit=${limit}`;
const url = `${BASE_URL}/products/${productId}/comments?${query}`;

try {
const response = await fetch(url);
if (!response.ok) {
throw new Error(
`상품댓글정보를 불러오는데 실패했습니다. Status: ${response.status}`
);
}
const body = await response.json();
return body;
} catch (error) {
throw new Error(
`상품댓글정보 요청에 실패했습니다. Error: ${error.message}`
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

깔끔하게 잘하셨어요! 👍 파라미터도 지금처럼 객체로 받으면 getProductComments 함수를 사용할 때 가독성이 올라가고, 확장성에 더 좋아요.

className={styles["file-input"]}
id="file"
type="file"
accept=".png, .jepg, .jpg"
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래처럼 jpeg 바꿔주세요~

accept=".png, .jpeg, .jpg"

Comment on lines +63 to +74
<div
className={styles["imgfile-preview"]}
style={{
backgroundImage: `url(${previewImg})`,
display: isFileSelected ? "block" : "none",
}}
>
<div
className={`${styles["imgfile-delete-btn"]} ${styles["inactive"]}`}
onClick={handleFileDelete}
/>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 CSS로 처리할 수도 있고, 조건부 랜더링으로 변경할 수도 있어요.

{isFileSelected ? (
          <div
            className={styles["imgfile-preview"]}
            style={{ backgroundImage: `url(${previewImg})` }}
          >
            <div
              className={`${styles["imgfile-delete-btn"]}`}
              onClick={handleFileDelete}
            />
          </div>
        ): null}

Comment on lines +10 to +11
const [postImg, setPostImg] = useState(null);
const [previewImg, setPreviewImg] = useState(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 상태는 파일 업로드 된 상태를 만약 다른 컴포넌트에도 공유해야되거나, ImgFileInput 컴포넌트 재사용성을 올리기 위해선 상태값을 상위로 올려서 props로 전달받는 방식으로 변경하는것도 고려하면 좋겠어요.

function ImgFileInput({ postImg, setPostImg, previewImg, setPreviewImg }) {

Comment on lines +65 to +67
useEffect(() => {
comment ? setIsCommentValid(true) : setIsCommentValid(false);
}, [comment]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

가독성이 아래 방법이 더 좋을 것 같아요.

  const [comment, setComment] = useState('');
  ...
  useEffect(() => {
    setIsCommentValid(comment.trim().length > 0);
  }, [comment]);

<CommentInputContainer>
<CommentTitle>문의하기</CommentTitle>
<CommentInput
as="textarea"
Copy link
Collaborator

Choose a reason for hiding this comment

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

as 문법 사용 굳! 👍

Comment on lines +3 to +17
const GrayBgInput = styled.input`
background-color: var(--cool-gray-100);
color: var(--cool-gray-400);
border: none;

&:focus {
border: solid 2px var(--cool-gray-400);
outline: none;
}

&::placeholder {
color: var(--cool-gray-400);
}
`;

Copy link
Collaborator

Choose a reason for hiding this comment

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

shared 부분 공통으로 뺀 부분 좋아요.

Comment on lines +7 to +19
const CommentsContainer = styled.ol`
width: 1200px;
margin: 0 auto;

@media screen and (max-width: 1199px) {
width: auto;
margin: 0 24px;
}

@media screen and (max-width: 767px) {
margin: 0 16px;
}
`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

styled-component에서 반응형 처리할 때 아래 처럼 많이 사용해요.

  1. styles 폴더 내 device.js 파일 생성
  2. 아래처럼 변수로 mobile,tablet, desktop 등 디바이스별 값 선언
const size = {
  mobile: "767px",
  tablet: "1199px",
};

export const device = {
  mobile: `(max-width: ${size.mobile})`,
  tablet: `(max-width: ${size.tablet})`,
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

이후 실제 적용하면 아래처럼 변경할 수 있어요.

const CommentsContainer = styled.ol`
  width: 1200px;
  margin: 0 auto;
  @media screen and ${device.tablet} {
    width: auto;
    margin: 0 24px;
  }
  @media screen and ${device.mobile} {
    margin: 0 16px;
  }
`;

Comment on lines +182 to +213
function ProductDetails({ details }) {
const { name, description, price, tags, favoriteCount, isFavorite, images } =
details;

return (
<DetailsContainer>
<ProductImg src={images}></ProductImg>
<DetailsTextContainer>
<KebabMenuBtn src={kebabMenuImg} alt="메뉴 더보기" />
<DetailsMainContainer>
<ProductName>{name}</ProductName>
<ProductPrice>{price.toLocaleString("ko-KR")}원</ProductPrice>
</DetailsMainContainer>
<DetailsSubContainer>
<SubDetailTitle>상품 소개</SubDetailTitle>
<ProductDescription>{description}</ProductDescription>
<SubDetailTitle>상품 태그</SubDetailTitle>
<ProductTagsContainer>
{tags.map((tag) => {
const hashedTag = "# " + tag;
return <ProductTag key={tag}>{hashedTag}</ProductTag>;
})}
</ProductTagsContainer>
<LikeCountContainer>
<HeartImg src={isFavorite ? heartActive : heartInActive}></HeartImg>
<LikeCount>{favoriteCount}</LikeCount>
</LikeCountContainer>
</DetailsSubContainer>
</DetailsTextContainer>
</DetailsContainer>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일처럼 스타일링 관련된 컴포넌트가 많아서, 중요한 컴포넌트에 관심을 가지기 어려운 상태가 있을 수 있어요.
그렇다면, 파일을 나누는 방법이 있어요.

ㄴ Market
    ㄴ ProductDetails
        ㄴ ProductDetails.js
        ㄴ ProductDetails.styled.js

Copy link
Collaborator

Choose a reason for hiding this comment

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

import React from "react";
import {
  CommentsContainer,
  NoCommentImg,
  CommentContainer,
  CommentContent,
  CommentInfoContainer,
  CommentUserPfp,
  CommentInfoTextContainer,
  CommentUserNickname,
  CommentElapseTime,
} from "./ProductDetails.styled";

function ProductDetailComments({ comments }) {
  return (
     ...
  )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

혹시 말씀하시는 것이 style만 하는 style components 만 다빼서 ProductDetails.styled.js에 넣고 현제 ProductDetails 컴포넌트의 return 값은 지금 이대로 쓰라는 뜻인가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@leehj322 네 맞습니다!

Comment on lines +66 to +70
useEffect(() => {
const productId = params.itemId;
handleProductDetailLoad({ productId });
handleProductComments({ productId });
}, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

의존성 디펜던시에 params.itemId 값이 추가되야해요~

  useEffect(() => {
    const productId = params.itemId;
    handleProductDetailLoad({ productId });
    handleProductComments({ productId });
  }, [params.itemId]);

@arthurkimdev
Copy link
Collaborator

styled component를 처음 사용하는데 이런식으로 사용하는게 맞을까요?

-> 관련해서 리뷰 남겨드렸습니다!

@arthurkimdev
Copy link
Collaborator

api 요청을 어디서 실행하는 것이 좋은지 잘 모르겠습니다. 찾아보니까 상위 컴포넌트에서 하는것을 추천하던데 이유가 있나요?

-> 하위에서 할 경우, 값 공유가 어렵기 때문이에요. 리액트는 하위에서 상위로 값을 보낼 수 있는 방법이 없거든요. 하지만, 상태관리 라이브러리를 사용하거나 react-query를 사용하게되면 반드시 상위에서 요청을 하지 않아도 돼요. 설계에 따라 변경 가능한 부분이고 절대적인 정답은 없습니다.

@arthurkimdev
Copy link
Collaborator

이번에 작성한 부분에 한해서라도 추상화 가능한 부분이나 가독성을 높일 수 있는 부분이 있다면 짚어주시면 감사하겠습니다.. 코드를 잘 작성해서 협업에 신경을 쓰고 싶습니다.

-> 가독성 부분에서 코멘트 남겨드렸습니다! 전반적으로 작성잘하셨어요! 노력이 느껴집니다..👍

@arthurkimdev arthurkimdev merged commit 93b13c4 into codeit-bootcamp-frontend:React-이형준 Jul 8, 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