-
Notifications
You must be signed in to change notification settings - Fork 22
평점/찜 기능 구현 및 프로젝트 구조 개선 / 이샘물 #45
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?
Conversation
- constants 폴더생성 및 상수 관리 체계화 - images.jsx: TMDB 이미지URL상수화 및 getImageUrl 유틸함수 적용 - menu.jsx: SideMenu 카테고리 데이터 분리 - Barrel Export 패턴 적용 (components/common/index.js) - SideMenu 구조개선(탭 아래 하위메뉴 배치)
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.
리뷰 한 번 확인 부탁드려요 !!!
| export const truncateText = (text, wordLimit) => { | ||
| if (!text) return ""; | ||
| const words = text.split(" "); | ||
| if (words.length <= wordLimit) return text; | ||
| return words.slice(0, wordLimit).join(" ") + "..."; | ||
| }; |
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.
이 부분은 utils로 파일 따로 빼서 사용하면 더 좋을 것 같아요 !
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.
넵 반영하겠습니다!
src/components/Banner/index.jsx
Outdated
| @@ -0,0 +1,58 @@ | |||
| import React from "react"; | |||
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.
요 부분은 React 17+부터는 JSX를 사용할 때 import React from 'react'가 필수가 아닙니다.
따라서 지워주시면 좋아요!
| {loading ? ( | ||
| <span style={{ color: "white" }}>...</span> | ||
| <Typography variant="bodySmall">...</Typography> | ||
| ) : user ? ( | ||
| <> | ||
| <UserMenu> | ||
| <Icon icon={faUser} className="user" label="사용자" /> | ||
| <Icon | ||
| name="user" | ||
| className="user-icon" | ||
| size="24px" | ||
| label="사용자" | ||
| /> | ||
| <div className="dropdown"> | ||
| <Link to="/profile">내 프로필</Link> | ||
| <button onClick={handleLogout}>로그아웃</button> | ||
| <Link to="/profile"> | ||
| <Typography variant="bodySmall">내 프로필</Typography> | ||
| </Link> | ||
| <button onClick={handleLogout}> | ||
| <Typography variant="bodySmall">로그아웃</Typography> | ||
| </button> | ||
| </div> | ||
| </UserMenu> | ||
| <Icon icon={faBell} className="bell" label="알림" /> | ||
| <BellIcon> | ||
| <Icon name="bell" size="24px" label="알림" /> | ||
| </BellIcon> | ||
| </> | ||
| ) : ( | ||
| <Link | ||
| to="/login" | ||
| style={{ color: "white", textDecoration: "none" }} | ||
| > | ||
| <Icon icon={faUser} className="user" label="로그인" /> | ||
| <Link to="/login" style={{ textDecoration: "none" }}> | ||
| <Icon | ||
| name="user" | ||
| size="24px" | ||
| label="로그인" | ||
| className="user-icon" | ||
| /> | ||
| </Link> | ||
| )} |
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.
이렇게 중첩 삼항 연산자 사용하는 것은 좋지 않습니다.
변경 예시
if (loading) {
return (
<Typography variant="bodySmall">...</Typography>
);
}
if (user) {
return (
<>
<UserMenu>
<Icon
name="user"
className="user-icon"
size="24px"
label="사용자"
/>
<div className="dropdown">
<Link to="/profile">
<Typography variant="bodySmall">내 프로필</Typography>
</Link>
<button onClick={handleLogout}>
<Typography variant="bodySmall">로그아웃</Typography>
</button>
</div>
</UserMenu>
<BellIcon>
<Icon name="bell" size="24px" label="알림" />
</BellIcon>
</>
);
}
return (
<Link to="/login" style={{ textDecoration: "none" }}>
<Icon
name="user"
size="24px"
label="로그인"
className="user-icon"
/>
</Link>
);
};
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 allMovies = data?.pages.flatMap((page) => page.results) || []; | ||
| const allMovies = | ||
| data?.pages | ||
| .flatMap((page) => page.results.slice(0, ITEMS_PER_PAGE)) |
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.
저번에 말씀 드렸던 flatMap 사용 좋습니다 !!! 굳굳
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에서 미리 데이터를 가공해서 가지고 나왔으면 컴포넌트에서 사용할 때 조금 더 깔끔해질 것 같아요 !
ex)
useInfiniteMovies 내부에서
const ITEMS_PER_PAGE = 18; 선언을 해준뒤
const allMovies =
data?.pages
.flatMap((page) => page.results.slice(0, ITEMS_PER_PAGE)).filter((movie) => movie.poster_path) || [];
데이터 가공을 해주고 return을 data 대신 allMovies를 return 시켜주는 방식으로 !
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.
아하 넵! 그리드 통합했으니, 아예 지정해서 사요하는걸로 수정하겠습니다!
src/components/SideMenu/index.jsx
Outdated
| {/* 하위 메뉴 (active된 탭만 보임) */} | ||
| <MenuList | ||
| $visible={activeCategory === key} | ||
| key={activeCategory === key ? key : undefined} |
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.
요 부분 이제 key 값은 반복문 안에서만 의미가 있는 부분인데 여기다 사용하신 이유가 궁금합니다 !!
제 생각에는 지워도 되지 않을까 싶어요 !
| const renderDisplayStars = (currentRating, size = "30px") => | ||
| [1, 2, 3, 4, 5].map((star) => ( | ||
| <div key={star} style={{ display: "flex" }}> | ||
| <Icon | ||
| name="starSolid" | ||
| size={size} | ||
| color={star <= currentRating ? "#ff1a66" : "d9d9d9"} | ||
| /> | ||
| </div> | ||
| )); |
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.
StarDisplay 컴포넌트로 분리하면 조금 더 깔끔해질 것 같아요 !
| {rating > 0 | ||
| ? `내가 준 평점: ${rating * 2}점` | ||
| : user | ||
| ? "아직 평점을 주지 않았습니다" | ||
| : "로그인 후 평점을 남겨보세요"} |
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.
중첩 삼항 사용은 좋지 않습니다 !
수정 방향 ex)
let message = "";
if (rating > 0) {
message = `내가 준 평점: ${rating * 2}점`;
} else if (user) {
message = "아직 평점을 주지 않았습니다";
} else {
message = "로그인 후 평점을 남겨보세요";
}
return <Typography variant="body">{message}</Typography>;
or
const getRatingMessage = () => {
if (rating > 0) return `내가 준 평점: ${rating * 2}점`;
if (user) return "아직 평점을 주지 않았습니다";
return "로그인 후 평점을 남겨보세요";
};
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.
오호...message...제가 생각보다 중첩삼항을 많이쓰네요ㅎㅎ;;;
src/pages/Layout/index.jsx
Outdated
| @@ -1,5 +1,5 @@ | |||
| import React from "react"; | |||
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.
이 부분 react 19에서는 사용 안해도 됩니다 !! 지워 주시는게 좋아요
src/pages/MainPage/index.jsx
Outdated
| @@ -1,6 +1,7 @@ | |||
| import React from "react"; | |||
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 Header from "@/components/Header"; | ||
| import Banner from "@/components/Banner"; | ||
| import PopularMovies from "@/components/PopularMovies"; | ||
| import TopRankedMovie from "@/components/TopRankedMovie"; |
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에 넣어서 한 번에 묶어 사용하면 깔끔해질 것 같아요!
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.
엇 이건 수정을 안했네요! 넵 이것도 반영하겠습니다
- MovieRating 중첩 삼항연산자 제거 및 함수 분리 - MovieOTT 모바일 반응형 레이아웃 수정 - SearchPage 초기화면 및 빈 결과 처리 추가 - StarRating 컴포넌트 통합 및 interactive prop 추가 - Header 컴포넌트에서 UserSection 컴포넌트 분리 - Util/text.jsx 분리해서 truncateText 관리개선
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.
리뷰 확인해주세요 !
| : null; | ||
| const { data, loading } = useFetchData(endpoint); | ||
|
|
||
| const results = data?.results || []; | ||
| const results = data?.results?.filter((movie) => movie.poster_path) || []; | ||
|
|
||
| const handleClick = (id) => { | ||
| navigate(`/movie/${id}`); | ||
| const renderEmptyState = () => { | ||
| if (loading) { | ||
| return <SearchEmpty type="loading" />; | ||
| } | ||
|
|
||
| if (!searchTerm || searchTerm.trim() === "") { | ||
| return <SearchEmpty type="empty" />; | ||
| } | ||
|
|
||
| if (results.length === 0) { | ||
| return <SearchEmpty type="noResults" searchTerm={searchTerm} />; | ||
| } | ||
| return null; | ||
| }; | ||
|
|
||
| if (loading) { | ||
| return <Typography>Loading...</Typography>; | ||
| } | ||
| const emptyState = renderEmptyState(); | ||
|
|
||
| if (!debouncedQuery) { | ||
| return <Typography>검색어를 입력해주세요.</Typography>; | ||
| if (emptyState) { | ||
| return <PageContainer>{emptyState}</PageContainer>; |
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 content = { | ||
| loading: { | ||
| title: "검색 중...", | ||
| description: null, | ||
| }, | ||
| empty: { | ||
| title: "영화를 검색해보세요.", | ||
| description: "제목, 장르, 감독 등으로 검색할 수 있습니다.", | ||
| }, | ||
| noResults: { | ||
| title: `"${searchTerm}"에 검색 결과가 없습니다.`, | ||
| description: "다른 검색어로 시도해보세요.", | ||
| }, | ||
| }; |
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.
이 부분은 constants로 분리 시켜주면 좋을 것 같아요 !
다만 noResults의 경우 props 기반 동적 값이라서 조금 다르게 분리를 시켜줘야 합니다.
힌트는 함수로 분리 입니다 !! 한 번 해보시면 좋을 것 같아요
| useEffect(() => { | ||
| const fetchReviews = async () => { | ||
| if (!movieId) return; | ||
|
|
||
| try { | ||
| const { data, error } = await supabase | ||
| .from("reviews") | ||
| .select("*") | ||
| .eq("movie_id", movieId) | ||
| .order("created_at", { ascending: false }); | ||
|
|
||
| if (error) throw error; | ||
|
|
||
| setReviews(data || []); | ||
|
|
||
| // 내 리뷰 찾기 | ||
| if (user) { | ||
| const mine = data?.find((r) => r.user_id === user.id); | ||
| setMyReview(mine || null); | ||
| } | ||
| } catch (error) { | ||
| console.error("Error fetching reviews:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchReviews(); | ||
| }, [movieId, user]); |
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.
이 부분 리팩토링 단계에서 tanstack query 사용해서 수정 해보시면 좋을 것 같아요!
- DetailPage 코드 간결화 및 탭 설정 분리 - MovieRating 반응형 개선 - MovieOTT 모바일 레이아웃 수정 - MovieReview 페이지 구현 - 가로 스크롤 문제 해결 - MovieGrid 컴포넌트 통합
평점과 찜 기능을 추가하고, 프로젝트 전체 구조를 개선했습니다.
구현 사항
1. 평점 시스템
2. 찜 기능
3. DetailPage 개선
어려웠던 점
(1) Supabase와 훅에 적용한 컬럼명 확인 후 수정
(2)
.single()→.maybeSingle()변경