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/#301 best piickle API #304

Merged
merged 15 commits into from
Jul 16, 2023
Merged

Feat/#301 best piickle API #304

merged 15 commits into from
Jul 16, 2023

Conversation

ilmerry
Copy link
Member

@ilmerry ilmerry commented Jul 10, 2023

  • 브랜치명, 브랜치 알맞게 설정
  • Reviewer, Assignees, Label, Milestone, Issue(PR 작성 후에) 붙이기
  • PR이 승인된 경우 해당 브랜치는 삭제하기 close Feat/#300 best piickle #301

📌 내용

  • 베스트 북마크 랭킹, 최근 북마크된 카드조회, 성별별 북마크 카드조회 API를 연결합니다.
  • 카드 클릭시 cardCollection으로 이동하는 로직을 구현합니다.

📌 내가 알게 된 부분

  • 서버에서 넘어오는 객체 형식이 다름에 유의해야 합니다. 예를들어, 성별별 북마크 카드조회는 data: CardList[]로 바로 넘어오지만, 최근 북마크 카드조회는 data: { cardResponseDtos: CardList[] } �형태로 넘어옵니다.

📌 질문할 부분

  • useCardLists 훅의 로직에 대한 질문입니다! BestCard 컴포넌트를 클릭해서 cardCollection으로 진입하게 되면, API를 한번 더 요청해서 카드를 받아오는 형식이 맞나요? 결론적으로 API를 두 번 호출하는 것 같은데 제가 이해한게 맞는지 알고 싶고 또 그렇게 한 이유가 궁금해요!

📸 스크린샷

@ilmerry ilmerry requested review from joohaem and NYeonK July 10, 2023 06:10
@ilmerry ilmerry self-assigned this Jul 10, 2023
@ilmerry ilmerry linked an issue Jul 10, 2023 that may be closed by this pull request
3 tasks
Copy link
Contributor

@NYeonK NYeonK left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다💘🥰👍

피클 코드 아직 적응 중일텐데 그냥 뚝딱해버리네!! ⭐멋있다

@@ -25,7 +25,7 @@ export default function RecommendItem(props: RecommendProps) {
bestPiickle={cards}
idx={idx}
canNavigate={!isDragging}
isLast={idx !== 3}
isLast={idx === 3}
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
Member Author

Choose a reason for hiding this comment

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

아 이거는 BestPiickleCard 컴포넌트 내부에서 isLast로 분기처리를 잘못해주고 있어서, !isLast로 바꾸면서 변경한겁니다!

Copy link
Member

Choose a reason for hiding this comment

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

여기도 숫자 const 상수로 선언해서 사용하면 좋을 것 같아요.
3을 직접 선언않고, 리스트 길이로 선언하는게, 데이터 정합성이 만족될 것 같고요!

Comment on lines 14 to 15
padding: 0.8rem 1.2rem;
padding-top: 2.1rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

padding: 2.9rem 1.2rem 0.8rem으로 합치면 되겠다!

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 감사해요!

</St.Content>
<St.LastCardButton>나머지 보기</St.LastCardButton>
</St.LastCard>
<LastBestPiickleCard />
Copy link
Contributor

Choose a reason for hiding this comment

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

따로 컴포넌트로 빼니까 더 깔끔하네요!-! 구웃

<St.ButtonWrapper>
<St.ContinueButton>이어서 베스트 피클 카드 보기</St.ContinueButton>
<St.ContinueButton onClick={() => navigateRankCollection(8)}>이어서 베스트 피클 카드 보기</St.ContinueButton>
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 왜 8인지 몰라서 기능명세서 보고 왔쟈나~ 똑순이!👑👍

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 +47 to +48
case LocationType.RECENT:
return data?.data.data.cardResponseDtos;
Copy link
Contributor

Choose a reason for hiding this comment

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

data?.data.data.cardResponseDtos 예리했다.. 라이브코딩 자주 해주세요✨✨

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 +13 to +14
CARDS_RECENT: "/recentlyBookmarkedCard",
CARDS_GENDER: "/cardByBookmarkedGender",
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 우리 같이 고민했었잖아👀
근데 코드 보다보니까 어차피 /cards/ 뒤에만 쓰이는 거라 간단하게 /recent, /gender 해도 될 것 같기도 하네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

근데 이게 서버 명세서 url이랑 동일해야하나봐! recent, gender로 하면 오류가 나네!

Copy link
Contributor

Choose a reason for hiding this comment

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

아 맞네🥲🥲🥲

Base automatically changed from feat/#300-best_piickle to release/2.1.0 July 16, 2023 12:40
@ilmerry ilmerry merged commit 7248757 into release/2.1.0 Jul 16, 2023
@ilmerry ilmerry deleted the feat/#301-best_piickle branch July 16, 2023 12:41
<RankItem cardId="1" content="상대방의 첫인상을 기억하나요?" rank={8} />
{bestPiickle &&
[...bestPiickle.data]
.slice(0, 8)
Copy link
Member

Choose a reason for hiding this comment

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

8이라는 숫자에 대해서 const BEST_PIICKLE_TOTAL_COUNT = ~ 처럼 정의해두면 읽기 좀 더 편하지 않을까 해요!

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 의견이에요! 다른 브랜치에서 반영해보겠습니다:)

@@ -25,7 +25,7 @@ export default function RecommendItem(props: RecommendProps) {
bestPiickle={cards}
idx={idx}
canNavigate={!isDragging}
isLast={idx !== 3}
isLast={idx === 3}
Copy link
Member

Choose a reason for hiding this comment

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

여기도 숫자 const 상수로 선언해서 사용하면 좋을 것 같아요.
3을 직접 선언않고, 리스트 길이로 선언하는게, 데이터 정합성이 만족될 것 같고요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Best Piickle ] 베스트피클 페이지 API 연동
3 participants