[유선향]-sprint7

[grimza99]

요구사항

기본

• 각 상품 클릭 시 상품 상세 페이지로 이동합니다.
• 상품 상세 페이지 주소는 “/items/{productId}” 입니다.
• response 로 받은 데이터로 화면을 구현합니다
• 목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 “/items” 으로 이동합니다
• 문의하기에 내용을 입력하면 등록 버튼의 색상은 “3692FF”로 변합니다.
• response 로 받은 데이터로 화면을 구현합니다

심화

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

주요 변경사항

-저번에 이어 이번에도 fileChanged 가 많습니다 😢😢

• 아마 미션 8때는 이전에 진행한 5,6 까지 리팩토링 후 8진행 예정이여서 더 많을것 같습니다 양해 부탁드려용..
  -주요 변경사항은 아래와 같습니다.

• 공통 컴포넌트
  -HeartBtn.jsx
  -select .jsx (sort , kebab 두가지로 나눴습니다.)
  -comment card.jsx

• 컴포넌트
  -comment.jsx
  -product info.jxs

스크린샷

멘토에게

• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[kiJu2]

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

[kiJu2]

-저번에 이어 이번에도 fileChanged 가 많습니다 😢😢

아마 미션 8때는 이전에 진행한 5,6 까지 리팩토링 후 8진행 예정이여서 더 많을것 같습니다 양해 부탁드려용..
-주요 변경사항은 아래와 같습니다.
공통 컴포넌트
-HeartBtn.jsx
-select .jsx (sort , kebab 두가지로 나눴습니다.)
-comment card.jsx

컴포넌트
-comment.jsx
-product info.jxs

---

아이구. 친절하게 설명까지해주시다니 감사드립니다 ! 😊
말씀주신 파일은 더 꼼꼼히 확인해볼게요 !

[kiJu2]

(제안) 다음과 같이 작성해도 같은 결과일 거라고 생각됩니다 !

