Skip to content

Conversation

@hyeonjiroh
Copy link
Collaborator

@hyeonjiroh hyeonjiroh commented Feb 19, 2025

요구사항

기본

상품 등록

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

심화

상품 등록

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

주요 변경사항

  • 폴더 구조 변경
  • CSS 기술 통일 - CSS Modules 사용
  • AllItemsSection의 검색창 input 태그에 search 타입 추가
  • 디바운싱을 사용한 브라우저 사이즈 감지 훅 생성
  • 페이지 바 기능 추가
    • 이전 페이지 없을 시 이전 버튼 비활성화
    • 다음 페이지 없을 시 다음 버튼 비활성화
    • 현재 페이지에 해당하는 페이지 버튼 파란색으로 표시
  • 아이템 정렬 드롭 다운 박스 구현
  • 반응형 디자인 수정
    • 모바일 화면에서는 글자만 있는 로고 표시
  • 랜딩 페이지(”/”)에서는 헤더에 로고와 로그인 버튼 표시
  • API 요청 시 axios 사용
  • API 요청 시 에러 처리 수정

멘토에게

  • 스프린트 미션 7 전에 컴포넌트 분리 시도해보겠습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@hyeonjiroh hyeonjiroh requested a review from kiJu2 February 19, 2025 03:34
@hyeonjiroh hyeonjiroh added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Feb 19, 2025
@kiJu2
Copy link
Collaborator

kiJu2 commented Feb 20, 2025

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

@kiJu2
Copy link
Collaborator

kiJu2 commented Feb 20, 2025

스프린트 미션 7 전에 컴포넌트 분리 시도해보겠습니다.

크으 좋습니다 ! 리팩토링 해보시면 좋은 경험이 되실거예요 😊

Comment on lines +8 to +24
function App() {
const location = useLocation();

return (
<>
{!["/signin", "/signup"].includes(location.pathname) && (
<Header className={styles.nav} />
)}
<div className={styles.body}>
<Outlet />
</div>
{["/"].includes(location.pathname) && (
<Footer className={styles.footer} />
)}
</>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

(제안/선택) 별칭을 사용하면 가독성을 향상시킬 수 있습니다 😊😊

Suggested change
function App() {
const location = useLocation();
return (
<>
{!["/signin", "/signup"].includes(location.pathname) && (
<Header className={styles.nav} />
)}
<div className={styles.body}>
<Outlet />
</div>
{["/"].includes(location.pathname) && (
<Footer className={styles.footer} />
)}
</>
);
}
function App() {
const location = useLocation();
const isAuthPage = ["/signin", "/signup"].includes(location.pathname);
const isHomePage = location.pathname === "/";
return (
<>
{!isAuthPage && <Header className={styles.nav} />}
<div className={styles.body}>
<Outlet />
</div>
{isHomePage && <Footer className={styles.footer} />}
</>
);
}

인라인 조건문을 변수로 추출하면 코드의 의도가 명확해지고 유지보수가 쉬워집니다! 이렇게 하면 가독성이 좋아지고, 새로운 경로가 추가될 때도 쉽게 관리할 수 있어요. 🚀

Comment on lines +4 to +25
function useWindowSize() {
const [windowSize, setWindowSize] = useState({
width: window.innerWidth,
height: window.innerHeight,
});

useEffect(() => {
const handleResize = debounce(() => {
setWindowSize({
width: window.innerWidth,
height: window.innerHeight,
});
}, 300);

window.addEventListener("resize", handleResize);
return () => {
window.removeEventListener("resize", handleResize);
};
}, []);

return windowSize;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

300이란 값을 파라메터로 받아볼 수 있어요 !:

크으 ~ 커스텀 훅으로 만드셨군요 ! 👍👍👍

Suggested change
function useWindowSize() {
const [windowSize, setWindowSize] = useState({
width: window.innerWidth,
height: window.innerHeight,
});
useEffect(() => {
const handleResize = debounce(() => {
setWindowSize({
width: window.innerWidth,
height: window.innerHeight,
});
}, 300);
window.addEventListener("resize", handleResize);
return () => {
window.removeEventListener("resize", handleResize);
};
}, []);
return windowSize;
}
function useWindowSize(delay = 300) {
const [windowSize, setWindowSize] = useState({
width: window.innerWidth,
height: window.innerHeight,
});
useEffect(() => {
const handleResize = debounce(() => {
setWindowSize({
width: window.innerWidth,
height: window.innerHeight,
});
}, delay);
window.addEventListener("resize", handleResize);
return () => {
window.removeEventListener("resize", handleResize);
};
}, []);
return windowSize;
}

import { useState, useEffect } from "react";
import debounce from "lodash.debounce";

function useWindowSize() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

(제안/심화/의견) debounceuseDebounce라는 커스텀 훅으로 만들어볼 수도 있을 것 같아요 😊

Comment on lines +26 to +38
const handleLoad = async (query) => {
try {
const { list, totalCount } = await getItems(query);
setItems(list);
setTotalItemCount(totalCount);
} catch (error) {
return;
}
};

useEffect(() => {
handleLoad({ page, pageSize, order });
}, [page, pageSize, order]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

handleLoaduseEffect 안에 선언하는건 어떨까요?

Suggested change
const handleLoad = async (query) => {
try {
const { list, totalCount } = await getItems(query);
setItems(list);
setTotalItemCount(totalCount);
} catch (error) {
return;
}
};
useEffect(() => {
handleLoad({ page, pageSize, order });
}, [page, pageSize, order]);
useEffect(() => {
const handleLoad = async () => {
try {
const { list, totalCount } = await getItems({ page, pageSize, order });
setItems(list);
setTotalItemCount(totalCount);
} catch (error) {
console.error("아이템을 불러오는 중 오류 발생:", error);
}
};
handleLoad();
}, [page, pageSize, order]);

handleLoaduseEffect에서만 사용되는 함수이므로, 굳이 컴포넌트 바깥에 선언할 필요가 없어요 ! useEffect 안에서 선언하면 불필요한 외부 접근을 방지하고, 코드 응집도가 높아져 관리가 쉬워집니다. 😊

Comment on lines +31 to +33
} catch (error) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

오류가 발생했음을 알려주는 건 어떨까요? 😊

Suggested change
} catch (error) {
return;
}
} catch (error) {
alert(error.message);
console.error('ERROR: ', error);
}

Comment on lines +12 to +22

function AllItems() {
const [order, setOrder] = useState("recent");
const [page, setPage] = useState(1);
const [pageSize, setPageSize] = useState(10);
const [isOpen, setIsOpen] = useState(false);
const [totalItemCount, setTotalItemCount] = useState(0);
const [pageBound, setPageBound] = useState(0);
const [items, setItems] = useState([]);
const MaxPageBound = Math.floor(totalItemCount / pageSize / 5);
const pageArr = [1, 2, 3, 4, 5];
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Suggested change
function AllItems() {
const [order, setOrder] = useState("recent");
const [page, setPage] = useState(1);
const [pageSize, setPageSize] = useState(10);
const [isOpen, setIsOpen] = useState(false);
const [totalItemCount, setTotalItemCount] = useState(0);
const [pageBound, setPageBound] = useState(0);
const [items, setItems] = useState([]);
const MaxPageBound = Math.floor(totalItemCount / pageSize / 5);
const pageArr = [1, 2, 3, 4, 5];
const PAGE_ARRAY = [1, 2, 3, 4, 5];
function AllItems() {
const [order, setOrder] = useState("recent");
const [page, setPage] = useState(1);
const [pageSize, setPageSize] = useState(10);
const [isOpen, setIsOpen] = useState(false);
const [totalItemCount, setTotalItemCount] = useState(0);
const [pageBound, setPageBound] = useState(0);
const [items, setItems] = useState([]);
const MaxPageBound = Math.floor(totalItemCount / pageSize / 5);

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

Comment on lines +75 to +77
if (width > 1200) {
newPageSize = 10; // PC
} else if (width > 768) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

(제안) 1200, 768과 같은 숫자가 많이 보이는군요 !

Suggested change
if (width > 1200) {
newPageSize = 10; // PC
} else if (width > 768) {
if (width > BREAKPOINTS.DESKTOP) {
newPageSize = 10; // PC
} else if (width > BREAKPOINTS.TABLET) {

앞으로도 재사용이 많을 것으로 예상되어 유지보수성과 가독성을 위해 상수로 정의해두면 어떨지 제안드려봅니다 !

<div className={styles.title}>전체 상품</div>
<div className={styles.menu}>
<form>
<img src={SearchIcon} className={styles.searchIcon} alt="검색" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

단순 장식용으로 있을 경우 ""로 나타낼 수 있습니다 !

Suggested change
<img src={SearchIcon} className={styles.searchIcon} alt="검색" />
<img src={SearchIcon} className={styles.searchIcon} alt="" />

장식 이미지는 페이지 콘텐츠에 정보를 추가하지 않습니다. 예를 들어, 이미지에서 제공하는 정보는 인접한 텍스트를 사용하여 이미 제공될 수도 있고, 웹 사이트를 시각적으로 더욱 매력적으로 만들기 위해 이미지가 포함될 수도 있습니다.

이러한 경우 스크린 리더와 같은 보조 기술에서 무시할 수 있도록 null(빈) alt텍스트를 제공해야 합니다( ). alt=""이러한 유형의 이미지에 대한 텍스트 값은 화면 판독기 출력에 청각적 혼란을 추가하거나 주제가 인접한 텍스트의 주제와 다른 경우 사용자의 주의를 산만하게 할 수 있습니다. 속성 을 생략하는 alt것도 옵션이 아닙니다. 속성이 제공되지 않으면 일부 화면 판독기가 이미지의 파일 이름을 대신 알려주기 때문입니다.

Decorative Images

@kiJu2
Copy link
Collaborator

kiJu2 commented Feb 20, 2025

현지님 ! 수고하셨습니다 😊
기초 프로젝트 이후로도 지치지않고 스프린트 미션을 수행해주셨군요 ! 👍 역시 현지님 !
코드에 종종 마법수(페이지네이션 5와 같은)가 종종 보이는 것 같아요. 이는 상수로 선언하여 별칭을 주면 (ex:MAX_PAGE) 가독성이 더 향상될 수 있어요 ㅎㅎ
기초 프로젝트 이 후 리액트 활용을 더욱 풍부하게 하신 것 같아 기분이 좋습니다 ! 👍

미션 수행하시느라 정말 고생 많으셨습니다 현지님 😊😊

@kiJu2 kiJu2 merged commit 76c4b00 into codeit-bootcamp-frontend:React-노현지 Feb 20, 2025
@hyeonjiroh hyeonjiroh self-assigned this Feb 20, 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