Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough무한 캐러셀 훅을 도입해 배너·인기 캐러셀의 드래그·자동재생 로직을 훅으로 이전하고, PopularCard 네비게이션을 앵커→클릭으로 변경했으며, 검색창 자동포커스·ButtonBar 비활성화 prop 및 여러 스타일·네이밍 조정이 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainBanner
participant useInfiniteCarousel
participant Logger
participant Router
User->>MainBanner: drag / swipe / click
MainBanner->>useInfiniteCarousel: attach handlers / forward pointer events
useInfiniteCarousel-->>MainBanner: slides, displayIndex, dragOffset, isSwiped, handlers
MainBanner->>Logger: logClickEvent (if click && !isSwiped)
MainBanner->>Router: navigate to detail (if click && !isSwiped)
Router-->>User: route change
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
🪷 Storybook 확인 🪷 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/hooks/useFilter.ts (2)
40-75:⚠️ Potential issue | 🟡 Minor
useCallback의존성 배열에setPrice누락 — 파이프라인 실패파이프라인 로그에서
react-hooks/exhaustive-deps경고가 발생하고 있습니다.setPrice를 의존성 배열에 추가해야 합니다. (useSetAtom이 반환하는 setter는 jotai에서 안정적인 참조를 보장하므로 불필요한 리렌더를 유발하지 않습니다.)🔧 수정 제안
- [router, addStorageValue], + [router, addStorageValue, setPrice],
78-82:⚠️ Potential issue | 🟠 Major
handleResetFilter에서priceAtom초기화 누락 —handleSearch와 불일치
handleSearch(Line 42-43)에서는filterListInstance.resetAllStates()와setPrice({ minPrice: 0, maxPrice: 30 })를 함께 호출하여 필터와 가격을 모두 초기화하지만,handleResetFilter에서는filterListInstance.resetAllStates()만 호출합니다. 필터 초기화 버튼을 눌렀을 때 가격 슬라이더가 리셋되지 않는 버그가 발생할 수 있습니다.🔧 수정 제안
const handleResetFilter = async () => { filterListInstance.resetAllStates(); + setPrice({ minPrice: 0, maxPrice: 30 }); queryClient.invalidateQueries({ queryKey: ['filteredList'], exact: false }); };src/app/homePage.css.ts (1)
31-41:⚠️ Potential issue | 🟡 Minor
modalOverlay의backgroundColor제거 확인 완료 - 일관성 개선 필요
backgroundColor는 의도적으로 제거되었으며, 배경색은Modal컴포넌트의modalBackdrop에서background: theme.COLORS.black60으로 제공됩니다. 그러나src/app/myPage/myPage.css.ts의modalOverlay는 여전히backgroundColor: theme.COLORS.black60을 포함하고 있어 두 파일 간 구현이 불일치합니다. 일관성을 위해myPage.css.ts의 정의를 정렬하거나, 둘 다 동일한 패턴을 따르도록 통일하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In `@src/components/banner/MainBanner.tsx`:
- Line 47: The map callback in MainBanner.tsx declares an unused parameter
"index" causing an ESLint error; remove the unused "index" parameter from the
slides.map callback (change slides.map((banner, index) => ...) to
slides.map(banner => ...)) so the component uses only banner and continues to
use banner.uniqueKey as the key.
- Around line 7-9: 현재 import 정렬이 ESLint `import/order` 규칙을 위반하고 있습니다: 이동해야 할 심볼은
useInfiniteCarousel이며 next/image보다 위에 있어야 하고 import 그룹 사이에 빈 줄이 필요합니다;
MainBanner.tsx에서 import 구문을 재정렬하여 `useInfiniteCarousel`을 `next/image`보다 먼저 불러오고
서로 다른 그룹(예: 외부 라이브러리, 절대/앱 레벨 모듈, 로컬 파일) 사이에 빈 줄을 추가해 BANNER_DATA와 styles import
그룹과 분리되도록 수정하세요.
In `@src/components/card/popularCard/PopularCard.tsx`:
- Line 18: Remove the unused "link" prop from the PopularCardProps interface and
from the PopularCard component's parameter destructuring (remove any reference
to link in PopularCard), and also stop passing "link" from the parent
PopularCarousel where it currently forwards that prop; since navigation is
handled via PopularCarousel's onClick, ensure no "link" remains in the props
interface, component signature, or the parent prop spread/prop list to eliminate
the ESLint no-unused-vars error.
- Line 48: The onKeyDown handler currently uses short-circuiting so onClick()
never runs for Enter; replace it with a proper conditional that calls onClick()
when e.key is 'Enter' or ' ' and call e.preventDefault() to stop page scrolling
for Space; update the onKeyDown prop (referencing onKeyDown, e.key and onClick)
to check both keys explicitly, call e.preventDefault() for the matched keys,
then invoke onClick().
In `@src/components/carousel/popularCarousel/PopularCarousel.tsx`:
- Around line 84-92: In the onClick handler for the PopularCarousel (the arrow
handling isSwiped, router.push and logClickEvent), reverse the call order so
logClickEvent('click_popularity_card', { label: temple.id }) runs before
initiating navigation with router.push(`/detail/${temple.id}`), or alternatively
make logging use a sendBeacon-style delivery (e.g., ensure logClickEvent uses
navigator.sendBeacon or a synchronous fallback) so the event is reliably sent
before the page unmounts; update the onClick that references isSwiped,
router.push and logClickEvent accordingly.
In `@src/components/common/button/buttonBar/ButtonBar.tsx`:
- Line 1: Move the local CSS import for buttonBar.css below the component
imports and separate import groups with a blank line to satisfy ESLint ordering:
ensure imports for external packages and components (e.g., the TextBtn import
from `@components/common/button/textBtn/TextBtn`) appear first, then add an empty
line, then import { buttonBarWrapper, buttonBarContainer } from
'./buttonBar.css' in ButtonBar.tsx.
In `@src/components/search/searchBar/SearchBar.tsx`:
- Around line 20-24: useEffect currently uses an empty dependency array which
triggers a react-hooks/exhaustive-deps warning for the autoFocus prop; update
the effect in SearchBar.tsx that calls inputRef.current.focus() so its
dependency array includes autoFocus (i.e. change [] to [autoFocus]) to satisfy
the linter while preserving behavior—do not add inputRef to deps since refs are
stable, and ensure the effect still guards with if (autoFocus &&
inputRef.current) before calling focus().
In `@src/components/search/searchHeader/SearchHeader.tsx`:
- Line 20: The pipeline fails due to jsx-a11y/no-autofocus being triggered on
the SearchBar's autoFocus prop; update the prop name to avoid the lint rule and
clarify intent: change the prop passed in SearchHeader from autoFocus to
shouldFocusOnMount and update the SearchBar component (and any other callers) to
accept and use shouldFocusOnMount (the SearchBar's existing useEffect +
ref.focus() behavior stays the same), or alternatively add an inline
eslint-disable-next-line jsx-a11y/no-autofocus comment immediately above the JSX
if you prefer the temporary bypass.
In `@src/hooks/useInfiniteCarousel.ts`:
- Around line 113-119: The handlers object currently only returns mouse events,
so mobile touch swipes won't work; add equivalent touch handlers to the same
handlers object that call the existing functions: onTouchStart should call
handleDragStart using e.touches[0].clientX, onTouchMove should call
handleDragMove using e.touches[0].clientX, onTouchEnd should call handleDragEnd,
and onTouchCancel should also call handleDragEnd; keep onTransitionEnd as-is
(handleTransitionEnd) and ensure the event types are handled safely (guard
touches length) so handleDrag* functions receive a numeric clientX just like
mouse handlers.
- Around line 8-18: The hook useInfiniteCarousel currently builds cloned slides
from data[0] / data[data.length-1] and sets totalOriginalSlides = data.length
without guarding empty arrays; add a defensive check so when data.length === 0
you return/produce safe defaults (e.g., slides = [], totalOriginalSlides = 0 and
any derived values like displayIndex, currentIndex, or autoplay timers handle
zero-length gracefully) and only build clone-first/clone-last when data.length >
0 (refer to the slides variable and totalOriginalSlides in useInfiniteCarousel
and ensure any logic that computes displayIndex or uses totalOriginalSlides
avoids division/mod by zero or invalid indexing).
🧹 Nitpick comments (8)
src/hooks/useFilter.ts (1)
22-22:getCookie가 모듈 스코프에서 한 번만 평가됨
isLoggedIn이 모듈 최상위에서 평가되므로, 사용자가 로그인/로그아웃 후에도 값이 갱신되지 않습니다. 이 PR에서 도입된 변경은 아니지만, 훅 내부 또는handleSearch내부로 이동하는 것이 안전합니다.♻️ 리팩터링 제안
-const isLoggedIn = getCookie('userNickname'); - const useFilter = () => { const { addStorageValue } = useLocalStorage(); const setPrice = useSetAtom(priceAtom); + const isLoggedIn = getCookie('userNickname');src/components/search/searchBar/searchBar.css.ts (1)
36-43:::placeholder에transform적용은 대부분의 브라우저에서 동작하지 않습니다.CSS
transform속성은::placeholder의사 요소에 적용되지 않습니다. input 요소 자체의transform: scale(0.9375)(Line 34)이 placeholder 텍스트에도 함께 적용되므로, placeholder 내의transform과transformOrigin은 불필요하며 제거해도 무방합니다.수정 제안
selectors: { '&::placeholder': { color: theme.COLORS.gray5, lineHeight: '1.5rem', - transform: 'scale(0.9375)', - transformOrigin: 'left center', }, },src/components/filter/filterBottomSheetModal/filterModalContent.css.ts (1)
16-21: Firefox에서 스크롤바가 여전히 표시될 수 있습니다.
::-webkit-scrollbar는 WebKit/Blink 기반 브라우저(Chrome, Safari)에서만 동작합니다. Firefox에서도 스크롤바를 숨기려면scrollbarWidth: 'none'을 추가하는 것이 좋습니다.🔧 Firefox 스크롤바 숨김 추가
export const main = style({ padding: '0 2rem', marginBottom: '7rem', overflowY: 'auto', maxHeight: 'calc(100vh - 280px)', height: '100vh', + scrollbarWidth: 'none', selectors: { '&::-webkit-scrollbar': { display: 'none', }, }, });src/components/filter/filterBottomSheetModal/FilterModalContent.tsx (1)
29-48:useMemo에서filtersState를 직접 사용하지 않아 ESLint 경고가 발생합니다.파이프라인 경고:
filtersState가useMemo콜백 내부에서 직접 참조되지 않고,filterListInstance.getGroupedSelectedFilters()를 대신 호출하고 있습니다.filtersState는 필터 변경 시 memo를 재계산하기 위한 트리거 역할인데, 이 패턴은 ESLintreact-hooks/exhaustive-deps규칙과 충돌합니다.
filtersState에서 직접 필터 값을 추출하거나, 불가능하다면 lint suppress 주석을 추가하는 것을 권장합니다.🔧 옵션 A: filtersState에서 직접 필터 값 추출
filtersState에서 선택된 필터를 직접 계산하면 ESLint 경고를 해소하면서filterListInstance싱글톤 직접 접근도 줄일 수 있습니다.🔧 옵션 B: lint suppress 주석 추가
현재 패턴을 유지하려면 의도를 명시하는 주석과 함께 suppress 처리합니다:
// eslint-disable-next-line react-hooks/exhaustive-deps -- filtersState is used as a recomputation trigger }, [price, filtersState, searchText]);src/styles/global.css.ts (1)
7-9:!important를 사용한 전역html패딩 오버라이드모달 라이브러리(예: body-scroll-lock 등)가 스크롤바 보정을 위해
html에paddingRight를 동적으로 주입하는 것을 방지하는 패턴으로 보입니다. 의도한 동작이라면 괜찮지만, 왜 이 규칙이 필요한지 간단한 주석을 남겨두면 향후 유지보수에 도움이 됩니다.💡 주석 추가 제안
+// 모달/오버레이 라이브러리가 스크롤바 보정으로 paddingRight를 주입하는 것을 방지 globalStyle('html', { paddingRight: '0px !important', });src/hooks/useInfiniteCarousel.ts (1)
55-94: 핸들러 함수 메모이제이션 고려
handleTransitionEnd,handleDragStart,handleDragMove,handleDragEnd가 매 렌더마다 새로 생성됩니다. 현재 사용 패턴에서는 큰 문제가 아니지만, 반환되는handlers객체가 매번 새로운 참조를 갖게 되므로, 소비 컴포넌트에서React.memo나 의존성 배열에handlers를 사용할 경우 불필요한 리렌더가 발생할 수 있습니다.향후 필요 시
useCallback과useMemo로 감싸는 것을 고려해 주세요.src/components/carousel/popularCarousel/popularCarousel.css.ts (1)
5-46:popularCard.css.ts와 스타일이 거의 완전히 중복됩니다.
container,slideList,slideItem,imageWrapper,templestayName,slideContent— 이 6개 스타일이src/components/card/popularCard/popularCard.css.ts에도 동일하게(또는 거의 동일하게) 정의되어 있습니다.PopularCard.tsx에서는popularCard.css를 import하고,PopularCarousel.tsx에서는popularCarousel.css를 import하는데, 실제로 카드 내부 레이아웃 스타일(slideItem,imageWrapper,slideContent,templestayName)은 카드 쪽에만 두고, 캐러셀 래퍼 스타일(container,slideList)만 이 파일에 유지하는 것이 DRY 원칙에 부합합니다.현재 구조에서는 스타일 변경 시 두 파일을 동시에 수정해야 하므로 유지보수 부담이 커집니다.
src/components/carousel/popularCarousel/PopularCarousel.tsx (1)
63-99: 캐러셀 슬라이드 너비가 하드코딩되어 CSS와 결합되어 있습니다.Line 68의
33.5 + 2는popularCarousel.css.ts의slideListwidth(33.5rem)와 gap(2rem)에 의존합니다. 이 값들이 CSS에서 변경되면 여기서도 수동으로 맞춰야 합니다. 당장 문제는 아니지만, 상수로 추출하여 한 곳에서 관리하면 유지보수가 편해집니다.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/search/searchBar/SearchBar.tsx (1)
62-66:⚠️ Potential issue | 🟡 Minor
useEffect의존성 배열에pathname누락.
pathname이 의존성 배열에 포함되지 않아react-hooks/exhaustive-deps경고가 발생할 수 있습니다. 마운트 시에만 실행하려는 의도라면 ESLint disable 주석을, 경로 변경에 반응하려면 의존성에 추가해야 합니다.수정 제안 (마운트 시 1회만 실행 의도인 경우)
useEffect(() => { if (pathname === '/search') { localStorage.setItem('prevPage', '/search'); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, []);수정 제안 (경로 변경에도 반응해야 하는 경우)
useEffect(() => { if (pathname === '/search') { localStorage.setItem('prevPage', '/search'); } - }, []); + }, [pathname]);
🤖 Fix all issues with AI agents
In `@src/components/carousel/popularCarousel/PopularCarousel.tsx`:
- Around line 6-10: Reorder the import statements to satisfy ESLint import
ordering: move the modules from external/package scopes so that
'@tanstack/react-query' (useQueryClient), 'cookies-next' (getCookie) and
internal hook alias '@hooks/useInfiniteCarousel' (useInfiniteCarousel) appear
before the Next import 'next/navigation' (useRouter); update the top of
PopularCarousel.tsx to import useQueryClient, getCookie and useInfiniteCarousel
above useRouter so ESLint no longer flags the import order.
In `@src/hooks/useInfiniteCarousel.ts`:
- Around line 131-144: The ternary expression assigning handlers when hasSlides
is true is mis-indented according to the ESLint indent rule; reformat the object
literal so its properties align with the surrounding indentation (fix
indentation for the block that contains onMouseDown, onMouseMove, onMouseUp,
onMouseLeave, onTouchStart, onTouchMove, onTouchEnd, onTransitionEnd) in
useInfiniteCarousel (the handlers assignment using hasSlides, and the functions
handleDragStart, handleDragMove, handleDragEnd, handleTransitionEnd). Ensure the
object braces and each property line follow the project's 8-space indenting
convention so ESLint no longer flags lines 133–143.
- Around line 75-97: When handleDragMove detects a vertical scroll and cancels
dragging, it only sets isDragging to false leaving isAnimate as false (set in
handleDragStart) so the carousel remains non-animated and auto-slide doesn't
resume; update handleDragMove to, when cancelling due to vertical scroll (the
branch that sets setIsDragging(false)), also call setIsAnimate(true) and restart
the auto-play (e.g., call the existing startAutoSlide or equivalent function
and/or undo stopAutoSlide) so animation and auto-slide are restored after the
cancel.
In `@src/styles/global.css.ts`:
- Around line 7-9: The globalStyle call currently hiding '::-webkit-scrollbar'
applies site-wide and harms accessibility; change it so you remove or stop using
the global '::-webkit-scrollbar' rule and instead apply scrollbar hiding only to
specific container selectors (e.g., carousel or modal container symbols) by
calling globalStyle with those container class/ID selectors; if you need broader
support include the Firefox equivalent by adding 'scrollbar-width: none' for
those scoped selectors and avoid a blanket globalStyle('::-webkit-scrollbar',
...) rule.
- Around line 11-13: The globalStyle override that force-removes paddingRight
via 'globalStyle' should be removed because it breaks react-modal-sheet's
scroll-lock gutter handling; instead stop overriding paddingRight and either add
the disableScrollLocking prop to your Sheet components (use the Sheet
component's disableScrollLocking attribute) so the library doesn't inject
padding, or ensure the scrollbar gutter is always present by setting body
overflow behavior at the global CSS level (e.g., apply an always-visible
vertical scrollbar via a global style like body { overflow-y: scroll }) to avoid
layout shift when modals open.
🧹 Nitpick comments (2)
src/components/carousel/popularCarousel/PopularCarousel.tsx (1)
67-69: 하드코딩된 슬라이드 너비 — CSS 값과 동기화 위험.
33.5 + 2는popularCard.css.ts의slideListwidth(33.5rem)와 gap(2rem)에서 유래한 매직 넘버입니다. CSS 값이 변경되면 이 계산도 함께 수정해야 하지만 놓치기 쉽습니다. 상수로 추출하여 CSS와 JS가 동일한 값을 참조하도록 하는 것을 권장합니다.♻️ 상수 추출 제안
+// 상수 정의 (공유 파일 또는 컴포넌트 상단) +const SLIDE_WIDTH_REM = 33.5; +const SLIDE_GAP_REM = 2; + // PopularCarousel 내부 style={{ - transform: `translateX(calc(-${currentIndex * (33.5 + 2)}rem + ${dragOffset}px))`, + transform: `translateX(calc(-${currentIndex * (SLIDE_WIDTH_REM + SLIDE_GAP_REM)}rem + ${dragOffset}px))`,src/components/card/popularCard/popularCard.css.ts (1)
4-39:popularCarousel.css.ts와 스타일 정의 중복.
slideItem,imageWrapper,slideContent,templestayName등의 스타일이popularCarousel.css.ts에도 동일하게 정의되어 있습니다(popularCarousel.css.tsLines 18-45 참고). 한쪽에서 스타일을 수정하면 다른 쪽과 불일치가 발생할 수 있으므로, 공통 스타일을 하나의 파일로 통합하거나 공유하는 것을 검토해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/banner/MainBanner.tsx`:
- Line 55: The onKeyDown handler in MainBanner.tsx currently triggers
handleBannerClick(banner) for Enter or Space but does not prevent the browser's
default behavior for Space (which can scroll the page); update the inline
handler (or the delegated key handler) to call e.preventDefault() when e.key ===
' ' (or e.code === 'Space') before invoking handleBannerClick(banner) so Space
activation doesn't cause page scroll while preserving Enter behavior.
2 participants
🛰️ 관련 이슈
🧑💻 작업 내용
🗯️ PR 포인트
🚀 알게된 점
📖 참고 자료 (선택)
📸 스크린샷 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
UI 개선