-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
✅ Deploy Preview for moabam-storybook ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
<div | ||
ref={intersectionRef} | ||
className="h-4" | ||
></div> | ||
</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.
고생하셨습니다 추후에 hasNextPage 속성을 이용해서 방이 더 이상 없다는 문구도 표시해주면 더 좋을 것 같습니다 ㄷㄷ
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.
아하 그렇네유 ! 문구 추천 부탁드립니다 꾸벅 ( _ _ )
더 이상 방이 없네요! 이거 .. ?
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.
고생하셨어요!! 세진님 😁😁
만드셨던 Deffer
컴포넌트와 useIntersectionObserver
훅을 활용해서 멋진 스켈레톤 UI와 무한 스크롤을 만들어주셨군요!! 😮👍👍
onObserve: VoidFunction; | ||
} | ||
|
||
const useIntersectionObserver = ({ |
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.
오.. IntersectionObserver API를 쉽게 활용할 수 있게 hooks로 만들어주셨군요! 👍👍
return { | ||
fetchNextPage, | ||
hasNextPage, | ||
results: data, | ||
isFetchingNextPage | ||
}; |
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.
P3:
UseSuspenseInfiniteQueryResult
타입의 데이터 중 일부만 추출하고 export
해주셨군요!! 😮
현재 export 되는 모든 데이터는 useSuspenseInfiniteQuery
를 호출한 결과만 바로 반환해도 가져올 수 있을 것 같은데, 특정 프로퍼티만 구조 분해 할당 한 뒤에 반환하셨던 의도가 있으셨는지 궁금해요!!
data
프로퍼티는results
라는 프로퍼티명으로 변경한 뒤에 반환되고 있는데, TanStack Query의useInfiniteQuery
에는 해당 프로퍼티가 없다보니 TanStack Query는 알고 있지만 이 커스텀 훅은 아직 모르는 개발자가 이 훅을 사용하는 곳의 코드를 봤을 때,results
라는 프로퍼티를 알기 위해서 다시 한번 훅의 인터페이스를 살펴봐야 하는 과정이 생길 수도 있을 것 같다는 매우매우 개인적인 의견입니다!!
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.
크게 생각하고 작성한 내용은 아닙니다만 허허 ,,!
커스텀 훅을 작성할 때 사용하는 값들만 리턴하는 습관이 있다 보니 (약간 가독성을 위한 ?) 그랬던 것 같아요!
이 페이지에서만 쓰는 훅이기 때문에, 쓰는 프로퍼티가 정해져 있어서 확장해서 쓸 생각은 안한 것도 있습니닷
그치만 어차피 컴포넌트 내에서 필요한 친구들만 빼서 쓰기 때문에 무관할 것 같네용
게다가 수정 횟수도 줄어들고.. ! 반영했습니다 굳.
data 도 사실 data.map => rooms.map 을 하는게 보기 어려울 것 같아서 네이밍을 바꿔치기 한 것이었는데,
상훈님이 말씀하신 걸 보니 라이브러리 표준 네이밍을 지키는 편이 훨씬 좋겠네요 !! 모두 반영합니닷 👍
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.
LGTM👍🏻 intersectionObserver 훅이 인상깊네요! 쏘 깔끔 ㅠㅠ
const ResultListFallback = ({ size = 10 }: ResultListFallbackProps) => { | ||
return ( | ||
<div className="flex flex-col gap-2 opacity-60"> | ||
{Array.from({ length: size }, (_, index) => ( |
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.
오 size를 인수로 받아 개수를 자유롭게 설정할 수 있군요👍🏻👍🏻
🧩 이슈 번호
✅ 작업 사항
2023._11._17._._5.13.21.mp4
영상에는 없긴 한데 불러오는 중간에 아코디언 펼쳤다 접었다 해도 잘 작동합니다 !
👩💻 공유 포인트 및 논의 사항
검색 기능은 아직 추가 안하고,
전체 방 불러오기 + 무한스크롤만 구현해놨습니닷
API 명세에 따라 다시 수정될 수도 있습니다 .. !