Conversation
# Conflicts: # app/build.gradle.kts # app/src/main/java/com/sseotdabwa/buyornot/ui/BuyOrNotApp.kt # core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/icon/BuyOrNotIcons.kt
|
Warning Rate limit exceeded
⌛ 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. Walkthrough업로드 화면에 후방 네비게이션 콜백을 추가하고 UploadScreen을 재구성했으며, 하단 네비게이션 바 관련 로직을 제거했습니다. 디자인시스템에는 새로운 Shape(BubbleShape), 아이콘( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavHost
participant NavController
participant UploadScreen
participant System as Activity/Launcher
User->>NavHost: 요청(업로드 화면 진입)
NavHost->>NavController: navigateToUpload()
NavController->>UploadScreen: composable 호출(onNavigateBack 제공)
UploadScreen->>System: 이미지 선택 요청 (Gallery/Camera launcher)
System-->>UploadScreen: 선택된 이미지 반환
User->>UploadScreen: 뒤로가기 또는 제출
alt 뒤로가기
UploadScreen->>NavController: onNavigateBack() -> popBackStack()
NavController-->>NavHost: 화면 복귀
else 제출
UploadScreen->>NavController: 제출 후 네비게이션(선택적)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt`:
- Around line 484-498: The touch target for the image remove button (the Box in
UploadScreen.kt that currently uses Modifier.size(20.dp) and .clickable {
onRemove() }) is too small; update the Modifier chain for that Box to preserve
the 20dp visual circle but expand the interactive area to at least 48dp by
applying minimumInteractiveComponentSize() (or by adding transparent padding
around the visual size) before .clickable so the onRemove() hit target meets
accessibility guidelines; keep the CircleShape background/clip and visual size
the same while ensuring the clickable area is enlarged.
In `@gradle/libs.versions.toml`:
- Line 39: libs.versions.toml에 추가된 materialNavigation("1.7.0")과 카탈로그 항목
androidx-compose-material 및 androidx-compose-material-navigation이 현재 어떤
build.gradle.kts에서도 임포트되어 사용되지 않습니다; 해당 아티팩트가 실제로 사용될 모듈에서 Compose Navigation 관련
의존성(import/implementation)을 추가하거나 사용 계획이 없다면 libs.versions.toml에서
materialNavigation 및 관련 카탈로그 항목(androidx-compose-material,
androidx-compose-material-navigation)을 삭제하여 버전 카탈로그를 정리하세요.
🧹 Nitpick comments (6)
core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/shape/BubbleShape.kt (1)
19-22:layoutDirection미사용 — RTL 레이아웃 고려현재
layoutDirection파라미터를 사용하지 않아, RTL 환경에서도 화살표가 항상 오른쪽을 가리킵니다. 한국어 앱 특성상 당장 문제는 없지만, 추후 다국어 지원 시 화살표 방향을layoutDirection에 따라 반전하는 처리가 필요할 수 있습니다.♻️ RTL 지원 예시
override fun createOutline( size: Size, layoutDirection: LayoutDirection, density: Density, ): Outline = Outline.Generic( Path().apply { val cornerRadiusPx = with(density) { cornerRadius.toPx() } val arrowWidthPx = with(density) { arrowWidth.toPx() } val arrowHeightPx = with(density) { arrowHeight.toPx() } + val isRtl = layoutDirection == LayoutDirection.Rtl - addRoundRect( - RoundRect( - rect = Rect(0f, 0f, size.width - arrowWidthPx, size.height), - cornerRadius = CornerRadius(cornerRadiusPx), - ), - ) + val bodyRect = if (isRtl) { + Rect(arrowWidthPx, 0f, size.width, size.height) + } else { + Rect(0f, 0f, size.width - arrowWidthPx, size.height) + } + addRoundRect(RoundRect(rect = bodyRect, cornerRadius = CornerRadius(cornerRadiusPx))) - moveTo(size.width - arrowWidthPx, size.height / 2 - arrowHeightPx / 2) - lineTo(size.width, size.height / 2) - lineTo(size.width - arrowWidthPx, size.height / 2 + arrowHeightPx / 2) + if (isRtl) { + moveTo(arrowWidthPx, size.height / 2 - arrowHeightPx / 2) + lineTo(0f, size.height / 2) + lineTo(arrowWidthPx, size.height / 2 + arrowHeightPx / 2) + } else { + moveTo(size.width - arrowWidthPx, size.height / 2 - arrowHeightPx / 2) + lineTo(size.width, size.height / 2) + lineTo(size.width - arrowWidthPx, size.height / 2 + arrowHeightPx / 2) + } close() }, )core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/BottomSheet.kt (1)
120-130: 드래그 핸들 색상이 하드코딩되어 있습니다.다른 모든 색상(
gray0,gray1000등)은BuyOrNotTheme.colors를 통해 참조하고 있는데, 드래그 핸들만Color(0xFFD9D9D9)로 하드코딩되어 있습니다. 다크 모드 지원이나 디자인 시스템 변경 시 누락될 수 있으므로, 가능하면 테마 색상을 사용하는 것이 좋습니다.♻️ 테마 색상 사용 제안
.background( - color = Color(0xFFD9D9D9), + color = BuyOrNotTheme.colors.gray300, // 또는 적절한 테마 색상 shape = RoundedCornerShape(18.dp), ),app/src/main/AndroidManifest.xml (1)
14-14:windowSoftInputMode를<activity>레벨로 이동하는 것을 권장합니다.현재
<application>레벨에 설정되어 있어 향후 추가되는 모든 Activity에 일괄 적용됩니다. 단일 Activity 앱이므로 당장 문제는 없지만, 의도를 명확히 하려면 Line 17의<activity>태그에 직접 지정하는 편이 낫습니다.제안
<application android:name=".BuyOrNotApplication" android:allowBackup="true" android:dataExtractionRules="@xml/data_extraction_rules" android:fullBackupContent="@xml/backup_rules" android:icon="@mipmap/ic_launcher" android:label="@string/app_name" android:roundIcon="@mipmap/ic_launcher_round" android:supportsRtl="true" - android:windowSoftInputMode="adjustResize" android:theme="@style/Theme.BuyOrNot"> <activity android:name=".MainActivity" android:exported="true" android:label="@string/app_name" - android:theme="@style/Theme.BuyOrNot"> + android:theme="@style/Theme.BuyOrNot" + android:windowSoftInputMode="adjustResize">app/src/main/java/com/sseotdabwa/buyornot/ui/BuyOrNotApp.kt (1)
59-67:bottomBarPadding함수명이 현재 역할과 맞지 않습니다.하단 네비게이션 바가 제거되었으므로 이 함수는 실질적으로 시스템 인셋 패딩 적용 여부를 결정합니다.
scaffoldPadding또는systemInsetsPadding등으로 이름을 변경하면 의도가 더 명확해집니다.feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt (2)
549-579:customShadow가public으로 노출되어 있습니다.이 파일 내
ToolTip에서만 사용되므로private으로 제한하거나, 재사용이 필요하다면core/designsystem모듈의 유틸리티 파일로 이동하는 것이 좋습니다.제안
-fun Modifier.customShadow( +private fun Modifier.customShadow(
265-323: 가격 입력 포매팅 로직이 잘 구현되어 있습니다.커서 위치 재계산이 정확하며, 10자리 제한(최대 ~99억)이
Long범위 내에 있어 안전합니다.한 가지 참고:
DecimalFormat("#,###")은 로케일에 따라 그룹 구분자가 달라질 수 있습니다. 한국 타겟 앱이므로 현재는 문제없지만, 다국어 지원 시DecimalFormat("#,###", DecimalFormatSymbols(Locale.KOREA))로 명시하면 더 안전합니다.
feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt
Show resolved
Hide resolved
DongChyeon
force-pushed
the
feat/#15-upload-ui
branch
from
e6dab92 to
27aabbe
Compare
| selectedImageUri = uri | ||
| } | ||
|
|
||
| val isSubmitEnabled = selectedCategory != null && priceRaw.isNotEmpty() && selectedImageUri != null |
There was a problem hiding this comment.
C: 현재 기능은 잘 동작합니다! :) 짱짱
다만 궁금한 사항이 있습니다!
현재 특정 필드만 수정되어도 다른 필드 및 부모까지 리컴포지션이 추가로 일어나는데 ,
isSubmitEnabled 변수가 UploadScreen 최상단에 있어서
여기서 참조하는 값이 하나라도 바뀌면 화면 전체가 리컴포지션 대상이 될 확률도 있을까요?
궁금해서 여쭤봅니다!!
There was a problem hiding this comment.
안녕하세요. 제 코드에 관심가져주셔서 감사합니다.
derivedStateOf를 사용하면 리컴포지션 횟수를 더 줄일 수 있는 점 찾아내서 반영했습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt`:
- Around line 421-468: The Surface currently uses Modifier.clickable which
prevents the ripple from being clipped to RoundedCornerShape(12.dp); remove the
Modifier.clickable(onClick = onClick) usage and instead use the Material3
Surface clickable overload by passing onClick directly to the Surface (i.e.,
Surface(onClick = onClick, shape = RoundedCornerShape(12.dp), ...)) while
keeping the size modifier and other properties; update any imports if needed and
ensure no duplicate clickable modifiers remain in Column or children.
🧹 Nitpick comments (5)
core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/OptionSheet.kt (1)
36-43: KDoc에sheetShape파라미터 설명이 누락되어 있습니다.새로 추가된
sheetShape파라미터에 대한@param문서가 빠져 있습니다. 다른 파라미터들은 모두 설명이 있으므로 일관성을 위해 추가해 주세요.feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt (4)
293-329: 가격 입력 커서 위치 계산 시 엣지 케이스 확인이 필요합니다.
digitsBeforeCursor > 0인데formattedText의 모든 자릿수를 순회해도digitCount가digitsBeforeCursor에 도달하지 못하면newCursorPos가0으로 남아 커서가 맨 앞으로 이동합니다. 현재 로직상formattedText는newDigits에서 파생되므로 이론적으로 불가능하지만, 방어 코드로 루프 종료 후newCursorPos를formattedText.length로 fallback 처리하면 더 안전합니다.🛡️ 방어 코드 제안
var digitCount = 0 var newCursorPos = 0 for (i in formattedText.indices) { if (formattedText[i].isDigit()) { digitCount++ } if (digitCount == digitsBeforeCursor) { newCursorPos = i + 1 break } } + // fallback: 루프에서 매칭되지 않은 경우 끝으로 + if (digitCount < digitsBeforeCursor) { + newCursorPos = formattedText.length + } if (digitsBeforeCursor == 0) { newCursorPos = 0 }
351-391:ContentInputField의BasicTextField에singleLine = false명시 또는 줄바꿈 제한 여부를 확인해 주세요.현재
BasicTextField에singleLine이 설정되지 않아 기본값false로 여러 줄 입력이 가능합니다. 의도된 동작이라면 괜찮지만, 글자 수 제한(100자)만 있고 줄 수 제한이 없어서 과도한 줄바꿈으로 레이아웃이 늘어날 수 있습니다.
106-112: 갤러리에서 받은Uri의 읽기 권한 지속성에 주의하세요.
ActivityResultContracts.GetContent()으로 받은Uri는 일시적 권한만 부여됩니다. 현재는 UI-only 구현이라 문제없지만, 추후 업로드 기능 구현 시ContentResolver.takePersistableUriPermission()을 호출하거나 즉시 파일을 복사하는 처리가 필요합니다.
262-267: 카테고리 텍스트에만clickable이 적용되어 있어 터치 영역이 좁습니다."카테고리 추가" 텍스트만 클릭 가능하지만, 사용자는 화살표 아이콘이나 "투표 등록" 텍스트까지 포함한 Row 전체를 탭할 것으로 기대할 수 있습니다.
clickable을Row에 적용하는 것을 고려해 주세요.
feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt
Show resolved
Hide resolved
2 participants
🛠 Related issue
closed #15
어떤 변경사항이 있었나요?
✅ CheckPoint
PR이 다음 요구 사항을 충족하는지 확인하세요.
✏️ Work Description
default.mp4
😅 Uncompleted Tasks
N/A
📢 To Reviewers
어제 수정한 BottomSheet 사용하니 하단 dim 영역이 의도대로 잡히지 않아서
수정해보다가... 결국 원상복구시켰습니다.
그리고 초반에 #13으로 잘못 커밋된 내용이 있긴 한데 고치려면 rebase 과정도 필요해서 이건 감안해주심 좋겠습니다.
📃 RCA 룰
Summary by CodeRabbit
릴리스 노트
새로운 기능
UI 개선
버그 수정
의존성