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

[Feat] 마이페이지 구매후기, 상품 상세 리뷰 모달, 이미지 리스트, 공유 버튼 구현 #158

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

ggjiny
Copy link
Contributor

@ggjiny ggjiny commented Jul 12, 2024

🚀 작업 내용

  • 마이페이지 구매 후기 (수정, 삭제 버튼 외 완료)
  • 상품 상세 이미지 리스트 구현
  • 페이지네이션 연결
  • 이미지 누르면 나오는 모달 구현
  • 공유 버튼 구현

📝 참고 사항

  • 리뷰 모달과 좋아요 옵티미스틱 업데이트 하다가 정신이 나가버릴거 같아서 보류했습니다..
    • 혹시 조언해주실 분 계시면 다 직접 설명해드릴 수 있으니,, 환영합니다 !!

🖼️ 스크린샷

image
image

🚨 관련 이슈 (이슈 번호)

✅ 체크리스트

  • Code Review 요청
  • Label 설정
  • PR 제목 규칙에 맞는지 확인

@ggjiny ggjiny added 📖 Page 페이지 제작 📬 API 서버 api 통신 ✨ Component 기능 개발 labels Jul 12, 2024
@ggjiny ggjiny self-assigned this Jul 12, 2024
@ggjiny ggjiny linked an issue Jul 12, 2024 that may be closed by this pull request
'Content-Type': 'application/json',
Authorization: `Bearer ${token}`,
const res = await fetch(
`${BASE_URL}/api/v1/order?page=0&size=10&startDate=2024-07-01T00:00:00&endDate=2024-07-30T23:59:59`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

이거 안넣으면 오류나서 넣었는데 필요없다면 지우겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

나중에 제가 수정할게요~

Comment on lines +34 to +38
useEffect(() => {
setIsChecked(isLiked);
setNewLikeCount(likeCount);
}, [isLiked, likeCount]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이걸 안넣으면 모달에서 오른쪽, 왼쪽으로 넘겼을 때 업데이트가 안되던데,,, 왤까요..?

Copy link
Contributor

Choose a reason for hiding this comment

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

오호,, 나중에 머지 되고 함 봐볼게욧

Copy link
Contributor

@armd482 armd482 Jul 15, 2024

Choose a reason for hiding this comment

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

사용처에서 key = id로 넣으면 해결 가능할 것 같아요!

//ReviewItem.tsx

<ReviewLikeButton id={id} key={id} isLiked={likedByUser} likeCount={likeCount} />

Copy link
Contributor

@bokeeeey bokeeeey left a comment

Choose a reason for hiding this comment

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

전체적으로 data라는 네이밍으로 사용하셨는데 어떤 데이터인지 더 명확하게 표현해주시면 좋을거같아요

'Content-Type': 'application/json',
Authorization: `Bearer ${token}`,
const res = await fetch(
`${BASE_URL}/api/v1/order?page=0&size=10&startDate=2024-07-01T00:00:00&endDate=2024-07-30T23:59:59`,
Copy link
Contributor

Choose a reason for hiding this comment

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

나중에 제가 수정할게요~

Comment on lines +28 to +34
const {
sort = 'createdAt',
page = '0',
size = '10',
startDate = '2024-01-01T00:00:00',
endDate = '2024-12-31T23:59:59',
} = params;
Copy link
Contributor

Choose a reason for hiding this comment

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

endDate는 현재시간 받아와서 사용하셔도 좋을거같아요~

Comment on lines +22 to +24
startDate: '2024-01-01T00:00:00',
endDate: '2024-12-31T23:59:59',
});
Copy link
Contributor

Choose a reason for hiding this comment

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

endDate는 현재시간을 받아와서 사용하셔도 좋을거같습니다~

Comment on lines +25 to +27

if (!productDetailData) return null;

Copy link
Contributor

Choose a reason for hiding this comment

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

null만 보여주는게 UX 차원에서 맞는걸까요?


if (!productDetailData) return null;

const { name, thubmnailList, categoryName } = productDetailData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const { name, thubmnailList, categoryName } = productDetailData;
const { name, thubmnailList, categoryName } = productDetailData ?? {};

이런식으로 empty case로 보여주는게 나을거같아요

size: searchParams.size || '16',
};

