Conversation
…log-upload-modifiy
# Conflicts: # core/navigation/src/main/java/com/twix/navigation/NavRoutes.kt # feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetail.kt # feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailViewModel.kt # feature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailUiState.kt # feature/photolog/editor/src/main/java/com/twix/photolog/editor/contract/PhotologEditorSideEffect.kt # feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailScreen.kt # feature/stats/detail/src/main/java/com/twix/stats/detail/navigation/StatsDetailGraph.kt # feature/task-certification/src/main/java/com/twix/task_certification/detail/component/TaskCertificationCardContent.kt # feature/task-certification/src/main/java/com/twix/task_certification/detail/contract/TaskCertificationDetailIntent.kt # feature/task-certification/src/main/java/com/twix/task_certification/navigation/TaskCertificationGraph.kt
📝 WalkthroughWalkthrough이 변경은 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 상세 검토 의견✅ 긍정적인 측면모듈 분리 구조의 개선: 일관된 네이밍 컨벤션: 💡 검토 시 주의할 사항1. 패키지 구조 변경 검증 필요 질문: 기존 codebase에서 2. 네비게이션 경로 일관성 제안: 다음 항목들의 변경이 모두 일관되게 적용되었는지 재확인하세요:
특히 3. DI 모듈 재등록 // 기존
val taskCertificationModule = module { ... }
// 신규
val photologCaptureModule = module { ... }
val photologDetailModule = module { ... }
val photologEditorModule = module { ... }확인 사항: 4. 리소스 문자열 마이그레이션 완성도 <!-- 변경 전 -->
<string name="task_certification_upload">업로드</string>
<!-- 변경 후 -->
<string name="photolog_upload">업로드</string>제안: 5. 삭제된 아이콘 리소스 질문: 이 아이콘이 더 이상 사용되지 않는다고 확인하셨나요? 혹시 다른 파일에서 🎯 권장 검수 절차
이 리팩토링은 구조적으로 잘 정리되었으나, 크기가 크기 때문에 세밀한 검증이 필수적입니다. 👍 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorViewModel.kt (1)
87-95:⚠️ Potential issue | 🟡 Minor
reduce내부에서 외부 스코프의 변수를 캡처하여 가독성이 떨어집니다.Line 90의
onSuccess = { reduce { it.toEditorUiState(argGoalId, argTargetDate) } }를 살펴보면:
- 외부
onSuccess람다에서 받은it(fetch 결과)을 reduce 블록 내부에서 캡처하고 있습니다- reduce 람다는
S.() -> S시그니처로 암시적 receiver(this)가 현재 state를 가리키는데, 여기서는 외부 클로저의it을 사용하고 있어 패턴이 일관되지 않습니다- Line 49의
reduce { copy(...) }처럼 암시적 receiver를 활용하는 방식과 달라서 의도가 불명확합니다개선 방안:
파라미터를 명시적으로 네이밍하면 훨씬 명확해집니다:
onSuccess = { photoLogs -> reduce { photoLogs.toEditorUiState(argGoalId, argTargetDate) } }또는 변환 로직을 별도 메서드로 추출하는 것도 좋은 방법입니다:
onSuccess = { reduce { it.copy(/* state 직접 수정 */) } }이렇게 수정하면 reduce 내부에서 암시적 receiver인 state와 매개변수를 명확히 구분할 수 있고, 코드의 의도가 더 명확해집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorViewModel.kt` around lines 87 - 95, The onSuccess lambda in fetchPhotolog captures the outer `it` inside the `reduce` receiver which mixes implicit state receiver with an external closure; change the lambda to name the fetch result (e.g., `photoLogs`) and pass that into reduce so it's explicit (e.g., onSuccess = { photoLogs -> reduce { photoLogs.toEditorUiState(argGoalId, argTargetDate) } }), or alternatively extract the transformation into a separate function (e.g., toEditorUiState) and call it from reduce to avoid capturing outer variables; update the fetchPhotolog method accordingly to use the named parameter and keep reduce's implicit receiver focused on state.feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt (1)
108-108:⚠️ Potential issue | 🟡 Minor사용되지 않는 변수
targetY가 선언되어 있습니다.
val targetY = 0f가 선언되었지만 이후 코드에서 사용되지 않습니다. 원래 Y축 애니메이션을 위해 추가했다가 사용하지 않게 된 것으로 보입니다. 코드 가독성과 유지보수를 위해 제거하는 것이 좋습니다.🧹 제거 제안
val targetX = if (offsetX.value > 0) { spec.dismissDistancePx } else { -spec.dismissDistancePx } - val targetY = 0f - /** * dismiss 애니메이션 완료 대기 */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt` at line 108, Remove the unused local variable `targetY` declared as `val targetY = 0f` inside the SwipeableCard component; locate the declaration in the SwipeableCard.kt code (within the SwipeableCard function/class) and delete that line to improve readability and remove dead code, ensuring no other references to `targetY` remain.feature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureUiState.kt (1)
25-43:⚠️ Potential issue | 🟠 Major
toggleTorch()가 불가능한 상태를 만들 수 있습니다.
showTorch는 후면 카메라 + 미촬영 상태에서만true인데, 현재toggleTorch()는 그 조건을 확인하지 않고 항상 토글합니다. 그래서 전면 렌즈이거나 이미 촬영된 뒤에도torch = On이 될 수 있고, 이후 UI와 카메라 설정이 서로 다른 상태를 보게 됩니다.!showTorch일 때는 항상TorchStatus.Off를 유지하도록 가드해 주세요.🔧 제안 코드
fun toggleTorch(): PhotologCaptureUiState { + if (!showTorch) return copy(torch = TorchStatus.Off) val newFlashMode = TorchStatus.Companion.toggle(torch) return copy(torch = newFlashMode) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureUiState.kt` around lines 25 - 43, toggleTorch() can set torch On even when showTorch is false (front camera or already captured); update PhotologCaptureUiState.toggleTorch to check showTorch first and if false return copy(torch = TorchStatus.Off), otherwise compute the new mode via TorchStatus.toggle(torch) and return copy(torch = newMode); reference methods/props: PhotologCaptureUiState.toggleTorch, PhotologCaptureUiState.showTorch, TorchStatus.toggle, and copy(...) to implement the guard.feature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailUiState.kt (1)
167-183:⚠️ Potential issue | 🟠 Majorgoal 조회 실패 fallback이 입력 컨텍스트를 잃습니다.
Line 170에서
PhotologDetailUiState()를 그대로 반환하면goalId,selectedDate,isCompletedGoal이 초기값으로 돌아갑니다. 그 상태에서 파생 값이 계산되면, 실제로는 조회 실패인데도 업로드/찌르기 버튼처럼 잘못된 UI가 열릴 수 있습니다. 최소한 전달받은 컨텍스트를 보존한 empty state를 반환하거나, 별도 error/empty 흐름으로 분리해 주세요.🔧 제안 코드
fun PhotoLogs.toUiState( goalId: Long, betweenUs: String, selectedDate: LocalDate, isCompletedGoal: Boolean, ): PhotologDetailUiState { + val currentShow = BetweenUs.valueOf(betweenUs) val currentGoalPhotolog = goals.firstOrNull { it.goalId == goalId - } ?: return PhotologDetailUiState() + } ?: return PhotologDetailUiState( + goalId = goalId, + currentShow = currentShow, + selectedDate = selectedDate, + isCompletedGoal = isCompletedGoal, + ) return PhotologDetailUiState( goalId = goalId, - currentShow = BetweenUs.valueOf(betweenUs), + currentShow = currentShow, selectedDate = selectedDate,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailUiState.kt` around lines 167 - 183, Returning a default PhotologDetailUiState() when goals.firstOrNull(...) fails strips important context (goalId, selectedDate, isCompletedGoal, nicknames) and can enable wrong UI actions; instead, when currentGoalPhotolog is null return a preserved/empty state that keeps the incoming context by constructing PhotologDetailUiState with goalId = goalId, selectedDate = selectedDate, isCompletedGoal = isCompletedGoal, myNickname = myNickname, partnerNickname = partnerNickname, currentShow = BetweenUs.valueOf(betweenUs) (or a safe default if betweenUs is invalid), and leave goal-specific fields (goalName, icon, myPhotolog, partnerPhotolog) empty or null; update the null-path in the code that uses goals.firstOrNull { it.goalId == goalId } (the currentGoalPhotolog check) to return that preserved state or alternatively emit a distinct error/empty state variant if your UI handles errors separately.feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/PhotologCaptureTopBar.kt (1)
32-39:⚠️ Potential issue | 🟠 Major닫기 액션이 접근성 요구사항을 충족하지 않습니다.
현재 클릭 가능한 Image에
contentDescription = null이 설정되어 있어 TalkBack 같은 보조 기술 사용자가 이 버튼을 인식할 수 없습니다. 이 화면을 종료하는 핵심 액션이므로 접근성이 필수적입니다.개선 방법:
contentDescription = null을contentDescription = "close"로 변경해주세요. 코드베이스의 다른 상호작용 버튼들("back", "menu" 등)과 동일한 패턴을 따릅니다.Image( imageVector = ImageVector.vectorResource(R.drawable.ic_close_c100), contentDescription = "close", // null 대신 설명 추가 modifier = Modifier .padding(24.dp) .align(Alignment.CenterEnd) .noRippleClickable(onClick = onClickClose), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/PhotologCaptureTopBar.kt` around lines 32 - 39, The Image composable used as the close button in PhotologCaptureTopBar (the Image with imageVector = ImageVector.vectorResource(R.drawable.ic_close_c100) and noRippleClickable(onClick = onClickClose)) sets contentDescription = null which breaks accessibility; change contentDescription to a descriptive string (e.g., "close") to match other interactive buttons ("back", "menu") so TalkBack and other assistive tech can announce the control.feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetail.kt (1)
177-191:⚠️ Potential issue | 🟡 Minor
isLoading네이밍이 의도를 명확히 반영하도록 개선하세요.현재 코드에서
isLoading은 일반적인 로딩 상태를 나타내는 것이 아니라, "초기값으로 인해 버튼이 렌더링되는 것을 막기 위한 변수"입니다(PhotologDetailUiState.kt 30-31줄 주석 참조).실제 로직:
isLoading = false→ 초기 상태, 콘텐츠 숨김isLoading = true→ 데이터 로드 완료, 콘텐츠 표시이는 일반적인
isLoading의 의미(true = 로딩 중, false = 로딩 완료)와 반대되어 혼동을 야기합니다.개선 방안:
isLoading을isDataReady,isContentReady, 또는isInitialized같은 이름으로 변경하면 의도가 명확해집니다. 또는hasInitialLoadCompleted처럼 명확한 의도를 담은 네이밍을 추천합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetail.kt` around lines 177 - 191, Rename the misleading boolean uiState.isLoading to a name that reflects "data ready" semantics (e.g., isDataReady, isContentReady, isInitialized, or hasInitialLoadCompleted) everywhere it's defined and used: update the PhotologDetailUiState declaration (current isLoading) and all usages in PhotologDetail.kt (the conditional that wraps PhotologCardContent and ReactionContent) so the conditional logic remains the same but the name reflects true = data loaded; also update any getters/setters, tests, and documentation/comments in PhotologDetailUiState.kt to match the new identifier so intent is clear and consistent across PhotologCardContent, ReactionContent, and related code paths.
🧹 Nitpick comments (10)
feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionBar.kt (1)
81-86: 매직 넘버4사용을 개선하면 유지보수성이 향상됩니다.현재
index < 4는ReactionUiModel.entries가 정확히 5개의 항목을 가진다고 가정합니다. 향후 리액션 타입이 추가되거나 삭제되면 이 조건도 함께 수정해야 하는데, 이를 놓치기 쉽습니다.
entries.size - 1을 사용하면 항목 수가 변경되어도 자동으로 적용되어 유지보수가 용이해집니다.♻️ 개선 제안
- if (index < 4) { + if (index < ReactionUiModel.entries.size - 1) { VerticalDivider( modifier = Modifier.fillMaxHeight(), color = GrayColor.C500, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionBar.kt` around lines 81 - 86, Replace the magic number check "index < 4" in ReactionBar (where VerticalDivider is conditionally shown) with a size-based check using ReactionUiModel.entries so dividers are shown for every item except the last (e.g., compare index with entries.size - 1); update the conditional that surrounds VerticalDivider to use that expression and reference ReactionUiModel.entries to avoid hard-coded counts.feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraPreviewBox.kt (2)
92-96:torchIcon값에remember적용을 고려해 보세요.현재
torch상태가 변경되지 않아도TorchIconcomposable이 recompose될 때마다ImageVector.vectorResource()가 매번 호출될 수 있습니다.remember를 사용하면 동일한torch값에 대해 불필요한 리소스 조회를 방지할 수 있습니다.다만,
vectorResource는 내부적으로 캐싱을 수행하므로 성능 영향이 크지 않을 수 있습니다. 팀의 코드 컨벤션에 따라 판단해 주세요.♻️ remember 적용 예시
- val torchIcon = - when (torch) { - TorchStatus.On -> ImageVector.vectorResource(id = R.drawable.ic_camera_torch_on) - TorchStatus.Off -> ImageVector.vectorResource(id = R.drawable.ic_camera_torch_off) + val torchIcon = remember(torch) { + when (torch) { + TorchStatus.On -> R.drawable.ic_camera_torch_on + TorchStatus.Off -> R.drawable.ic_camera_torch_off } + }.let { ImageVector.vectorResource(id = it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraPreviewBox.kt` around lines 92 - 96, The torchIcon lookup currently calls ImageVector.vectorResource(...) on every recomposition of the composable; wrap the computation in remember keyed by the torch state so the vector is only resolved when torch changes (e.g., replace the current val torchIcon = when (torch) { ... } with a remembered expression like remember(torch) { when (torch) { TorchStatus.On -> ImageVector.vectorResource(id = R.drawable.ic_camera_torch_on) TorchStatus.Off -> ImageVector.vectorResource(id = R.drawable.ic_camera_torch_off) } } ), referencing the existing torch, torchIcon and TorchStatus symbols.
108-121: Preview Composable이 제공되어 있습니다. ✨코딩 가이드라인에서 요구하는 Preview Composable이 적절히 제공되어 있어 UI 개발 시 빠른 피드백을 받을 수 있습니다.
추가로,
CaptureStatus.Captured상태에 대한 Preview도 함께 제공하면 두 가지 상태 모두를 쉽게 확인할 수 있어 더 좋을 것 같습니다. 필요하시다면 추가를 고려해 보세요.♻️ Captured 상태 Preview 추가 예시
`@Preview` `@Composable` fun CameraPreviewBoxCapturedPreview() { TwixTheme { CameraPreviewBox( capture = CaptureStatus.Captured(uri = Uri.EMPTY), showTorch = false, torch = TorchStatus.Off, previewRequest = null, onClickFlash = {}, onPositioned = {}, ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraPreviewBox.kt` around lines 108 - 121, Add a Preview composable for the captured state so designers can see both states; create a new function (e.g., CameraPreviewBoxCapturedPreview) that wraps TwixTheme and calls CameraPreviewBox with capture = CaptureStatus.Captured(uri = Uri.EMPTY), showTorch set appropriately (e.g., false), torch = TorchStatus.Off, previewRequest = null, and no-op lambdas for onClickFlash and onPositioned to mirror the existing CameraPreviewBoxNotCapturedPreview setup.feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt (1)
34-41: Preview Composable 추가를 고려해 주세요.Coding guidelines에 따르면 Compose UI에서 Preview Composable을 제공하는 것이 권장됩니다. 스와이프 카드의 다양한 상태(기본, 스와이프 중 등)를 Preview로 제공하면 UI 개발과 디자인 검증이 더 편리해집니다.
혹시 Preview가 다른 파일에 별도로 관리되고 있다면 무시하셔도 됩니다.
💡 Preview 예시
`@Preview`(showBackground = true) `@Composable` private fun SwipeableCardPreview() { SwipeableCard( onSwipe = {}, isDisplayingMyPhoto = true, ) { // Preview content Box( modifier = Modifier .size(200.dp) .background(Color.LightGray) ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt` around lines 34 - 41, Add one or more `@Preview` composable functions for SwipeableCard to aid UI validation: create a private `@Preview` function (e.g., SwipeableCardPreview and optionally SwipeableCardSwipingPreview) that calls SwipeableCard(onSwipe = {}, isDisplayingMyPhoto = true/false, spec = SwipeCardSpec() as needed) and supplies simple preview content (Box with size and background) to represent default and swiping states; place these previews in the same file under the SwipeableCard composable so designers can quickly see baseline and interaction states in Android Studio.feature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/CaptureCamera.kt (1)
138-139: IMAGE_NAME도 Photolog로 통일하는 것을 고려해 주세요.
IMAGE_PATH는"Pictures/Photolog"로 적절히 변경되었지만,IMAGE_NAME은 여전히"task_%d"형식입니다. 네이밍 일관성을 위해"photolog_%d"로 변경하는 것을 고려해 보시겠어요?💡 네이밍 일관성을 위한 제안
- private const val IMAGE_NAME = "task_%d" + private const val IMAGE_NAME = "photolog_%d"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/CaptureCamera.kt` around lines 138 - 139, IMAGE_NAME is still "task_%d" while IMAGE_PATH was renamed to "Photolog"; update IMAGE_NAME in CaptureCamera.kt to "photolog_%d" to keep naming consistent (change the private const val IMAGE_NAME value), and search for usages of IMAGE_NAME/format calls in the class (e.g., where String.format or "%d" is applied) to ensure filenames are generated with the new prefix and no other code assumes "task_".feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.kt (1)
70-95: 접근성(Accessibility) 개선을 고려해 보세요.
MyReactionBadge의Image컴포넌트들이contentDescription = null로 설정되어 있습니다. 시각 장애 사용자를 위한 스크린 리더 지원을 위해 의미 있는 contentDescription을 제공하는 것이 좋습니다.♻️ 접근성 개선 제안
Image( painter = painterResource(R.drawable.ic_my_reaction_union), - contentDescription = null, + contentDescription = "내 리액션 배지 배경", ) Image( painter = painterResource(it.imageResources), - contentDescription = null, + contentDescription = "내 리액션: ${it.name}",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.kt` around lines 70 - 95, MyReactionBadge currently sets both Image contentDescription to null which breaks screen-reader support; update MyReactionBadge to provide meaningful contentDescription text: use the reaction parameter (e.g., reaction.name or reaction.label) for the foreground image and a descriptive label for the background union image (or mark it as decorative only if truly non-informative) and fall back to an empty string or stringResource when reaction is null; ensure to reference MyReactionBadge and ReactionUiModel so the content text is derived from the model or passed in as an explicit parameter.core/design-system/src/main/java/com/twix/designsystem/components/photolog/ForegroundCard.kt (1)
47-60: Preview에 다양한 케이스 추가를 고려해 보세요.현재 Preview가
isCertificated = true,currentShow = BetweenUs.ME케이스만 보여주고 있습니다. 다양한 상태를 확인할 수 있도록PreviewParameter를 활용하거나 추가 Preview 함수를 작성하면 UI 검증에 도움이 됩니다.♻️ 추가 Preview 예시
`@Preview` `@Composable` private fun ForegroundCardPartnerNotCertificatedPreview() { TwixTheme { ForegroundCard( isCertificated = false, nickName = "파트너", imageUrl = null, comment = null, currentShow = BetweenUs.PARTNER, rotation = 0f, ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/photolog/ForegroundCard.kt` around lines 47 - 60, Add additional Composable previews to cover different ForegroundCard states so designers/devs can verify UI variations; create new `@Preview` functions (or use PreviewParameter) that call ForegroundCard with combinations such as isCertificated=false, imageUrl=null, currentShow=BetweenUs.PARTNER, different rotation values and nickName/comment cases; name them clearly (e.g., ForegroundCardPartnerNotCertificatedPreview) and keep TwixTheme wrapping so previews render consistently; ensure you include at least one preview for the non-certified partner state and one for nil image/comment scenarios.feature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureViewModel.kt (1)
130-146: Content-Type 상수 추출을 고려해 보세요.Line 137에서
"image/jpeg"가 하드코딩되어 있습니다. 이 값이 다른 곳에서도 사용될 수 있으므로, 상수로 추출하면 유지보수성이 향상됩니다.♻️ 상수 추출 제안
companion object { private const val ERROR_DISPLAY_DURATION_MS = 1500L + private const val CONTENT_TYPE_JPEG = "image/jpeg" }photologRepository.uploadPhotologImage( goalId = navArgs.goalId, bytes = image, - contentType = "image/jpeg", + contentType = CONTENT_TYPE_JPEG, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureViewModel.kt` around lines 130 - 146, The hardcoded content type "image/jpeg" in upload() (used in photologRepository.uploadPhotologImage) should be extracted to a named constant (e.g., IMAGE_JPEG or CONTENT_TYPE_JPEG) so it can be reused and maintained; create the constant either as a private const val in the PhotologCaptureViewModel companion object or as a top-level constant, replace the literal in upload() with that constant, and ensure any other usages (future or existing) reference the same constant name.feature/photolog/editor/src/main/java/com/twix/photolog/editor/navigation/PhotologEditorNavGraph.kt (1)
32-46: 콜백 이름은 이번 기회에 Photolog 도메인으로 같이 맞추는 편이 좋겠습니다.라우트와 모듈 이름은 모두
Photolog로 정리됐는데navigateToCertification만 이전 표현을 남기고 있어서 흐름을 읽을 때 한 번 더 해석이 필요합니다.PhotologEditorRoute의 파라미터명까지 함께 바꿔 두면 이번 리팩터링 의도가 더 일관되게 보입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/editor/src/main/java/com/twix/photolog/editor/navigation/PhotologEditorNavGraph.kt` around lines 32 - 46, Rename the callback param navigateToCertification to a Photolog-consistent name (e.g., navigateToPhotologDetail or navigateToPhotolog) in the PhotologEditorRoute declaration and where it is passed/used inside PhotologEditorNavGraph; update the lambda binding that constructs NavRoutes.PhotologRoute.createRoute (the lambda taking goalId, photologId, comment, selectedDate) to match the new parameter name and adjust any callers of PhotologEditorRoute to use the new identifier so the route and module naming are consistent with the Photolog domain.feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorRoute.kt (1)
56-60: 콜백 이름도 photolog/capture 기준으로 맞춰 주세요.왜 문제가 되냐면, 이 파일만 아직
navigateToCertification가 남아 있어서 리팩터링 후 공개 API를 읽을 때 task-certification 용어가 섞여 보입니다.navigateToCapture처럼 현재 모듈 구조에 맞는 이름으로 바꿔보면 어떨까요?✏️ 이름 정리 예시
fun PhotologEditorRoute( navigateToBack: () -> Unit, - navigateToCertification: (Long, Long, String, LocalDate) -> Unit, + navigateToCapture: (Long, Long, String, LocalDate) -> Unit, toastManager: ToastManager = koinInject(), viewModel: PhotologEditorViewModel = koinViewModel(), ) { @@ - navigateToCertification( + navigateToCapture( uiState.goalId, uiState.photologId, uiState.comment.value, uiState.selectedDate, ) @@ - navigateToCertification( + navigateToCapture( uiState.goalId, uiState.photologId, uiState.comment.value, uiState.selectedDate, )Also applies to: 90-95, 128-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorRoute.kt` around lines 56 - 60, Rename the callback parameter navigateToCertification to navigateToCapture in PhotologEditorRoute (keep the same function type signature (Long, Long, String, LocalDate) -> Unit) and update every internal usage/forwarding inside this file (the other occurrences around the second and third blocks referenced) to use navigateToCapture; ensure any places that call PhotologEditorRoute are updated to pass the renamed parameter and update any public API/docs/comments in this file that mention "certification" to "capture" to keep the module terminology consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/design-system/src/main/res/values/strings.xml`:
- Around line 132-154: The strings.xml rename to photolog_* keys is good but
since only the default Korean values folder exists, ensure any future addition
of other locale resource files (e.g., values-en) includes the exact same
photolog_* keys (photolog_upload, photolog_image_capture_fail,
photolog_take_picture, photolog_editor_modify_success, etc.) and matching
comment groupings (인증샷 촬영 화면, 보기 화면, 수정 화면) so translations stay consistent;
when adding locales, copy this strings.xml as the base and translate each
photolog_* entry rather than creating new keys or leaving gaps.
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/navigation/PhotologCaptureNavGraph.kt`:
- Around line 31-42: 현재 popUpTo가 NavRoutes.PhotologDetailRoute.route를 가리켜 이전의 다른
detail 엔트리를 지울 수 있으니, navController.navigate(
NavRoutes.PhotologDetailRoute.createRoute(...) ) 호출 내의 popUpTo 대상을 현재 화면인
NavRoutes.PhotologRoute.route로 변경해 현재 캡처 화면을 백스택에서 제거하도록 수정하세요; 즉 navigate 블록에서
popUpTo(...) { inclusive = true } 호출이 NavRoutes.PhotologRoute.route를 사용하도록 바꿔서
화면 교체 목적을 달성하도록 합니다.
---
Outside diff comments:
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/PhotologCaptureTopBar.kt`:
- Around line 32-39: The Image composable used as the close button in
PhotologCaptureTopBar (the Image with imageVector =
ImageVector.vectorResource(R.drawable.ic_close_c100) and
noRippleClickable(onClick = onClickClose)) sets contentDescription = null which
breaks accessibility; change contentDescription to a descriptive string (e.g.,
"close") to match other interactive buttons ("back", "menu") so TalkBack and
other assistive tech can announce the control.
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureUiState.kt`:
- Around line 25-43: toggleTorch() can set torch On even when showTorch is false
(front camera or already captured); update PhotologCaptureUiState.toggleTorch to
check showTorch first and if false return copy(torch = TorchStatus.Off),
otherwise compute the new mode via TorchStatus.toggle(torch) and return
copy(torch = newMode); reference methods/props:
PhotologCaptureUiState.toggleTorch, PhotologCaptureUiState.showTorch,
TorchStatus.toggle, and copy(...) to implement the guard.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt`:
- Line 108: Remove the unused local variable `targetY` declared as `val targetY
= 0f` inside the SwipeableCard component; locate the declaration in the
SwipeableCard.kt code (within the SwipeableCard function/class) and delete that
line to improve readability and remove dead code, ensuring no other references
to `targetY` remain.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailUiState.kt`:
- Around line 167-183: Returning a default PhotologDetailUiState() when
goals.firstOrNull(...) fails strips important context (goalId, selectedDate,
isCompletedGoal, nicknames) and can enable wrong UI actions; instead, when
currentGoalPhotolog is null return a preserved/empty state that keeps the
incoming context by constructing PhotologDetailUiState with goalId = goalId,
selectedDate = selectedDate, isCompletedGoal = isCompletedGoal, myNickname =
myNickname, partnerNickname = partnerNickname, currentShow =
BetweenUs.valueOf(betweenUs) (or a safe default if betweenUs is invalid), and
leave goal-specific fields (goalName, icon, myPhotolog, partnerPhotolog) empty
or null; update the null-path in the code that uses goals.firstOrNull {
it.goalId == goalId } (the currentGoalPhotolog check) to return that preserved
state or alternatively emit a distinct error/empty state variant if your UI
handles errors separately.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetail.kt`:
- Around line 177-191: Rename the misleading boolean uiState.isLoading to a name
that reflects "data ready" semantics (e.g., isDataReady, isContentReady,
isInitialized, or hasInitialLoadCompleted) everywhere it's defined and used:
update the PhotologDetailUiState declaration (current isLoading) and all usages
in PhotologDetail.kt (the conditional that wraps PhotologCardContent and
ReactionContent) so the conditional logic remains the same but the name reflects
true = data loaded; also update any getters/setters, tests, and
documentation/comments in PhotologDetailUiState.kt to match the new identifier
so intent is clear and consistent across PhotologCardContent, ReactionContent,
and related code paths.
In
`@feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorViewModel.kt`:
- Around line 87-95: The onSuccess lambda in fetchPhotolog captures the outer
`it` inside the `reduce` receiver which mixes implicit state receiver with an
external closure; change the lambda to name the fetch result (e.g., `photoLogs`)
and pass that into reduce so it's explicit (e.g., onSuccess = { photoLogs ->
reduce { photoLogs.toEditorUiState(argGoalId, argTargetDate) } }), or
alternatively extract the transformation into a separate function (e.g.,
toEditorUiState) and call it from reduce to avoid capturing outer variables;
update the fetchPhotolog method accordingly to use the named parameter and keep
reduce's implicit receiver focused on state.
---
Nitpick comments:
In
`@core/design-system/src/main/java/com/twix/designsystem/components/photolog/ForegroundCard.kt`:
- Around line 47-60: Add additional Composable previews to cover different
ForegroundCard states so designers/devs can verify UI variations; create new
`@Preview` functions (or use PreviewParameter) that call ForegroundCard with
combinations such as isCertificated=false, imageUrl=null,
currentShow=BetweenUs.PARTNER, different rotation values and nickName/comment
cases; name them clearly (e.g., ForegroundCardPartnerNotCertificatedPreview) and
keep TwixTheme wrapping so previews render consistently; ensure you include at
least one preview for the non-certified partner state and one for nil
image/comment scenarios.
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraPreviewBox.kt`:
- Around line 92-96: The torchIcon lookup currently calls
ImageVector.vectorResource(...) on every recomposition of the composable; wrap
the computation in remember keyed by the torch state so the vector is only
resolved when torch changes (e.g., replace the current val torchIcon = when
(torch) { ... } with a remembered expression like remember(torch) { when (torch)
{ TorchStatus.On -> ImageVector.vectorResource(id =
R.drawable.ic_camera_torch_on) TorchStatus.Off -> ImageVector.vectorResource(id
= R.drawable.ic_camera_torch_off) } } ), referencing the existing torch,
torchIcon and TorchStatus symbols.
- Around line 108-121: Add a Preview composable for the captured state so
designers can see both states; create a new function (e.g.,
CameraPreviewBoxCapturedPreview) that wraps TwixTheme and calls CameraPreviewBox
with capture = CaptureStatus.Captured(uri = Uri.EMPTY), showTorch set
appropriately (e.g., false), torch = TorchStatus.Off, previewRequest = null, and
no-op lambdas for onClickFlash and onPositioned to mirror the existing
CameraPreviewBoxNotCapturedPreview setup.
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/CaptureCamera.kt`:
- Around line 138-139: IMAGE_NAME is still "task_%d" while IMAGE_PATH was
renamed to "Photolog"; update IMAGE_NAME in CaptureCamera.kt to "photolog_%d" to
keep naming consistent (change the private const val IMAGE_NAME value), and
search for usages of IMAGE_NAME/format calls in the class (e.g., where
String.format or "%d" is applied) to ensure filenames are generated with the new
prefix and no other code assumes "task_".
In
`@feature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureViewModel.kt`:
- Around line 130-146: The hardcoded content type "image/jpeg" in upload() (used
in photologRepository.uploadPhotologImage) should be extracted to a named
constant (e.g., IMAGE_JPEG or CONTENT_TYPE_JPEG) so it can be reused and
maintained; create the constant either as a private const val in the
PhotologCaptureViewModel companion object or as a top-level constant, replace
the literal in upload() with that constant, and ensure any other usages (future
or existing) reference the same constant name.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.kt`:
- Around line 70-95: MyReactionBadge currently sets both Image
contentDescription to null which breaks screen-reader support; update
MyReactionBadge to provide meaningful contentDescription text: use the reaction
parameter (e.g., reaction.name or reaction.label) for the foreground image and a
descriptive label for the background union image (or mark it as decorative only
if truly non-informative) and fall back to an empty string or stringResource
when reaction is null; ensure to reference MyReactionBadge and ReactionUiModel
so the content text is derived from the model or passed in as an explicit
parameter.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionBar.kt`:
- Around line 81-86: Replace the magic number check "index < 4" in ReactionBar
(where VerticalDivider is conditionally shown) with a size-based check using
ReactionUiModel.entries so dividers are shown for every item except the last
(e.g., compare index with entries.size - 1); update the conditional that
surrounds VerticalDivider to use that expression and reference
ReactionUiModel.entries to avoid hard-coded counts.
In
`@feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.kt`:
- Around line 34-41: Add one or more `@Preview` composable functions for
SwipeableCard to aid UI validation: create a private `@Preview` function (e.g.,
SwipeableCardPreview and optionally SwipeableCardSwipingPreview) that calls
SwipeableCard(onSwipe = {}, isDisplayingMyPhoto = true/false, spec =
SwipeCardSpec() as needed) and supplies simple preview content (Box with size
and background) to represent default and swiping states; place these previews in
the same file under the SwipeableCard composable so designers can quickly see
baseline and interaction states in Android Studio.
In
`@feature/photolog/editor/src/main/java/com/twix/photolog/editor/navigation/PhotologEditorNavGraph.kt`:
- Around line 32-46: Rename the callback param navigateToCertification to a
Photolog-consistent name (e.g., navigateToPhotologDetail or navigateToPhotolog)
in the PhotologEditorRoute declaration and where it is passed/used inside
PhotologEditorNavGraph; update the lambda binding that constructs
NavRoutes.PhotologRoute.createRoute (the lambda taking goalId, photologId,
comment, selectedDate) to match the new parameter name and adjust any callers of
PhotologEditorRoute to use the new identifier so the route and module naming are
consistent with the Photolog domain.
In
`@feature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorRoute.kt`:
- Around line 56-60: Rename the callback parameter navigateToCertification to
navigateToCapture in PhotologEditorRoute (keep the same function type signature
(Long, Long, String, LocalDate) -> Unit) and update every internal
usage/forwarding inside this file (the other occurrences around the second and
third blocks referenced) to use navigateToCapture; ensure any places that call
PhotologEditorRoute are updated to pass the renamed parameter and update any
public API/docs/comments in this file that mention "certification" to "capture"
to keep the module terminology consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: efb56190-9657-4b9d-9143-b48921b15ac5
📒 Files selected for processing (84)
app/build.gradle.ktsapp/src/main/java/com/yapp/twix/di/FeatureModules.ktcore/design-system/src/main/java/com/twix/designsystem/components/photolog/ForegroundCard.ktcore/design-system/src/main/res/drawable/ic_keepi_singing.xmlcore/design-system/src/main/res/values/strings.xmlcore/navigation/src/main/java/com/twix/navigation/AppNavHost.ktcore/navigation/src/main/java/com/twix/navigation/NavRoutes.ktcore/navigation/src/main/java/com/twix/navigation/args/DetailNavArgs.ktcore/util/src/main/java/com/twix/util/bus/PhotologRefreshBus.ktcore/util/src/main/java/com/twix/util/di/UtilModule.ktfeature/main/src/main/java/com/twix/main/navigation/MainNavGraph.ktfeature/notification/src/main/java/com/twix/notification/navigation/NotificationNavGraph.ktfeature/photolog/capture/.gitignorefeature/photolog/capture/build.gradle.ktsfeature/photolog/capture/consumer-rules.profeature/photolog/capture/proguard-rules.profeature/photolog/capture/src/main/AndroidManifest.xmlfeature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureScreen.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureViewModel.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraControlBar.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CameraPreviewBox.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/component/CommentErrorText.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/component/LoadingContent.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/component/PhotologCaptureTopBar.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureIntent.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureSideEffect.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/contract/PhotologCaptureUiState.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/di/PhotologCaptureModule.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/model/CaptureStatus.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/model/TorchStatus.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/Camera.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/CameraPreview.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/model/camera/CaptureCamera.ktfeature/photolog/capture/src/main/java/com/twix/photolog/capture/navigation/PhotologCaptureNavGraph.ktfeature/photolog/detail/.gitignorefeature/photolog/detail/build.gradle.ktsfeature/photolog/detail/consumer-rules.profeature/photolog/detail/proguard-rules.profeature/photolog/detail/src/main/AndroidManifest.xmlfeature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetail.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailViewModel.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologDetailTopBar.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionBar.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionContent.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionEffect.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionEffectSpec.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionParticle.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/reaction/ReactionUiModel.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeCardSpec.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/swipe/SwipeableCard.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailIntent.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailSideEffect.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/contract/PhotologDetailUiState.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/di/PhotologDetailModule.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/navigation/PhotologDetailNavGraph.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/preview/PhotologDetailPreviewProvider.ktfeature/photolog/editor/.gitignorefeature/photolog/editor/build.gradle.ktsfeature/photolog/editor/consumer-rules.profeature/photolog/editor/proguard-rules.profeature/photolog/editor/src/main/AndroidManifest.xmlfeature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorRoute.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/PhotologEditorViewModel.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/component/PhotologEditorTopBar.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/component/RetakeButton.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/contract/PhotologEditorIntent.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/contract/PhotologEditorSideEffect.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/contract/PhotologEditorUiState.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/di/PhotologEditorModule.ktfeature/photolog/editor/src/main/java/com/twix/photolog/editor/navigation/PhotologEditorNavGraph.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailScreen.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/contract/StatsDetailSideEffect.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/navigation/StatsDetailGraph.ktfeature/task-certification/.gitignorefeature/task-certification/consumer-rules.profeature/task-certification/proguard-rules.profeature/task-certification/src/main/java/com/twix/task_certification/detail/contract/TaskCertificationDetailIntent.ktfeature/task-certification/src/main/java/com/twix/task_certification/di/TaskCertificationModule.ktfeature/task-certification/src/main/java/com/twix/task_certification/editor/contract/TaskCertificationEditorIntent.ktfeature/task-certification/src/main/java/com/twix/task_certification/editor/contract/TaskCertificationEditorSideEffect.ktfeature/task-certification/src/main/java/com/twix/task_certification/navigation/TaskCertificationGraph.ktsettings.gradle.kts
💤 Files with no reviewable changes (8)
- feature/task-certification/.gitignore
- feature/task-certification/src/main/java/com/twix/task_certification/navigation/TaskCertificationGraph.kt
- core/design-system/src/main/res/drawable/ic_keepi_singing.xml
- feature/task-certification/src/main/java/com/twix/task_certification/editor/contract/TaskCertificationEditorSideEffect.kt
- feature/task-certification/src/main/java/com/twix/task_certification/di/TaskCertificationModule.kt
- feature/task-certification/src/main/java/com/twix/task_certification/editor/contract/TaskCertificationEditorIntent.kt
- feature/task-certification/src/main/java/com/twix/task_certification/detail/contract/TaskCertificationDetailIntent.kt
- feature/task-certification/proguard-rules.pro
이슈 번호
작업내용
feature/task-certification구조를feature/photolog/{capture,detail,editor}로 분리NavRoutes및 관련 사용처 반영)리뷰어에게 추가로 요구하는 사항 (선택)