Skip to content

[문지영] sprint7 #228

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

Conversation

sinyroom
Copy link
Collaborator

@sinyroom sinyroom commented Jul 10, 2025

요구사항

기본

  • 상품 상세

  • 상품 상세 페이지 주소는 "/items/{productId}" 입니다.

  • response 로 받은 아래의 데이터로 화면을 구현합니다.
    => favoriteCount : 하트 개수
    => images : 상품 이미지
    => tags : 상품태그
    => name : 상품 이름
    => description : 상품 설명

  • 목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 "/items" 으로 이동합니다

  • 상품 문의 댓글

  • 문의하기에 내용을 입력하면 등록 버튼의 색상은 "3692FF"로 변합니다.

  • response 로 받은 아래의 데이터로 화면을 구현합니다
    => image : 작성자 이미지
    => nickname : 작성자 닉네임
    => content : 작성자가 남긴 문구
    => description : 상품 설명
    => updatedAt : 문의글 마지막 업데이트 시간

심화

  • 모든 버튼에 자유롭게 Hover효과를 적용하세요.

주요 변경사항

  • 미션7 기본 요구사항에 맞게 페이지를 구현했습니다.
  • 코멘트 부분 페이지네이션은 /items에서 사용했던 페이지네이션을 사용해 구현했습니다.

스크린샷

모바일형
테블릿형
데스크탑

멘토에게

  • 제품 상세 페이지의 컴포넌트를 크게 두가지로 나누게 되었습니다. 만들다보니 렌더링되는 리턴 부분이 복잡해보여서 더 작게 나누고 싶었는데 어떤걸 분리시켜야 좋을지 감이 잡히질 않아 이 부분에서 코멘트 주시면 리팩토링 해보고 싶습니다.
  • 댓글 등록까지 구현해야하는 줄 알고 로그인 상태를 유지시키려고 했었다가, 이후 미션에서 진행하는 단계인걸 깨닫고 그 부분에서는 마무리하진 못했습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@sinyroom sinyroom requested a review from addiescode-sj July 10, 2025 06:49
@sinyroom sinyroom added the 순한맛🐑 마음이 많이 여립니다.. label Jul 10, 2025
@sinyroom sinyroom changed the base branch from main to React-문지영 July 10, 2025 06:53
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.

수고하셨습니다!
지영님 점점 코드 퀄리티가 좋아지는게 보이네요 :) 👍
제가 드리는 리팩토링 포인트 보시고, 더 깊이 공부해보시면 도움 되실거예요! :)

주요 리뷰 포인트

  • api 연결 시점 생각해보며 컴포넌트 구조 바꾸기
  • 더 많은 요구사항을 품게 될 경우를 대비해 합성 컴포넌트 기반으로 설계 바꿔보기
  • useEffect 내부에서 사용되는 함수 메모리 누수 줄이는 방법
  • 리턴문 가독성 높이기
  • 공통적으로 반복 처리되는 패턴을 묶어 재사용 가능한 함수로 분리하기 (커스텀 훅)