const data = await getUserProductReviews(initialParams);
Copy link
Contributor

Choose a reason for hiding this comment

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

개인적으로 data라는 네이밍이 조금 애매한거같아요

Copy link
Contributor

Choose a reason for hiding this comment

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

페이지에서 prefetch 하고계신건가요? 캐싱으로 prefetch 하시는게 더 나을거같아요

@@ -19,7 +19,7 @@ interface TabData {
interface DetailTabProps {
detailsImg: string;
reviewData: ProductReviewType;
productId: string;
productId: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

productId.toString() 메서드로 받으시는경우 있는거같은데

Suggested change
productId: number;
productId: number | string;

그냥 함수에서 둘다받아도 상관없을거같아요

Comment on lines +27 to +31
const { data } = useQuery<ProductReviewType>({
queryKey: ['review', productId, currentReviewId],
queryFn: () => getProductReviews({ productId }),
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const { data } = useQuery<ProductReviewType>({
queryKey: ['review', productId, currentReviewId],
queryFn: () => getProductReviews({ productId }),
});
const { data: 이건 어떤 데이터인가요? } = useQuery<ProductReviewType>({
queryKey: ['review', productId, currentReviewId],
queryFn: () => getProductReviews({ productId }),
});

import { useOutsideClick } from '@/hooks/useOutsideClick';
import { KakaoIcon, LinkCopyIcon } from '@/public/index';
import { ProductType } from '@/types/ProductTypes';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { ProductType } from '@/types/ProductTypes';
import type { ProductType } from '@/types/ProductTypes';

@@ -1,14 +1,19 @@
'use client';

import { ShareIcon } from '@/public/index';
import { ProductType } from '@/types/ProductTypes';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { ProductType } from '@/types/ProductTypes';
import type { ProductType } from '@/types/ProductTypes';

Copy link
Contributor

@minjeong9919 minjeong9919 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 MyReviewProduct({ productId, updatedAt, switchOption }: MyReviewProductProps) {
const { data: productDetailData } = useQuery<ProductType>({
queryKey: ['product-detail', productId],
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 저 모든 키를 camel 로 작성하긴 했는데 통일시키지 않아도 상관없겟져?!!

Copy link
Contributor

@armd482 armd482 Jul 15, 2024

Choose a reason for hiding this comment

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

다른 getProductDetail 불러오는 함수 query키 일치시키는 게 좋을 것 같아요.
OptionEditModal 컴포넌트의 경우 아래처럼 사용하고 있어요

  queryKey: [`product-${productId}`],
  queryFn: () => getProductDetail(String(productId)),

두 방식 중에 하나로 하는 게 좋을 것 같습니다~

}

export default function ProductReviewList({ data, productId }: ProductReviewListProps) {
const { reviewDtoList, ...previewData } = data;
const allReviewImgs = reviewDtoList.flatMap((review) => review.reviewImgs);
const allReviewImgs = reviewDtoList.flatMap((review) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

flatMap이라닛 첨 보는 신기한 친구군뇨

disabled: isNextDisabled,
})}
>
{'>'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Icon안에 추가적으로 text 넣으신 이유가 무엇인가용?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

예리하시네유... ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ div로 했다가 바꿔서 남아있나봐용,,

Comment on lines +34 to +38
useEffect(() => {
setIsChecked(isLiked);
setNewLikeCount(likeCount);
}, [isLiked, likeCount]);

Copy link
Contributor

Choose a reason for hiding this comment

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

오호,, 나중에 머지 되고 함 봐볼게욧

const boxRef = useRef<HTMLDivElement>(null);

const shareUrl = window.location.href;

useOutsideClick(boxRef, () => {
handleClick();
Copy link
Contributor

Choose a reason for hiding this comment

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

prop으로 받으시는거면 OnClickClose,,? 같은 동작의 의미가 담겨져있는 네이밍ㅇ은 호옥시 어떠신지,,

Copy link
Contributor

@armd482 armd482 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~ 낙관적 업데이트는 잘 안되는 부분 알려주시면 아는데까진 도움을 줄 수 있습니다...(사실 저도 잘 모름/)

const [currentSectionIndex, setCurrentSectionIndex] = useState(0);

const isPrevDisabled = currentSectionIndex === 0;
const isNextDisabled = currentSectionIndex === Math.floor(reviewImgs.length / SECTION_SIZE) - 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

1 안 빼도 될 것 같아요. reviewImgs가 빈 배열일 때 -1이라서 제대로 표시가 안되고 있어요!

Suggested change
const isNextDisabled = currentSectionIndex === Math.floor(reviewImgs.length / SECTION_SIZE) - 1;
const isNextDisabled = currentSectionIndex === Math.floor(reviewImgs.length / SECTION_SIZE);

Comment on lines +37 to +41
const handleMovePrevSection = () => {
if (currentSectionIndex > 0) setCurrentSectionIndex((prev) => prev - 1);
};
const handleMoveNextSection = () => {
if (currentSectionIndex < Math.floor(reviewImgs.length / SECTION_SIZE)) setCurrentSectionIndex((prev) => prev + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

isPrevDisabled하고 isNextDisabled를 등호가 아닌 부등호로 하면 다음과 같이 적용할 수 있을 것 같아요!

Suggested change
const handleMovePrevSection = () => {
if (currentSectionIndex > 0) setCurrentSectionIndex((prev) => prev - 1);
};
const handleMoveNextSection = () => {
if (currentSectionIndex < Math.floor(reviewImgs.length / SECTION_SIZE)) setCurrentSectionIndex((prev) => prev + 1);
//const isPrevDisabled = currentSectionIndex <= 0;
//const isNextDisabled = currentSectionIndex >= Math.floor(reviewImgs.length / SECTION_SIZE) ;
const handleMovePrevSection = () => {
if (!isPrevDisabled) {
setCurrentSectionIndex((prev) => prev - 1);
}
};
const handleMoveNextSection = () => {
if (!isNextDisabled) {
setCurrentSectionIndex((prev) => prev + 1);
}

Comment on lines +34 to +38
useEffect(() => {
setIsChecked(isLiked);
setNewLikeCount(likeCount);
}, [isLiked, likeCount]);

Copy link
Contributor

@armd482 armd482 Jul 15, 2024

Choose a reason for hiding this comment

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

사용처에서 key = id로 넣으면 해결 가능할 것 같아요!

//ReviewItem.tsx

<ReviewLikeButton id={id} key={id} isLiked={likedByUser} likeCount={likeCount} />

Copy link
Contributor

@Young2un Young2un left a comment

Choose a reason for hiding this comment

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

고민인 부분이 어딜까용?? ㅠㅠ 고생하셨어용 ㅠㅠ
공식문서 당연히 보셨겠지만,, 일단 첨부

Comment on lines +7 to +11
declare global {
interface Window {
Kakao: Kakao;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

오홍 신기해요 카카오에서 쓰란 방식이겠죠?

Comment on lines +36 to +42
export interface ReviewSearchParams {
totalElements: number;
totalPages: number;
first: boolean;
last: boolean;
currentPage: number;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

페이지네이션 타입으로 함써보까나요?

Comment on lines +56 to +60
totalElements: number;
totalPages: number;
first: boolean;
last: boolean;
currentPage: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

위에랑 겹치는고 같아용!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📬 API 서버 api 통신 ✨ Component 기능 개발 📖 Page 페이지 제작
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 마이페이지 구매후기
5 participants