Skip to content

Conversation

@1971123-seongmin
Copy link
Collaborator

@1971123-seongmin 1971123-seongmin commented Sep 20, 2025

🧨 Issue

💻 Work Description

  • 장소 상세 화면 대표메뉴 가격 패딩 수치 통일
  • 장소 상세 화면 가게 이미지가 없는 경우, 문구 리컴포지션 되는 문제 수정
  • 공유 버튼 중복 클릭 주기 수정

Summary by CodeRabbit

  • Bug Fixes

    • Spot 상세 화면의 “매장 없음” 안내 문구가 재구성 시마다 바뀌던 문제를 수정해, 화면 유지 동안 일관되게 표시되도록 했습니다.
  • Improvements

    • 공유 버튼 연타 방지 간격을 1초에서 3초로 늘려 중복 실행을 줄였습니다.
    • 시그니처 메뉴 항목의 레이아웃을 조정해 텍스트 정렬, 너비 일관성 및 줄바꿈/잘림 처리를 개선했습니다.

@coderabbitai
Copy link

coderabbitai bot commented Sep 20, 2025

Walkthrough

장소 상세 화면의 컴포저블에서 UI 레이아웃과 소수의 동작을 조정했다. SignatureMenu의 타입 참조와 텍스트 레이아웃 제약을 변경했고, Success 상태의 무매장 문구를 composition에 고정했으며 공유 버튼의 탭 스로틀 시간을 늘렸다.

Changes

Cohort / File(s) Change Summary
SignatureMenu 타입·레이아웃 조정
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SignatureMenu.kt
파라미터 제네릭을 FQCN에서 공개 SignatureMenu 타입으로 변경. widthIn 제거 및 Box(고정 160.dp) + Text.fillMaxWidth() + Alignment.CenterStart로 레이아웃 재구성. 일부 임포트 정리(예: fillMaxWidth 추가).
성공 상태 무매장 문구 안정화
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SpotDetailScreen.kt
noStoreText.random() 호출을 remember { noStoreText.random() }로 래핑해 컴포지션 간 텍스트가 고정되도록 변경. remember 임포트 추가.
공유 버튼 클릭 스로틀 확장
feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/StoreFloatingButtonSet.kt
공유 버튼의 클릭 경로에서 throttleTime1000L3000L로 상향 조정해 연속 탭 수용 간격을 확대.

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: 동일 문구 표시
Loading
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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 톡—기억은 한 번만 꺼내놓고
글자는 상자 안에 가지런히 기대고
탭은 잠깐 쉬어 3초의 숨을 고르고
메뉴는 고요히 정렬되어 빛나네
깡충—작은 손질, 큰 안정감 ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check ❓ Inconclusive 연결된 이슈 #249에는 구체적인 요구사항이나 수락 기준이 기록되어 있지 않아 PR 변경사항이 이슈의 주요 요구를 완전히 충족하는지 검증할 수 없습니다. PR 설명과 raw_summary는 패딩 통일, 이미지 없음 시 리컴포지션 문제 수정, 공유 버튼 중복 클릭 주기 수정 등 목표를 제시하며 변경사항도 이에 부합합니다. 다만 이슈 자체의 불명확성 때문에 최종적 합치 여부는 확정할 수 없습니다. 이 이슈에 대한 명확한 수락 기준(예: 기대 동작, 화면 캡처, 단위/UI 테스트)을 이슈 또는 PR 본문에 추가하거나 PR 작성자가 변경 사항이 이슈 요구를 어떻게 만족하는지 명시해 주세요.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 제목 "[Mod/#249] 장소 상세 화면 UI 수정"은 변경사항의 주된 목적(장소 상세 화면 UI 수정)을 명확히 요약하고 있어 PR 내용과 직접 연관됩니다. 숫자 태그(#249)가 포함되어 있어 추적성도 확보됩니다.
Out of Scope Changes Check ✅ Passed 검토된 변경사항은 모두 feature/spot 내의 장소 상세 화면 관련 파일에 국한되며 레이아웃(대표메뉴 패딩·정렬), 이미지 없음 시 리컴포지션 방지, 공유 버튼 클릭 스로틀 조정 등 PR 목적과 직접 연관되어 있어 범위 외 변경은 확인되지 않습니다. SignatureMenu의 타입 축약(import 정리)은 내부 구현 범위로 보이나 호출부 영향 여부는 간단히 확인할 필요가 있습니다.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mod/#249-spot-detail-edit

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e21dc0 and 61646fa.

📒 Files selected for processing (1)
  • feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SpotDetailScreen.kt (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • feature/spot/src/main/java/com/acon/acon/feature/spot/screen/spotdetail/composable/SpotDetailScreen.kt
⏰ 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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between fea6c17 and 7e21dc0.

📒 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

@1971123-seongmin 1971123-seongmin merged commit c328b2e into develop Sep 20, 2025
2 checks passed
@1971123-seongmin 1971123-seongmin deleted the mod/#249-spot-detail-edit branch September 20, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants