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

Conversation

devmanta
Copy link
Collaborator

요구사항

기본

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

심화

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

  • 베스트 상품을 prefetch 기능 해보려고했는데 이렇게 되면 미리 그려놓는거라 반응형으로 pageSize를 적절하게 사용할수 없어서 css로 처리했습니다

@devmanta devmanta self-assigned this Oct 25, 2024
@devmanta devmanta added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Oct 25, 2024
@devmanta devmanta requested a review from GANGYIKIM October 25, 2024 20:23
rgb(var(--background-end-rgb))
)
rgb(var(--background-start-rgb));
.max-container {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

global에 max-container 클래스 두고 필요할때마다 여기저기서 가져다 쓰려고했는데,
이번미션부터 module css를 사용해서.. 약간 아래처럼 사용해야하는 상황인데

className={`${style.xxx} max-container`}

혹시 css모듈에서 글로벌하게 사용하는 좋은방법 있을까요? (:global 어쩌고.. 이렇게 자주 쓰이나요..? 그러면.. 애초에 global.css 도 모듈로 사용해야하는데 일관성잇게 global도 모듈쓰는 방향으로 가는게 좋을까요)

Copy link
Collaborator

Choose a reason for hiding this comment

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

max-container가 다른 컴포넌트에서 사용되는 경우가 없어서 우선 다른 방법을 쓰시기를 추천드립니다.
css 모듈이 스타일의 중첩을 방지하기 위한 방식이라 global 하게 사용하는 좋은 방식은 없는 것으로 압니다 🥹
말씀해주신 목적을 구현하기 위한 다양한 접근 방식이 있는데, 이중 희진님이 하신 것처럼 globalStyle 파일을 만드시는 것도 포함되어 잇습니다.
다만 어떤것이 더 적절한가는 usage에 따라 다를 것 같습니다.

Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

희진님 이번 스프린트 미션 고생하셨습니다~
깔끔하게 작업하셔서 리뷰드릴게 많이 없었네요!

export default function Header() {
return (
<header className={styles.header}>
<div className={`${styles.headerContainer}`}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
단일 클래스라면 아래처럼 하시는 걸 추천드려요~

Suggested change
<div className={`${styles.headerContainer}`}>
<div className={styles.headerContainer}>

aria-label="홈으로 이동"
/>
<nav className={styles.nav}>
<Link href={'/boards'}>자유게시판</Link>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
해당 메뉴의 주소일 때 색이 변하게 해주시면 더 좋을 것 같습니다~
참고하실만한 링크 남깁니다

#next Link 태그 문서
https://nextjs.org/docs/app/api-reference/components/link#checking-active-links

<>
<Header />
<main className={styles.main}>
<div className={'max-container'}>{children}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
멘토링때도 얘기나눴던 사항이지만
max-container 클래스가 단일 컴포넌트에서만 사용되니 globalStyle 파일에서 GlobalLayout.module.css 로 옮기면 좋을 것 같습니다~


interface PrimaryButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
children: React.ReactNode;
className?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
ButtonHTMLAttributes<HTMLButtonElement>에 이미 classname 이 정의되어 있으니 아래처럼 하시는걸 추천드려요~

Suggested change
className?: string;


export default function Button({
children,
className,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:
classname이 undefined일 수 있고 그러면 ${classname}일 때 undefined가 들어갈 수 있으니 빈 문자열로 초기화해주세요~

Suggested change
className,
className = '',

Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
👍

props: {
boards: list || [],
},
revalidate: 600, // Re-generate the page every 600 seconds (ISR)
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
적절하게 잘 사용하셨네요! 관련해서 읽어보면 좋을 글이 있어서 첨부합니다~

https://velog.io/@dldngus5/nextjs-revalidate

Comment on lines +7 to +11
interface BestBoardCardProps
extends Pick<
Board,
'title' | 'image' | 'writer' | 'likeCount' | 'createdAt'
> {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
추가로 정의할 타입이 있지 않다면 아래처럼 하시는 것을 추천드립니다.

Suggested change
interface BestBoardCardProps
extends Pick<
Board,
'title' | 'image' | 'writer' | 'likeCount' | 'createdAt'
> {}
type BestBoardCardProps = Pick<
Board,
'title' | 'image' | 'writer' | 'likeCount' | 'createdAt'
>;

@GANGYIKIM GANGYIKIM merged commit c4034c1 into codeit-bootcamp-frontend:Next-김희진 Oct 29, 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