Comment on lines +8 to +19
function CommentEditList({ onEditClick, commentId, onDeleteClick }) {
const [isOpen, setIsOpen] = useState(false);

const handleEdit = () => {
onEditClick(commentId);
setIsOpen(false);
};

const handleDelete = () => {
onDeleteClick(commentId);
setIsOpen(false);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금은 comments 배열을 부모 컴포넌트에서 state로 관리하고있고, 수정/삭제 될때마다 해당 배열을 업데이트해야하다보니 이런 구조를 쓰는게 맞지만, 추후에 관심사 분리 관점에서 api 연결을 통해 수정/삭제할때도 여전히 이 구조를 유지하는지 맞을지에 대해서는 고민해볼 필요가 있겠네요! :)

해당 컴포넌트에서 담당하는 일을

  • 수정 삭제 옵션을 isOpen state에 따라 보여주는것
  • 수정, 삭제 완료 시 기존 state 업데이트와 함께 UI 갱신

이렇게 api를 연결하는 시점에선 두가지를 담당할수있도록 리팩토링해봅시다! :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 이 컴포넌트가 프로그램이 복잡해지고 규모가 커지면서 더 많은 요구사항을 품게 된다면 어떻게 될까요?
props가 늘어날테고, 분기도 늘어날테고, 전체적인 복잡도가 크게 증가하겠죠?
하나의 컴포넌트에서 너무 많은 요구사항을 처리하려하면 이런 일이 쉽게 발생할 수 있습니다.
React는 상속보다 조합에 특화되어있습니다.

컴포넌트를 기능과 역할에 맞게 잘게 쪼게 합성(조합)하는 방식으로 바꿔보는건 어떨까요?
예시를 보여드릴게요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • 합성 컴포넌트 패턴을 사용한 예시
import { useState } from "react";
import styled from "styled-components";
import searchIcon from "../../assets/images/icons/ic_search.svg";
import closeEye from "../../assets/images/icons/ic_visibility_off.png";
import openEye from "../../assets/images/icons/ic_visibility_on.png";
import { applyFontStyles } from "../../styles/mixins";
import { ColorTypes, FontTypes } from "../../styles/theme";

// Base Input Container
const InputContainer = ({ children, label, error, id }) => (
  <StyledContainer>
    {label && <label htmlFor={id}>{label}</label>}
    {children}
    {error && <ErrorMessage>{error}</ErrorMessage>}
  </StyledContainer>
);

// Main Input Component
function Input({ children, label, error, id, ...props }) {
  return (
    <InputContainer label={label} error={error} id={id} {...props}>
      {children}
    </InputContainer>
  );
}

// Text Input
Input.Text = ({
  id,
  type = "text",
  placeholder,
  value,
  onChange,
  onBlur,
  autoComplete,
  ...props
}) => (
  <InputWrapper>
    <input
      id={id}
      type={type}
      placeholder={placeholder}
      value={value}
      onChange={onChange}
      onBlur={onBlur}
      autoComplete={autoComplete}
      {...props}
    />
  </InputWrapper>
);

// Password Input
Input.Password = ({ id, placeholder, value, onChange, onBlur, ...props }) => {
  const [isPwVisible, setIsPwVisible] = useState(false);

  return (
    <InputWrapper $hasPasswordToggle={true}>
      <input
        id={id}
        type={isPwVisible ? "text" : "password"}
        placeholder={placeholder}
        value={value}
        onChange={onChange}
        onBlur={onBlur}
        autoComplete="current-password"
        {...props}
      />
      <EyeButton
        type="button"
        onClick={() => setIsPwVisible((prev) => !prev)}
        tabIndex={-1}
      >
        <img
          src={isPwVisible ? openEye : closeEye}
          alt={isPwVisible ? "비밀번호 숨기기" : "비밀번호 보기"}
        />
      </EyeButton>
    </InputWrapper>
  );
};

// Email Input
Input.Email = ({ id, placeholder, value, onChange, onBlur, ...props }) => (
  <Input.Text
    id={id}
    type="email"
    placeholder={placeholder}
    value={value}
    onChange={onChange}
    onBlur={onBlur}
    autoComplete="username"
    {...props}
  />
);

// Nickname Input
Input.Nickname = ({ id, placeholder, value, onChange, onBlur, ...props }) => (
  <Input.Text
    id={id}
    type="text"
    placeholder={placeholder}
    value={value}
    onChange={onChange}
    onBlur={onBlur}
    autoComplete="nickname"
    {...props}
  />
);

// Search Input
Input.Search = ({
  id,
  placeholder,
  value,
  onChange,
  onBlur,
  onSearch,
  ...props
}) => (
  <InputWrapper $hasSearchIcon={true}>
    <input
      id={id}
      type="text"
      placeholder={placeholder}
      value={value}
      onChange={onChange}
      onBlur={onBlur}
      {...props}
    />
    <SearchButton type="button" onClick={onSearch} tabIndex={-1}>
      <img src={searchIcon} alt="검색" />
    </SearchButton>
  </InputWrapper>
);

// Number Input
Input.Number = ({
  id,
  placeholder,
  value,
  onChange,
  onBlur,
  min,
  max,
  step,
  ...props
}) => (
  <Input.Text
    id={id}
    type="number"
    placeholder={placeholder}
    value={value}
    onChange={onChange}
    onBlur={onBlur}
    min={min}
    max={max}
    step={step}
    {...props}
  />
);

// TextArea Input
Input.TextArea = ({
  id,
  placeholder,
  value,
  onChange,
  rows = 10,
  ...props
}) => (
  <InputWrapper>
    <textarea
      id={id}
      placeholder={placeholder}
      value={value}
      onChange={onChange}
      rows={rows}
      {...props}
    />
  </InputWrapper>
);

// Error Message Component
Input.ErrorMessage = ({ children }) => <StyledError>{children}</StyledError>;

// Styled Components
const StyledContainer = styled.div`
  display: flex;
  flex-direction: column;
  gap: 8px;
  width: 100%;
`;

const InputWrapper = styled.div`
  position: relative;
  width: 100%;

  input {
    width: 100%;
    padding-right: ${({ $hasPasswordToggle, $hasSearchIcon }) =>
      $hasPasswordToggle ? "30px" : $hasSearchIcon ? "40px" : "0"};
    border: 1px solid
      ${({ $hasError, theme }) =>
        $hasError
          ? theme.colors[ColorTypes.ERROR]
          : theme.colors[ColorTypes.SECONDARY_GRAY_100]};
  }

  textarea {
    width: 100%;
    border: 1px solid
      ${({ theme }) => theme.colors[ColorTypes.SECONDARY_GRAY_100]};
    resize: vertical;
  }
`;

const EyeButton = styled.button`
  position: absolute;
  right: 10px;
  top: 50%;
  transform: translateY(-50%);
  background: none;
  border: none;
  padding: 0;
  cursor: pointer;
  display: flex;
  align-items: center;

  img {
    width: 20px;
    height: 18px;
    margin-right: 18px;
  }
`;

const SearchButton = styled.button`
  position: absolute;
  right: 10px;
  top: 50%;
  transform: translateY(-50%);
  background: none;
  border: none;
  padding: 0;
  cursor: pointer;
  display: flex;
  align-items: center;

  img {
    width: 16px;
    height: 16px;
  }
`;

const StyledError = styled.p`
  ${applyFontStyles(FontTypes.SEMIBOLD14, ColorTypes.ERROR)}
`;

export default Input;
  • 실제 사용
// 커스텀 에러 메시지
<Input label="나이" id="age">
  <Input.Number
    placeholder="나이를 입력하세요"
    value={age}
    onChange={setAge}
    min={1}
    max={120}
  />
  <Input.ErrorMessage>
    나이는 1세 이상 120세 이하여야 합니다.
  </Input.ErrorMessage>
</Input>

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 +18 to +22
const fetchProductDetail = async () => {
const res = await getProductDetail(productId);
setProduct(res);
setIsTags(res.tags.length > 0);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

React 공식문서상으로는 useEffect 내부에서 사용하는 함수는 의존성에 추가하고, useCallback으로 감싸서 사용할것을 권고하고있습니다 :)

이유는, useEffect가 실행될때마다 fetchProductDetail 함수의 인스턴스가 새로 생성되는데 함수 자체는 생성될때마다 동일한 로직이지만 메모리상은 새로운 객체로 취급되고, 또한 비동기 함수가 실행 중일때 컴포넌트가 언마운트되더라도 함수가 계속 실행 될 가능성이 있어, 메모리 누수 가능성이 있기 때문입니다.

따라서, useCallback을 사용해 의존성이 변경되지않는 한 동일한 함수 참조를 유지하며 AbortController를 사용해 cleanup까지 고려해주시면 좋습니다.

const fetchProductDetail = useCallback(async () => {
  const abortController = new AbortController();
  
  try {
    const res = await getProductDetail(productId, { signal: abortController.signal });
    setProduct(res);
    setIsTags(res.tags.length > 0);
  } catch (error) {
    if (error.name !== 'AbortError') {
      console.error('Failed to fetch product:', error);
    }
  }
  
  return () => abortController.abort();
}, [productId, getProductDetail]);

useEffect(() => {
  const cleanup = fetchProductDetail();
  return cleanup;
}, [fetchProductDetail]);

이런 작은 디테일의 변화들도 놓치지않고 신경써서 코딩한다면 최적화 경험을 늘리면서도 좋은 개발 습관을 들일 수 있을거예요! :)

