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: 전체 방 조회 API 연결 및 기능 구현 #164

Merged
merged 11 commits into from
Nov 18, 2023

Conversation

chasj0326
Copy link
Member

@chasj0326 chasj0326 commented Nov 17, 2023

🧩 이슈 번호

✅ 작업 사항

  • 전체 방 조회 msw handler 작성
  • 전체 방 조회 api 함수 및 쿼리 작성
  • 무한스크롤 기능 추가
  • 로딩 스켈레톤 UI 추가
2023._11._17._._5.13.21.mp4

영상에는 없긴 한데 불러오는 중간에 아코디언 펼쳤다 접었다 해도 잘 작동합니다 !

👩‍💻 공유 포인트 및 논의 사항

검색 기능은 아직 추가 안하고,
전체 방 불러오기 + 무한스크롤만 구현해놨습니닷
API 명세에 따라 다시 수정될 수도 있습니다 .. !

@chasj0326 chasj0326 added ☁️ FE 프론트 레포지토리에서의 작업 ✨ feature 새로운 기능에 대한 작업 labels Nov 17, 2023
@chasj0326 chasj0326 added this to the 3차 스프린트 milestone Nov 17, 2023
@chasj0326 chasj0326 self-assigned this Nov 17, 2023
Copy link

netlify bot commented Nov 17, 2023

Deploy Preview for moabam-storybook ready!

Name Link
🔨 Latest commit 58f4589
🔍 Latest deploy log https://app.netlify.com/sites/moabam-storybook/deploys/655799ba43736a0008258d0a
😎 Deploy Preview https://deploy-preview-164--moabam-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<div
ref={intersectionRef}
className="h-4"
></div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

고생하셨습니다 추후에 hasNextPage 속성을 이용해서 방이 더 이상 없다는 문구도 표시해주면 더 좋을 것 같습니다 ㄷㄷ

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 그렇네유 ! 문구 추천 부탁드립니다 꾸벅 ( _ _ )
더 이상 방이 없네요! 이거 .. ?

Copy link
Member

@bbearcookie bbearcookie left a comment

Choose a reason for hiding this comment

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

고생하셨어요!! 세진님 😁😁
만드셨던 Deffer 컴포넌트와 useIntersectionObserver 훅을 활용해서 멋진 스켈레톤 UI와 무한 스크롤을 만들어주셨군요!! 😮👍👍

onObserve: VoidFunction;
}

const useIntersectionObserver = ({
Copy link
Member

Choose a reason for hiding this comment

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

오.. IntersectionObserver API를 쉽게 활용할 수 있게 hooks로 만들어주셨군요! 👍👍

Comment on lines 23 to 28
return {
fetchNextPage,
hasNextPage,
results: data,
isFetchingNextPage
};
Copy link
Member

Choose a reason for hiding this comment

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

P3:
UseSuspenseInfiniteQueryResult 타입의 데이터 중 일부만 추출하고 export 해주셨군요!! 😮
현재 export 되는 모든 데이터는 useSuspenseInfiniteQuery 를 호출한 결과만 바로 반환해도 가져올 수 있을 것 같은데, 특정 프로퍼티만 구조 분해 할당 한 뒤에 반환하셨던 의도가 있으셨는지 궁금해요!!

data 프로퍼티는 results 라는 프로퍼티명으로 변경한 뒤에 반환되고 있는데, TanStack Query의 useInfiniteQuery 에는 해당 프로퍼티가 없다보니 TanStack Query는 알고 있지만 이 커스텀 훅은 아직 모르는 개발자가 이 훅을 사용하는 곳의 코드를 봤을 때, results 라는 프로퍼티를 알기 위해서 다시 한번 훅의 인터페이스를 살펴봐야 하는 과정이 생길 수도 있을 것 같다는 매우매우 개인적인 의견입니다!!

Copy link
Member Author

Choose a reason for hiding this comment

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

크게 생각하고 작성한 내용은 아닙니다만 허허 ,,!
커스텀 훅을 작성할 때 사용하는 값들만 리턴하는 습관이 있다 보니 (약간 가독성을 위한 ?) 그랬던 것 같아요!
이 페이지에서만 쓰는 훅이기 때문에, 쓰는 프로퍼티가 정해져 있어서 확장해서 쓸 생각은 안한 것도 있습니닷

그치만 어차피 컴포넌트 내에서 필요한 친구들만 빼서 쓰기 때문에 무관할 것 같네용
게다가 수정 횟수도 줄어들고.. ! 반영했습니다 굳.

data 도 사실 data.map => rooms.map 을 하는게 보기 어려울 것 같아서 네이밍을 바꿔치기 한 것이었는데,
상훈님이 말씀하신 걸 보니 라이브러리 표준 네이밍을 지키는 편이 훨씬 좋겠네요 !! 모두 반영합니닷 👍

Copy link
Contributor

@nayeon-hub nayeon-hub left a comment

Choose a reason for hiding this comment

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

LGTM👍🏻 intersectionObserver 훅이 인상깊네요! 쏘 깔끔 ㅠㅠ

const ResultListFallback = ({ size = 10 }: ResultListFallbackProps) => {
return (
<div className="flex flex-col gap-2 opacity-60">
{Array.from({ length: size }, (_, index) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

오 size를 인수로 받아 개수를 자유롭게 설정할 수 있군요👍🏻👍🏻

@chasj0326 chasj0326 merged commit e4224aa into main Nov 18, 2023
5 checks passed
@chasj0326 chasj0326 deleted the feat/#157/search-page-api branch November 18, 2023 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☁️ FE 프론트 레포지토리에서의 작업 ✨ feature 새로운 기능에 대한 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

방 검색 페이지 API 연결
4 participants