feat: 해당하는 페이지에서 하단 탭 터치시 스크롤 최상단으로 이동 #162
Conversation
wjsdncl
requested review from
CitrusSoda,
YehaYoo,
un0211 and
heony704
as code owners
Walkthrough이 풀 리퀘스트는 앱의 하단 탭 탐색 경험을 개선하기 위한 변경 사항을 포함하고 있습니다. 주요 기능은 하단 탭(홈, 친구, 마이페이지)을 반복적으로 탭할 때 해당 페이지의 최상단으로 스크롤하고 데이터를 새로고침하는 것입니다. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
역시 대 인 싸 예하님 이십니다... 친구 요청에서 동작하지 않은 이유는 친구 요청이 쌓일 일이 없을거 같아서 였는데 있는게 좋을까요? |
제가.. 인싸?? 또 저를?놀린다는?생각밖에는 안들고? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
hooks/useScrollToTop.ts (2)
9-12: 함수 시그니처의 목적이 명확하게 드러나도록 주석을 고려해 보세요.
useScrollToTop훅에 어떤 인자를 받으며 어떤 동작을 하는지 코드만으로 파악 가능하지만, 다른 개발자가 빠르게 이해할 수 있도록 문서 주석(JSDoc)을 추가하면 가독성과 유지보수에 도움이 됩니다.
18-19: 테스트나 프로덕션 환경에서console.log를 남겨둘지 재검토하십시오.
개발 중 디버깅용으로 유용하지만, 프로덕션 환경에서는 불필요한 로그가 될 수 있습니다. 상황에 따라 제거하거나 디버그 레벨 로깅으로 대체하는 편을 권장합니다.components/PostGrid.tsx (1)
18-18:refetch프로퍼티를 필수로 지정한 이유를 명시해두면 좋습니다.
PostGridProps인터페이스에서refetch가 선택적이지 않으므로, 사용자가PostGrid를 사용할 때 무조건 제공해야 합니다. 필수 사용 의도가 분명하다면, 주석이나 문서화로 의도를 분명히 밝혀주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/(protected)/(tabs)/_layout.tsx(4 hunks)app/(protected)/(tabs)/friend/index.tsx(2 hunks)app/(protected)/(tabs)/home.tsx(2 hunks)app/(protected)/(tabs)/mypage.tsx(2 hunks)components/PostGrid.tsx(3 hunks)components/SearchLayout.tsx(4 hunks)hooks/useScrollToTop.ts(1 hunks)
🔇 Additional comments (21)
hooks/useScrollToTop.ts (1)
15-20: 이벤트 리스너 제거 로직이 적절히 동작하므로 메모리 누수를 방지할 수 있습니다.
subscription.remove()를 통해 이벤트 리스너를 해제하는 처리가 잘 되어 있어, 메모리 누수나 중복 호출 문제가 발생하지 않을 것으로 보입니다.app/(protected)/(tabs)/mypage.tsx (2)
23-23:refetch변수를 명시적으로 반환해주는 로직이 적절합니다.
useFetchData훅에서 정상적으로 refetch 함수를 반환받아 사용하는 부분이 깔끔합니다. 추후 요청이나 리프레시 로직 확장이 용이할 것으로 보입니다.
47-47:PostGrid컴포넌트에refetch를 전달할 때 타입 충돌이 없는지 확인하십시오.
현재로서는PostGrid의refetch프로퍼티 타입과 일치하므로 문제없어 보입니다. 다만, 추후refetch구현부가 변경될 때 해당 부분 역시 타입이 일치하는지 확인이 필요합니다.components/SearchLayout.tsx (5)
2-2: 새로운 훅useScrollToTop을 올바르게 임포트했습니다.
훅 호출 시점과 위치가 적절하므로 큰 문제는 없어 보입니다.
15-15:SearchLayoutProps에refetch프로퍼티가 추가되어 확장성 확보에 도움이 됩니다.
이로 인해 검색 레이아웃에서 스크롤 이벤트와 행사 로직을 좀 더 유연하게 처리할 수 있습니다.
Line range hint
25-33: 함수 시그니처를 제네릭 형태로 유지하면서refetch를 추가한 점이 적절합니다.
추후SearchLayout이 다양한 데이터 타입을 다루더라도 스크롤 상단 이동 및 데이터 새로고침 기능을 일관되게 제공할 수 있어 보입니다.
34-37: 이벤트 문자열이 중복되지 않도록 주의하세요.
"SCROLL_FRIEND_TO_TOP"이벤트를 다른 곳에서도 쓰게 된다면, 스크롤이 의도치 않게 여러 화면에 영향을 줄 수 있습니다. 이벤트 이름 충돌에 주의하십시오.
42-42:ref속성에 커스텀 훅 반환값을 연결한 점은 적절하며, 스크롤 기능을 명확히 구현합니다.
FlatList에 바로 할당되어 훅과 긴밀히 연동되는 흐름이 일관적입니다.components/PostGrid.tsx (4)
2-2:useScrollToTop훅 임포트가 올바르게 이루어졌습니다.
추가된 훅과 함께 스크롤 및 데이터 갱신 로직이 명확해졌습니다.
23-23: 함수 시그니처에refetch가 잘 추가되었습니다.
데이터 로딩 흐름이 명확하고 향후 확장성도 높아 보입니다.
54-57: 구현한 이벤트가 의도대로 동작하는지 여러 시나리오에서 테스트해 보세요.
지금은"SCROLL_HOME_TO_TOP"에만 연결되어 있지만, 같은 화면 내 다른 스크롤 뷰가 있거나 이벤트가 중복 사용되는 경우를 주의해야 합니다.
62-62:ref속성에 훅 반환값을 바로 연결해 직관적입니다.
상단으로 스크롤 이동 및refetch로직이 긴밀하게 연결되어 있어, 개발자가 흐름을 이해하기 편리합니다.app/(protected)/(tabs)/friend/index.tsx (2)
26-26: refetch 프로퍼티 추가 확인
해당 부분은 친구 목록을 다시 불러오기 위한refetch변수를 훅에서 받아오는 로직으로 보이며, 정상적으로 동작할 것으로 예상됩니다.
84-84: SearchLayout으로 refetch 함수 전달
SearchLayout에refetch함수를 넘겨주어 데이터 갱신 시점을 통합 관리하는 방식이 좋아 보입니다. 유지보수 관점에서도 깔끔한 설계입니다.app/(protected)/(tabs)/_layout.tsx (4)
2-2: DeviceEventEmitter 사용
DeviceEventEmitter를 통해 이벤트를 발행하는 로직을 도입하셨습니다. 탭을 재터치했을 때 화면 상단으로 이동시키는 시나리오에 적합하며, 구현 의도에 부합해 보입니다.
84-91: Home 탭 재터치 이벤트 처리
현재 탭이 활성화된 상태에서 같은 탭을 다시 누르면"SCROLL_HOME_TO_TOP"이벤트를 발생시키는 구조로 구현하셨습니다. 반복 탭 시 상단 스크롤 요구사항에 잘 부합합니다.
100-107: Friend 탭 재터치 이벤트 처리
해당 탭이 이미 활성화된 상황에서 다시 탭을 누르면"SCROLL_FRIEND_TO_TOP"이벤트를 발생시키도록 처리하셨습니다. 동일한 로직 패턴이 일관성 있게 적용되어 있습니다.
138-145: MyPage 탭 재터치 이벤트 처리
사용자가 이미 활성화된 마이페이지 탭을 재탭했을 때"SCROLL_MY_PAGE_TO_TOP"이벤트가 전달되도록 코드를 작성하였습니다. 필요한 로직이 잘 구현된 것으로 보입니다.app/(protected)/(tabs)/home.tsx (3)
11-12: 스크롤 기능 및 타입 정의 추가
최상단 스크롤 기능을 위해useScrollToTop훅을 불러오고,Post타입을 명시적으로 가져오는 방식이 적절합니다. 타입 안정성과 기능 분리가 명확합니다.
96-99: useScrollToTop 사용 로직
해당 훅에서SCROLL_HOME_TO_TOP이벤트를 수신해refetch를 호출하는 설계가 직관적입니다. 별도 스크롤 최상단 이동과 데이터 갱신을 동시에 처리하기 좋아 보입니다.
104-104: FlatList의 ref 연결
flatListRef를 FlatList의ref에 바인딩하여 스크롤 동작을 직접 제어할 수 있도록 한 구현이 적절합니다. 유지보수 측면에서도 사용자가 의도하는 탭 재터치 로직과 잘 연계되어 있습니다.
|
@coderabbitai 새해 복 많이 받아! |
|
Tip For best results, initiate chat on the files or code changes. @YehaYoo 새해 복 많이 받으세요! 이번 한 해도 즐겁고 행복한 일만 가득하시길 바랍니다. |
|
친구 목록, 친구 요청 탭 둘다 하단 친구 탭 누르면 해당하는 페이지 상단으로 이동하고 리패치 하게 했습니다. 추가로 친구 요청에서 useFocusEffect가 게속 동작하길래 useCallback 걸어뒀습니다 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
app/(protected)/(tabs)/friend/request.tsx (3)
1-1: useCallback, useRef 등 훅 추가 사용에 대한 의도 확인
이 훅들을 활용하여 컴포넌트가 효율적으로 렌더링되도록 잘 구성한 것으로 보입니다. 성능 측면에서 문제될 부분은 없어 보이지만, 혹시 의도하지 않은 리렌더링이 발생하는지 확인해 주세요.
24-24: FlatList 참조 관리
flatListRef를 통해 프로그램matically 스크롤 제어가 가능해졌습니다. 향후 동일한 로직이 여러 컴포넌트에서 반복된다면 공용 모듈화도 검토해 보세요.
68-69: 스크롤 및 refetch 이벤트 연동
하단 탭에서 이벤트를 받아 FlatList 위치를 최상단으로 이동하고, 추가로refetch를 호출하여 데이터를 갱신하는 플로우가 직관적입니다. 로딩 상태 로직도 함께 고려해 보시면 사용자 경험이 더 좋아질 수 있습니다.hooks/useScrollToTop.ts (1)
1-2: DeviceEventEmitter 사용에 대한 검토
React Native에서DeviceEventEmitter는 비동기 이벤트 전달에 유용하지만, 추후 다른 이벤트 관리 방식(예: Context, Redux, Recoil 등)을 고려할 수도 있습니다. 유지 보수와 확장성을 위해 장단점을 검토해 보세요.components/SearchLayout.tsx (2)
Line range hint
25-33: 의미 있는 상태 관리
검색 키워드에 따라onChangeKeyword를 호출하고,refetch를 사용해서 데이터 갱신이 가능하도록 한 로직이 격리되어 있어 가독성이 좋습니다.검색 결과가 없을 때와 에러 핸들링 시, 추가적으로 UX가 개선될 여지가 있는지 고민해 보세요.
45-45: ref 설정
FlatList에ref={flatListRef}를 지정함으로써 제어가 가능해졌습니다. 다른 컴포넌트에서도 비슷한 로직을 반복하지 않도록 차후에 커스텀 컴포넌트를 분리하는 방안도 고려해 볼 수 있습니다.components/PostGrid.tsx (3)
24-26: 에러 상태 처리
에러 상태 혹은 게시물이 없을 때의 예외 처리를 별도의 컴포넌트로 분리하는 것이 유지보수에 좋습니다. 현재는 그대로 둬도 문제없지만, 향후 예외 케이스가 늘어나면 관리가 어려워질 가능성이 있습니다.
56-57: 스크롤 및 데이터 갱신 이벤트
SCROLL_MY_PAGE_TO_TOP이벤트 발생 시 FlatList를 제어하고 데이터 갱신하는 구조가 단순하고 명확합니다. UI 로딩 상태 표시 로직도 함께 고려해 주시면 UX를 더욱 개선할 수 있습니다.
61-61: ref 사용을 통한 리스트 제어
ref속성을 지정함으로써 외부에서 FlatList 스크롤을 쉽게 조정할 수 있습니다. 다만, FlatList를 매우 빈번하게 업데이트하면 성능 이슈가 발생할 수 있으므로 주기적인 모니터링을 권장합니다.app/(protected)/(tabs)/home.tsx (1)
105-105: FlatList에 ref가 올바르게 연결되었습니다!성능 최적화를 위해 다음 사항을 고려해보시면 좋겠습니다:
FlatList의windowSize프로퍼티 추가를 통해 렌더링 성능을 개선할 수 있습니다. 특히 많은 수의 포스트를 표시할 때 유용할 수 있습니다.<FlatList ref={flatListRef} + windowSize={5} data={data?.pages.flatMap((page) => page.data) ?? []}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/(protected)/(tabs)/_layout.tsx(4 hunks)app/(protected)/(tabs)/friend/request.tsx(6 hunks)app/(protected)/(tabs)/home.tsx(3 hunks)components/PostGrid.tsx(3 hunks)components/SearchLayout.tsx(4 hunks)hooks/useScrollToTop.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/(protected)/(tabs)/_layout.tsx
🔇 Additional comments (12)
app/(protected)/(tabs)/friend/request.tsx (3)
10-10: useScrollToTop 훅의 도입
하단 탭 터치 시 스크롤을 최상단으로 이동시키는 기능을 구현하기 위해useScrollToTop훅을 도입한 점이 적절해 보입니다. 재사용성도 높아질 것으로 기대됩니다.
33-33: refetch 의존성 주의
refetch를 여러 훅에서 사용하는 경우, 데이터 무한 로드와의 의존성 충돌 여부를 주시하세요. 간헐적으로 중복 호출이 발생할 수 있으므로 확인이 필요합니다.
42-48: useFocusEffect로 쿼리 무효화
포커스될 때마다 쿼리를 무효화하여 최신 데이터를 불러오는 로직이 적절합니다. 다만isFetchingNextPage가true인 상황에서 별도의 UI 안내가 필요한지 검토해 보시는 것도 좋겠습니다.hooks/useScrollToTop.ts (1)
4-21: 공용 훅으로서의 구조
이 훅은 FlatList에 대한 스크롤 컨트롤과 데이터 새로고침 로직을 이벤트로 연결합니다. 전역 리스너 사용 시 메모리 누수나 중복 등록에 주의해야 합니다. 현재subscription.remove()를 통해 제거하고 있으니 괜찮지만, 다른 곳에서 이벤트 이름이 겹치지 않도록 관리가 필요합니다.components/SearchLayout.tsx (3)
2-2: useScrollToTop 훅을 통한 스크롤 제어
검색 페이지에서도 탭 이벤트로 스크롤 행동을 제어할 수 있어 일관된 사용자 경험을 제공할 것으로 기대됩니다.
15-15: refetch를 옵셔널 프로퍼티로 관리
refetch함수를 옵셔널로 처리하여 유연성을 높인 점이 좋습니다. 혹시나 null-safe 처리가 필요한 부분이 있다면,?? (() => {})처럼 안전장치를 설정한 부분이 잘 반영되었습니다.
34-40: FlatList 참조 활용
flatListRef와useScrollToTop을 연결해서 원하는 이벤트 시점에 스크롤을 제어하는 방식이 일관적입니다. 한편, 서드파티 라이브러리(예: React Navigation)와의 충돌 가능성을 체크해 보세요.components/PostGrid.tsx (2)
2-2: useScrollToTop 훅을 통해 일관된 기능 제공
마이페이지(게시물 그리드)에서도 동일한 스크롤-to-top + refetch 패턴을 유지해 주어, 사용자 입장에서 일관된 경험을 줄 수 있습니다.
19-19: refetch 비즈니스 로직 주의
게시물 목록을 갱신할 때,refetch가 어떤 데이터를 다시 불러오는지 명확히 구분해 두세요. 여러 목록이 동시에 존재하는 경우에는 충돌이 없는지 확인이 필요합니다.app/(protected)/(tabs)/home.tsx (3)
11-11: 필요한 훅들이 올바르게 임포트되었습니다!스크롤 기능 구현을 위한
useScrollToTop훅과useRef가 적절하게 추가되었습니다.Also applies to: 15-15
37-37: FlatList 참조가 올바르게 생성되었습니다!
FlatList컴포넌트를 위한 ref가 적절한 타입과 함께 생성되었습니다.
96-100: 스크롤 투 탑 기능이 적절하게 구현되었습니다!
useScrollToTop훅이 올바른 파라미터들과 함께 구현되었습니다. 다만, 다음 사항들을 확인해 주시면 좋겠습니다:
- 탭을 여러 번 터치할 때 스크롤과 데이터 새로고침이 정상적으로 작동하는지
- 스크롤 애니메이션이 부드럽게 동작하는지
✅ Verification successful
useScrollToTop훅이 일관되게 구현되어 있습니다
useScrollToTop훅의 구현을 검토한 결과, 다음과 같은 사항들이 확인되었습니다:
home.tsx와friend/request.tsx모두 동일한 파라미터 구조를 따르고 있습니다- 각 탭별로 고유한 이벤트 이름을 사용하고 있습니다 (
SCROLL_HOME_TO_TOP,SCROLL_REQUEST_TO_TOP)- 훅의 내부 구현이 적절하게 되어 있으며, 스크롤 애니메이션과 데이터 새로고침 기능이 포함되어 있습니다
따라서 이전 리뷰 코멘트에서 요청드린 수동 확인은 불필요할 것으로 판단됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if useScrollToTop hook is consistently implemented across tabs # Look for similar implementations in other tab files rg -l "useScrollToTop" "app/(protected)/(tabs)"Length of output: 121
Script:
#!/bin/bash # Check implementation details of useScrollToTop in both files rg "useScrollToTop" "app/(protected)/(tabs)/home.tsx" -A 3 -B 1 rg "useScrollToTop" "app/(protected)/(tabs)/friend/request.tsx" -A 3 -B 1 # Find the hook implementation ast-grep --pattern 'const useScrollToTop = $_'Length of output: 1723
|
친구요청 페이지 뭔가 자꾸 호출되고 친구 순서가 바뀌는 이슈 하나가 있고 좋아요한 친구 페이지 들어갈려니 |
ㅠㅠ 확인해보겠습니다.. |
|
제 환경에서는 오류가 안나와서 수정됐는지는 확인 부탁드립니다... 친구페이지에서 둘 다 동작하는건 확인했습니다 🫡 친구 목록에서도 useFocusEffect가 게속 동작하길래 useCallback 걸어뒀습니다 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/PostGrid.tsx (1)
56-66: 이벤트 리스너 구현이 잘 되었습니다만, 의존성 배열을 확인해주세요.이벤트 구독 및 정리가 잘 구현되었습니다. 하지만
useEffect의 의존성 배열에refetch만 포함되어 있는데, 이는refetch함수가 변경될 때마다 이벤트 리스너가 재생성됨을 의미합니다.
refetch함수를useCallback으로 메모이제이션하는 것을 고려해보세요:+ const memoizedRefetch = useCallback(() => { + refetch(); + }, [refetch]); useEffect(() => { const subscription = DeviceEventEmitter.addListener( "SCROLL_MY_PAGE_TO_TOP", () => { flatListRef.current?.scrollToOffset({ offset: 0, animated: true }); - refetch(); + memoizedRefetch(); }, ); return () => subscription.remove(); - }, [refetch]); + }, [memoizedRefetch]);app/(protected)/(tabs)/friend/request.tsx (1)
Line range hint
97-116: FlatList 구현에서 성능 최적화를 고려해보세요.FlatList의 구현은 전반적으로 잘 되어 있습니다만, 성능 최적화를 위한 몇 가지 제안사항이 있습니다.
다음과 같은 성능 최적화 속성들을 추가하는 것을 고려해보세요:
<FlatList ref={flatListRef} className="w-full grow px-8" data={hasRequests ? requestData.pages.flatMap((page) => page.data) : []} keyExtractor={(request) => String(request.requestId)} renderItem={({ item: request }) => ( <FriendRequest {...request} isLoading={isLoading} /> )} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} + initialNumToRender={6} onEndReached={loadMore} contentContainerStyle={hasRequests ? {} : { flex: 1 }} ListHeaderComponent={<View className="h-2" />} ListFooterComponent={ isFetchingNextPage ? ( <ActivityIndicator size="large" className="py-4" color={colors.primary} /> ) : ( <View className="h-4" /> ) } ListEmptyComponent={ <ErrorScreen errorMessage="친구 요청이 없습니다." /> } />components/SearchLayout.tsx (1)
Line range hint
1-81: 성능 최적화 관련 제안사항PR 코멘트에서 언급된 반복적인 API 호출 문제와 관련하여, 다음 사항들을 고려해보시면 좋을 것 같습니다:
useMemo를 사용하여renderItem콜백 메모이제이션FlatList의maxToRenderPerBatch및windowSize속성 최적화- 스크롤 이벤트 발생 시 데이터 새로고침 전에 사용자 상호작용 제한
예시 코드:
const memoizedRenderItem = useMemo( () => renderItem, [renderItem] ); // FlatList 속성 추가 <FlatList maxToRenderPerBatch={10} windowSize={5} renderItem={memoizedRenderItem} ... />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/(protected)/(tabs)/friend/index.tsx(4 hunks)app/(protected)/(tabs)/friend/request.tsx(6 hunks)app/(protected)/(tabs)/home.tsx(3 hunks)components/PostGrid.tsx(3 hunks)components/SearchLayout.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/(protected)/(tabs)/friend/index.tsx
- app/(protected)/(tabs)/home.tsx
🔇 Additional comments (8)
components/PostGrid.tsx (2)
19-19: Props 인터페이스가 적절하게 업데이트되었습니다.
refetch함수를 필수 prop으로 추가한 것이 데이터 새로고침 기능 구현에 적합합니다.Also applies to: 24-24
71-71: FlatList ref 구현이 적절합니다.
flatListRef를 통한 스크롤 제어 구현이 잘 되어있습니다.app/(protected)/(tabs)/friend/request.tsx (2)
46-52: useFocusEffect 최적화가 잘 되었습니다.
useCallback을 사용하여useFocusEffect콜백을 메모이제이션한 것이 좋습니다. 의존성 배열도 적절하게 설정되어 있습니다.
72-82: 이벤트 리스너 구현이 PostGrid와 일관성이 있습니다.스크롤 및 데이터 새로고침 기능이 PostGrid 컴포넌트와 동일한 패턴으로 구현되어 있어 일관성이 좋습니다.
components/SearchLayout.tsx (4)
3-3: 새로운 기능을 위한 import 구문이 적절히 추가되었습니다.필요한 모든 의존성이 올바르게 import되었습니다:
useRef와useEffecthooksDeviceEventEmitterAlso applies to: 6-6
15-15: 선택적 refetch 속성이 적절히 정의되었습니다.
refetch속성이 선택적(?)으로 정의되어 있어 기존 코드와의 호환성을 유지하면서도 새로운 기능을 지원할 수 있습니다.
51-51: FlatList ref가 적절히 구현되었습니다.
flatListRef가 FlatList 컴포넌트에 올바르게 연결되어 있습니다.
25-25: refetch 매개변수가 적절히 추가되었습니다.컴포넌트 매개변수에
refetch가 추가되어 새로고침 기능이 올바르게 지원됩니다.
components/SearchLayout.tsx
Outdated
| const flatListRef = useRef<FlatList>(null); | ||
|
|
||
| useEffect(() => { | ||
| const subscription = DeviceEventEmitter.addListener( | ||
| "SCROLL_FRIEND_TO_TOP", | ||
| () => { | ||
| flatListRef.current?.scrollToOffset({ offset: 0, animated: true }); | ||
| refetch?.(); | ||
| }, | ||
| ); | ||
|
|
||
| return () => subscription.remove(); | ||
| }, [refetch]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스크롤 이벤트 처리 로직 개선이 필요합니다.
현재 구현에서 몇 가지 잠재적인 문제가 있습니다:
- 이벤트 리스너가
refetch의존성으로 인해 불필요하게 재생성될 수 있습니다. - 연속적인 이벤트 발생 시 과도한 API 호출이 발생할 수 있습니다.
다음과 같이 개선하는 것을 제안드립니다:
const flatListRef = useRef<FlatList>(null);
+const handleScrollToTop = useCallback(() => {
+ flatListRef.current?.scrollToOffset({ offset: 0, animated: true });
+ refetch?.();
+}, [refetch]);
useEffect(() => {
const subscription = DeviceEventEmitter.addListener(
"SCROLL_FRIEND_TO_TOP",
- () => {
- flatListRef.current?.scrollToOffset({ offset: 0, animated: true });
- refetch?.();
- },
+ handleScrollToTop,
);
return () => subscription.remove();
-}, [refetch]);
+}, [handleScrollToTop]);추가로 디바운싱 처리를 고려해보시는 것도 좋을 것 같습니다:
const debouncedHandleScrollToTop = useMemo(
() => debounce(handleScrollToTop, 300),
[handleScrollToTop]
);
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/SearchLayout.tsx (1)
34-48:⚠️ Potential issue이벤트 핸들러 구현에 심각한 오류가 있습니다.
- 이벤트 핸들러가 함수 참조만 반환하고 실행되지 않습니다:
- () => handleScrollToTop, + handleScrollToTop,
이전 리뷰 코멘트에서 언급된 것처럼 디바운싱 처리가 필요합니다.
스크롤 동작 실패에 대한 에러 처리가 필요합니다:
const handleScrollToTop = useCallback(() => { - flatListRef.current?.scrollToOffset({ offset: 0, animated: true }); + try { + if (!flatListRef.current) { + console.warn('FlatList ref가 유효하지 않습니다'); + return; + } + flatListRef.current.scrollToOffset({ offset: 0, animated: true }); + refetch?.(); + } catch (error) { + console.error('스크롤 동작 중 오류 발생:', error); + } - refetch?.(); }, [refetch]);
🧹 Nitpick comments (3)
app/(protected)/(tabs)/home.tsx (1)
113-113: FlatList에 ref가 올바르게 연결되었습니다!성능 최적화를 위한 제안: 긴 목록을 스크롤할 때 성능을 개선하기 위해
windowSize,maxToRenderPerBatch,updateCellsBatchingPeriod속성 추가를 고려해보세요.다음과 같이 성능 최적화 속성을 추가할 수 있습니다:
<FlatList ref={flatListRef} + windowSize={5} + maxToRenderPerBatch={10} + updateCellsBatchingPeriod={50} data={data?.pages.flatMap((page) => page.data) ?? []} ... />components/SearchLayout.tsx (2)
3-3: import 구문 및 props 타입 정의 개선이 필요합니다.import 구문과 props 타입이 잘 정의되어 있지만, 몇 가지 개선사항이 있습니다:
- import 구문을 그룹화하여 가독성을 높일 수 있습니다
refetchprop에 대한 JSDoc 문서화가 필요합니다다음과 같이 개선하는 것을 제안드립니다:
- import { useCallback, useEffect, useRef, useState } from "react"; + // React hooks + import { useCallback, useEffect, useRef, useState } from "react"; + + // React Native components import { ActivityIndicator, DeviceEventEmitter, FlatList, type ListRenderItemInfo, View, } from "react-native"; interface SearchLayoutProps { + /** 데이터 새로고침을 위한 콜백 함수 */ refetch?: () => void;Also applies to: 6-6, 15-15
53-53: FlatList 성능 최적화가 필요합니다.FlatList에 ref가 올바르게 추가되었지만, 성능 최적화를 위한 추가 props를 고려해볼 수 있습니다.
다음과 같은 성능 최적화 props 추가를 제안드립니다:
<FlatList ref={flatListRef} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} className="w-full grow px-6"이러한 설정은 긴 리스트를 렌더링할 때 메모리 사용량과 성능을 개선할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/(protected)/(tabs)/friend/request.tsx(6 hunks)app/(protected)/(tabs)/home.tsx(3 hunks)components/PostGrid.tsx(3 hunks)components/SearchLayout.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/(protected)/(tabs)/friend/request.tsx
- components/PostGrid.tsx
🔇 Additional comments (2)
app/(protected)/(tabs)/home.tsx (2)
14-14: 필요한 import들이 올바르게 추가되었습니다!새로운 기능 구현을 위한
useRef와DeviceEventEmitterimport가 적절하게 추가되었습니다.Also applies to: 17-17
37-37: FlatList ref가 올바르게 정의되었습니다!
FlatList컴포넌트를 제어하기 위한 ref가 적절한 타입과 함께 선언되었습니다.
app/(protected)/(tabs)/home.tsx
Outdated
| const handleScrollToTop = useCallback(() => { | ||
| flatListRef.current?.scrollToOffset({ offset: 0, animated: true }); | ||
| refetch(); | ||
| }, [refetch]); |
There was a problem hiding this comment.
이벤트 리스너 콜백 함수 구현에 오류가 있습니다.
이벤트 리스너에서 handleScrollToTop 함수를 직접 실행하지 않고 함수 참조만 반환하고 있습니다.
다음과 같이 수정해주세요:
useEffect(() => {
const subscription = DeviceEventEmitter.addListener(
"SCROLL_HOME_TO_TOP",
- () => handleScrollToTop,
+ handleScrollToTop,
);
return () => subscription.remove();
}, [handleScrollToTop]);Also applies to: 101-108
| @@ -20,6 +22,7 @@ interface SearchLayoutProps { | |||
| } | |||
|
|
|||
| export function SearchLayout<T>({ | |||
| refetch, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
제네릭 타입 T가 사용되지 않고 있습니다.
컴포넌트에 제네릭 타입 T가 선언되어 있지만 실제로 사용되지 않고 있습니다. 또한 UserProfile 타입이 하드코딩되어 있어 재사용성이 제한됩니다.
다음과 같이 개선하는 것을 제안드립니다:
- export function SearchLayout<T>({
+ export function SearchLayout<T extends { id: string }>({
refetch,
- data: UserProfile[],
+ data: T[],
...
- renderItem: (itemInfo: ListRenderItemInfo<UserProfile>) => React.ReactElement;
+ renderItem: (itemInfo: ListRenderItemInfo<T>) => React.ReactElement;Committable suggestion skipped: line range outside the PR's diff.
|
이유는 모르겠는데 친구 페이지 상단 탭이 반대쪽으로 이동하는 문제가 있어서 수정하겠습니다 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/(protected)/(tabs)/_layout.tsx (2)
2-2: 이벤트 타입 안전성 개선이 필요합니다
DeviceEventEmitter를 사용한 구현은 작동하지만, 타입 안전성과 유지보수성 측면에서 개선이 필요합니다.다음과 같은 개선사항을 고려해보세요:
// events.ts export const TabEvents = { SCROLL_HOME_TO_TOP: 'SCROLL_HOME_TO_TOP', SCROLL_FRIEND_TO_TOP: 'SCROLL_FRIEND_TO_TOP', SCROLL_REQUEST_TO_TOP: 'SCROLL_REQUEST_TO_TOP', SCROLL_MY_PAGE_TO_TOP: 'SCROLL_MY_PAGE_TO_TOP', } as const; export type TabEventType = typeof TabEvents[keyof typeof TabEvents];또는 더 나은 대안으로 Context API나 Recoil과 같은 상태 관리 도구의 사용을 고려해보세요.
84-91: 탭 상태 확인 로직의 재사용성 개선이 필요합니다현재 탭의 활성 상태를 확인하는 로직이 여러 탭에서 반복되고 있습니다.
다음과 같이 공통 함수로 추출하는 것을 추천드립니다:
const isActiveTab = (navigation: any, routeName: string) => { const state = navigation.getState(); return state.routes[state.index].name === routeName; }; // 사용 예시 listeners={({ navigation, route }) => ({ tabPress: () => { if (isActiveTab(navigation, route.name)) { DeviceEventEmitter.emit("SCROLL_HOME_TO_TOP"); } }, })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/(protected)/(tabs)/_layout.tsx(4 hunks)app/(protected)/(tabs)/friend/request.tsx(6 hunks)app/(protected)/(tabs)/home.tsx(3 hunks)components/PostGrid.tsx(3 hunks)components/SearchLayout.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/SearchLayout.tsx
- app/(protected)/(tabs)/friend/request.tsx
- components/PostGrid.tsx
- app/(protected)/(tabs)/home.tsx
🔇 Additional comments (1)
app/(protected)/(tabs)/_layout.tsx (1)
152-159: 마이페이지 탭 리스너의 코드 중복 개선이 필요합니다홈 탭과 동일한 로직이 반복되고 있습니다.
앞서 제안된
isActiveTab함수를 사용하여 코드 중복을 제거해주세요.
app/(protected)/(tabs)/_layout.tsx
Outdated
| listeners={({ navigation, route }) => ({ | ||
| tabPress: () => { | ||
| const rootState = navigation.getState(); | ||
| if (rootState.routes[rootState.index].name === route.name) { | ||
| const nestedState = rootState.routes[rootState.index].state as { | ||
| index: number; | ||
| routeNames: string[]; | ||
| }; | ||
| if (nestedState) { | ||
| const topTabIndex = nestedState.index; | ||
| const topTabName = nestedState.routeNames[topTabIndex]; | ||
| if (topTabName === "index") { | ||
| DeviceEventEmitter.emit("SCROLL_FRIEND_TO_TOP"); | ||
| } else if (topTabName === "request") { | ||
| DeviceEventEmitter.emit("SCROLL_REQUEST_TO_TOP"); | ||
| } | ||
| } else { | ||
| DeviceEventEmitter.emit("SCROLL_FRIEND_TO_TOP"); | ||
| } | ||
| } | ||
| }, | ||
| })} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
친구 탭 리스너의 타입 안전성과 복잡성 개선이 필요합니다
현재 구현에서 몇 가지 개선이 필요한 부분이 있습니다:
- 네비게이션 상태의 타입 캐스팅이 안전하지 않습니다
- 중첩된 조건문이 복잡합니다
- 117번 줄의 else 분기가 불필요할 수 있습니다
다음과 같은 개선된 구현을 제안드립니다:
type NestedState = {
index: number;
routeNames: string[];
};
listeners={({ navigation, route }) => ({
tabPress: () => {
const rootState = navigation.getState();
if (rootState.routes[rootState.index].name !== route.name) return;
const nestedState = rootState.routes[rootState.index].state as NestedState;
if (!nestedState?.routeNames) {
DeviceEventEmitter.emit("SCROLL_FRIEND_TO_TOP");
return;
}
const topTabName = nestedState.routeNames[nestedState.index];
const eventMap = {
index: "SCROLL_FRIEND_TO_TOP",
request: "SCROLL_REQUEST_TO_TOP",
};
const eventName = eventMap[topTabName as keyof typeof eventMap];
if (eventName) {
DeviceEventEmitter.emit(eventName);
}
},
})}
There was a problem hiding this comment.
마이페이지는 글이 별로 없어서 확인 못했지만, 나머지는 해당 탭일 때 탭 클릭하면 위로 잘 이동합니다.
뭔가 한꺼번에 유틸이나 컴포넌트로 될 것 같아서 부탁드렸는데 각자 다 해주셨네요. 감사합니다!!!
친구페이지 useCallback도 다 걸어주셔서 감사합니다.
@YehaYoo 친구 탭의 경우에는 상단 탭도 있는데, 상단 탭 클릭했을 때 효과가 나타나는 게 나을까요..?
(인스타 참고해봤는데 인스타는 친구탭은 눌러도 새로고침이나 상단으로 이동 안되는 것 같긴 하네요..)
그리고 친구검색 페이지가 user/search에 있는데, 친구 페이지랑 일관성있게 동작하면 좋을 것 같아요. 여긴 탭이 없네요..
저는 홈에서 글쓴이 (+ 좋아요 누른 사람) 클릭해서 해당 유저 페이지 들어가려 할 때 Rendered more hooks than during the previous render. 에러 나네요.. 댓글은 괜찮은데
| @@ -81,6 +81,14 @@ export default function TabsLayout() { | |||
| title: "Home", | |||
| tabBarIcon: ({ color }) => <TabIcon color={color} name="HOME" />, | |||
| }} | |||
| listeners={({ navigation, route }) => ({ | |||
There was a problem hiding this comment.
상단탭까지 손가락이 올라가는 것보다는 하단탭 두드리는 게 나을 것 같아요!!
확인 감사합니다 수정해볼게요 🫡 |
📝 PR 설명
해당하는 페이지에서 하단 탭 터치하면 최상단으로 스크롤 되고 리패치 하도록 추가했습니다.
기록이랑 업로드는 필요 없을거 같아서 홈, 친구, 마이 페이지에만 추가했습니다.
친구는 친구 목록만 스크롤 되고 친구 요청에서는 동작안합니다.
제가 친구랑 글이 적어서 테스트를 못했는데 많은 계정 있으면 확인 부탁드립니다.
📸 스크린샷
🔗 관련 이슈
Summary by CodeRabbit
새로운 기능
성능 개선
useCallback및useRef훅 도입버그 수정