[FIX/#1591] 앱잼탬프 QA 사항 반영#1598
Conversation
Summary by CodeRabbit릴리스 노트
둘러보기이 PR은 앱잼 탬프 QA 피드백을 반영하여 버튼 상태 제어, 뒤로가기 연속 클릭 방지, 미션 수정 시 변경 감지 기반 버튼 활성화, 그리고 상대 시간 표시 정확도를 개선합니다. 변경사항앱잼 탬프 QA 개선사항
예상 코드 리뷰 난이도🎯 3 (보통) | ⏱️ ~25분 추천 리뷰어
🐰 변경 축하 래빗의 시
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces submission state tracking and validation to the mission detail screen, replaces standard click modifiers with ripple-free and throttled alternatives, and refines the relative time formatting logic. Feedback focuses on ensuring that initial snapshots are updated upon successful mission submission, caching SimpleDateFormat instances to prevent recomposition overhead, optimizing LaunchedEffect keys to avoid redundant coroutine restarts, and conditionally applying click modifiers on the button component to improve accessibility when disabled.
| initSnapshotImageModel = ImageModel.Remote(imageModel.url), | ||
| initSnapshotDate = date, | ||
| initSnapshotContent = content, |
There was a problem hiding this comment.
modifyMission()에서는 초기 스냅샷 값(initSnapshotImageModel, initSnapshotDate, initSnapshotContent)을 올바르게 업데이트하고 있지만, 새로운 미션을 작성하는 submitMission()에서는 성공적으로 제출된 후 이 스냅샷 값들을 업데이트하지 않고 있습니다.
이로 인해 사용자가 새 미션을 제출한 직후 상세 화면에서 바로 수정을 시도할 경우, 초기 스냅샷 값이 여전히 비어 있는 상태(기본값)로 유지되어 isSubmitEnabled 조건이 오동작하게 됩니다(예: 제출된 미션과 비교해 아무런 변경 사항이 없음에도 완료 버튼이 활성화됨).
submitMission()의 onSuccess 블록에서도 이 스냅샷 값들을 최신 상태로 업데이트하도록 보완해 주세요.
| val dateFormat = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.KOREA) | ||
| dateFormat.timeZone = TimeZone.getTimeZone("Asia/Seoul") | ||
| val monthDayFormat = SimpleDateFormat("M월d일", Locale.KOREA).apply { | ||
| timeZone = TimeZone.getTimeZone("Asia/Seoul") | ||
| } | ||
|
|
||
| val date = dateFormat.parse(this) ?: return "" | ||
| val date = runCatching { dateFormat.parse(this) }.getOrNull() ?: return "" | ||
| val currentDate = Date() | ||
|
|
||
| val diffMillis = currentDate.time - date.time | ||
| if (diffMillis < 0) return "1분 전" | ||
| if (diffMillis < 0) return "방금 전" | ||
|
|
||
| val minutes = TimeUnit.MILLISECONDS.toMinutes(diffMillis) | ||
| val hours = TimeUnit.MILLISECONDS.toHours(diffMillis) | ||
| val days = TimeUnit.MILLISECONDS.toDays(diffMillis) | ||
|
|
||
| return when { | ||
| minutes == 0L -> "1분 전" | ||
| minutes in 1..59 -> "${minutes}분 전" | ||
| hours in 1..24 -> "${hours}시간 전" | ||
| else -> { | ||
| val days = TimeUnit.MILLISECONDS.toDays(diffMillis) | ||
| "${days}일 전" | ||
| } | ||
| minutes < 10L -> "방금 전" | ||
| minutes < 60L -> "${minutes}분 전" | ||
| hours < 25L -> "${hours}시간 전" | ||
| days < 7L -> "${days}일 전" | ||
| days < 35L -> "${days / 7}주 전" | ||
| else -> monthDayFormat.format(date) | ||
| } | ||
| } |
There was a problem hiding this comment.
toRelativeTime() 함수가 호출될 때마다 SimpleDateFormat 인스턴스를 매번 생성하는 것은 비효율적입니다. 특히 이 함수는 리스트 아이템의 Jetpack Compose 리컴포지션 중에 호출되므로, SimpleDateFormat 인스턴스 생성 비용이 누적되어 성능 저하를 유발할 수 있습니다.
성능을 최적화하고 불필요한 객체 할당을 피하기 위해 포맷터를 캐싱하는 것이 좋습니다. SimpleDateFormat은 스레드 안전하지 않으므로, 파일 수준의 private Holder 객체 내부에 ThreadLocal을 사용하여 안전하게 캐싱할 수 있습니다.
val date = runCatching { Holder.dateFormat.get().parse(this) }.getOrNull() ?: return ""
val currentDate = Date()
val diffMillis = currentDate.time - date.time
if (diffMillis < 0) return "방금 전"
val minutes = TimeUnit.MILLISECONDS.toMinutes(diffMillis)
val hours = TimeUnit.MILLISECONDS.toHours(diffMillis)
val days = TimeUnit.MILLISECONDS.toDays(diffMillis)
return when {
minutes < 10L -> "방금 전"
minutes < 60L -> "${minutes}분 전"
hours < 25L -> "${hours}시간 전"
days < 7L -> "${days}일 전"
days < 35L -> "${days / 7}주 전"
else -> Holder.monthDayFormat.get().format(date)
}
}
private object Holder {
val dateFormat = object : ThreadLocal<SimpleDateFormat>() {
override fun initialValue() = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.KOREA).apply {
timeZone = TimeZone.getTimeZone("Asia/Seoul")
}
}
val monthDayFormat = object : ThreadLocal<SimpleDateFormat>() {
override fun initialValue() = SimpleDateFormat("M월d일", Locale.KOREA).apply {
timeZone = TimeZone.getTimeZone("Asia/Seoul")
}
}
}| LaunchedEffect(showPostSubmissionBadge, !uiState.isLoading, progress) { | ||
| if (showPostSubmissionBadge && progress >= 0.99f && !uiState.isLoading) { | ||
| delay(500L) | ||
| viewModel.updateShowPostSubmissionBadge() | ||
| navigateUp() | ||
| } | ||
| } |
There was a problem hiding this comment.
progress를 LaunchedEffect의 키로 사용하면, progress >= 0.99f 조건이 참이 된 이후에도 Lottie 애니메이션의 매 프레임마다(즉, progress 값이 미세하게 변경될 때마다) 이펙트가 취소되고 재시작됩니다. 이로 인해 불필요한 코루틴 취소 및 재시작이 발생하며, 애니메이션이 완전히 멈출 때까지 delay(500L)가 계속 뒤로 밀리게 됩니다.
대신 LaunchedEffect의 키로는 showPostSubmissionBadge만 사용하고, 내부에서 snapshotFlow를 통해 progress와 loading 조건을 대기하도록 변경하면 불필요한 재시작 없이 부드럽게 동작합니다.
참고: androidx.compose.runtime.snapshotFlow와 kotlinx.coroutines.flow.first 임포트가 필요합니다.
LaunchedEffect(showPostSubmissionBadge) {
if (showPostSubmissionBadge) {
snapshotFlow { progress >= 0.99f && !uiState.isLoading }
.first { it }
delay(500L)
viewModel.updateShowPostSubmissionBadge()
navigateUp()
}
}| .background( | ||
| color = SoptTheme.colors.primary, | ||
| color = if (isEnabled) SoptTheme.colors.primary else SoptTheme.colors.onSurface300, | ||
| shape = RoundedCornerShape(9.dp), | ||
| ) | ||
| .clickable(onClick = onClicked), | ||
| .noRippleClickable(onClick = { if (isEnabled) onClicked() }), |
There was a problem hiding this comment.
isEnabled가 false일 때 버튼은 시각적으로 비활성화되지만, 클릭 한정자(noRippleClickable)는 여전히 연결되어 있어 클릭 이벤트를 가로챕니다(람다 내부에서 아무 동작도 하지 않더라도). 이는 TalkBack과 같은 접근성 서비스나 키보드 탐색 시 비활성화된 버튼이 여전히 클릭 가능한 것처럼 인식되는 문제를 유발할 수 있습니다.
버튼이 비활성화되었을 때는 클릭 리스너 자체가 연결되지 않도록 .let을 사용하여 noRippleClickable 한정자를 조건부로 적용하는 것이 좋습니다.
.background(
color = if (isEnabled) SoptTheme.colors.primary else SoptTheme.colors.onSurface300,
shape = RoundedCornerShape(9.dp),
)
.let {
if (isEnabled) it.noRippleClickable(onClick = onClicked) else it
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@feature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/ranking/component/Top3RecentRankingMission.kt`:
- Around line 179-180: The week-based formatting branch in
Top3RecentRankingMission (the days-based when/else that currently uses `days <
35L`) incorrectly includes 28–34 days as "n주 전"; change the condition to limit
week display to 27 days (e.g., `days < 28L` or `days <= 27L`) so that any date
>= 28 days falls through to the else branch and uses monthDayFormat.format(date)
per the spec.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a13e6071-d273-44d7-91a9-60047a92a5e3
📒 Files selected for processing (6)
feature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/component/AppjamtampButton.ktfeature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/component/BackButtonHeader.ktfeature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/missiondetail/MissionDetailRoute.ktfeature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/missiondetail/MissionDetailState.ktfeature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/missiondetail/MissionDetailViewModel.ktfeature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/ranking/component/Top3RecentRankingMission.kt
| days < 35L -> "${days / 7}주 전" | ||
| else -> monthDayFormat.format(date) |
There was a problem hiding this comment.
4주 이후 포맷 요구사항과 분기 조건이 어긋납니다.
Line 179의 days < 35L 조건 때문에 28~34일이 "4주 전"으로 표시됩니다. PR 명세(4주 이후는 "n월 n일")에 맞추려면 주 단위 표기는 27일까지로 제한되어야 합니다.
수정 제안 diff
- days < 35L -> "${days / 7}주 전"
+ days < 28L -> "${days / 7}주 전"
else -> monthDayFormat.format(date)🤖 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
`@feature/appjamtamp/src/main/java/org/sopt/official/feature/appjamtamp/ranking/component/Top3RecentRankingMission.kt`
around lines 179 - 180, The week-based formatting branch in
Top3RecentRankingMission (the days-based when/else that currently uses `days <
35L`) incorrectly includes 28–34 days as "n주 전"; change the condition to limit
week display to 27 days (e.g., `days < 28L` or `days <= 27L`) so that any date
>= 28 days falls through to the else branch and uses monthDayFormat.format(date)
per the spec.
1 participant
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
스토리 카드 시간 표기 기준은 조금 많이 변경되었는데 지난 QA 때 의견이 나와서 4주 이후에는 n월 n일로 수정하기로 해서 변경했습니다..!