-
Notifications
You must be signed in to change notification settings - Fork 9
[6주차] Team 팝업사이클 김철흥&송아영 미션 제출합니다. #15
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
base: master
Are you sure you want to change the base?
Conversation
Feature: detail 페이지 구현
Feat: 검색 페이지 구현
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 코드 감사합니다! 덕분에 많이 배워가는 것 같습니다!
별개로 현재 merge conflict가 있는 거 같아서 말씀드립니다!
public/svgs/home-logo.svg
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기본적으로 camel-case 사용해서 네이밍한 것으로 보이는데, svg파일에만 kebab-case 네이밍 적용하신 이유가 있을까요?
app/layout.tsx
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usePathName hook 때문에 clientLayout으로 분리한 것 같은데, 잘 배워갑니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좀 생각해봤는데 이렇게 하는 거 대신에 nav-bar(tab-bar) 컴포넌트를 CSR로 처리해서 그 안에서 return !isSplash ? <div> : null;
이런 식으로 하는 건 어떠신가요? 그러면 layout.tsx 파일이 동일한 레벨에 하나만 생겨서 좀 더 깔끔할 거 같은데 저도 생각만 해본 거라 잘 모르겠네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 동일하게 생각합니다! 동욱 님이 말씀하신 대로 작성해도 잘 동작하고, 저도 그렇게 작성하곤 합니닷 👍👍
@@ -0,0 +1,5 @@ | |||
import Splash from '@/components/layouts/Splash'; | |||
|
|||
export default async function SplashPage() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 여기서 async function을 사용하신 이유가 있을까요?
} | ||
], | ||
"paths": { | ||
"@/*": ["./*"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 설정하면 path alias로 절대경로 사용할 수 있는데, 한 번 찾아보시면 더 좋을 것 같습니다!
e.g.
"@components": ["components/*"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 예전 프로젝트에서는 @components 같은 형태로 절대경로를 사용했는데, 폴더가 추가될 때마다 경로 설정을 추가해야 하는 부분이 많이 번거롭더라고요! 그래서 @/components 같은 형식으로 처리되는 데 만족하고 있습니다.
const PlayButton = () => { | ||
return ( | ||
<button | ||
className="w-full h-fit py-2 px-4 flex items-center justify-center gap-2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 prettier 사용해도 동일한 text string 내에서는 분리가 안되던데 이거 따로 플러그인이 있나요? 아니면 position 관련 클래스랑 이외로 해서 임의로 나누신 건가요? 궁금해서 여쭤봅니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
임의로 개행한 거예요! 너무 길어지면 보기가 힘들어서 적당히 맥락에 따라서 개행하는 편입니다
alt={movie.title} | ||
fill | ||
priority={index === 0} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 이렇게 했을 때 dev mode에서 sizes property 채우라고 warning 떴었는데, 혹시 안 뜨시나요? (제가 aspect-ratio 클래서 안 써서 그럴 수도 있을 거 같은데, 잘 몰라서 여쭤봅니다)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sizes가 width height와는 별도의 프로퍼티인 건가요? 그렇다면 우선 저는 sizes 를 사용해 본 적이 없습니다. Image 컴포넌트를 사용하면서 본 경고는 width height를 적용하지 않았을 때 정도네요!
Image 컴포넌트의 width height는 이미지가 로드되기 전 공간을 확보하여, 비어 있던 공간에 이미지가 추가되면서 발생 가능한 ui 깜빡거림(용어가 기억이 안 나네요...) 현상을 방지하기 위한 요소입니다. 그래서 width height를 명시적으로 설정하거나, fill 속성을 부여하고 부모 요소에 relative와 적절한 width height를 설정할 수 있다고 알고 있습니다.
정확히 어떤 이유로 sizes를 채우라는 오류가 떴던 건지 세션 때 더 얘기 나눠보아요!
}, []); | ||
|
||
return ( | ||
<ul className="w-full aspect-[5/7] relative"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aspect class 사용해서 조절하는 거 좋은 것 같습니다! 배워갑니다!
video: boolean; | ||
vote_average: number; | ||
vote_count: number; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 따로 trending-movie interface를 정의하신 이유가 있으실까요? getTrendingMovies 반환할 때 필요한 속성만 반환할 수 있으면 더 좋을 것 같습니다!
fill | ||
src={'https://image.tmdb.org/t/p/w500' + movie.poster_path} | ||
alt={`${movie.title} poster`} | ||
className={clsx('bg-gray-200 object-cover', isOriginal ? 'rounded-[0.125rem]' : 'rounded-[0.0625rem]')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clsx 사용하면 이렇게 tailwind class를 동적으로 처리할 수 있군요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞습니다! className을 조건부, 동적으로 처리할 때 유용한 라이브러리입니닷
<Image | ||
unoptimized | ||
fill | ||
src={'https://image.tmdb.org/t/p/w500' + movie.poster_path} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 이거 https://image.tmdb.org/t/p/w500 없이 그냥 poster_path 해도 되었던 거 같은데 혹시 에러 뜨셨나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엇 제 기억에 동욱 님 과제에서는 response 자체에서 baseURL을 붙여서 return했던 것 같은데요, 아마 그래서 클라이언트 사이드에서는 별도의 처리가 없어도 되지 않았을까 싶습니다. 없으면 오류 나더라고요!
Fix: 검색 기능 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그제 어제 아파서 이제야 코드리뷰하게되었습니다ㅠㅠㅠㅠㅠ파일구조에서 신경쓰신게 보여서 감탄하면서 보았습니다
404에러 페이지 관련 코드를 못찾아서 여기서 쓰게 됐는데 그것도 width 제한 통일하면 완성도가 높았을 것 같았습니다! 수고하셨습니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
page 기본값이 1이라서, 따로 선언하지 안하도 적용이 되어서 trending/movie/day
까지만 선언해도 될 것 같습니다!
export default async function Page({ params }: { params: Promise<{ type: 'tv' | 'movie'; id: string }> }) { | ||
const { type, id } = await params; | ||
|
||
const response = await getDetail(`/${type}/${id}`); | ||
if (!response) return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
검색하다보니 type에 person도 나오더라구요, tv와 movie 둘 중 type선언하신거 같은데 어떻게 처리가 된걸까요? 의도된게 아니라면 사람 검색이 안되게 해야할 것 같습니다
export { default as PlusIcon } from '@/public/svgs/Banner/plus.svg'; | ||
export { default as InfoIcon } from '@/public/svgs/Banner/info.svg'; | ||
export { default as Top10Icon } from '@/public/svgs/Banner/top10.svg'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 따로 선언한 이유가 있을까요?
@@ -0,0 +1,24 @@ | |||
import { getMoviesApi } from '@/services/tmdb'; | |||
|
|||
import { categoryEndpointMap } from '@/constants/categoryEndpointMap'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 endpoiontMap을 사용해서 가져오는 방식이굉장히 재밌는 것 같습니다
배포 링크
Netflix Clone
Key Questions
1. 정적 라우팅(Static Routing)/동적 라우팅(Dynamic Routing)
정적 라우팅 (Static Routing)
ex. app/login/page.tsx
generateStaticParams
함수 안에서 params의 값을 미리 선언하여 정적 라우팅으로 제한하거나, 일부를 정적 라우팅과 함께 사용할 수 있습니다.동적 라우팅 (Dynamic Routing)
와 같이 여러 개의 id를 받아야 하는 경우
app/detail/[...id].tsx`처럼 ...을 붙여 사용합니다.2. 무한 스크롤과 Intersection Observer API의 특징
무한 스크롤 (Infinite Scroll)
Intersection Observer API
3. TanStack Query의 사용 이유와 특징
TanStack Query란? (구 React Query)
기존 상태 관리 라이브러리와의 차이점
TanStack Query를 사용하는 이유
isLoading
,isError
,isSuccess
등 상태를 명확하게 분리useInfiniteQuery
,getNextPageParam
등 제공사용 예시 (React, Next.js 공통)