<div>
<StyledOwnerNickname>{product?.ownerNickname}</StyledOwnerNickname>
<StyledProductCreatedAt>
{product?.createdAt.split('T')[0].split('-').join('. ')}
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 +39 to +71
const handleEmailChange = (e) => {
const value = e.target.value;
setEmail(value);
setErrors((prev) => ({
...prev,
email: validateField('email', value, { email: value, password }, 'login'),
}));
};

const handlePasswordChange = (e) => {
const value = e.target.value;
setPassword(value);
setErrors((prev) => ({
...prev,
password: validateField('password', value, { email, password: value }, 'login'),
}));
};

const handleEmailBlur = (e) => {
const value = e.target.value;
setErrors((prev) => ({
...prev,
email: validateField('email', value, { email: value, password }, 'login'),
}));
};

const handlePasswordBlur = (e) => {
const value = e.target.value;
setErrors((prev) => ({
...prev,
password: validateField('password', value, { email, password: value }, 'login'),
}));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

핸들러에서 공통적으로 처리되는 패턴이 반복되는것같은데,
코드 중복을 줄이고 간소화하기위해 기존에 쓰고있던 커스텀 훅에 필드 변경이나 블러 이벤트를 처리하는 함수들을 재사용성있게 만들어 추가해보는건 어떨까요?

  // 필드 변경 핸들러 생성 함수
  const createFieldHandler = useCallback(
    (fieldName) => {
      return (e) => {
        const value = e.target.value;
        const newValues = { ...values, [fieldName]: value };
        setValues(newValues);
        setErrors((prev) => ({
          ...prev,
          [fieldName]: validateField(fieldName, value, newValues, mode),
        }));
      };
    },
    [values, mode]
  );

  // 필드 블러 핸들러 생성 함수
  const createBlurHandler = useCallback(
    (fieldName) => {
      return (e) => {
        const value = e.target.value;
        setErrors((prev) => ({
          ...prev,
          [fieldName]: validateField(fieldName, value, values, mode),
        }));
      };
    },
    [values, mode]
  );

  // 폼 제출 핸들러 생성 함수
  const createSubmitHandler = useCallback(
    (onSubmit) => {
      return (e) => {
        e.preventDefault();
        const validationResult = validate(values);
        if (Object.keys(validationResult).length > 0) return;
        onSubmit(values);
      };
    },
    [values, validate]
  );

  // 필드별 props 생성 함수
  const getFieldProps = useCallback(
    (fieldName) => {
      return {
        value: values[fieldName] || "",
        onChange: createFieldHandler(fieldName),
        onBlur: createBlurHandler(fieldName),
        error: errors[fieldName],
      };
    },
    [values, errors, createFieldHandler, createBlurHandler]
  );

  // 폼 상태 확인 함수들
  const hasError = Object.values(errors).some((v) => !!v);
  const isFilled = Object.keys(values).every(
    (key) => values[key] && values[key].trim()
  );
  const isFormValid = !hasError && isFilled;
  • 실제 사용 예시
     ...
        <InputField
          label="이메일"
          type="email"
          placeholder="이메일을 입력해주세요"
          isTextArea={false}
          {...getFieldProps("email")}
        />
        <InputField
          label="비밀번호"
          type="password"
          placeholder="비밀번호를 입력해주세요"
          isTextArea={false}
          {...getFieldProps("password")}
        />
   ...

이런식으로 기존 핸들러 함수를 통해 개별 필드별 props를 생성하는 함수를 만들고, createSubmitHandler() 함수로 폼 제출 로직을 관리하는 함수를 만들면 모든 폼에서 동일한 패턴을 사용해 일관성을 높여줄수도있고, 새로운 인풋 필드를 추가한다고해도 getFieldProps('fieldName')만 호출하면 되니까 확장성 측면에서도 유리해질 수 있겠죠? :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 페이지에서도 LoginPage에서 드렸던 코멘트 보시고 리팩토링 참고해보세요! :)

@addiescode-sj
Copy link
Collaborator

질문에 대한 답변

멘토에게

  • 제품 상세 페이지의 컴포넌트를 크게 두가지로 나누게 되었습니다. 만들다보니 렌더링되는 리턴 부분이 복잡해보여서 더 작게 나누고 싶었는데 어떤걸 분리시켜야 좋을지 감이 잡히질 않아 이 부분에서 코멘트 주시면 리팩토링 해보고 싶습니다.

ProductInfo 컴포넌트가 제일 복잡해보이네요!
분리 기준은 독립적인 기능을 하는 단위라면 분리의 대상이 될 수 있다고 봐도 무방합니다.
시각적으로 구분되는 영역을 가지고 구분하자면 <ProductImage />, <ProductHeader />, <ProductDescription />, <ProductOwner /> 등으로 쪼개봐도 괜찮겠네요 :)

  • 댓글 등록까지 구현해야하는 줄 알고 로그인 상태를 유지시키려고 했었다가, 이후 미션에서 진행하는 단계인걸 깨닫고 그 부분에서는 마무리하진 못했습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@addiescode-sj addiescode-sj merged commit 49edf86 into codeit-bootcamp-frontend:React-문지영 Aug 6, 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