-
Notifications
You must be signed in to change notification settings - Fork 22
Mission 5 박한별 #54
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: 박한별
Are you sure you want to change the base?
Mission 5 박한별 #54
Conversation
…ssion-2 [mission2] 2단계 미션 구현 - 안지선 / 1팀
[Mission 2] 2단계 미션 구현 - 박진우A / 2팀
han-nun0107
left a comment
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.
코드리뷰 확인해 주세요 !
추가로 전체적인 파일 구조가 컴포넌트 안에 api 호출, 비즈니스 로직 등이 너무 과하게 있어 분리하는 작업이 필요합니다. 각 폴더 별로 나눠서 로직 분리 작업 진행해 주시면 좋을 것 같아요 !
| import Layout from "@components/Layout"; | ||
| import OAuthCallback from "@components/OAuthCallback"; |
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에 barrel 만들어서 사용하면 더 좋을 것 같아요
| import OAuthCallback from "@components/OAuthCallback"; | ||
|
|
||
| import { useSupabaseAuth } from "@sb"; | ||
| import { UserContext } from "@sbCtx/UserContext"; |
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.
이 부분은 context라는 폴더로 만들어서 관리하면 좋습니다! sbCtx라는 명은 너무 불명확 합니다.
| import { useNavigate } from "react-router-dom"; | ||
| import { useUser } from "@sbCtx/UserContext"; | ||
| import { useBookmarks } from "@/context/BookmarkContext"; | ||
| import { Button } from "."; |
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.
절대 경로로 불러오면 더 좋을 것 같아요 !
| import { useBookmarks } from "@/context/BookmarkContext"; | ||
| import { Button } from "."; | ||
|
|
||
| const baseUrl = "https://image.tmdb.org/t/p/w500"; |
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.
baseUrl은 constants 폴더에 Urls라는 파일 만들어서 관리하면 더 깔끔해질 것 같아요 !
| e.stopPropagation(); | ||
|
|
||
| if (!user) { | ||
| alert("로그인이 필요합니다!"); |
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.
alert는 ux 적으로 좋지 않기 때문에 react toastify와 같은 토스트 라이브러리 사용하는 것이 좋아요!
|
|
||
| {/* 평점 */} | ||
| <span className="absolute bottom-2 left-2 bg-black/70 text-white text-sm px-2 py-1 rounded"> | ||
| ⭐ {vote_average} |
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.
평점의 경우 toFixed 넣어줘서 반올림 해주면 보기 더 좋습니다 !
| const [bookmarks, setBookmarks] = useState([]); | ||
|
|
||
| // localStorage에서 복원 | ||
| useEffect(() => { | ||
| const saved = localStorage.getItem("bookmarks"); | ||
| if (saved) setBookmarks(JSON.parse(saved)); | ||
| }, []); | ||
|
|
||
| // 저장 | ||
| useEffect(() => { | ||
| localStorage.setItem("bookmarks", JSON.stringify(bookmarks)); | ||
| }, [bookmarks]); | ||
|
|
||
| const toggleBookmark = (movie) => { | ||
| setBookmarks((prev) => { | ||
| const exists = prev.some((m) => m.id === movie.id); | ||
| if (exists) { | ||
| return prev.filter((m) => m.id !== movie.id); | ||
| } | ||
| return [...prev, movie]; | ||
| }); | ||
| }; | ||
|
|
||
| const removeBookmark = (id) => { | ||
| setBookmarks((prev) => prev.filter((m) => m.id !== id)); | ||
| }; | ||
|
|
||
| const isBookmarked = (id) => bookmarks.some((m) => m.id === id); |
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.
이 부분은 비즈니스 로직 분리 해주면 좋을 것 같아요 !
| const [featuredMovies, setFeaturedMovies] = useState([]); | ||
| const [movies, setMovies] = useState([]); | ||
| const [loading, setLoading] = useState(true); | ||
| const movieIdsRef = useRef(new Set()); | ||
| const [page, setPage] = useState(1); | ||
| const [loading, setLoading] = useState(false); | ||
| const [isEnd, setIsEnd] = useState(false); | ||
|
|
||
| const navigate = useNavigate(); | ||
| const loaderRef = useRef(null); | ||
|
|
||
| const loadFeaturedMovies = async () => { | ||
| const popular = await fetchPopularMovies(1); | ||
| const shuffled = popular.sort(() => Math.random() - 0.5); | ||
| setFeaturedMovies(shuffled.slice(0, 20)); | ||
| }; | ||
|
|
||
| const loadMovies = useCallback(async () => { | ||
| if (loading || isEnd) return; | ||
|
|
||
| setLoading(true); | ||
| const newMovies = await fetchPopularMovies(page); | ||
|
|
||
| if (!newMovies || newMovies.length === 0) { | ||
| setIsEnd(true); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| const filtered = newMovies.filter( | ||
| (movie) => !movieIdsRef.current.has(movie.id) | ||
| ); | ||
|
|
||
| if (filtered.length === 0) { | ||
| setPage((prev) => prev + 1); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| filtered.forEach((movie) => movieIdsRef.current.add(movie.id)); | ||
|
|
||
| setMovies((prev) => [...prev, ...filtered]); | ||
| setPage((prev) => prev + 1); | ||
|
|
||
| setLoading(false); | ||
| }, [page, loading, isEnd]); | ||
|
|
||
| useEffect(() => { | ||
| fetchPopularMovies() | ||
| .then((data) => setMovies(data)) | ||
| .finally(() => setLoading(false)); | ||
| loadFeaturedMovies(); | ||
| loadMovies(); | ||
| }, []); | ||
|
|
||
| if (loading) { | ||
| return ( | ||
| <div className="min-h-screen flex items-center justify-center"> | ||
| <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-yellow-400"></div> | ||
| </div> | ||
| useEffect(() => { | ||
| const observer = new IntersectionObserver( | ||
| (entries) => { | ||
| const target = entries[0]; | ||
| if (target.isIntersecting && !loading) { | ||
| loadMovies(); | ||
| } | ||
| }, | ||
| { threshold: 0.5 } | ||
| ); | ||
| } | ||
|
|
||
| if (loaderRef.current) observer.observe(loaderRef.current); | ||
| return () => loaderRef.current && observer.unobserve(loaderRef.current); | ||
| }, [loadMovies]); |
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.
이 부분도 컴포넌트 안에 비즈니스 로직이 너무 많아요 분리 해주는 작업이 좋습니다 !
useInfiniteMovies.ts, useFeaturedMovies.ts
이런식으로 분리 작업 해주면 좋습니다
구현 사항
어려웠던 점
구현 이미지