[refactor] #196 PhotoDetail 메모 UI를 HorizontalPager 밖으로 분리#197
Merged
ikseong00 merged 6 commits intoApr 20, 2026
Conversation
- PhotoDetailState의 memoModes: Map<Long, MemoMode> 제거 - 단일 memoMode: MemoMode 필드로 대체 - currentMemoMode / memoModeOf 헬퍼 제거 - ViewModel의 메모 인텐트 처리 단순화 - 페이지 전환 시 memo 내용만 갱신하고 모드는 유지
- PhotoDetailScreen에서 Pager/Dim/MemoTextField를 감싸는 PhotoDetailPagerArea 컴포저블 추출 - dim 오버레이와 MemoTextField를 각 페이지 안이 아닌 pager 밖 공용 Box에 배치 - PhotoDetailImageItem 시그니처 단순화 (memo/MemoMode/액션 핸들러 제거, 이미지 + 좌우 탭만 유지) - 메모 관련 UI가 화면 전역 단일 인스턴스로 동작하여 페이지 전환 시 모드는 유지되고 내용만 갱신
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 0 seconds. ⌛ 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPhotoDetail의 메모 상태를 사진별 맵에서 전역 단일 Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Screen as PhotoDetailScreen
participant Pager as HorizontalPager
participant ViewModel as PhotoDetailViewModel
participant State as PhotoDetailState
User->>Screen: 메모 아이콘 클릭
Screen->>ViewModel: ClickMemoIcon()
ViewModel->>State: state.copy(memoMode = Preview/Expanded/Editing)
State-->>Screen: uiState.memoMode 업데이트
Screen->>Screen: 페이저 외부 오버레이(스크림/에디터) 렌더
User->>Screen: 메모 편집 진입
Screen->>ViewModel: ClickMemoText() / ClickMemoMore()
ViewModel->>State: state.copy(memoMode = Editing)
State-->>Pager: userScrollEnabled = false
User->>Pager: 페이지 스와이프 시도
Pager->>Screen: isScrollInProgress = true
Screen->>ViewModel: PageScrollStarted()
ViewModel->>State: state.copy(memoMode = Closed)
State-->>Screen: uiState.memoMode = Closed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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.
🧹 Nitpick comments (1)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt (1)
232-237: Dim 오버레이 색상을 디자인 시스템 토큰으로 추출 권장 (선택)
Color(0x80202227)하드코딩된 매직 ARGB를NekiTheme.colorScheme의 scrim/dim 토큰으로 빼두면 다크모드 대응과 다른 오버레이와의 일관성 유지가 쉬워집니다. 프로젝트에 해당 토큰이 아직 없다면 이번 PR 범위 밖으로 미뤄도 무방합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt` around lines 232 - 237, 현재 PhotoDetailScreen의 Box에서 하드코딩된 오버레이 색상 Color(0x80202227)을 직접 사용하고 있어 다크모드/일관성 문제가 생길 수 있으니, Box(… .background(Color(0x80202227)))를 NekiTheme.colorScheme의 scrim/dim 같은 디자인 토큰으로 교체하도록 변경하세요: 정의된 토큰이 있다면 해당 토큰(예: NekiTheme.colorScheme.scrim 또는 .dim)을 사용하도록 background를 바꾸고, 토큰이 없으면 디자인팀과 협의해 이름을 정한 후 토큰을 추가해 적용하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt`:
- Around line 232-237: 현재 PhotoDetailScreen의 Box에서 하드코딩된 오버레이 색상
Color(0x80202227)을 직접 사용하고 있어 다크모드/일관성 문제가 생길 수 있으니, Box(…
.background(Color(0x80202227)))를 NekiTheme.colorScheme의 scrim/dim 같은 디자인 토큰으로
교체하도록 변경하세요: 정의된 토큰이 있다면 해당 토큰(예: NekiTheme.colorScheme.scrim 또는 .dim)을 사용하도록
background를 바꾸고, 토큰이 없으면 디자인팀과 협의해 이름을 정한 후 토큰을 추가해 적용하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 56515d0f-99f0-49d5-a721-40ec82197050
📒 Files selected for processing (4)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailContract.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailViewModel.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/component/PhotoDetailImageItem.kt
💤 Files with no reviewable changes (1)
- feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/component/PhotoDetailImageItem.kt
Ojongseok
approved these changes
Apr 19, 2026
Member
Ojongseok
left a comment
Ojongseok
left a comment
There was a problem hiding this comment.
고생하셨습니다~! Before/After로 비교해보니 Before가 더 자연스러운 것 같은 생각이 들지만 요구사항이 그러하다면 이렇게 진행하시죠 😅
- PhotoDetailIntent.PageScrollStarted 인텐트 추가 - PhotoDetailRoute에서 pagerState.isScrollInProgress 관찰 LaunchedEffect 추가 - 스와이프가 시작되는 순간 memoMode를 Closed로 전환 (settle까지 기다리지 않음) - 다음 사진의 메모를 보려면 메모 아이콘을 다시 탭해야 함
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt (1)
79-91: 두 LaunchedEffect를 하나로 병합 고려(선택).
pagerState를 key로 하는 LaunchedEffect가 두 개 있습니다. 동일한 key이므로 하나의LaunchedEffect(pagerState)내부에서 두snapshotFlow를launch { ... }로 나눠 수집해도 동일하게 동작하며, 관련 로직을 한 곳에서 파악할 수 있습니다. 필수는 아닙니다.♻️ 예시 리팩터
- LaunchedEffect(pagerState) { - snapshotFlow { pagerState.settledPage }.collect { page -> - viewModel.store.onIntent(PhotoDetailIntent.PageChanged(page)) - } - } - - LaunchedEffect(pagerState) { - snapshotFlow { pagerState.isScrollInProgress } - .filter { it } - .collect { - viewModel.store.onIntent(PhotoDetailIntent.PageScrollStarted) - } - } + LaunchedEffect(pagerState) { + launch { + snapshotFlow { pagerState.settledPage }.collect { page -> + viewModel.store.onIntent(PhotoDetailIntent.PageChanged(page)) + } + } + launch { + snapshotFlow { pagerState.isScrollInProgress } + .filter { it } + .collect { viewModel.store.onIntent(PhotoDetailIntent.PageScrollStarted) } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt` around lines 79 - 91, There are two LaunchedEffect(pagerState) blocks collecting snapshotFlow { pagerState.settledPage } and snapshotFlow { pagerState.isScrollInProgress } separately; merge them by using a single LaunchedEffect(pagerState) and inside it launch separate coroutines to collect each snapshotFlow so both handlers run concurrently: keep using snapshotFlow for settledPage to call viewModel.store.onIntent(PhotoDetailIntent.PageChanged(page)) and for isScrollInProgress to filter true and call viewModel.store.onIntent(PhotoDetailIntent.PageScrollStarted), ensuring you reference pagerState, settledPage, isScrollInProgress, and the viewModel.store.onIntent/PhotoDetailIntent symbols unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt`:
- Around line 79-91: There are two LaunchedEffect(pagerState) blocks collecting
snapshotFlow { pagerState.settledPage } and snapshotFlow {
pagerState.isScrollInProgress } separately; merge them by using a single
LaunchedEffect(pagerState) and inside it launch separate coroutines to collect
each snapshotFlow so both handlers run concurrently: keep using snapshotFlow for
settledPage to call
viewModel.store.onIntent(PhotoDetailIntent.PageChanged(page)) and for
isScrollInProgress to filter true and call
viewModel.store.onIntent(PhotoDetailIntent.PageScrollStarted), ensuring you
reference pagerState, settledPage, isScrollInProgress, and the
viewModel.store.onIntent/PhotoDetailIntent symbols unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8badb4d9-9959-4e3b-8949-5e28d6f99a7f
📒 Files selected for processing (3)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailContract.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailViewModel.kt
- PhotoDetailPagerArea의 HorizontalPager에 Editing 시 bottom padding 추가 - ActionBar가 숨겨질 때 pager 영역이 그만큼 확장되어 이미지가 커 보이던 문제 해결
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt`:
- Around line 215-225: The pager swipe blocking incorrectly uses a combined
flag; replace the combined isMemoActive logic with a dedicated isMemoEditing
flag (e.g., val isMemoEditing = uiState.memoMode == MemoMode.Editing) and use
that for pager interaction: set userScrollEnabled = !isMemoEditing on the
HorizontalPager (pagerState) so only Editing blocks swipes while
Preview/Expanded allow them; keep the dim visibility/clickable behavior separate
(introduce a separate isDimVisible or isMemoExpanded flag tied to
MemoMode.Expanded for the full-screen clickable dim and use that in the
clickable modifier) so dim rendering does not implicitly block pager drag.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 06acd76a-2881-49f3-90f4-67de8032f8cc
📒 Files selected for processing (1)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt
- PhotoDetailActionBar에서 참조하는 icon_memo_filled, icon_memo_stroked 드로어블 커밋 누락 수정 - CI 빌드 실패 해결
There was a problem hiding this comment.
♻️ Duplicate comments (1)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt (1)
216-250:⚠️ Potential issue | 🟠 MajorExpanded에서도 pager 스와이프/탭이 차단됩니다 — 이슈
#196요구사항과 불일치.Line 216–217에서
Expanded와Editing이 여전히isMemoActive하나로 묶여 있고, 그 플래그가 Line 226의userScrollEnabled, Line 234의isTapEnabled에 그대로 사용되고 있어서 Expanded 상태에서도 스와이프/탭이 막힙니다. 추가로 Line 245–250의 전체화면 dimBox가fillMaxSize() + noRippleClickableSingle로 Expanded에서도 포인터 이벤트를 가로채기 때문에,userScrollEnabled를 Editing 전용으로 풀어도 드래그가 dim에 먹혀 pager로 전달되지 않을 수 있습니다.Linked issue
#196목표("Editing에서만 pager 스와이프 차단, Preview/Expanded는 허용")에 맞추려면 "dim 표시 여부"와 "pager interaction 차단 여부"를 별도 플래그로 분리해 주세요.🛠 참고 수정안
- val isMemoActive = uiState.memoMode == MemoMode.Expanded || - uiState.memoMode == MemoMode.Editing + val isMemoDimVisible = uiState.memoMode == MemoMode.Expanded || + uiState.memoMode == MemoMode.Editing + val isPagerInteractionBlocked = uiState.memoMode == MemoMode.Editing @@ - userScrollEnabled = !isMemoActive, + userScrollEnabled = !isPagerInteractionBlocked, @@ - isTapEnabled = !isMemoActive, + isTapEnabled = !isPagerInteractionBlocked, @@ - AnimatedVisibility( - visible = isMemoActive, + AnimatedVisibility( + visible = isMemoDimVisible, enter = fadeIn(), exit = fadeOut(), ) { Box( modifier = Modifier .fillMaxSize() .background(Color(0x80202227)) - .noRippleClickableSingle { onIntent(PhotoDetailIntent.ClickMemoFold) }, + .then( + if (isPagerInteractionBlocked) { + Modifier.noRippleClickableSingle { + onIntent(PhotoDetailIntent.ClickMemoFold) + } + } else { + Modifier + }, + ), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt` around lines 216 - 250, Separate the combined isMemoActive into two booleans: one for showing the dim (e.g., showDim = uiState.memoMode == MemoMode.Expanded || uiState.memoMode == MemoMode.Editing) and one for blocking interactions (e.g., blockInteractions = uiState.memoMode == MemoMode.Editing); then use blockInteractions for pager/user interaction flags (set HorizontalPager userScrollEnabled = !blockInteractions and PhotoDetailImageItem isTapEnabled = !blockInteractions) and use showDim to control AnimatedVisibility, but apply noRippleClickableSingle (the ClickMemoFold handler) only when blockInteractions is true so the dim in Expanded no longer intercepts pointer events; update references to uiState.memoMode, MemoMode.Expanded, MemoMode.Editing, pagerState, PhotoDetailImageItem, and the ClickMemoFold/noRippleClickableSingle usage accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.kt`:
- Around line 216-250: Separate the combined isMemoActive into two booleans: one
for showing the dim (e.g., showDim = uiState.memoMode == MemoMode.Expanded ||
uiState.memoMode == MemoMode.Editing) and one for blocking interactions (e.g.,
blockInteractions = uiState.memoMode == MemoMode.Editing); then use
blockInteractions for pager/user interaction flags (set HorizontalPager
userScrollEnabled = !blockInteractions and PhotoDetailImageItem isTapEnabled =
!blockInteractions) and use showDim to control AnimatedVisibility, but apply
noRippleClickableSingle (the ClickMemoFold handler) only when blockInteractions
is true so the dim in Expanded no longer intercepts pointer events; update
references to uiState.memoMode, MemoMode.Expanded, MemoMode.Editing, pagerState,
PhotoDetailImageItem, and the ClickMemoFold/noRippleClickableSingle usage
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9d2b886b-1aaa-4ac3-9884-0f70c98bdc5f
📒 Files selected for processing (2)
feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailScreen.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/component/PhotoDetailActionBar.kt
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
2 participants
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.
🔗 관련 이슈
📙 작업 설명
memoModes: Map<Long, MemoMode>를 전역 단일memoMode: MemoMode로 단순화currentMemoMode/memoModeOf(photoId)헬퍼 제거toImmutableMap체인 제거)MemoTextField) + dim 오버레이를HorizontalPager바깥으로 이동PhotoDetailPagerArea컴포저블 추출: pager + dim + 메모 박스를 감싸는 BoxPhotoDetailImageItem시그니처 축소 (imageUrl,isScrollInProgress,isTapEnabled,onClickLeft,onClickRight만 유지)MemoTextField내부LaunchedEffect(memo, memoMode)가 내부TextFieldState를 새memo로 재동기화🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
KakaoTalk_Video_2026-04-20-00-01-34.mp4
123.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
memoMode로 단순화 (state/ViewModel/Screen 호출부 마이그레이션)HorizontalPager밖으로 분리 (UI 구조 변경 +PhotoDetailImageItem시그니처 단순화)noRippleClickableSingle유지. Expanded/Editing 에서는 pager 스와이프가 차단되어 있어 드래그 전파 이슈가 없음.MemoTextField본체는 수정하지 않고 재사용 (내부TextFieldState동기화 로직이 그대로 활용됨)Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
동작 개선