Repository navigation
Fix/486 groups logic - #489
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds login-modal gating for unauthenticated users across groups list/detail pages, group tab navigation, and simplifies BottomNav/Header by removing prior login gating on the 모임 item. Separately, it introduces a reusable SwipeCarousel component and refactors NewsListBanner and NewsBannerSlider to use it. ChangesGuest Auth Gating for Groups and Navigation
Estimated code review effort: 3 (Moderate) | ~25 minutes SwipeCarousel Component and News Banner Refactor
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Guest
participant GroupsPageClient
participant AuthStore
participant LoginModal
participant Router
Guest->>GroupsPageClient: click "모임 생성하기" / visit / apply
GroupsPageClient->>AuthStore: check isLoggedIn
alt not logged in
GroupsPageClient->>LoginModal: openLoginModal()
else logged in
GroupsPageClient->>Router: navigate to target route
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors group pages and layouts to enforce authentication checks for guests, and extracts a reusable SwipeCarousel component from the news banners. Feedback focuses on optimizing the new SwipeCarousel component, specifically by adding a unique key prop to the rendered cards to prevent rendering bugs, and memoizing event handlers and helper functions with useCallback to ensure stable references and satisfy dependency rules.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const renderCard = (item: CarouselItem, position: string) => { | ||
| return ( | ||
| <div | ||
| className="absolute top-0 bottom-0 w-full flex-shrink-0" | ||
| style={{ left: position }} | ||
| > |
There was a problem hiding this comment.
In React, rendering dynamic lists or sibling elements without a unique key prop can lead to rendering bugs, inefficient DOM updates, and unexpected behavior during transitions (especially with Framer Motion). Since this is a virtualized carousel where cards change positions and content, we should add a unique key to the outer div returned by renderCard.
Using the position parameter (e.g., "-100%", "0%", "100%") as the key is highly suitable here because:
- It is guaranteed to be unique among the three rendered cards.
- It prevents duplicate key issues when
slideCountis 2 (where the previous and next indices point to the same item). - It allows React to efficiently reuse the DOM nodes at each absolute position rather than recreating them.
const renderCard = (item: CarouselItem, position: string) => {
return (
<div
key={position}
className="absolute top-0 bottom-0 w-full flex-shrink-0"
style={{ left: position }}
>
| @@ -0,0 +1,237 @@ | |||
| "use client"; | |||
|
|
|||
| import { useState, useEffect, useRef } from "react"; | |||
There was a problem hiding this comment.
Import useCallback from "react" to support the memoization of the helper functions and event handlers.
| import { useState, useEffect, useRef } from "react"; | |
| import { useState, useEffect, useRef, useCallback } from "react"; |
| // React 상태와 Ref를 즉각적으로 동기화하는 헬퍼 함수 | ||
| const updateRenderIndex = (newIndex: number) => { | ||
| renderIndexRef.current = newIndex; | ||
| setRenderIndex(newIndex); | ||
| }; |
There was a problem hiding this comment.
Wrap updateRenderIndex in useCallback to ensure its reference remains stable across renders. This is important because it is used as a dependency in other memoized functions (like navigateToSlide and jumpTo).
| // React 상태와 Ref를 즉각적으로 동기화하는 헬퍼 함수 | |
| const updateRenderIndex = (newIndex: number) => { | |
| renderIndexRef.current = newIndex; | |
| setRenderIndex(newIndex); | |
| }; | |
| // React 상태와 Ref를 즉각적으로 동기화하는 헬퍼 함수 | |
| const updateRenderIndex = useCallback((newIndex: number) => { | |
| renderIndexRef.current = newIndex; | |
| setRenderIndex(newIndex); | |
| }, []); |
| useMotionValueEvent(x, "change", (latestX) => { | ||
| if (!containerRef.current || slideCount <= 1) return; | ||
| const containerWidth = containerRef.current.offsetWidth; | ||
|
|
||
| let offsetIndex = 0; | ||
| if (latestX <= -containerWidth / 2) { | ||
| offsetIndex = 1; | ||
| } else if (latestX >= containerWidth / 2) { | ||
| offsetIndex = -1; | ||
| } | ||
|
|
||
| const newIndicatorIndex = wrap(0, slideCount, renderIndexRef.current + offsetIndex); | ||
| if (newIndicatorIndex !== indicatorIndex) { | ||
| setIndicatorIndex(newIndicatorIndex); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The inline callback passed to useMotionValueEvent is recreated on every render and references indicatorIndex. Because Framer Motion's useMotionValueEvent internally re-registers the event listener whenever the callback reference changes, this listener is unsubscribed and resubscribed on every single render (and on every frame of dragging when indicatorIndex changes).
We can optimize this by:
- Extracting the callback into a memoized function using
useCallback. - Using a functional state update for
setIndicatorIndexto remove the dependency onindicatorIndexfrom the closure.
This ensures the event listener is only registered once and remains stable throughout the drag interaction.
| useMotionValueEvent(x, "change", (latestX) => { | |
| if (!containerRef.current || slideCount <= 1) return; | |
| const containerWidth = containerRef.current.offsetWidth; | |
| let offsetIndex = 0; | |
| if (latestX <= -containerWidth / 2) { | |
| offsetIndex = 1; | |
| } else if (latestX >= containerWidth / 2) { | |
| offsetIndex = -1; | |
| } | |
| const newIndicatorIndex = wrap(0, slideCount, renderIndexRef.current + offsetIndex); | |
| if (newIndicatorIndex !== indicatorIndex) { | |
| setIndicatorIndex(newIndicatorIndex); | |
| } | |
| }); | |
| // x 좌표를 실시간 추적하여 절반 이상 넘어갔을 때 인디케이터를 선제적으로 교체 | |
| const handleXChange = useCallback((latestX: number) => { | |
| if (!containerRef.current || slideCount <= 1) return; | |
| const containerWidth = containerRef.current.offsetWidth; | |
| let offsetIndex = 0; | |
| if (latestX <= -containerWidth / 2) { | |
| offsetIndex = 1; | |
| } else if (latestX >= containerWidth / 2) { | |
| offsetIndex = -1; | |
| } | |
| const newIndicatorIndex = wrap(0, slideCount, renderIndexRef.current + offsetIndex); | |
| setIndicatorIndex((prev) => (newIndicatorIndex !== prev ? newIndicatorIndex : prev)); | |
| }, [slideCount]); | |
| useMotionValueEvent(x, "change", handleXChange); |
| const navigateToSlide = (direction: number) => { | ||
| if (slideCount <= 1) return; | ||
| const containerWidth = containerRef.current?.offsetWidth || 0; | ||
| const targetX = direction > 0 ? -containerWidth : containerWidth; | ||
|
|
||
| animate(x, targetX, { | ||
| type: "spring", | ||
| stiffness: 300, | ||
| damping: 30, | ||
| onComplete: () => { | ||
| // 애니메이션이 끝나면 렌더링 인덱스를 동기화하고 x를 즉시 초기화 | ||
| const newRenderIndex = renderIndexRef.current + direction; | ||
| updateRenderIndex(newRenderIndex); // Ref를 즉시 변경하여 꼬임 방지 | ||
| setIndicatorIndex(wrap(0, slideCount, newRenderIndex)); | ||
| x.set(0); // 이 순간 change 이벤트가 트리거되어도 최신 Ref를 바라봄 | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Wrap navigateToSlide in useCallback to avoid stale closures and satisfy the react-hooks/exhaustive-deps rule in the autoplay useEffect. Currently, navigateToSlide is recreated on every render, which would cause the interval to be cleared and recreated on every single render if it were listed as a dependency, or lead to stale closures if it is omitted.
| const navigateToSlide = (direction: number) => { | |
| if (slideCount <= 1) return; | |
| const containerWidth = containerRef.current?.offsetWidth || 0; | |
| const targetX = direction > 0 ? -containerWidth : containerWidth; | |
| animate(x, targetX, { | |
| type: "spring", | |
| stiffness: 300, | |
| damping: 30, | |
| onComplete: () => { | |
| // 애니메이션이 끝나면 렌더링 인덱스를 동기화하고 x를 즉시 초기화 | |
| const newRenderIndex = renderIndexRef.current + direction; | |
| updateRenderIndex(newRenderIndex); // Ref를 즉시 변경하여 꼬임 방지 | |
| setIndicatorIndex(wrap(0, slideCount, newRenderIndex)); | |
| x.set(0); // 이 순간 change 이벤트가 트리거되어도 최신 Ref를 바라봄 | |
| }, | |
| }); | |
| }; | |
| const navigateToSlide = useCallback((direction: number) => { | |
| if (slideCount <= 1) return; | |
| const containerWidth = containerRef.current?.offsetWidth || 0; | |
| const targetX = direction > 0 ? -containerWidth : containerWidth; | |
| animate(x, targetX, { | |
| type: "spring", | |
| stiffness: 300, | |
| damping: 30, | |
| onComplete: () => { | |
| // 애니메이션이 끝나면 렌더링 인덱스를 동기화하고 x를 즉시 초기화 | |
| const newRenderIndex = renderIndexRef.current + direction; | |
| updateRenderIndex(newRenderIndex); // Ref를 즉시 변경하여 꼬임 방지 | |
| setIndicatorIndex(wrap(0, slideCount, newRenderIndex)); | |
| x.set(0); // 이 순간 change 이벤트가 트리거되어도 최신 Ref를 바라봄 | |
| }, | |
| }); | |
| }, [slideCount, x, updateRenderIndex]); |
| const jumpTo = (targetIdx: number) => { | ||
| if (slideCount <= 1) return; | ||
| const diff = targetIdx - indicatorIndex; | ||
| if (diff === 0) return; | ||
|
|
||
| // 인디케이터 클릭 시엔 즉각적으로 두 인덱스를 모두 동기화 | ||
| const newRenderIndex = renderIndexRef.current + diff; | ||
| updateRenderIndex(newRenderIndex); | ||
| setIndicatorIndex(targetIdx); | ||
| x.set(0); | ||
| }; |
There was a problem hiding this comment.
Wrap jumpTo in useCallback to ensure its reference remains stable across renders, preventing unnecessary re-renders of the indicator buttons that receive it as an event handler.
| const jumpTo = (targetIdx: number) => { | |
| if (slideCount <= 1) return; | |
| const diff = targetIdx - indicatorIndex; | |
| if (diff === 0) return; | |
| // 인디케이터 클릭 시엔 즉각적으로 두 인덱스를 모두 동기화 | |
| const newRenderIndex = renderIndexRef.current + diff; | |
| updateRenderIndex(newRenderIndex); | |
| setIndicatorIndex(targetIdx); | |
| x.set(0); | |
| }; | |
| const jumpTo = useCallback((targetIdx: number) => { | |
| if (slideCount <= 1) return; | |
| const diff = targetIdx - indicatorIndex; | |
| if (diff === 0) return; | |
| // 인디케이터 클릭 시엔 즉각적으로 두 인덱스를 모두 동기화 | |
| const newRenderIndex = renderIndexRef.current + diff; | |
| updateRenderIndex(newRenderIndex); | |
| setIndicatorIndex(targetIdx); | |
| x.set(0); | |
| }, [slideCount, indicatorIndex, updateRenderIndex, x]); |
| useEffect(() => { | ||
| if (slideCount <= 1 || autoPlayInterval <= 0) return; | ||
|
|
||
| if (isDragging) return; | ||
|
|
||
| const timer = setInterval(() => { | ||
| navigateToSlide(1); | ||
| }, autoPlayInterval); | ||
|
|
||
| return () => clearInterval(timer); | ||
| }, [slideCount, renderIndex, autoPlayInterval, isDragging]); |
There was a problem hiding this comment.
Add navigateToSlide to the dependency array of the autoplay useEffect to satisfy ESLint rules and ensure the interval always uses the latest memoized version of the function.
| useEffect(() => { | |
| if (slideCount <= 1 || autoPlayInterval <= 0) return; | |
| if (isDragging) return; | |
| const timer = setInterval(() => { | |
| navigateToSlide(1); | |
| }, autoPlayInterval); | |
| return () => clearInterval(timer); | |
| }, [slideCount, renderIndex, autoPlayInterval, isDragging]); | |
| useEffect(() => { | |
| if (slideCount <= 1 || autoPlayInterval <= 0) return; | |
| if (isDragging) return; | |
| const timer = setInterval(() => { | |
| navigateToSlide(1); | |
| }, autoPlayInterval); | |
| return () => clearInterval(timer); | |
| }, [slideCount, renderIndex, autoPlayInterval, isDragging, navigateToSlide]); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/components/base-ui/News/NewsListBanner.tsx (2)
10-30: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
useMemodependencynewsListis always a new reference, defeating the cache.
data?.pages.flatMap(...)creates a new array on every render, so[newsList]changes every render and the memo never caches. Move thenewsListderivation inside theuseMemoand depend ondata?.pagesinstead.⚡ Fix useMemo dependency
- const newsList = data?.pages.flatMap((page) => page.basicInfoList) || []; - - // 'PROMOTION' 태그가 붙은 소식만 필터링 및 게시 종료 날짜 순 정렬 (최대 5개) const carouselItems = useMemo(() => { + const newsList = data?.pages.flatMap((page) => page.basicInfoList) || []; return newsList .filter((news) => news.carousel === "PROMOTION") .sort((a, b) => new Date(b.publishEndAt).getTime() - new Date(a.publishEndAt).getTime()) .slice(0, 5) .map((news) => { const isValidSrc = news.thumbnailUrl && news.thumbnailUrl !== "string" && (news.thumbnailUrl.startsWith("/") || news.thumbnailUrl.startsWith("http")); return { id: news.newsId, imageUrl: isValidSrc ? news.thumbnailUrl : "/news_sample.svg", title: news.title, description: news.description, link: `/news/${news.newsId}`, }; }); - }, [newsList]); + }, [data?.pages]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/base-ui/News/NewsListBanner.tsx` around lines 10 - 30, The `useMemo` in `NewsListBanner` is not caching because `newsList` is recreated on every render by `data?.pages.flatMap(...)`, so the dependency always changes. Move the `newsList` derivation inside the `useMemo` callback and make the memo depend on `data?.pages` (or the source pagination data) instead, while keeping the existing filter/sort/slice/map logic in `carouselItems`.
18-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared news-to-CarouselItem mapping to avoid duplication.
The
isValidSrcvalidation andCarouselItemmapping (lines 18–28) is identical toNewsBannerSlider.tsxlines 13–24. Extracting a shared helper would centralize the fallback logic and prevent drift.♻️ Shared helper extraction
// e.g., src/utils/newsCarousel.ts import type { CarouselItem } from "`@/components/common/SwipeCarousel`"; export function mapNewsToCarouselItem(news: { newsId: string | number; thumbnailUrl: string; title: string; description: string; }): CarouselItem { const isValidSrc = news.thumbnailUrl && news.thumbnailUrl !== "string" && (news.thumbnailUrl.startsWith("/") || news.thumbnailUrl.startsWith("http")); return { id: news.newsId, imageUrl: isValidSrc ? news.thumbnailUrl : "/news_sample.svg", title: news.title, description: news.description, link: `/news/${news.newsId}`, }; }Then in both components:
- .map((news) => { - const isValidSrc = news.thumbnailUrl && news.thumbnailUrl !== "string" && - (news.thumbnailUrl.startsWith("/") || news.thumbnailUrl.startsWith("http")); - return { - id: news.newsId, - imageUrl: isValidSrc ? news.thumbnailUrl : "/news_sample.svg", - title: news.title, - description: news.description, - link: `/news/${news.newsId}`, - }; - }); + .map(mapNewsToCarouselItem);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/base-ui/News/NewsListBanner.tsx` around lines 18 - 28, The news-to-CarouselItem mapping and thumbnail fallback logic are duplicated between NewsListBanner and NewsBannerSlider; extract the shared `isValidSrc` check plus item construction into a reusable helper (for example a `mapNewsToCarouselItem` utility) and have both components call it so the fallback behavior stays centralized and consistent.src/components/base-ui/home/News/NewsBannerSlider.tsx (2)
10-25: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
useMemodependencynewsListis always a new reference, defeating the cache.Same issue as
NewsListBanner.tsx:data?.pages.flatMap(...)creates a new array every render, so[newsList]always changes and the memo never caches. Move the derivation inside theuseMemoand depend ondata?.pages.⚡ Fix useMemo dependency
- const newsList = data?.pages.flatMap((page) => page.basicInfoList) || []; - const carouselItems = useMemo(() => { + const newsList = data?.pages.flatMap((page) => page.basicInfoList) || []; return newsList.slice(0, 5).map((news) => { const isValidSrc = news.thumbnailUrl && news.thumbnailUrl !== "string" && (news.thumbnailUrl.startsWith("/") || news.thumbnailUrl.startsWith("http")); return { id: news.newsId, imageUrl: isValidSrc ? news.thumbnailUrl : "/news_sample.svg", title: news.title, description: news.description, link: `/news/${news.newsId}`, }; }); - }, [newsList]); + }, [data?.pages]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/base-ui/home/News/NewsBannerSlider.tsx` around lines 10 - 25, The memo in NewsBannerSlider is ineffective because newsList is recreated on every render from data?.pages.flatMap(...), so useMemo always sees a new dependency. Move the newsList derivation inside the useMemo that builds carouselItems, and change the dependency to data?.pages so the cache can actually be reused. Use the existing NewsBannerSlider component and its carouselItems memo as the place to make this change.
13-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate news-to-CarouselItem mapping logic.
The
isValidSrcvalidation andCarouselItemmapping here (lines 13–24) is identical toNewsListBanner.tsxlines 18–28. See the extraction suggestion in theNewsListBannerreview for a sharedmapNewsToCarouselItemhelper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/base-ui/home/News/NewsBannerSlider.tsx` around lines 13 - 24, The news-to-CarouselItem mapping in NewsBannerSlider duplicates the same validation and field mapping used elsewhere, so extract and reuse a shared helper instead of keeping a second copy. Move the thumbnail validation and object निर्माण into the existing shared mapper suggested by NewsListBanner, then call that helper from the NewsBannerSlider transform so both components use the same logic and stay consistent.src/components/common/SwipeCarousel.tsx (1)
173-236: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider adding keyboard navigation for accessibility.
The carousel supports drag/swipe and indicator-dot clicks but lacks keyboard arrow-key navigation. Adding
tabIndex={0}and anonKeyDownhandler for ArrowLeft/ArrowRight would let keyboard users navigate slides. Anaria-roledescription="carousel"on the container andaria-live="polite"on the slide region would also improve screen-reader announcements.[accessibility]
♿ Suggested keyboard navigation addition
<div ref={containerRef} className={`relative ${heightClass} ${maxWidthClass} mx-auto overflow-hidden rounded-[10px] shadow-sm`} + tabIndex={0} + role="region" + aria-roledescription="carousel" + onKeyDown={(e) => { + if (e.key === "ArrowRight") { + e.preventDefault(); + navigateToSlide(1); + } else if (e.key === "ArrowLeft") { + e.preventDefault(); + navigateToSlide(-1); + } + }} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/common/SwipeCarousel.tsx` around lines 173 - 236, The SwipeCarousel container and slide region currently support drag and dot clicks but not keyboard or screen-reader navigation. Update the main wrapper in SwipeCarousel to be focusable with tabIndex and add aria-roledescription="carousel"; then add an onKeyDown handler to navigate with ArrowLeft/ArrowRight using the existing jumpTo/navigateToSlide logic. Also mark the visible slide region as aria-live="polite" so slide changes are announced to assistive tech.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(main)/groups/[id]/layout.tsx:
- Around line 149-154: The tab click guard in the group layout is using only
isLoggedIn, which can incorrectly open the login modal before auth has finished
initializing. Update the onClick handler for the notice/bookcase tabs to also
check the auth initialization state from useAuthStore (isInitialized), matching
the other auth-gated flows, so the modal is only shown after auth has settled.
In `@src/app/`(main)/groups/GroupsPageClient.tsx:
- Line 93: `isGuestDefaultView` in `GroupsPageClient` is currently based only on
`isLoggedIn` and `isSearchMode`, which allows a transient guest state before
auth bootstrapping completes. Update the guard to also check the auth
initialization state from `AuthProvider` (use `isInitialized` alongside
`isLoggedIn` and `isSearchMode`) so authenticated users don’t briefly take the
guest search path and trigger the clubs-search fetch.
---
Nitpick comments:
In `@src/components/base-ui/home/News/NewsBannerSlider.tsx`:
- Around line 10-25: The memo in NewsBannerSlider is ineffective because
newsList is recreated on every render from data?.pages.flatMap(...), so useMemo
always sees a new dependency. Move the newsList derivation inside the useMemo
that builds carouselItems, and change the dependency to data?.pages so the cache
can actually be reused. Use the existing NewsBannerSlider component and its
carouselItems memo as the place to make this change.
- Around line 13-24: The news-to-CarouselItem mapping in NewsBannerSlider
duplicates the same validation and field mapping used elsewhere, so extract and
reuse a shared helper instead of keeping a second copy. Move the thumbnail
validation and object निर्माण into the existing shared mapper suggested by
NewsListBanner, then call that helper from the NewsBannerSlider transform so
both components use the same logic and stay consistent.
In `@src/components/base-ui/News/NewsListBanner.tsx`:
- Around line 10-30: The `useMemo` in `NewsListBanner` is not caching because
`newsList` is recreated on every render by `data?.pages.flatMap(...)`, so the
dependency always changes. Move the `newsList` derivation inside the `useMemo`
callback and make the memo depend on `data?.pages` (or the source pagination
data) instead, while keeping the existing filter/sort/slice/map logic in
`carouselItems`.
- Around line 18-28: The news-to-CarouselItem mapping and thumbnail fallback
logic are duplicated between NewsListBanner and NewsBannerSlider; extract the
shared `isValidSrc` check plus item construction into a reusable helper (for
example a `mapNewsToCarouselItem` utility) and have both components call it so
the fallback behavior stays centralized and consistent.
In `@src/components/common/SwipeCarousel.tsx`:
- Around line 173-236: The SwipeCarousel container and slide region currently
support drag and dot clicks but not keyboard or screen-reader navigation. Update
the main wrapper in SwipeCarousel to be focusable with tabIndex and add
aria-roledescription="carousel"; then add an onKeyDown handler to navigate with
ArrowLeft/ArrowRight using the existing jumpTo/navigateToSlide logic. Also mark
the visible slide region as aria-live="polite" so slide changes are announced to
assistive tech.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f81f077f-c35a-4bfe-b38c-3c4994d31a1f
📒 Files selected for processing (8)
src/app/(main)/groups/GroupsPageClient.tsxsrc/app/(main)/groups/[id]/GroupDetailClient.tsxsrc/app/(main)/groups/[id]/layout.tsxsrc/components/base-ui/News/NewsListBanner.tsxsrc/components/base-ui/home/News/NewsBannerSlider.tsxsrc/components/common/SwipeCarousel.tsxsrc/components/layout/BottomNav.tsxsrc/components/layout/Header.tsx
| onClick={(e) => { | ||
| if ((tab.id === "notice" || tab.id === "bookcase") && !isLoggedIn) { | ||
| e.preventDefault(); | ||
| openLoginModal(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'isInitialized|setInitialized' src/store/useAuthStore.tsRepository: checkmo2025/FE
Length of output: 1077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## src/app/(main)/groups/[id]/layout.tsx\n'
sed -n '120,180p' src/app/'(main)'/groups/'[id]'/layout.tsx
printf '\n## src/store/useAuthStore.ts\n'
sed -n '1,220p' src/store/useAuthStore.ts
printf '\n## auth initialization references\n'
rg -n --no-heading 'isInitialized|setInitialized|openLoginModal|isLoggedIn' src | sed -n '1,220p'Repository: checkmo2025/FE
Length of output: 20560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## src/components/auth/AuthProvider.tsx\n'
sed -n '1,180p' src/components/auth/AuthProvider.tsx
printf '\n## src/hooks/useAuthGuard.ts\n'
sed -n '1,120p' src/hooks/useAuthGuard.ts
printf '\n## src/app/(main)/groups/GroupsPageClient.tsx\n'
sed -n '70,230p' src/app/'(main)'/groups/GroupsPageClient.tsxRepository: checkmo2025/FE
Length of output: 9849
Gate these tabs on auth initialization.
useAuthStore starts with isLoggedIn: false and only settles after AuthProvider finishes loading the session. On a hard refresh, an authenticated user can click notice/bookcase before that and get the login modal instead of navigating. Check isInitialized here, like the other auth-gated flows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/groups/[id]/layout.tsx around lines 149 - 154, The tab click
guard in the group layout is using only isLoggedIn, which can incorrectly open
the login modal before auth has finished initializing. Update the onClick
handler for the notice/bookcase tabs to also check the auth initialization state
from useAuthStore (isInitialized), matching the other auth-gated flows, so the
modal is only shown after auth has settled.
| const [region, setRegion] = useState(false); | ||
| const [appliedParams, setAppliedParams] = useState<Omit<ClubSearchParams, "cursorId"> | null>(null); | ||
| const isSearchMode = appliedParams !== null; | ||
| const isGuestDefaultView = !isLoggedIn && !isSearchMode; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C4 '(setInitialized|isInitialized|isLoggedIn)\s*[:=]' src/store/useAuthStore.ts
rg -nP -C4 'setInitialized|isInitialized' --type=ts -g '!**/*.test.*'Repository: checkmo2025/FE
Length of output: 1161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== GroupsPageClient.tsx ==\n'
ast-grep outline src/app/'(main)'/groups/GroupsPageClient.tsx --view expanded || true
printf '\n-- relevant lines --\n'
sed -n '1,180p' src/app/'(main)'/groups/GroupsPageClient.tsx | cat -n
printf '\n== Auth store ==\n'
ast-grep outline src/store/useAuthStore.ts --view expanded || true
printf '\n-- relevant lines --\n'
sed -n '1,140p' src/store/useAuthStore.ts | cat -n
printf '\n== Auth initialization references ==\n'
rg -n "setInitialized\\(|isInitialized|isLoggedIn|useAuthStore" src -g '!**/*.test.*' | sed -n '1,220p'Repository: checkmo2025/FE
Length of output: 29402
Gate isGuestDefaultView on auth init (src/app/(main)/groups/GroupsPageClient.tsx:93)
Until AuthProvider finishes bootstrapping, isLoggedIn is still false, so authenticated users can briefly hit the guest search path and trigger an unnecessary clubs-search fetch before switching to recommendations. Add isInitialized to this guard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/groups/GroupsPageClient.tsx at line 93, `isGuestDefaultView`
in `GroupsPageClient` is currently based only on `isLoggedIn` and
`isSearchMode`, which allows a transient guest state before auth bootstrapping
completes. Update the guard to also check the auth initialization state from
`AuthProvider` (use `isInitialized` alongside `isLoggedIn` and `isSearchMode`)
so authenticated users don’t briefly take the guest search path and trigger the
clubs-search fetch.
…rame drops This fixes an issue where the drag listener was unmounted and remounted on every render, causing jank during continuous swiping.
…rtualization bug This prevents identical DOM nodes from mixing states when replacing items during a swipe, ensuring reliable rendering of edge cases.
📌 개요 (Summary)
비로그인 유저(Guest)의 모임(/groups) 탐색 경험 개선 및 접근 제어 로직 리팩토링을 진행했습니다.
기존에는 모임 탭 진입 시 전역 useEffect를 통해 비회원을 강제로 메인 화면으로 리다이렉트(Hard-Gating)하여, 모임 리스트조차 둘러볼 수 없는 UX적인 문제와 비로그인 상태에서 빈 화면이 나타나는 Cold Start 이슈가 있었습니다.
이를 **컴포넌트 레벨의 이벤트 인터셉션(Soft-Gating)**과 API Dual-Track 전략으로 개편하여, 누구나 자유롭게 모임을 탐색하되 실제 행동(방문, 가입, 생성 등)을 취할 때만 즉각적인 로그인 모달을 띄우도록 기술적 최적화를 완료했습니다.
공통 캐러셀 컴포넌트 개발: Framer Motion을 기반으로 빈 여백이 발생하지 않는
연속 기차(Continuous Track)원리의<SwipeCarousel />공통 컴포넌트를 신규 개발했습니다.기존 페이지 리팩토링:
NewsBannerSlider및NewsListBanner컴포넌트 내부에 중복되어 있던 슬라이더 로직을 모두 제거하고, 신규 공통 컴포넌트를 주입하여 유지보수성을 극대화했습니다.실시간 반응형 버그 픽스: 스와이프 중 인디케이터(점)가 뒤늦게 갱신되는 현상 및 방치 시 순서가 꼬이는 비동기 상태 엇갈림(Race Condition) 버그를 수정했습니다.
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)🔬 기술적 상세 사항 (Technical Details)
1. 연속 트랙(Continuous Track) 및 가상화(Virtualization)
기존
AnimatePresence방식은 카드가 교체되는 찰나에 DOM에서 카드가 지워지면서 하얀 여백이 노출되는 치명적 한계가 있었습니다. 이를 해결하기 위해 카드를 가로로 묶은 연속 기차(Continuous Track) 구조로 변경했습니다. 메모리 과부하를 막기 위해 실제 데이터가 수십 개여도, 트랙에는 단 3장(이전, 현재, 다음)만 절대 좌표로 띄워두고 재활용하는 **가상화(Virtualization)**를 적용했습니다.2. 투 트랙(Two-Track) 렌더링 최적화
카드가 애니메이션으로 날아가는 도중에 React 상태 인덱스를 교체해버리면, 카드가 갑자기 눈앞에서 순간 이동을 하는 시각적 버그(Layout Shift)가 발생합니다. 하지만 인덱스 갱신을 늦추면 사용자 눈에 보이는 인디케이터(점)도 한 박자 늦게 켜지는 딜레마가 있습니다.
이를 DOM 렌더링용 인덱스와 시각용 점등 인덱스로 완전히 분리(Decoupling)하여 해결했습니다.
renderIndex: DOM 카드 재배치용 (애니메이션 완료 후에만 안전하게 변경)indicatorIndex: 하단 점 표시용 (useMotionValueEvent로 x 좌표를 감시하여, 손가락으로 50% 이상 드래그 시 즉시 반응)sequenceDiagram participant User as 사용자 (드래그) participant Motion as Framer Motion (x좌표) participant Indicator as indicatorIndex (점) participant Render as renderIndex (DOM) User->>Motion: 스와이프 발생 Motion-->>Indicator: 실시간 x 좌표 1px 단위로 관찰 alt x 좌표가 화면 너비의 절반(50%) 이상 넘어감 Indicator->>Indicator: indicatorIndex 즉시 변경 Note over Indicator: 하단 점(Dot) 즉각 불 켜짐! end Motion->>Motion: 카드 스냅 애니메이션 종료 (onComplete) Motion->>Render: renderIndex 업데이트 (DOM 교체) Render->>Indicator: 상태 최종 동기화 및 0점 정렬3. 상태 엇갈림 버그(Race Condition) 동기적 해결
React의 비동기 업데이트 타이밍과 Framer Motion의 즉각적 이벤트 실행 타이밍 간의 경합 조건(Race Condition)으로 인해 점의 위치가 과거로 돌아가버리는 버그가 있었습니다.
updateRenderIndex라는 별도의 헬퍼 함수를 만들어, React 화면 렌더링 대기열에 들어가기 전에 메모리(useRef)에 최신 인덱스를 동기적(Synchronous)으로 꽂아 넣는 기법을 사용하여 상태 충돌을 원천 차단했습니다.graph LR A[애니메이션 완료] -->|1. Ref에 즉시 주입| B(renderIndexRef.current = newIndex) B -->|2. React에 상태 갱신 예약| C(setRenderIndex) B -->|3. X 좌표 0으로 초기화| D[x.set 0] D -.->|4. X 변화 감지기 동기적 발동| E{X 감지기} E -->|이미 최신화된 Ref를 읽음!| F[올바른 점등 상태 유지] style B stroke:#ffeb3b,stroke-width:3px4. 모바일 방어 대책
touch-action: pan-y를 부여하여 가로 스와이프 중에는 브라우저 세로 스크롤이 원천 차단되도록 했습니다. (모바일 네이티브와 동일한 터치감 확보)draggable={false},pointer-events-none처리를 완료했습니다.5. 전역 라우팅 가드 제거 및 컴포넌트 이벤트 인터셉션 패턴 도입
6. Dual-Track 전략을 통한 비회원 초기 렌더링 최적화
백엔드의 /api/v1/clubs/recommendations API는 유저의 토큰을 기반으로 초개인화된 모임을 추천하므로 비회원에게는 401 에러를 반환합니다.
이에 isGuestDefaultView (비회원이면서 검색어가 없는 상태)라는 명시적인 뷰 렌더링 상태를 추가하고, 해당 상태일 경우 백엔드의 범용 검색 API(GET /api/v1/clubs/search?outputFilter=ALL)를 Fallback으로 호출하도록 React Query 옵션을 재조정했습니다.
7. 상세 페이지(/groups/:id) 단계적 권한 제어 (Progressive Gating)
방문하기 버튼 차단: 비회원이 방문하기 버튼 클릭 시 401 에러 화면이 나타나는 것을 막기 위해 진입 자체를 모달로 방어했습니다.
Home 탭 접근 완화: 로그인한 유저라면 해당 모임의 가입자가 아니더라도 Home 탭에서 모임 소개글을 확인하고 가입 신청을 할 수 있도록 layout.tsx의 useClubAccessGuard 권한을 완화했습니다.
내부 탭 철저 차단: Notice, Bookcase 등 멤버 전용 탭에 대해서만 렌더링 전에 requiresMemberAccess를 true로 설정하여 비정상적인 URL 접근을 원천 차단했습니다.
Summary by CodeRabbit
New Features
Bug Fixes