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

V1.1.0 Develop - 상세페이지 북마크 #89

Merged
merged 9 commits into from
Sep 11, 2024
Merged

V1.1.0 Develop - 상세페이지 북마크 #89

merged 9 commits into from
Sep 11, 2024

Conversation

seondal
Copy link
Member

@seondal seondal commented Sep 3, 2024

What is this PR? 🔍

상세페이지 북마크 기능 구현

Changes 📝

  • 클라이언트 쿠키는 window 객체가 존재할 때만 접근하도록 변경
  • 상세페이지 메타데이터 생성용으로 만들어뒀던 코드는 잠시 주석처리했습니다
  • 앱 설치 배너가 있을 때 상세페이지에서 아래 영역이 확보되지 않던 이슈를 해결했습니다
  • 다른페이지에 갓다가 마이포즈로 돌아오면 마이포즈 탭 카운트가 0이 되어버리는 이슈를 수정했습니다.

Screenshot 📷

https://discord.com/channels/1156286498400899082/1272798460357246977/1272798705136963717

@seondal seondal added ✨ Feat 새로운 기능 개발 MyPose 🔖 북마크 labels Sep 3, 2024
@seondal seondal requested a review from guesung September 3, 2024 19:18
@seondal seondal self-assigned this Sep 3, 2024
@seondal seondal added Detail 👀 상세페이지 and removed MyPose 🔖 북마크 labels Sep 3, 2024
Copy link
Collaborator

@guesung guesung left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ~

코드확인했습니다 !

export const usePoseDetailQuery = (
{ poseId }: { poseId: number },
options?: UseQueryOptions<PoseDetailResponse>
) => useQuery<PoseDetailResponse>(['poseId', poseId], () => getPoseDetail(poseId), { ...options });
Copy link
Collaborator

@guesung guesung Sep 10, 2024

Choose a reason for hiding this comment

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

{...options}로 넘겨준 이유가 따로 있을까요 ?

options로 넘겨줘도 괜찮을 거 같아서요 !

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 이전에 다른 속성을 함께 작성했다가 지웠는데 {}를 없애는걸 까먹고 있었네요 ...! 알려주셔서 감사합니다! 반영했습니다 :)

Comment on lines +12 to +13
width?: number;
height?: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

width와 height는 선택 조건으로 바뀐건가요 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

네! 백엔드에서 같은 포즈데이터에 대해 상세페이지에서는 width와 height를 보내주지 않고, 피드에서는 보내주는 형태라서 변경했습니다 :) 현재 백엔드에 요청드린 상태인데, 추후에 상세페이지에도 사이즈 정보를 같이 보내주게 바꿔주시면 그때 다시 바꿀 에정입니답

@@ -20,6 +20,7 @@ export function MainFooter({ children, grow = true }: MainFooterI) {
{isIOS() && <AppDownloadBanner />}
</div>
<div className="h-88" />
{isIOS() && <div className="h-62" />}
Copy link
Collaborator

Choose a reason for hiding this comment

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

iOS는 높이가 62px 더 필요한가요 ?

Copy link
Member Author

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 13
// 상세페이지 메타데이터 생성
// export async function generateMetadata({ params }: { params: { id: string } }): Promise<Metadata> {
// const id = parseInt(params.id);
// const {
// poseInfo: { peopleCount, frameCount, tagAttributes },
// } = await getPoseDetail(id);
// const description = `${tagAttributes},${frameCount}컷,${peopleCount}인 포즈추천`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

머지될 PR에는 주석이 없는 게 좋을 거 같아요.

나중에 제거될 코드인가요, 혹은 수정할 코드인가요 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

기존에 있던 코드인데 해당 기능이 완성되기 전까지는 작동하지 않는 부분이라 빌드할 때 경고가 떠서 주석처리하였습니다ㅠㅠ 메인에 머지할때는 지우겠습니답

@@ -5,10 +5,12 @@ export const setClientCookie = (key: string, value: string, options?: { expires?
};

export const getClientCookie = (key: string) => {
const value = `; ${document.cookie}`;
const parts = value.split(`; ${key}=`);
if (typeof window !== 'undefined') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이미 만들어둔 isServer를 활용해도 좋을 것 같아요.

if (isServer) {

Copy link
Member Author

Choose a reason for hiding this comment

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

있는지 몰랐네요..! 반영하겠습니다아!!

@seondal seondal requested a review from guesung September 11, 2024 07:23
Copy link
Collaborator

@guesung guesung left a comment

Choose a reason for hiding this comment

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

확인했습니다 ! 수고하셨습니다 :)

@seondal seondal merged commit bb74e27 into develop Sep 11, 2024
1 check passed
@seondal seondal deleted the v1.1.0 branch September 11, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Detail 👀 상세페이지 ✨ Feat 새로운 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants