[Feature/#64] 모임방 상세 페이지 UI#70
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough이 PR은 진행 중이거나 완료된 모임방의 상세 페이지 기능을 구현합니다. GroupDetailScreen과 Expo Router 레이아웃 및 동적 [roomId] 라우트를 추가하고, 타입 정의(DTO)와 DUMMY_GROUP_DETAIL을 포함합니다. 헤더, 바텀시트, 확인 모달, 책 정보 컴포넌트, 기록장 요약, 투표 카루셀, 일일 인사말 컴포넌트 등을 새로 추가했으며, RecruitingGroupInfo를 범용 GroupInfo로 리팩토링하고 여러 화면의 네비게이션과 ISBN 파라미터 전달을 정규화했습니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/ui/book-info-bar/index.tsx (1)
20-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winP4 - 완료된 TODO는 제거/갱신해주세요
클릭 시 상세 이동은 이미 구현되어 있어서 현재 TODO 문구가 오해를 만들 수 있습니다. TODO를 제거하거나 실제 남은 작업으로 갱신해 주세요.
As per coding guidelines 가독성, 예외처리, 테스트/확장/유지보수성을 기반으로 리뷰해야 하며 팀 스타일 통일성도 확인해야 합니다.
🤖 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/shared/ui/book-info-bar/index.tsx` at line 20, Remove or update the misleading TODO comment "// TODO: 클릭 시 책 상세 정보 페이지로 이동되도록" in src/shared/ui/book-info-bar/index.tsx (component BookInfoBar) because the click-to-detail navigation is already implemented; either delete the TODO or replace it with an accurate note about any remaining work (or a short FIXME with exact next steps) so the source and BookInfoBar's click handler no longer imply incomplete functionality.
🧹 Nitpick comments (3)
src/shared/ui/group-info/index.tsx (1)
90-128: ⚡ Quick win[P4] 분기 UI 회귀 방지용 단위 테스트 2~3개를 같이 추가하면 안정성이 크게 올라갑니다
변경 분기가 많아서 테스트 한 번만 깔아두면 이후 리팩터링이 훨씬 편합니다. 추천 케이스:
isRecruiting=true일 때 독서메이트 영역 disabled + 화살표 미노출isRecruiting=false+onPressReadingMate전달 시 클릭 콜백 호출recruitEndDate유무에 따른 모집 칩 노출/미노출As per coding guidelines
src/**: "미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요."🤖 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/shared/ui/group-info/index.tsx` around lines 90 - 128, Add unit tests for the GroupInfo UI to prevent regression around the recruiting/display branches: write tests for the component exported in src/shared/ui/group-info/index.tsx (the Pressable block/reading mate area and the recruit end-date chip). Specifically: (1) render with isRecruiting=true and assert the Pressable is disabled and the right-arrow icon (IcRightRight) is not present; (2) render with isRecruiting=false, pass a mock onPressReadingMate handler (or simulate handleToReadingMateList via the component prop) and assert pressing the Pressable invokes the callback; (3) render twice for recruitEndDate present vs absent and assert the recruit chip (the View with styles.endDateCategoryItem and the text containing recruitEndDate) is shown when provided and not rendered when undefined. Use the component name from the file and test-library utilities to query by text/role and simulate press.screens/group/components/my-group-carousel/my-group-carousel-item.tsx (1)
137-137: P4: TODO — 이미지 영역 포함 탭 UX 이슈이미지 영역까지 눌렀을 때 네비게이션이 동작하지 않는 문제가 남아있습니다. 별도 이슈로 트래킹하면 좋을 것 같습니다.
새 이슈로 등록하거나 원인 분석이 필요하시면 말씀해 주세요! (
Pressable의hitSlop또는Animated.View의 레이아웃 충돌이 원인일 가능성이 있습니다.)🤖 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 `@screens/group/components/my-group-carousel/my-group-carousel-item.tsx` at line 137, The carousel item’s image isn’t triggering navigation because the touchable area likely doesn’t cover the image or an Animated.View is intercepting touches; update MyGroupCarouselItem so the Pressable fully wraps the image view (or move the Animated.View inside the Pressable) and add a conservative hitSlop on the Pressable to expand its tappable area, remove/adjust any pointerEvents or touch handling on the Animated.View that could block touches, then verify navigation works when tapping the image; check the Pressable usage and Animated.View containment in the component to locate the exact spot to change.screens/group-detail/group-detail-screen.tsx (1)
67-79: ⚡ Quick winP4: 네비게이션 폴백 로직은 헬퍼로 추출하면 유지보수성이 더 좋아집니다
Line 74
78과 Line 101105가 동일해서 이후 경로 정책 변경 시 누락 위험이 있습니다. 작은 헬퍼로 묶어두면 액션 추가 시에도 안전합니다. 디테일 잘 만드셨고, 여기까지만 정리하면 더 탄탄해집니다 🙂♻️ 제안 리팩터링
+ const navigateBackOrGroup = () => { + if (router.canGoBack()) { + router.back(); + } else { + router.push("/group"); + } + }; const handleReportGroup = () => { setIsBottomSheetVisible(false); Toast.show({ type: "default", text1: "모임방 신고를 완료했어요.", }); - if (router.canGoBack()) { - router.back(); - } else { - router.push("/group"); - } + navigateBackOrGroup(); }; ... - if (router.canGoBack()) { - router.back(); - } else { - router.push("/group"); - } + navigateBackOrGroup(); };Also applies to: 87-106
🤖 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 `@screens/group-detail/group-detail-screen.tsx` around lines 67 - 79, Extract the repeated navigation fallback into a small helper (e.g., navigateBackOrPushHome or goBackOrToGroup) and replace the duplicated logic in handleReportGroup and the other identical locations (the blocks using router.canGoBack(), router.back(), router.push("/group")) with a single call to that helper; the helper should accept the router instance (and optionally a fallback path like "/group") and perform the canGoBack/back or push(fallback) decision so future path-policy changes are made in one place.
🤖 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 `@screens/group-detail/components/daily-greeting-button/index.tsx`:
- Around line 14-16: handleToDailyGreeting currently only console.logs and
leaves the button non-functional; remove the console.log and implement the
intended behavior or a safe interim: either call the actual navigation helper
(e.g., invoke the app router/navigation function with roomId — references:
handleToDailyGreeting and roomId) to navigate to the daily-greeting page, or
disable the button and show a "준비 중" toast/modal until navigation is
implemented. Ensure you update the button's disabled state and remove any
console.log from handleToDailyGreeting so production code has no stray logs.
In `@screens/group-detail/components/group-detail-modal/index.tsx`:
- Around line 24-28: The modal currently shows the deletion warning
unconditionally because the AppText blocks use a fixed string; update the
conditional rendering in the group-detail-modal component so the descriptive
text and button label vary by the prop type (the existing type variable) — when
type === "delete" keep the current deletion warning and confirm copy, but when
type === "leave" show a leave-specific message (e.g., explain leaving the room
or losing access) and change the negative button text to "아니요" (also adjust the
confirm button copy appropriately for each action). Locate and update the two
AppText instances that render the secondary message and the buttons around the
same area (the AppText blocks and the button labels referenced near the existing
type === "delete" conditional) so they branch on type === "delete" vs "leave".
In `@screens/group-detail/components/record-book-overview/index.tsx`:
- Around line 45-59: Clamp the progress value before rendering so the progress
bar and displayed percentage never break layout; compute a clampedPercentage =
Math.min(100, Math.max(0, userPercentage)) and use clampedPercentage for both
the displayed text and the style width (i.e., in the View with
styles.currentProgress and the AppText showing userPercentage) so the bar width
is always between 0% and 100% and the shown number matches the bar.
In `@src/shared/ui/group-info/index.tsx`:
- Around line 40-42: The CTA currently uses a no-op console.log in
handleToReadingMateList which breaks the user flow; change the component to
accept a callback prop (e.g., onOpenReadingMateList?: (roomId: string) => void)
and replace the body of handleToReadingMateList to call that prop with roomId
(and optionally fall back to existing navigation behavior only if the prop is
not provided). Update the component props/type signature to include
onOpenReadingMateList, call it from handleToReadingMateList, and ensure callers
(parent components) pass the real navigation handler so the CTA performs the
intended navigation instead of logging.
---
Outside diff comments:
In `@src/shared/ui/book-info-bar/index.tsx`:
- Line 20: Remove or update the misleading TODO comment "// TODO: 클릭 시 책 상세 정보
페이지로 이동되도록" in src/shared/ui/book-info-bar/index.tsx (component BookInfoBar)
because the click-to-detail navigation is already implemented; either delete the
TODO or replace it with an accurate note about any remaining work (or a short
FIXME with exact next steps) so the source and BookInfoBar's click handler no
longer imply incomplete functionality.
---
Nitpick comments:
In `@screens/group-detail/group-detail-screen.tsx`:
- Around line 67-79: Extract the repeated navigation fallback into a small
helper (e.g., navigateBackOrPushHome or goBackOrToGroup) and replace the
duplicated logic in handleReportGroup and the other identical locations (the
blocks using router.canGoBack(), router.back(), router.push("/group")) with a
single call to that helper; the helper should accept the router instance (and
optionally a fallback path like "/group") and perform the canGoBack/back or
push(fallback) decision so future path-policy changes are made in one place.
In `@screens/group/components/my-group-carousel/my-group-carousel-item.tsx`:
- Line 137: The carousel item’s image isn’t triggering navigation because the
touchable area likely doesn’t cover the image or an Animated.View is
intercepting touches; update MyGroupCarouselItem so the Pressable fully wraps
the image view (or move the Animated.View inside the Pressable) and add a
conservative hitSlop on the Pressable to expand its tappable area, remove/adjust
any pointerEvents or touch handling on the Animated.View that could block
touches, then verify navigation works when tapping the image; check the
Pressable usage and Animated.View containment in the component to locate the
exact spot to change.
In `@src/shared/ui/group-info/index.tsx`:
- Around line 90-128: Add unit tests for the GroupInfo UI to prevent regression
around the recruiting/display branches: write tests for the component exported
in src/shared/ui/group-info/index.tsx (the Pressable block/reading mate area and
the recruit end-date chip). Specifically: (1) render with isRecruiting=true and
assert the Pressable is disabled and the right-arrow icon (IcRightRight) is not
present; (2) render with isRecruiting=false, pass a mock onPressReadingMate
handler (or simulate handleToReadingMateList via the component prop) and assert
pressing the Pressable invokes the callback; (3) render twice for recruitEndDate
present vs absent and assert the recruit chip (the View with
styles.endDateCategoryItem and the text containing recruitEndDate) is shown when
provided and not rendered when undefined. Use the component name from the file
and test-library utilities to query by text/role and simulate press.
🪄 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: edf06c48-01bb-47cb-8060-576932832b35
📒 Files selected for processing (26)
app/_layout.tsxapp/group-detail/[roomId].tsxapp/group-detail/_layout.tsxscreens/group-detail/components/daily-greeting-button/index.tsxscreens/group-detail/components/group-book/index.tsxscreens/group-detail/components/group-detail-bottom-sheet/index.tsxscreens/group-detail/components/group-detail-header/index.tsxscreens/group-detail/components/group-detail-modal/index.tsxscreens/group-detail/components/index.tsscreens/group-detail/components/record-book-overview/index.tsxscreens/group-detail/components/votes-carousel/index.tsxscreens/group-detail/constants/index.tsscreens/group-detail/group-detail-screen.tsxscreens/group-detail/index.tsscreens/group-detail/types/index.tsscreens/group/components/my-group-carousel/my-group-carousel-item.tsxscreens/join-group/components/index.tsscreens/join-group/components/join-group-book/index.tsxscreens/join-group/join-group-screen.tsxscreens/saved/components/saved-book/index.tsxscreens/search/components/most-searched-book-item/index.tsxscreens/search/components/searched-book-item/index.tsxsrc/shared/ui/book-info-bar/index.tsxsrc/shared/ui/group-info/index.tsxsrc/shared/ui/index.tssrc/shared/ui/my-group-card/index.tsx
💤 Files with no reviewable changes (1)
- screens/join-group/components/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
screens/group-detail/components/daily-greeting-button/index.tsx (1)
14-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick win[P2]
오늘의 한마디버튼이 실제 상세 플로우를 열지 않습니다지금은
alert플레이스홀더만 실행되어, PR 목표(오늘의 한마디 페이지 흐름 구현) 기준으로 사용자 액션이 기능으로 이어지지 않습니다. 실제 라우트로 연결하거나, 미구현 상태라면 버튼 비활성화+명시적 안내로 동작을 고정해 주세요.빠른 수정 예시
+import { router } from "expo-router"; ... const handleToDailyGreeting = () => { - alert(`${roomId}번 오늘의 한마디 페이지 준비 중`); + router.push({ + pathname: "/daily-greeting/[roomId]", + params: { roomId: String(roomId) }, + }); };🤖 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 `@screens/group-detail/components/daily-greeting-button/index.tsx` around lines 14 - 18, The button's onPress handler handleToDailyGreeting currently only shows an alert placeholder and doesn't navigate to the daily-greeting flow; replace the placeholder with the real routing call (e.g., use the app navigation to navigate to the DailyGreeting screen with roomId) or, if the flow isn't implemented yet, disable the Pressable (set disabled prop and apply visual disabled style in styles.container) and change handleToDailyGreeting to show a clear message like "오늘의 한마디는 준비 중입니다" so the UX is explicit; update references in this file to use the app's navigation helper (e.g., useNavigation()/navigate) or adjust Pressable disabled behavior and styles accordingly.
🤖 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/shared/ui/group-info/index.tsx`:
- Around line 89-102: The right-arrow affordance (IcRightRight) is shown even
when the Pressable is disabled, which misleads users; update the rendering logic
in the GroupInfo component so IcRightRight is only rendered when the Pressable
is active (i.e., when onPressReadingMate exists and isRecruiting is false).
Locate the Pressable block using the Pressable, onPressReadingMate and
isRecruiting symbols and change the conditional around IcRightRight so the arrow
is omitted when the control is disabled.
---
Duplicate comments:
In `@screens/group-detail/components/daily-greeting-button/index.tsx`:
- Around line 14-18: The button's onPress handler handleToDailyGreeting
currently only shows an alert placeholder and doesn't navigate to the
daily-greeting flow; replace the placeholder with the real routing call (e.g.,
use the app navigation to navigate to the DailyGreeting screen with roomId) or,
if the flow isn't implemented yet, disable the Pressable (set disabled prop and
apply visual disabled style in styles.container) and change
handleToDailyGreeting to show a clear message like "오늘의 한마디는 준비 중입니다" so the UX
is explicit; update references in this file to use the app's navigation helper
(e.g., useNavigation()/navigate) or adjust Pressable disabled behavior and
styles accordingly.
🪄 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: e932533c-354f-4577-af10-cc9a90bbf6e6
📒 Files selected for processing (5)
screens/group-detail/components/daily-greeting-button/index.tsxscreens/group-detail/components/group-detail-modal/index.tsxscreens/group-detail/components/record-book-overview/index.tsxscreens/group-detail/group-detail-screen.tsxsrc/shared/ui/group-info/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- screens/group-detail/components/record-book-overview/index.tsx
- screens/group-detail/components/group-detail-modal/index.tsx
- screens/group-detail/group-detail-screen.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.
📌 Related Issues
📄 Tasks
📷 Screenshot
Summary by CodeRabbit