Merged
Conversation
Walkthrough장소 상세 화면의 컴포저블에서 UI 레이아웃과 소수의 동작을 조정했다. SignatureMenu의 타입 참조와 텍스트 레이아웃 제약을 변경했고, Success 상태의 무매장 문구를 composition에 고정했으며 공유 버튼의 탭 스로틀 시간을 늘렸다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as SpotDetailScreen
participant Compose as Compose Runtime
User->>Screen: 화면 진입 (Success 상태)
Note over Screen: noStoreText에서 1개 선택
Screen->>Compose: remember { noStoreText.random() }
Compose-->>Screen: 동일 컴포지션 동안 값 유지
User->>Screen: 재조합 트리거(스크롤/상호작용)
Screen->>Compose: remember 조회
Compose-->>Screen: 이전 값 반환
Screen-->>User: 동일 문구 표시
sequenceDiagram
autonumber
actor User
participant Button as ShareButton
participant Handler as ClickHandler
participant Throttle as ThrottleGate(3s)
participant Action as onClickButton
User->>Button: 탭
Button->>Handler: onClick
Handler->>Throttle: canProceed()?
alt 첫 탭 또는 3초 경과
Throttle-->>Handler: true
Handler->>Action: invoke()
else 3초 내 연속 탭
Throttle-->>Handler: false
Handler-->>User: 무시됨
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/StoreFloatingButtonSet.kt (2)
97-101: 공유 버튼 롱프레스 해제 시 상태 변수 오타
isShareLongPressed가 아니라isMenuBoardLongPressed를 false로 설정하고 있어 공유 아이콘이 눌린 상태로 남거나 다른 버튼 상태에 영향을 줄 수 있습니다.아래처럼 수정하세요.
- onReleaseAfterLongPress = { - onClickShare() - isMenuBoardLongPressed = false - }, + onReleaseAfterLongPress = { + onClickShare() + isShareLongPressed = false + },
142-149: 공유 스로틀이 롱프레스 경로에는 적용되지 않음현재 스로틀은
onTap에만 적용되어 롱프레스 후 손을 뗄 때는 제한 없이 공유가 실행됩니다. 동일한 스로틀을 롱프레스 해제 경로에도 적용하세요. 또한 단조 증가 시간(모노토닉)을 사용해 시스템 시계 변경 영향을 피하는 것을 권장합니다.아래 패치를 적용하면 롱프레스 해제 시에도 동일 스로틀이 적용됩니다.
+import android.os.SystemClock @@ - detectTapGestures( + detectTapGestures( onPress = { isLongPressed = false val pressSucceeded = tryAwaitRelease() if (pressSucceeded && isLongPressed) { - onReleaseAfterLongPress() + onReleaseAfterLongPress() + // 롱프레스 해제도 클릭으로 간주하여 동일 스로틀 적용 + val now = SystemClock.elapsedRealtime() + if (isShare) { + if (now - lastClickTime >= throttleTime) { + lastClickTime = now + onClickButton() + } + } else { + onClickButton() + } } }, onTap = { - val currentTime = System.currentTimeMillis() + val now = SystemClock.elapsedRealtime() if (isShare) { - if (currentTime - lastClickTime >= throttleTime) { - lastClickTime = currentTime + if (now - lastClickTime >= throttleTime) { + lastClickTime = now onClickButton() } } else { onClickButton() } },Also applies to: 150-159
🧹 Nitpick comments (5)
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/StoreFloatingButtonSet.kt (1)
118-118: 스로틀 값 하드코딩 최소화(파라미터화 제안)테스트/AB 설정을 고려해 스로틀 값을 파라미터로 받을 수 있게 하면 재배포 없이 조정이 수월합니다.
private fun StoreDetailButton( @@ - isShare: Boolean = false + isShare: Boolean = false, + throttleMillis: Long = 3000L ) { @@ - val throttleTime = 3000L + val throttleTime = throttleMillis호출부(공유 버튼)에서 필요 시
throttleMillis = 3000L전달로 명시성도 높일 수 있습니다.feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SpotDetailScreen.kt (2)
126-126: Recomposition 안정성 향상: 키를 spotId로 설정가게 전환 시(다른 Spot)만 문구를 다시 선택하도록 키를 부여하세요. 현재 구현은 동일 Spot 내 재컴포지션에서도 고정되지만, 의도를 더 명확히 하고 상태 복원 시 예측 가능성이 좋아집니다.
- val rememberedNoStoreText = remember { noStoreText.random() } + val rememberedNoStoreText = remember(state.spotDetail.spotId) { noStoreText.random() }
68-68: 내부 API 의존 제거: okhttp3.internal.immutableListOf 사용 지양OkHttp 내부 API는 호환성 보장이 없습니다. 표준
listOf로 대체하세요.-import okhttp3.internal.immutableListOf +// (제거)- val noStoreText = immutableListOf( + val noStoreText = listOf( stringResource(R.string.no_store_image_verified), stringResource(R.string.no_store_image_secret), stringResource(R.string.no_store_image_mystery) )Also applies to: 88-92
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SignatureMenu.kt (2)
21-21: 동일 명칭(함수/타입) 충돌 혼동 완화: import 별칭 사용 제안Composable
SignatureMenu와 모델 타입SignatureMenu가 동일 이름이라 가독성이 떨어집니다. 별칭을 권장합니다.-import com.acon.acon.core.model.model.spot.SignatureMenu +import com.acon.acon.core.model.model.spot.SignatureMenu as SpotSignatureMenu @@ -internal fun SignatureMenu( - signatureMenuList: List<SignatureMenu> +internal fun SignatureMenu( + signatureMenuList: List<SpotSignatureMenu> )Also applies to: 26-27
48-61: 레이아웃 단순화: Box 제거하고 Text에 직접 폭 지정현재 Box(width=160.dp) 내부에
fillMaxWidth()텍스트를 두고 있어 계층이 불필요합니다. 동일 결과를 더 단순하게 표현할 수 있습니다.- Box( - modifier = Modifier.width(160.dp) - ) { - Text( - text = menuName, - color = AconTheme.color.White, - style = AconTheme.typography.Body1, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier - .fillMaxWidth() - .align(Alignment.CenterStart) - ) - } + Text( + text = menuName, + color = AconTheme.color.White, + style = AconTheme.typography.Body1, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.width(160.dp) + )참고: 더 유연한 정렬이 필요하면
Modifier.weight(1f).widthIn(max = 160.dp)패턴도 고려해볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SignatureMenu.kt(3 hunks)feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SpotDetailScreen.kt(3 hunks)feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/StoreFloatingButtonSet.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
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.
🧨 Issue
💻 Work Description
Summary by CodeRabbit
Bug Fixes
Improvements