Suggested change

	export default function BtnHeart({ value, ...props }) {
	const { border, active, onClick, small, ...rest } = props;
	export default function BtnHeart({ value, border, active, onClick, small, ...rest }) {

굳이 스프레드 연산자를 두 번 사용하지 않아도 될 것 같아서 제안드립니다 😊😊

[kiJu2]

해당 함수는 필요 없을 것으로 보입니다 !:

Suggested change

	const onClickChange = (e) => {
	onClick(e);
	};
	return (
	<>
	{active ? (
	<S.ActiveBtnHeart
	onClick={onClickChange}
	$border={border}
	$small={small}
	{...rest}
	>
	<S.HeartImg src={activeHeart} $small={small ? small : undefined} />
	{value}
	</S.ActiveBtnHeart>
	) : (
	<S.InactiveBtnHeart
	onClick={onClickChange}
	return (
	<>
	{active ? (
	<S.ActiveBtnHeart
	onClick={onClick}
	$border={border}
	$small={small}
	{...rest}
	>
	<S.HeartImg src={activeHeart} $small={small ? small : undefined} />
	{value}
	</S.ActiveBtnHeart>
	) : (
	<S.InactiveBtnHeart
	onClick={onClick}

없어져도 로직상 똑같은 결과가 나오지 않을까 사료됩니다 😊

[kiJu2]

(이어서/ 더 나아가서) 결국 onClick과 관련된 선언들을 다 지우셔도 될 것 같아요.

Suggested change

	export default function BtnHeart({ value, ...props }) {
	const { border, active, onClick, small, ...rest } = props;
	const onClickChange = (e) => {
	onClick(e);
	};
	return (
	<>
	{active ? (
	<S.ActiveBtnHeart
	onClick={onClickChange}
	$border={border}
	$small={small}
	{...rest}
	>
	<S.HeartImg src={activeHeart} $small={small ? small : undefined} />
	{value}
	</S.ActiveBtnHeart>
	) : (
	<S.InactiveBtnHeart
	onClick={onClickChange}
	$border={border}
	$small={small}
	{...rest}
	export default function BtnHeart({ value, ...props }) {
	const { border, active, small, ...rest } = props;
	return (
	<>
	{active ? (
	<S.ActiveBtnHeart
	$border={border}
	$small={small}
	{...rest}
	>
	<S.HeartImg src={activeHeart} $small={small ? small : undefined} />
	{value}
	</S.ActiveBtnHeart>
	) : (
	<S.InactiveBtnHeart
	$border={border}
	$small={small}
	{...rest}

rest 안에 onClick이 포함되므로 전과 똑같이 동작할 것으로 사료됩니다 😊

[kiJu2]

(제안) small을 props로 주어지지 않으면 결국 undefined로 오게될 것으로 예상됩니다 !

Suggested change

	<S.HeartImg src={inactiveHeart} $small={small ? small : undefined} />
	<S.HeartImg src={inactiveHeart} $small={small} />

따라서 위와 같이 작성될 수 있을 것 같아요.

[kiJu2]

논리식에 오타가 있는 것 같아요 !

Suggested change

	const [selected, setSelected] = useState(options[0] | "options배열 필요");
	const [selected, setSelected] = useState(options[0] ?? "options배열 필요");

현재 작성주신 |는 비트 연산자(OR)입니다 !

[kiJu2]

(제안) options는 컴포넌트의 자원을 사용하지 않기에 상수로 표현할 수 있으며 컴포넌트 바깥에 선언하실 수 있습니다 !

Suggested change

	//
	export function SortSelect({ onChange, ...props }) {
	const options = ["최신순", "좋아요순"];
	const OPTIONS = ["최신순", "좋아요순"];
	export function SortSelect({ onChange, ...props }) {

options는 컴포넌트 내부 상태, props와 연관이 없으므로 컴포넌트 바깥에 선언해볼 수 있어요. 이렇게 하시면 컴포넌트의 내용들은 컴포넌트의 자원과 관련된 로직들로만 구성되며, 리렌더링에 의한 불필요한 재선언을 방지할 수 있습니다 😊

[kiJu2]

콜백을 통하여 접근하시면 가장 신선한 '이 전 상태'를 참고하실 수 있습니다 ! 😊

Suggested change

	<S.KebobButton $isOpen={isOpen} onClick={() => setIsOpen(!isOpen)}>
	<S.KebobButton $isOpen={isOpen} onClick={() => setIsOpen((prev) => !prev)}>

[kiJu2]

return은 크게 의미가 없는 것 같아요 !

Suggested change

	const handleOnChange = (option) => {
	if (option === button.edit) {
	setIsEditing(data.id);
	}
	if (option === button.delete) {
	setIsDelete(data.id);
	}
	return;
	};
	const handleOnChange = (option) => {
	if (option === button.edit) {
	setIsEditing(data.id);
	}
	if (option === button.delete) {
	setIsDelete(data.id);
	}
	};

함수 마지막에 존재하며 반환하는 값이 없기에 return이 있으나 없으나 같은 결과일 것으로 보입니다 ! 😊

[kiJu2]

useEffect 안에 handleLoad를 선언해볼 수 있어요. 😊

Suggested change

	const handleLoad = async () => {
	const data = await getProductComments(productId);
	setComments(data);
	};
	const handleChange = () => {
	setIsDisabled(false);
	};
	const handleClickSubmit = () => {};
	useEffect(() => {
	handleLoad();
	}, []);
	const handleChange = () => {
	setIsDisabled(false);
	};
	const handleClickSubmit = () => {};
	useEffect(() => {
	const handleLoad = async () => {
	const data = await getProductComments(productId);
	setComments(data);
	};
	handleLoad();
	}, []);

handleLoad 함수는 오직 useEffect 내부에서만 호출되므로, 굳이 컴포넌트 외부에서 선언할 필요가 없어요. handleLoad가 useEffect 바깥에 선언되면, useEffect가 handleLoad를 의존성으로 인식할 가능성이 있어요.
부가적으로 handleLoad가 useEffect 안에 있으면, "이 함수는 오직 useEffect에서 실행되는 비동기 데이터 로딩 함수다" 라는 의도가 명확해져서 가독성이 좋아집니다! 👀👀

[kiJu2]

ㅠㅠㅠ 언제나 빠르고 착실하게 미션을 수행하시는 선향님 🥺🥺
이번에도 레벨업 하셨군요 ! 스프린트 미션이 끝났음에도 지치지 않고 성실하게 미션을 수행하시는군요..
코드를 보아하니 선향님께서 문제를 해결하기 위해 곰곰히 생각해보고 더 나은 코드가 어떤 것인지 고민하신 흔적이 보여요. 분명 이대로면 누구보다 빠르게 성장하실거라 생각합니다 !

스프린트 미션 수행하시느라 정말 수고 많으셨습니다 선향님 !!