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

[이형준]sprint9 #270

Merged

Conversation

leehj322
Copy link
Collaborator

@leehj322 leehj322 commented Aug 9, 2024

사이트 배포 링크

요구사항

피그마 링크

기본 요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • 기존의 React, Typescript로 구현한 프로젝트와 별도로 진행합니다.
  • Next.js를 사용합니다

체크리스트 (기본)

  • 자유 게시판 페이지 주소는 "/boards" 입니다.
  • 전체 게시글에서 드롭 다운으로 "최신 순" 또는 "좋아요 순"을 선택해서 정렬을 할 수 있습니다.
  • 게시글 목록 조회 api를 사용하여 베스트 게시글, 게시글을 구현합니다.
  • 게시글 title에 검색어가 일부 포함되면 검색이 됩니다.

체크리스트 (심화)

  • 반응형으로 보여지는 베스트 게시판 개수를 다르게 설정할때 서버에 보내는 pageSize값을 적절하게 설정합니다.
  • next의 prefetch 기능을 사용해봅니다.

피드백 반영

  • 모바일 사이즈 최소값 제거
  • alias path 사용

추가 예정 사항

  • map 돌린 부분 시멘틱태그 ul, li 로 수정
  • 게시글 전체 목록 무한 스크롤 구현
  • GlobalNavBar isMain prop 없애기
  • axios 에러 핸들링 추가

주요 변경사항

  • sprint mission 9 구현중

멘토에게

  • 해당 페이지를 getServerSideProps를 이용해서 SSR로 구현하려다가 화면 사이즈가 변함에 따라서 pageSize를 다르게 해서 api에 요청하는 것을 구현할때 꼼수로 구현하는 방법 밖에 찾지 못해서 CSR로 구현하였습니다. 아마 다음 스프린트 미션에는 충분히 SSR로 구현이 가능할 것 같아서 그때 사용하게 될 것 같습니다.
  • 원래 styled-components를 이용하려 했는데 next에서 사용할 경우 추가적인 세팅이 많이 필요하고 이후에 기능에 제한이 있을것 같아 emotion, vanila css, tailwind중 tailwind로 갈아 탔습니다.
  • src/components/boards/BestPostCard.tsx 에서 badge를 이번에는 귀찮아서 image로 사용하긴 했지만 실제로 사이트들을 보면 css를 이용해서 만들 수 있는 것들도 image로 로드하는 경우가 종종 있던데 왜그런건가요?
  • useCurrentDevice 커스텀 훅에서 useCallback을 사용했는데 올바르게 사용하고 있는 것인지 잘 모르겠습니다. 이외의 컴포넌트도 확인 부탁드립니다.
  • axios에서 예외 및 에러 처리를 어떤식으로 해야하는지 잘 모르겠습니다. fetch(기존 에러 처리)와는 약간 다르게 동작하는거 같은데 고민중입니다. 추가로 인스턴스에 content-type을 application/json으로 설정해주는 경우도 있던데 default(application/x-www-form-urlencoded)를 쓸지 json을 쓸지 선택을 어떻게 하나요?
  • 폰트가 pretendard로 적용이 안되는데 어떤 이유일까요? tailwind를 처음 적용하다 보니 원인을 찾는데 어려움이 있습니다.
  • Dropdown 공통 컴포넌트에서 li태그를 map돌리는 부분에 Object.keys 를 이용해서 리스트를 새로 만들다보니 제네릭으로 정해준 타입을 인식하지 못해서 as를 썼는데 적절할까요?
  • (해결완료: 0a2dfc2) Dropdown 에서 hydration error가 발생했는데 해당 부분을 어떤식으로 수정해아할지 잘 모르겠습니다. 우선은 suppressHydrationWarning={true}를 적용한 상태인데 useEffect를 제외하고는 더 좋은 방법의 개선방법이 있을까요..?
    image
    image

@leehj322 leehj322 requested a review from jyh0521 August 9, 2024 14:01
@leehj322 leehj322 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 9, 2024
@leehj322 leehj322 removed the 미완성🫠 죄송합니다.. label Aug 10, 2024
Copy link
Collaborator

@jyh0521 jyh0521 left a comment

Choose a reason for hiding this comment

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

과제하느라 고생하셨습니다! next.js의 다양한 기능들을 잘 활용해보셨네요 👍

@@ -0,0 +1,48 @@
import instance from "./instance";
Copy link
Collaborator

Choose a reason for hiding this comment

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

instance 이름은 axiosInstance 같은 좀 더 구체적인 이름을 추천드릴게요. 나중에는 다른 instance가 생길 수도 있거든요!

import useToggle from "@/hooks/useToggle";
import Image from "next/image";

type DropdownProps<T extends string> = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

제네릭 활용해보신거 좋네요!

Copy link
Collaborator Author

@leehj322 leehj322 Aug 12, 2024

Choose a reason for hiding this comment

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

export default function Dropdown<T extends string>({
  itemDict,
  currentItemValue,
  onItemChange,
}: DropdownProps<T>) {
  const [isOpen, toggleIsOpen] = useToggle();

  const handleItemClick = (value: T) => {
    onItemChange(value);
    toggleIsOpen();
  };
  ...

컴포넌트 쪽에서 이런식으로 썼는데 이것도 문제 없을까요?
구글링 해서 찾아 쓰긴 한건데 왜 function Dropdown<T extends string> 형태로 컴포넌트 옆에도 다시 정의 해줘야하는지를 모르겠습니다.
그러니까 정확히는 props type에서 선언한 제네릭 T를 함수에서 받아서 사용할 수 없는걸까요?

Comment on lines +17 to +18
const router = useRouter();
const path = router.pathname;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 두 줄은 다음과 같이 요약할 수 있을 것 같습니다.

const { pathname } = useRouter();

className="hidden md:block"
width={153}
height={51}
priority
Copy link
Collaborator

Choose a reason for hiding this comment

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

priority 옵션 활용 좋네요!

import React, { useState } from "react";

export interface SearchBarProps {
onKeywordChange: React.Dispatch<React.SetStateAction<string>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

setState 함수를 바로 받아오는 것 보다 setState를 포함하고있는 () => void 타입의 함수를 받아오신다면 좀 더 유용하게 로직을 작성할 수 있게 됩니다.

Comment on lines +11 to +13
mobilePageSize: number,
tabletPageSize: number,
desktopPageSize: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

요런 경우에는 객체로 파라미터를 받아오게 하는 것도 추천드립니다. usePageSize(1, 2, 3) 이렇게 사용되면 어떤 파라미터가 어떤 역할인지 알기 어려울 것 같아서요!


export default function PostList() {
const [articles, setArticles] = useState<Article[]>([]);
const [orderOption, setOrderOption] = useState<orderOption>("recent");
Copy link
Collaborator

Choose a reason for hiding this comment

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

orderOption 타입 이름이라면 파스칼 케이스로 사용하시는 것을 추천드립니다.

@jyh0521 jyh0521 merged commit 3e3478a into codeit-bootcamp-frontend:Next-이형준 Aug 12, 2024
@jyh0521
Copy link
Collaborator

jyh0521 commented Aug 12, 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