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

Conversation

m-4verick
Copy link

공통

  • Github에 스프린트 미션 PR을 만들어 주세요.

  • React, Express를 사용해 진행합니다.
    프론트엔드 구현 요구사항
    랜딩 페이지

  • HTML과 CSS로 구현한 랜딩페이지를 React로 마이그레이션하세요.

  • 랜딩 페이지 url path는 "/"로 설정하세요.

중고마켓 페이지

  • 중고마켓 페이지 url path를 "/items"으로 설정하세요.
  • 페이지 주소가 "/items" 일 때 상단내비게이션바의 "중고마켓" 버튼의 색상은 "3692FF"입니다.
  • 중고마켓 페이지 판매 중인 상품은 본인이 만든 GET 메서드를 사용해 주세요.
  • 다만 좋아요 순 정렬 기능은 제외해 주세요.
  • 사진은 디폴트 이미지로 프론트엔드에서 처리해주세요.
  • 베스트 상품 목록 조회는 구현하지 않습니다.
  • '상품 등록하기' 버튼을 누르면 "/registration" 로 이동합니다. ( 빈 페이지 )

상품 등록 페이지

  • PC, Tablet, Mobile 디자인에 해당하는 상품 등록 페이지를 만들어 주세요.
  • 상품 등록 url path는 "/registration"입니다.
  • 상품 등록은 본인이 만든 POST 메서드를 사용해 주세요.
  • 등록 성공 시, 해당 상품 상세 페이지로 이동합니다. (빈페이지)

링크
https://lovely-puffpuff-7b374b.netlify.app/

감사합니다.

@m-4verick m-4verick requested review from sprint-edu and rjc1704 and removed request for sprint-edu January 23, 2025 13:03
const pathname = usePathname();

return (
<span className="flex gap-[20px] text-[18px] font-bold text-[#4B5563] md:gap-[30px]">
Copy link
Collaborator

Choose a reason for hiding this comment

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

모바일 일 때의 텍스트사이즈는 16px입니다. 보통 모바일일 때와 데스크탑일 때의 네비게이션 메뉴의 텍스트 사이즈가 다른 경우가 많으니 고려해서 눈여겨 보시면 좋을 것 같습니다.

return () => {
window.removeEventListener("scroll", handleScroll);
};
}, [loading, hasMore]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

의존성배열에 loading, hasMore 는 없는 것이 좋습니다. 이벤트리스너는 컴포넌트 마운트 시 단 한번만 실행시켜놓게 하고, 페이지 이동으로 인해 언마운트 시 이벤트리스너 메모리 해제되는 로직으로만 해도 충분합니다. 현재는 로딩상태나 hasMore 값이 바뀔때마다 새로운 이벤트리스너를 또 실행시키는 형국이라 메모리누수에 해당됩니다.

return (
<>
<div className="mb-[160px] mt-8 grid grid-cols-2 gap-2 md:grid-cols-3 md:gap-4 xl:grid-cols-5 xl:gap-6">
{products.map((product, index) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

map 으로 뿌려지는 ProductCard UI도 컴포넌트로 별도로 분리해주시는 것을 권장 드립니다.

@@ -0,0 +1,5 @@
import ItemsPage from "@/views/ItemsPage/ItemsPage";

export default function items() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

파일명이 소문자인 page.tsx 더라도 컴포넌트명은 항상 파스칼케이스를 따르는 것이 좋고 페이지 컴포넌트면 뒤에 Page 를 붙여서 ItemsPage 정도로 컴포넌트명을 만들어주시면 좋겠습니다.

@rjc1704
Copy link
Collaborator

rjc1704 commented Jan 27, 2025

모든 프론트엔드 만족사항들을 잘 만족시켜주셨습니다. 고생하셨습니다.

  • Next.js 를 벌써 사용해서 프론트엔드 코드를 구성하셨군요. Next.js 의 App Router 에서는 서버 컴포넌트를 고려해야해서 러닝커브가 높은 편인데 첫 구성이 나쁘지 않습니다.
  • SearchNav 컴포넌트의 경우 상품 등록하기 버튼 클릭 이벤트핸들러 처리를 위해 client component 로 처리되었는데 Link 컴포넌트 사용하시면 서버 컴포넌트로 구성이 가능합니다. Next.js 에서는 클라이언트 컴포넌트 비중을 최소화하는 방향이 번들최적화 측면에서 성능을 최적화하는 방향과도 비례합니다.
  • .gitignore 에 보면 package-lock.json 파일이 있는데, 이 파일은 깃으로 관리해야하는 파일입니다. 개발 당시의 의존성 버전과 추후 시간이 흘러 의존성 버전이 변경됨에 따라 기존 코드가 동작하지 않는 일을 방지해줍니다.
  • 상품등록폼에 유효성검사 없이도 등록버튼 클릭 시 api 요청이 나가고 있습니다. form 제출은 항상 각 input의 유효성검사를 모두 통과했을 때만 제출되도록 통제해주는 것이 중요합니다.
  • Next.js 에서 폴더구조는 아래와 같은 구조를 갖는 것을 권장 드립니다.
    src/
    ├── app/ # Next.js App Router
    │ ├── api/ # Route Handlers
    │ │ ├── products/
    │ │ │ └── route.ts # GET, POST 등 API 핸들러
    │ │ └── auth/
    │ │ └── route.ts
    │ ├── (auth)/ # 인증 관련 라우트 그룹
    │ ├── (main)/ # 메인 레이아웃 라우트 그룹
    │ └── layout.tsx

    ├── components/ # 공통 컴포넌트
    │ ├── ui/ # 기본 UI 컴포넌트
    │ └── shared/ # 공통으로 사용되는 복합 컴포넌트

    ├── lib/ # 유틸리티 및 설정
    │ ├── utils/
    │ ├── constants/
    │ └── config/

    ├── hooks/ # 커스텀 훅

    ├── types/ # TypeScript 타입 정의

    └── services/ # 외부 API 통신 관련 (클라이언트 사이드)

@rjc1704 rjc1704 merged commit f32e515 into codeit-sprint-fullstack:react-신진호 Jan 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants