feat(map): add overlapping feed marker stack#11
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 29 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughPR은 지도에서 근접 피드를 그룹으로 묶어 덱 기반 오버레이로 표시하는 기능을 추가합니다. 하버사인 기반 거리 계산으로 피드를 클러스터링하고, 제스처 인식 및 애니메이션 상수화를 통해 카드 덱 UI를 개선하며, 마커 시각화와 스타일을 feed-group 변형으로 확장합니다. Changes피드 마커 그룹화 및 덱 기반 카드 시스템
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 (4)
src/features/map/ui/ClusterBlob.tsx (1)
378-398: ⚖️ Poor tradeoff전역 keyframe 이름 중복 정의
spot-cluster-core-breathe,spot-feed-group-core-shift등의 keyframe이 각ClusterBlob인스턴스마다<style>태그로 재정의됩니다. 기능적으로는 문제없지만, 마커가 많을 때 DOM에 중복 스타일이 누적됩니다.성능에 민감한 경우 keyframe을 전역 CSS 파일로 추출하거나
useInsertionEffect로 한 번만 삽입하는 방식을 고려할 수 있습니다.🤖 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/features/map/ui/ClusterBlob.tsx` around lines 378 - 398, The inline <style> block in the ClusterBlob component is redefining keyframes (spot-cluster-core-breathe, spot-cluster-discovery-breathe, spot-feed-group-core-shift, spot-cluster-satellite-drift) for every instance, causing redundant DOM styles; fix it by extracting these keyframes to a single global CSS file or inject them once per app lifecycle (e.g., add a one-time insertion using useInsertionEffect or a module-level flag inside ClusterBlob) so the keyframes are defined only once and remove the per-instance <style> block.src/features/feed/model/feed-marker-group.ts (1)
65-83: 💤 Low valueSingle-linkage 클러스터링으로 인한 체이닝 효과 가능성
현재 알고리즘은 그룹 내 어떤 좌표라도 임계값 내에 있으면 해당 그룹에 합류합니다. 이로 인해 A↔B(10m), B↔C(10m)인 경우 A와 C가 20m 떨어져 있어도 같은 그룹이 됩니다.
18m 임계값에서는 실제로 큰 문제가 되지 않을 수 있지만, 피드가 밀집된 지역에서 예상보다 큰 그룹이 생성될 수 있습니다. 의도된 동작인지 확인이 필요합니다.
🤖 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/features/feed/model/feed-marker-group.ts` around lines 65 - 83, The current single-linkage logic in the grouping loop (using resolveCoord, getDistanceMeters and thresholdMeters) causes chaining (A–B and B–C within threshold but A–C may exceed it); change the membership test so a new coord is compared to the group's representative instead of any member to avoid chaining—e.g., maintain and update a group's centroid (or representative coord) on groups entries and use getDistanceMeters(representative, coord) <= thresholdMeters when deciding membership, then update the representative when adding the new coord; ensure the representative is initialized when pushing a new group and recalculated (average or weighted) after pushing.src/features/map/client/MapClient.tsx (1)
187-191: 💤 Low value덱 열림 상태에서 클러스터 변경 시 pager 상태 리셋 누락 가능성
isMapMarkerDeckOpen만 의존성에 있어서 덱이 이미 열린 상태(true)에서 다른 클러스터를 선택해도 이 effect가 재실행되지 않습니다.현재 UX상 덱 간 전환 시 pager 상태 유지가 의도된 것이라면 문제없지만, 매 클러스터 선택마다 리셋이 필요하다면
selectedClusterId도 의존성에 추가해야 합니다.🤖 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/features/map/client/MapClient.tsx` around lines 187 - 191, The effect that resets pager state (useEffect containing setPagerPromotedCount and setPagerSnap) only depends on isMapMarkerDeckOpen, so when the deck is already open and selected cluster changes the effect won’t rerun; if you want to reset pager on every cluster selection as well, include selectedClusterId in the dependency array (i.e., change the hook to depend on [isMapMarkerDeckOpen, selectedClusterId]) so that setPagerPromotedCount(0) and setPagerSnap('peek') run whenever the open deck and selectedClusterId change.src/features/map/model/feed-stack-gesture.test.ts (1)
14-46: ⚡ Quick win속도 임계값 분기도 테스트로 고정해두는 편이 좋습니다.
지금 스위트는 거리 기반 좌우 스와이프만 검증해서,
absVx >= SWIPE_SIDE_VELOCITY_THRESHOLD분기가 깨져도 놓칩니다. 거리 부족 + 고속 스와이프 케이스 하나를 추가해 두면 회귀를 막기 쉽습니다.🤖 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/features/map/model/feed-stack-gesture.test.ts` around lines 14 - 46, Add tests covering the velocity branch of resolveFeedStackGesture: the suite currently only checks distance-based side swipes and can miss the absVx >= SWIPE_SIDE_VELOCITY_THRESHOLD path. Add at least one short-distance, high-velocity case (simulate dx small, dy small, and set vx such that absVx >= SWIPE_SIDE_VELOCITY_THRESHOLD) for both left and right to assert 'detail-left' and 'detail-right' respectively; reference resolveFeedStackGesture, absVx and SWIPE_SIDE_VELOCITY_THRESHOLD to locate the logic to protect against regressions.
🤖 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/features/map/ui/ClusterBlob.tsx`:
- Around line 542-554: The CSS animation duration is hardcoded to 2.8s while the
framer-motion transition uses isFeedGroup ? 3.2 : 2.8, causing mismatch; in the
ClusterBlob component compute a single animationDuration variable (e.g., const
animationDuration = isFeedGroup ? 3.2 : 2.8) and use that variable for both the
framer-motion transition.duration and the inline CSS animation string (replace
the hardcoded 2.8 in the template `${2.8 + i * 0.22}s ...` with
`${animationDuration + i * 0.22}s ...`) so both animations run at the same base
speed.
In `@src/features/map/ui/MapCardDeckOverlay.tsx`:
- Around line 72-79: closeDeck and dismissTop should not call setTimeout to
guess when exit animations finish; instead they should mark the "closing" intent
and let AnimatePresence's onExitComplete execute the parent close. Change
closeDeck/dismissTop to only call setExitOverride(...) and
setIsClosingDeck(true) (preserve prefersReducedMotion path by calling
onCloseAction immediately if prefersReducedMotion is true), and remove the
window.setTimeout(onCloseAction, ...) logic; then update onExitComplete to check
the isClosingDeck flag and, when true, call onCloseAction and clear closing
state (setIsClosingDeck(false) and setExitOverride(null)) so the actual close
happens only after the exit animation completes. Ensure you reference the same
symbols: closeDeck, dismissTop, setExitOverride, setIsClosingDeck,
onExitComplete, onCloseAction, prefersReducedMotion, visibleItems, and
DECK_ANIMATION.
---
Nitpick comments:
In `@src/features/feed/model/feed-marker-group.ts`:
- Around line 65-83: The current single-linkage logic in the grouping loop
(using resolveCoord, getDistanceMeters and thresholdMeters) causes chaining (A–B
and B–C within threshold but A–C may exceed it); change the membership test so a
new coord is compared to the group's representative instead of any member to
avoid chaining—e.g., maintain and update a group's centroid (or representative
coord) on groups entries and use getDistanceMeters(representative, coord) <=
thresholdMeters when deciding membership, then update the representative when
adding the new coord; ensure the representative is initialized when pushing a
new group and recalculated (average or weighted) after pushing.
In `@src/features/map/client/MapClient.tsx`:
- Around line 187-191: The effect that resets pager state (useEffect containing
setPagerPromotedCount and setPagerSnap) only depends on isMapMarkerDeckOpen, so
when the deck is already open and selected cluster changes the effect won’t
rerun; if you want to reset pager on every cluster selection as well, include
selectedClusterId in the dependency array (i.e., change the hook to depend on
[isMapMarkerDeckOpen, selectedClusterId]) so that setPagerPromotedCount(0) and
setPagerSnap('peek') run whenever the open deck and selectedClusterId change.
In `@src/features/map/model/feed-stack-gesture.test.ts`:
- Around line 14-46: Add tests covering the velocity branch of
resolveFeedStackGesture: the suite currently only checks distance-based side
swipes and can miss the absVx >= SWIPE_SIDE_VELOCITY_THRESHOLD path. Add at
least one short-distance, high-velocity case (simulate dx small, dy small, and
set vx such that absVx >= SWIPE_SIDE_VELOCITY_THRESHOLD) for both left and right
to assert 'detail-left' and 'detail-right' respectively; reference
resolveFeedStackGesture, absVx and SWIPE_SIDE_VELOCITY_THRESHOLD to locate the
logic to protect against regressions.
In `@src/features/map/ui/ClusterBlob.tsx`:
- Around line 378-398: The inline <style> block in the ClusterBlob component is
redefining keyframes (spot-cluster-core-breathe, spot-cluster-discovery-breathe,
spot-feed-group-core-shift, spot-cluster-satellite-drift) for every instance,
causing redundant DOM styles; fix it by extracting these keyframes to a single
global CSS file or inject them once per app lifecycle (e.g., add a one-time
insertion using useInsertionEffect or a module-level flag inside ClusterBlob) so
the keyframes are defined only once and remove the per-instance <style> block.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4263806c-121c-4fe4-ae43-4033a019cf8e
📒 Files selected for processing (22)
src/app/providers/theme-provider.tsxsrc/features/feed/model/card-deck-animation.tssrc/features/feed/model/feed-marker-group.test.tssrc/features/feed/model/feed-marker-group.tssrc/features/feed/ui/MapFeedCardPager.tsxsrc/features/map/client/MapClient.tsxsrc/features/map/model/feed-stack-gesture.test.tssrc/features/map/model/feed-stack-gesture.tssrc/features/map/model/hotspot-feed-overlap.test.tssrc/features/map/model/hotspot-feed-overlap.tssrc/features/map/model/marker-visual-mode.test.tssrc/features/map/model/marker-visual-mode.tssrc/features/map/model/types.tssrc/features/map/ui/ClusterBlob.test.tsxsrc/features/map/ui/ClusterBlob.tsxsrc/features/map/ui/MapBottomStack.tsxsrc/features/map/ui/MapCardDeckOverlay.tsxsrc/features/map/ui/MapFeedInfoCard.tsxsrc/features/map/ui/MapFeedStackCard.tsxsrc/features/map/ui/MapTutorialOverlay.tsxsrc/features/map/ui/SpotInfoCard.tsxsrc/features/my/client/MyPageClient.tsx
💤 Files with no reviewable changes (1)
- src/features/my/client/MyPageClient.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Checks
Notjing Final Gate
Summary by CodeRabbit
릴리스 노트
New Features
UI/Style