Conversation
Week2/yunsl m1
There was a problem hiding this comment.
Vite로 생성 시 기본적으로 제공되는 스타일은 사용되지 않으니 제거해 주시는 것이 좋습니다 !
There was a problem hiding this comment.
현재 .env 파일이 저장소에 포함되어 API 키가 노출되어있습니다! 보안상 위험하기 때문에, 기존 키는 폐기하시고 새로 발급받아 사용해주세요 🙂 또한 재발급 하시고 .env 파일이 GitHub에 업로드되지 않도록 .gitignore에 .env를 추가해주시면 이 파일은 업로드되지 않을 겁니다 !
There was a problem hiding this comment.
현재 폴더 구조에서 components/ 폴더가 mission1/ 하위에 생성된 것으로 보이는데, components/는 src/ 하위에 위치하도록 구성해주시는 것이 더 적절해 보입니다 🙂 추가로 파일 이름도 MovieCard.tsx와 같이 컴포넌트명과 일치하도록 수정해주시면, 협업 시 파일 역할을 파악하기 쉽고 일관성 있는 구조를 유지하는 데 도움이 될 것 같습니다 !
There was a problem hiding this comment.
pages/ 폴더도 마찬가지로 src/ 하위에 위치하도록 이동해주세요 !
| /> | ||
|
|
||
| {isHovered && ( | ||
| <div className='absoulte inset-0 bg-gradient-to-t from-black/50 to-transparent backdrop-blur-md flex flex-col justify-center items-center text-white p-4 ' > |
There was a problem hiding this comment.
오타로 인해 오버레이가 의도대로 동작하지 않을 수 있습니다 ! absoulte → absolute 로 수정해주세요 !
| //axios 를 활용하면 {const result=await response.json(); //json 형태로 변환 및 풀어줌} 이거 안해줘도 됨 | ||
| setMovies(data.results); //영화 데이터는 data 안에 results라는 배열로 담겨있음, 그것을 movies 상태에 저장 | ||
| }; //데이터를 호출할 때 useEffect 사용 | ||
| //const response = fetch(url); // 이렇게 쓰면 Promise라서 바로 값을 못 씀/ 그래서 async, await 사용해서 값을 받아옴 |
There was a problem hiding this comment.
현재 API 호출 시 에러 처리가 없는 것으로 보입니다. try-catch를 활용해주시면 네트워크 오류 발생 시 원인을 파악하기 더 수월할 것 같습니다 🙂 또한 API 호출뿐만 아니라, 데이터를 불러오는 동안 로딩 상태를 함께 처리해주시면 사용자 입장에서 더 직관적인 화면을 제공할 수 있을 것 같습니다 !
| // 밑에 className은 특정 크기에 따라 컬럼 수가 달라지는 그리드 레이아웃 설정 | ||
| return ( | ||
| <div className="grid grid-cols-2 sm:grid-cols-3 md:grid-cols-4 lg:grid-cols-5 xl:grid-cols-6 gap-4 p-10"> | ||
| {movies && |
There was a problem hiding this comment.
movies는 배열이라 빈 배열도 true를 반환합니다 그래서 별도의 조건문 없이 바로 렌더링하셔도 괜찮을 것 같습니다 !
{movies.map((movie) => (
<MovieCard key={movie.id} movie={movie} />
))}
| import MoviePage from '../pages/MoviePage'; | ||
|
|
||
| function App() { | ||
| console.log(import.meta.env.VITE_TMDB_KEY); // 콘솔에 환경변수 출력 |
There was a problem hiding this comment.
console문은 PR 올리기 전에 제거해주시는 것이 좋습니다 🙂 특히 환경변수와 관련된 출력은 API 키와 같은 민감한 정보가 노출될 수 있어 보안상 위험하므로 꼭 확인 후 제거해주세요 !
There was a problem hiding this comment.
현재 HomePage는 실제 메인 페이지라기보다는 Navbar와 Outlet을 포함한 레이아웃 역할을 하고 있어, HomePage보다는 Layout/ 폴더를 생성해서 Layout과 같은 이름으로 변경해서 관리하는 것이 더 적절해 보입니다 !
| onClick={()=>setPage(prev=>prev+1)}>{'>'}</button> | ||
| <span>{page} 페이지</span> | ||
| </div> | ||
| {isPending &&( |
There was a problem hiding this comment.
이렇게 하셔도 기능적으로는 문제가 없지만 구조가 조금 중복되어 가독성 떨어질 수 있습니다 !
{isPending && <div className='flex justify-center items-center h-dvh'><LoadingSpinner /></div>}
<div className='p-10 grid gap-4 grid-cols-2 sm:grid-cols-3 ...'>
{movies.map((movie) => (
<MovieCard key={movie.id} movie={movie} />
))}
</div>
이렇게 정리하시면 더 깔끔한 코드를 사용할 수 있습니다 !
| import { useEffect, useState } from "react"; | ||
| import axios from "axios"; | ||
|
|
||
| type MovieDetail = { |
There was a problem hiding this comment.
페이지 내부에 타입을 직접 정의해주신 부분이 확인되는데, 타입은 types/ 폴더로 분리하여 관리해주시면 재사용성과 가독성 측면에서 더 좋은 구조가 될 것 같습니다 🙂
| fetchData(); | ||
| }, [movieId]); | ||
|
|
||
| if (!movie) return <div className="text-white p-10">로딩중...</div>; |
There was a problem hiding this comment.
현재 movie가 없는 경우를 로딩 상태로 처리해주셨는데, 이 경우 에러 상황과 로딩 상태가 구분되지 않을 수 있습니다 ! isLoading, iserror와 같이 상태를 분리하여 처리해주시면 더 명확한 UI를 구성할 수 있을 것 같습니다 🙂 또한 로딩 컴포넌트를 제작해주셨으니, 새롭게 로딩 스타일을 작성하기보다는 해당 컴포넌트를 재사용해주시면 코드 중복을 줄이고 일관된 UI를 유지하는 데 더 좋을 것 같습니다 !
2 participants
📚 주차 / 미션
📌 작업 내용
---카테고리를 누르면 그와 관련된 영화들이 다르게 뜨고 버튼을 눌러 페이지가 넘어가게 진행. 그리고 해당 영화를 누르면 그 영화와 관련된 상세 페이지로 넘어감
✨ 상세 작업 내용
📸 스크린샷
❓ 리뷰어가 알아야 할 사항 / 질문
✅ 체크리스트