[feat] #10 QR 스캐너 화면 1차 구현#14
Conversation
- QR 스캔 화면의 UI 구성 요소인 `QRScannerContent`를 구현했습니다. - 스캔 영역 UI, 상단 닫기 버튼, 안내 텍스트, 하단 플래시 버튼을 포함합니다. - UI에 필요한 `icon_close`, `icon_qr_light` 벡터 아이콘을 추가했습니다.
- buildConfig 사용을 위해 필요합니다.
- 요구사항이 모두 나오지 않은 경우에 린트 오류가 걸려 제거했습니다.
Walkthrough이 변경사항은 QR 코드 스캐너 기능을 구현하기 위해 새로운 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant QRScanScreen
participant QRScanViewModel
participant MviIntentStore
participant CameraX
participant MLKit as ML Kit<br/>Barcode Scanner
participant WebView
User->>QRScanScreen: 화면 진입
QRScanScreen->>QRScanViewModel: ViewModel 주입
QRScanViewModel->>MviIntentStore: 초기 상태 설정<br/>(QRScanState)
QRScanScreen->>CameraX: 카메라 초기화
CameraX->>MLKit: 카메라 프레임 전송
loop QR 코드 스캔
User->>CameraX: QR 코드 카메라에 비춤
CameraX->>MLKit: ImageProxy 분석 요청
MLKit-->>CameraX: URL 바코드 감지
CameraX-->>QRScanScreen: onQRCodeScanned(url)
QRScanScreen->>QRScanViewModel: ScanQRCode(scannedUrl) intent 전송
QRScanViewModel->>MviIntentStore: 상태 업데이트<br/>(viewType → WEB_VIEW)
end
QRScanScreen->>WebView: scannedUrl 로드
WebView->>WebView: HTML/이미지 렌더링
User->>WebView: 페이지 상호작용
alt 이미지 URL 감지
WebView-->>QRScanScreen: onDetectImageUrl(url)
QRScanScreen->>QRScanViewModel: DetectImageUrl intent 전송
end
User->>QRScanScreen: 종료 버튼 클릭
QRScanScreen->>QRScanViewModel: ClickCloseQRScan intent 전송
QRScanViewModel->>MviIntentStore: NavigateBack 사이드이펙트 발생
MviIntentStore-->>QRScanScreen: 네비게이션 콜백
QRScanScreen->>User: 이전 화면으로 이동
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 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
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: 9
🤖 Fix all issues with AI Agents
In @app/src/main/AndroidManifest.xml:
- Line 7: The manifest currently declares a mandatory camera feature via the
uses-feature element (android.hardware.camera) which blocks installs on devices
without a camera; change the uses-feature to be optional and more flexible by
replacing the element with one using android.hardware.camera.any and adding
android:required="false" (update the existing <uses-feature
android:name="android.hardware.camera" /> entry to <uses-feature
android:name="android.hardware.camera.any" android:required="false" />) and
ensure runtime checks use
PackageManager.hasSystemFeature("android.hardware.camera.any") before attempting
camera/QR operations to handle devices without cameras gracefully.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebView.kt:
- Around line 15-26: The WebView is created with JavaScript always enabled and
is not cleaned up on Composable disposal; update the WebView creation in
PhotoWebView (the WebView(context).apply block that sets
settings.javaScriptEnabled, settings.domStorageEnabled, webViewClient =
PhotoWebViewClient { onDetectImageUrl(...) } and loadUrl(scannedUrl)) to only
enable JavaScript after validating that scannedUrl is from a trusted source (or
make JS conditional via a parameter) and wrap the WebView in a DisposableEffect
that onDispose stops loading, clears clients and listeners (set webViewClient =
null), removes the view, calls removeAllViews(), clearHistory() and destroy() to
prevent memory leaks.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt:
- Around line 87-91: The two Icon usages in QRScannerContent (the close and QR
light icons) set contentDescription = null; update them to provide meaningful,
localized descriptions by replacing null with stringResource(...) (e.g.,
contentDescription = stringResource(R.string.cd_close) and contentDescription =
stringResource(R.string.cd_qr_light)) and add the corresponding string resources
(cd_close, cd_qr_light) to strings.xml; ensure you import
androidx.compose.ui.res.stringResource and use these contentDescription values
in the Icon calls inside QRScannerContent.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/PhotoWebViewClient.kt:
- Line 17: In PhotoWebViewClient, the expression request?.url.toString() can
yield the literal "null" when url is null; change the extraction to handle null
explicitly (e.g., use request?.url?.toString() ?: "" or perform a null-check/let
on request?.url) so subsequent checks against url operate on a real empty string
or are skipped when null; update any logic that compares url (used later in the
class) to use this null-safe value or guard early with an if (request?.url ==
null) return.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.kt:
- Line 21: The BarcodeScanner created via
BarcodeScanning.getClient(qrScannerOptions) (assigned to scanner in
QRImageAnalyzer) is Closeable and is never closed; implement Closeable on
QRImageAnalyzer (or add a public close() method) and call scanner.close() when
the analyzer is disposed, then update the caller (QRScanner.kt) to invoke this
cleanup (e.g., via DisposableEffect) so the scanner resource is released.
- Around line 32-42: The analyzer calls onQRCodeScanned for every frame with a
URL barcode, causing duplicate callbacks when the same QR remains visible;
update QRImageAnalyzer to deduplicate by tracking the last scanned value and/or
a timestamp cooldown before invoking onQRCodeScanned (e.g., store lastScannedUrl
and lastScanTime in the class, compare incoming url in the scanner.process
success loop and only call onQRCodeScanned if different or cooldown has
elapsed), ensure you still close imageProxy in addOnCompleteListener, and reset
the tracking appropriately when a different barcode or no barcode is seen to
allow future scans.
🧹 Nitpick comments (12)
detekt-config.yml (1)
53-54:UnusedParameter규칙 전체 비활성화는 지나치게 광범위합니다.전체 규칙을 비활성화하기보다는, 다른 규칙(예:
LongMethod,CyclomaticComplexMethod)처럼ignoreAnnotated옵션으로 특정 어노테이션(예:Composable)만 제외하는 것이 더 적절합니다. 이를 통해 실제 불필요한 파라미터를 감지할 수 있으며, 새로운 모듈들에서 코드 품질을 유지할 수 있습니다.특히 MVI 패턴과 새로운 Compose UI 컴포넌트에서 모든 파라미터가 의도적으로 사용되는지 확인해야 합니다.
🔎 제안된 수정 방안
style: UnusedParameter: - active: false + ignoreAnnotated: [ 'Composable' ]또는, Composable이 아닌 일반 함수에서 UnusedParameter를 의도적으로 무시해야 하는 특정한 경우가 있다면, 그 이유를 문서화하고 규칙을 비활성화하기 전에 코드 리뷰에서 확인하시기 바랍니다.
core/designsystem/src/main/res/drawable/icon_qr_light.xml (2)
2-5: 비정사각형 아이콘 크기로 인한 스케일링 고려 필요아이콘이 21dp x 23dp로 정사각형이 아닙니다. UI에서 사용 시 예상치 못한 스케일링이나 정렬 문제가 발생할 수 있습니다. 아이콘을 정사각형 캔버스(예: 24dp x 24dp)로 조정하는 것을 고려해보세요.
8-8: 테마 색상 사용 권장하드코딩된 white 색상 대신 테마 속성이나 틴트(tint)를 사용하는 것을 권장합니다. 이렇게 하면 다크 모드나 다양한 테마를 지원할 때 더 유연하게 대응할 수 있습니다.
🔎 제안하는 개선 방안
아이콘을 사용하는 곳에서
android:tint속성으로 색상을 지정하거나, 색상을 투명하게 설정하고 외부에서 틴트를 적용하는 방식을 고려해보세요:- android:fillColor="#ffffff"/> + android:fillColor="@android:color/white"/>또는 ImageView/Icon 컴포저블에서 틴트를 적용:
Icon( painter = painterResource(R.drawable.icon_qr_light), contentDescription = "QR", tint = MaterialTheme.colorScheme.onPrimary )core/designsystem/src/main/res/drawable/icon_close.xml (1)
1-18: 전체적으로 양호하나 테마 색상 사용 권장닫기 아이콘의 구조와 구현은 적절합니다. 다만 하드코딩된 white 색상(
#ffffff) 대신 테마 속성이나 외부 틴트를 사용하면 다양한 테마 환경에서 더 유연하게 대응할 수 있습니다.🔎 개선 제안
아이콘 사용 시 외부에서 틴트를 적용하는 방식을 권장합니다:
Icon( painter = painterResource(R.drawable.icon_close), contentDescription = "닫기", tint = MaterialTheme.colorScheme.onSurface )또는 색상 리소스 사용:
- android:strokeColor="#ffffff" + android:strokeColor="@android:color/white"core/ui/src/main/java/com/neki/android/core/ui/Spacer.kt (1)
14-18: 기본값 0.dp를 필수 파라미터로 변경하는 것을 고려하세요.
HorizontalSpacer와VerticalSpacer의 기본값이0.dp로 설정되어 있습니다. 0dp 간격은 시각적 효과가 없으므로, 실수로 인한 잘못된 사용을 방지하기 위해 파라미터를 필수로 만드는 것이 더 명확할 수 있습니다.🔎 필수 파라미터로 변경하는 제안
@Composable fun HorizontalSpacer( - width: Dp = 0.dp, + width: Dp, ) { Spacer(modifier = Modifier.width(width)) } @Composable fun VerticalSpacer( - height: Dp = 0.dp, + height: Dp, ) { Spacer(modifier = Modifier.height(height)) }Also applies to: 28-32
feature/photo-upload/impl/build.gradle.kts (1)
2-2: 불필요한 import 제거
kotlin.apply는 Kotlin 표준 라이브러리 함수로 명시적 import가 필요하지 않습니다.import java.util.Properties -import kotlin.applycore/ui/build.gradle.kts (1)
16-16: 버전 카탈로그 사용 권장
lifecycle-viewmodel-ktx버전이 하드코딩되어 있습니다. 프로젝트의 다른 의존성들과 일관성을 유지하기 위해libs.versions.toml에서 관리하는 것이 좋습니다.🔎 제안된 수정
gradle/libs.versions.toml에 추가:[versions] lifecycle = "2.10.0" [libraries] androidx-lifecycle-viewmodel-ktx = { group = "androidx.lifecycle", name = "lifecycle-viewmodel-ktx", version.ref = "lifecycle" }
core/ui/build.gradle.kts수정:- implementation("androidx.lifecycle:lifecycle-viewmodel-ktx:2.10.0") + implementation(libs.androidx.lifecycle.viewmodel.ktx)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (1)
27-27: ShowToast 사이드 이펙트 미구현
ShowToast처리가 비어있습니다. ViewModel에서 토스트 메시지를 발송하지만 UI에서 처리되지 않습니다. TODO 주석을 추가하거나 구현을 완료하세요.해당 기능 구현을 도와드릴까요, 아니면 이를 추적할 새 이슈를 생성할까요?
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (1)
46-46: 매직 넘버 대신 상수 사용 권장
0.82f가 여러 곳에서 사용됩니다.QRLayoutConst.CUTOUT_WIDTH_FRACTION(≈0.827)과 약간 다릅니다. 일관성을 위해 상수를 사용하세요.🔎 제안된 수정
+import com.neki.android.feature.photo_upload.impl.qrscan.const.QRLayoutConst.CUTOUT_WIDTH_FRACTION + @Composable internal fun QRScannerContent( ... ) { - val cutoutSize = LocalConfiguration.current.screenWidthDp.dp * 0.82f + val cutoutSize = LocalConfiguration.current.screenWidthDp.dp * CUTOUT_WIDTH_FRACTIONAlso applies to: 60-60, 68-68
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.kt (1)
39-39: DetectImageUrl Intent 미구현
DetectImageUrl핸들러가 비어있습니다. PR 목표에 "이미지 리퀘스트 인터셉터 구현(미완료)"이 언급되어 있으므로, TODO 주석을 추가하여 향후 구현을 추적하세요.🔎 제안된 수정
- is QRScanIntent.DetectImageUrl -> {} + is QRScanIntent.DetectImageUrl -> { + // TODO: 이미지 URL 감지 후 처리 로직 구현 + }이 기능 구현을 추적할 새 이슈를 생성할까요?
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/const/QRLayoutConst.kt (1)
11-11: 타입 일관성: Double vs Float
FRAME_CORNER_RADIUS가Double로 추론됩니다..dp확장 함수와 함께 사용 시Float로 변환이 필요할 수 있습니다. 명시적으로Float타입을 사용하세요.🔎 제안된 수정
- internal const val FRAME_CORNER_RADIUS = 28.96 + internal const val FRAME_CORNER_RADIUS = 28.96ffeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.kt (1)
54-63: 선택적 최적화: 불필요한 상태 업데이트 방지
onGloballyPositioned는 매 레이아웃 패스마다 호출됩니다. 값이 변경되지 않아도 상태를 업데이트하면 불필요한 리컴포지션이 발생할 수 있습니다.🔎 최적화 제안
cutout( Modifier.onGloballyPositioned { coords -> val pos = coords.positionInParent() val size = coords.size - cutoutRect = CutoutRectPx( - left = pos.x, - top = pos.y, - width = size.width.toFloat(), - height = size.height.toFloat(), - ) + val newRect = CutoutRectPx( + left = pos.x, + top = pos.y, + width = size.width.toFloat(), + height = size.height.toFloat(), + ) + if (cutoutRect != newRect) { + cutoutRect = newRect + } }, )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlbuild-logic/src/main/java/com/neki/android/buildlogic/extensions/Android.ktbuild-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureImplConventionPlugin.ktcore/designsystem/src/main/res/drawable/icon_close.xmlcore/designsystem/src/main/res/drawable/icon_qr_light.xmlcore/ui/.gitignorecore/ui/build.gradle.ktscore/ui/src/main/java/com/neki/android/core/ui/Flow.ktcore/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.ktcore/ui/src/main/java/com/neki/android/core/ui/Spacer.ktdetekt-config.ymlfeature/photo-upload/api/.gitignorefeature/photo-upload/api/build.gradle.ktsfeature/photo-upload/api/src/main/java/com/neki/android/feature/photo_upload/api/PhotoUploadNavKey.ktfeature/photo-upload/impl/.gitignorefeature/photo-upload/impl/build.gradle.ktsfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/di/PhotoUploadEntryProvider.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanContract.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebView.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/ScanCornerFrame.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/const/QRLayoutConst.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/PhotoWebViewClient.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.ktgradle/libs.versions.tomlsettings.gradle.kts
🧰 Additional context used
🧬 Code graph analysis (5)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.kt (1)
onQRCodeScanned(13-47)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (4)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
QRScanner(36-136)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.kt (1)
DimExceptContent(37-100)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/ScanCornerFrame.kt (1)
ScanCornerFrame(24-103)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.kt (1)
core/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.kt (2)
mviIntentStore(53-60)postSideEffect(39-41)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/di/PhotoUploadEntryProvider.kt (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (1)
QRScanRoute(15-34)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (4)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (1)
QRScannerContent(38-139)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.kt (1)
onIntent(18-41)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebView.kt (1)
PhotoWebView(8-34)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
⏰ 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: ci-build
🔇 Additional comments (31)
feature/photo-upload/api/src/main/java/com/neki/android/feature/photo_upload/api/PhotoUploadNavKey.kt (3)
1-6: LGTM!패키지 선언과 임포트가 적절합니다. 필요한 네비게이션 및 직렬화 타입이 정확히 임포트되었습니다.
7-11: LGTM!Sealed interface 패턴을 사용한 타입 안전 네비게이션 키 정의가 올바릅니다.
@Serializable어노테이션도 navigation3 라이브러리의 요구사항에 맞게 적용되었습니다.
13-15: LGTM!확장 함수를 통한 네비게이션 API가 깔끔하고 읽기 쉽습니다. 코틀린 관례에 맞는 구현입니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/di/PhotoUploadEntryProvider.kt (3)
1-14: LGTM!패키지 선언과 임포트가 적절합니다. Dagger/Hilt 모듈 구성에 필요한 타입들이 정확히 임포트되었습니다.
15-24: LGTM!Dagger Hilt 모듈 설정이 올바릅니다.
ActivityRetainedComponent에 설치하고@IntoSet으로 멀티바인딩을 사용하는 것이 네비게이션 엔트리 제공에 적합합니다.
26-33: 빈 navigateToHome 콜백을 확인해주세요.
navigateToHome이 빈 람다로 구현되어 있습니다.QRScanRoute는QRScanSideEffect.NavigateToHome을 처리하도록 설계되었으나, 현재는 아무 동작도 하지 않습니다.이것이 의도된 것인지(향후 구현 예정) 확인이 필요합니다. PR 설명에서 언급된 미완료 작업과 관련이 있을 수 있습니다.
feature/photo-upload/impl/.gitignore (1)
1-1: 표준 빌드 아티팩트 무시 패턴이 올바르게 설정됨.Gradle 모듈의 빌드 디렉토리를 무시하는 표준 패턴이 적절하게 구성되었습니다.
core/ui/.gitignore (1)
1-1: 표준 빌드 아티팩트 무시 패턴이 올바르게 설정됨.새로운 core:ui 모듈의 빌드 디렉토리 무시 설정이 적절합니다.
feature/photo-upload/api/.gitignore (1)
1-1: 표준 빌드 아티팩트 무시 패턴이 올바르게 설정됨.photo-upload API 모듈의 빌드 디렉토리 무시 설정이 적절합니다.
core/ui/src/main/java/com/neki/android/core/ui/Flow.kt (1)
10-21: 구현이 올바릅니다!Lifecycle을 고려한 Flow 수집 패턴이 정확하게 구현되었습니다.
LaunchedEffect의 키와repeatOnLifecycle사용이 적절하며, Compose UI에서 안전하게 Flow를 수집할 수 있는 표준 패턴을 제공합니다.core/ui/src/main/java/com/neki/android/core/ui/Spacer.kt (1)
13-39: 깔끔하고 일관된 Spacer 유틸리티 구현입니다.고정 크기와 가중치 기반 간격을 모두 지원하는 편리한 헬퍼 함수들이 잘 구현되었습니다. Compose 레이아웃에서 간격을 일관되게 처리할 수 있는 좋은 API를 제공합니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/PhotoWebViewClient.kt (1)
19-22: BuildConfig 상수가 올바르게 정의되어 있습니다.
BuildConfig.PHOTOISM_URL과BuildConfig.PHOTOISM_IMG_URL_END상수는feature/photo-upload/impl/build.gradle.kts의defaultConfig블록에 명시적으로 정의되어 있으므로 컴파일 오류가 발생하지 않습니다.gradle/libs.versions.toml (2)
4-4: CameraX 버전 1.5.2는 최신 안정 버전이며 보안 취약점이 없습니다.웹 검색 결과 확인됨: CameraX 1.5.2는 2025년 12월 기준 최신 안정 버전이며, 이 버전에 대한 알려진 보안 취약점(CVE)은 없습니다.
29-29: ML Kit 바코드 스캐닝 버전 확인 완료버전 17.3.0은 최신 버전이며 알려진 보안 취약점이 없습니다. 추가 조치가 필요하지 않습니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanContract.kt (1)
1-29: LGTM! MVI 계약 구조가 깔끔합니다.State, Intent, SideEffect, ViewType이 명확하게 분리되어 있고, 기본값도 적절하게 설정되어 있습니다.
core/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.kt (1)
43-50:onIntent에서 state 접근 패턴 확인 필요
_uiState.value를 직접 읽어서onIntent콜백에 전달하고 있습니다. 여러 intent가 빠르게 연속 발생하면 전달된state값이 최신이 아닐 수 있습니다. 다만reduce함수가MutableStateFlow.update를 사용하므로 상태 업데이트 자체는 원자적입니다.현재 QR 스캔 기능에서는 문제가 되지 않을 것으로 보이나, 복잡한 시나리오에서는 reduce 함수 내에서 최신 상태를 참조하도록 주의가 필요합니다.
feature/photo-upload/api/build.gradle.kts (1)
1-7: LGTM!API 모듈 빌드 설정이 표준 구조를 따르고 있습니다.
build-logic/src/main/java/com/neki/android/buildlogic/extensions/Android.kt (1)
16-18: LGTM!
minSdk설정을 빌드 로직에 중앙화하여 모든 모듈에서 일관성을 보장합니다.build-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureImplConventionPlugin.kt (1)
20-20: LGTM!모든 feature impl 모듈에서 MVI 유틸리티를 사용할 수 있도록
core:ui의존성을 추가한 것은 적절합니다.settings.gradle.kts (1)
40-42: LGTM!새로운 모듈 포함이 기존 프로젝트 구조와 일관되게 추가되었습니다.
app/build.gradle.kts (1)
22-22: LGTM!새로운 모듈 의존성이 기존 패턴과 일관되게 추가되었습니다.
Also applies to: 29-30
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebView.kt (1)
28-32: URL 비교 로직 검토
webView.url은 초기에null일 수 있으므로,scannedUrl과의 비교가 예상대로 동작합니다. 다만,scannedUrl이 빈 문자열이 아닌 경우에만 로드하는 현재 로직은 적절합니다.feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (2)
15-34: LGTM!
QRScanRoute의 lifecycle-aware 상태 수집 및 side effect 처리가 적절합니다.
8-8: 현재 코드의 import는 올바릅니다.androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 deprecated되지 않았으며, 오히려androidx.hilt.navigation.compose.hiltViewModel이 deprecated되었습니다. Hilt ViewModel 함수들이androidx.hilt.navigation.compose에서androidx.hilt.lifecycle.viewmodel.compose로 이동했으므로, 현재 import는 최신의 권장 방식입니다. 변경이 필요하지 않습니다.Likely an incorrect or invalid review comment.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (1)
48-76: LGTM!QR 스캐너 레이아웃 구조가 잘 구성되어 있습니다. 레이어링이 적절하고 modifier 사용이 일관됩니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.kt (2)
34-37: SetViewType에서 항상 토스트 표시현재
SetViewType이 항상 에러 토스트를 표시합니다.scannedUrl이 null인 fallback 상황에서만 호출되므로 현재는 적절하지만, 향후 다른 용도로 사용될 경우 조건부 로직이 필요할 수 있습니다.
9-16: LGTM!MVI 패턴이 깔끔하게 적용되었고, Hilt 주입 및 스토어 초기화가 적절합니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/const/QRLayoutConst.kt (1)
3-11: LGTM!레이아웃 상수가 잘 구성되어 있습니다. 관련 값들이 한 곳에 모여있어 유지보수가 용이합니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/ScanCornerFrame.kt (1)
24-103: 구현이 잘 되었습니다!Canvas를 사용한 코너 프레임 렌더링 로직이 명확하고, 각 코너에 대해
quadraticTo를 사용한 둥근 모서리 처리가 적절합니다. 상수를QRLayoutConst에서 가져와 일관성을 유지한 점도 좋습니다.feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
91-132: 제스처 구현이 적절합니다탭-투-포커스와 핀치-투-줌 제스처 처리가 올바르게 구현되었습니다. 좌표 변환을 통한 포커스 포인트 계산과 줌 비율 제한 로직이 정확합니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.kt (1)
37-99: 깔끔한 구현입니다!
CompositingStrategy.Offscreen을 사용하여BlendMode.Clear가 올바르게 작동하도록 한 점,@Immutable어노테이션으로 리컴포지션 최적화를 고려한 점이 좋습니다. cutout 패딩 및 radius 처리 로직도 정확합니다.
`PhotoWebView`를 `PhotoWebViewContent`로 리네이밍하고, `DisposableEffect`를 사용하여 `onDispose` 시점에서 WebView를 destroy 하도록 수정했습니다. 이를 통해 QR 코드 스캔 후 웹뷰 화면을 벗어날 때 발생할 수 있는 메모리 누수를 방지합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.kt:
- Around line 14-20: The local mutable webView variable is reinitialized on
recomposition, so change it to a remembered state that survives recompositions
(e.g., remember { mutableStateOf<WebView?>(null) }) and assign the WebView
instance inside the AndroidView factory to that remembered state (e.g.,
webViewState.value = createdWebView); then use webViewState.value inside
DisposableEffect to call destroy() on dispose. Ensure you update all references
from webView to the remembered state (webViewState.value) so DisposableEffect
closes over the stable remembered reference.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt:
- Around line 58-89: LaunchedEffect and DisposableEffect use mismatched keys
causing camera to not reinit when lifecycleOwner changes, and QRImageAnalyzer
never closes the ML Kit BarcodeScanner; implement Closeable on QRImageAnalyzer
to call scanner.close(), and change the camera lifecycle effect in QRScanner to
use the same key (use lifecycleOwner as the LaunchedEffect key or consolidate
into one DisposableEffect tied to lifecycleOwner) so you bind/unbind provider
consistently; on dispose call provider?.unbindAll() and call analyzer.close()
(the QRImageAnalyzer.close() that closes BarcodeScanning.getClient()) before
unbinding to avoid native resource leaks.
🧹 Nitpick comments (3)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
47-47: 사용되지 않는 변수 제거 필요
scope변수가 선언되었지만 코드 어디에서도 사용되지 않습니다.🔎 제안된 수정
- val scope = rememberCoroutineScope() - var provider by remember { mutableStateOf<ProcessCameraProvider?>(null) }feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.kt (1)
16-20: WebView 정리 로직 보완 고려현재
destroy()호출만으로도 메모리 누수 방지의 핵심은 처리되지만, 더 안전한 정리를 위해 추가 메서드 호출을 고려할 수 있습니다.🔎 선택적 개선안
DisposableEffect(Unit) { onDispose { - webView?.destroy() + webViewRef.value?.apply { + stopLoading() + webViewClient = null + clearHistory() + removeAllViews() + destroy() + } } }feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (1)
56-60: 코드 일관성을 위한 중괄호 추가 권장단일 표현식이지만 가독성과 일관성을 위해 if 문에 중괄호를 추가하는 것을 고려하세요.
🔎 제안된 수정
- if (scannedUrl != null) + if (scannedUrl != null) { PhotoWebViewContent( scannedUrl = scannedUrl, onDetectImageUrl = { imageUrl -> onIntent(QRScanIntent.DetectImageUrl(imageUrl)) }, ) - else { + } else { LaunchedEffect(Unit) { onIntent(QRScanIntent.SetViewType(viewType = QRScanViewType.QR_SCAN)) } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt
🧰 Additional context used
🧬 Code graph analysis (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (4)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (1)
QRScannerContent(38-139)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanViewModel.kt (1)
onIntent(18-41)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.kt (1)
PhotoWebViewContent(9-44)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.kt (1)
onQRCodeScanned(13-47)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
⏰ 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: ci-build
🔇 Additional comments (4)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
91-93: LGTM!토치 제어 로직이 올바르게 구현되었습니다.
isTorchEnabled와camera모두를 의존성으로 사용하여 적절하게 반응합니다.
95-147: LGTM!탭 포커스와 핀치 줌 제스처 구현이 올바릅니다:
- 좌표 변환을 통해 Compose 좌표를 카메라 표면 좌표로 정확히 변환합니다.
- 줌 비율을 유효 범위 내로 제한하여 안전하게 처리합니다.
- null 체크와 early return으로 안정성을 확보했습니다.
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt (2)
61-65: 이전 리뷰 이슈 해결 확인
LaunchedEffect를 사용하여 이전 리뷰에서 지적된 composition 중 side effect 호출 문제가 올바르게 해결되었습니다.scannedUrl이 null일 때 안전하게 intent를 전달합니다.
22-30: LGTM - 적절한 생명주기 인식 상태 수집
collectAsStateWithLifecycle()과collectWithLifecycle을 사용하여 생명주기를 고려한 안전한 상태 및 사이드 이펙트 수집이 구현되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt:
- Around line 56-87: The camera resource handling bug: change
LaunchedEffect(Unit) to LaunchedEffect(lifecycleOwner) (or combine the effects)
so camera reinitializes when lifecycleOwner changes, and modify QRImageAnalyzer
to implement Closeable and expose a close() that calls barcodeScanner.close()
(or equivalent native close), then keep a reference to the analyzer instance
created in the LaunchedEffect and call analyzer.close() inside
DisposableEffect's onDispose (alongside provider?.unbindAll()) to ensure the
BarcodeScanner is closed and native resources released.
- Around line 107-109: The call to the non-existent extension offset.transform()
will not compile; replace it with the CameraX API call
coordinateTransformer.transform(offset) (i.e., call transform on the
CoordinateTransformer with the Offset) or alternatively implement an extension
function like fun Offset.transform(coordinateTransformer: CoordinateTransformer)
in the core:ui module and then call offset.transform(coordinateTransformer);
update the assignment to surfacePoint accordingly (variable names: offset,
coordinateTransformer, surfacePoint).
🧹 Nitpick comments (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
121-124: 매직 넘버를 상수로 추출하는 것을 고려해 보세요포커스 자동 취소 시간(3초)을 상수로 추출하면 가독성과 유지보수성이 향상됩니다.
🔎 상수 추출 제안
QRLayoutConst 또는 별도의 상수 파일에 다음을 추가:
const val FOCUS_AUTO_CANCEL_DURATION_SECONDS = 3L그런 다음:
val action = FocusMeteringAction.Builder( focusPoint, FocusMeteringAction.FLAG_AF or FocusMeteringAction.FLAG_AE, -).setAutoCancelDuration(3, TimeUnit.SECONDS).build() +).setAutoCancelDuration(FOCUS_AUTO_CANCEL_DURATION_SECONDS, TimeUnit.SECONDS).build()
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.kt (1)
onQRCodeScanned(13-47)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
⏰ 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: ci-build
🔇 Additional comments (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
103-141: 탭-투-포커스 및 핀치-투-줌 구현이 잘 되어 있습니다제스처 처리 로직이 올바르게 구현되었습니다:
SurfaceOrientedMeteringPointFactory를 사용한 적절한 좌표 변환- 줌 비율을
minZoomRatio와maxZoomRatio범위로 제한- 3초 자동 취소와 함께 AF/AE 플래그 사용
좌표 변환 API 검증만 확인되면 문제없습니다.
89-91: 토치 제어 로직이 올바르게 구현되었습니다
LaunchedEffect의 키로isTorchEnabled와camera를 모두 사용하여 적절히 반응하도록 구현되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt:
- Around line 106-108: The code calls a non-existent extension
Offset.transform() causing a compile error; replace the call with the CameraX
API method by invoking coordinateTransformer.transform(offset) (i.e. change the
expression using the coordinateTransformer variable to call its transform method
with the offset), or alternatively add an extension fun
Offset.transform(coordinateTransformer: CoordinateTransformer) that delegates to
coordinateTransformer.transform(this) if you prefer the extension style; update
the usage in QRScanner.kt (where surfacePoint is computed) to use the chosen
approach.
- Around line 55-86: LaunchedEffect in QRScanner is using Unit as its key while
DisposableEffect uses lifecycleOwner, and QRImageAnalyzer's internal
BarcodeScanner isn't closed, causing lifecycle binding bugs and native resource
leaks; fix by making QRImageAnalyzer implement Closeable with a close() that
calls scanner.close(), create and store the analyzer instance when calling
setAnalyzer (e.g., val analyzer = QRImageAnalyzer { ... }), change
LaunchedEffect's key to lifecycleOwner (or consolidate the effects) so the
camera rebinds when lifecycleOwner changes, and in DisposableEffect.onDispose
call provider?.unbindAll() and analyzer.close() to ensure the analyzer's
resources are released.
🧹 Nitpick comments (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
120-123: 매직 넘버 상수화 고려3초 자동 취소 시간이 하드코딩되어 있습니다. PR 설명에서 상수화 관련 리뷰를 요청하셨는데, 이런 값은 상수로 추출하면 유지보수성이 향상됩니다.
♻️ 상수 추출 제안
파일 상단에 companion object 또는 별도 상수 파일에 추가:
private object CameraConst { const val FOCUS_AUTO_CANCEL_SECONDS = 3L }사용:
val action = FocusMeteringAction.Builder( focusPoint, FocusMeteringAction.FLAG_AF or FocusMeteringAction.FLAG_AE, - ).setAutoCancelDuration(3, TimeUnit.SECONDS).build() + ).setAutoCancelDuration(FOCUS_AUTO_CANCEL_SECONDS, TimeUnit.SECONDS).build()
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (2)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/util/QRImageAnalyzer.kt (1)
onQRCodeScanned(13-47)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
⏰ 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: ci-build
🔇 Additional comments (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
147-153: 프리뷰 구현이 적절합니다디자인 타임 프리뷰가 올바르게 구현되어 있습니다.
Ojongseok
left a comment
Ojongseok
left a comment
There was a problem hiding this comment.
고생 많으셨습니다!!
Q. Const 로 주요 값? 들을 상수화했는데 어떤가요 ??
A. 좋습니다! 상수 뿐만 아니라 feature 내에서 쓰이는 Const(QRLayoutConst), Util(PhotoWebViewClient, QRImageAnlyzer)은 :photo-upload 모듈에 적용되어 있는 것 처럼 하시죠
Q. 텍스트로 표시하는 문자열 같은 경우에 1차 MVP 개발 단계에서도 strings.xml 로 추출해서 stringResource 로 사용할까요 ??
A. 흐음, 처음부터 string.xml에 정의하고 stringResource로 사용한다고 생각을 했는데 말씀을 듣고보니 저희가 1차 MVP 이후 디자인 시스템이나 컴포넌트를 다듬는 과정이 있을테니 그 떄 stringResource도 같이 손을 볼까요??
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.kt:
- Around line 40-44: update 블록에서 webView.url != scannedUrl 비교는 null, 리다이렉션,
프래그먼트/쿼리 차이로 오작동하므로 webView.url 직접 비교를 제거하고 AndroidView의 update 람다(변수명 update)
내부에서 scannedUrl이 비어있지 않고 이전에 로드한 URL과 다른지 판단하는 추적 변수(lastLoadedUrl 등, remember {
mutableStateOf("") }로 선언)로 검사하도록 변경하세요; 조건을 만족하면 webView.loadUrl(scannedUrl)을
호출하고 lastLoadedUrl을 scannedUrl로 갱신하여 불필요한 재로딩을 방지합니다.
- Around line 28-29: PhotoWebViewContent currently enables JavaScript and DOM
storage unconditionally (settings.javaScriptEnabled /
settings.domStorageEnabled) which is unsafe for arbitrary scanned URLs; before
calling loadUrl() in PhotoWebViewContent, validate the incoming URL's scheme
(allow only https:// and optionally http:// if needed) and check the host
against a trusted whitelist (implement a trustedDomains set), enable
settings.javaScriptEnabled and settings.domStorageEnabled only after the URL
passes these checks (or keep them disabled by default), and ensure
PhotoWebViewClient continues to filter resources but does not rely on it for
initial navigation safety.
In
@feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt:
- Around line 94-117: The Text composable in QRScannerContent.kt contains
hardcoded Korean strings inside buildAnnotatedString ("QR을 스캔" and "하면\n보관함에 자동
저장돼요!"); extract these into string resources (e.g., qr_scan_title_highlight and
qr_scan_title_description in strings.xml) and replace the hardcoded append calls
with stringResource(...) usage (or pass the strings into buildAnnotatedString
from stringResource() before appending) so the UI uses i18n-managed text;
optionally move the gradient color literals used in the SpanStyle to theme
constants (e.g., NekiTheme or named color constants) and reference those instead
for consistency.
🧹 Nitpick comments (7)
core/ui/src/main/java/com/neki/android/core/ui/compose/Spacer.kt (1)
20-25: weight 파라미터 검증 고려weight 기반 Spacer 함수들이 잘 구현되어 있습니다. 다만, Compose의
weightmodifier는 일반적으로 양수 값을 기대하므로, 방어적 코딩 차원에서require(weight > 0f)검증을 추가하는 것을 고려해볼 수 있습니다.♻️ 선택적 개선안
@Composable fun RowScope.HorizontalSpacer( weight: Float, ) { + require(weight > 0f) { "weight must be positive" } Spacer(modifier = Modifier.weight(weight)) }@Composable fun ColumnScope.VerticalSpacer( weight: Float, ) { + require(weight > 0f) { "weight must be positive" } Spacer(modifier = Modifier.weight(weight)) }Also applies to: 34-39
core/ui/src/main/java/com/neki/android/core/ui/compose/Flow.kt (1)
10-21:reified수정자 제거를 권장합니다.함수 구현에서 타입 파라미터
T의 런타임 타입 정보가 사용되지 않으므로reified수정자는 불필요합니다.♻️ reified 수정자 제거
@Composable -inline fun <reified T> Flow<T>.collectWithLifecycle( +inline fun <T> Flow<T>.collectWithLifecycle( minActiveState: Lifecycle.State = Lifecycle.State.STARTED, noinline action: suspend (T) -> Unit, ) {참고:
androidx.lifecycle.compose.collectAsStateWithLifecycle과는 다른 목적입니다.collectAsStateWithLifecycle은 Flow를 Compose State로 수집하는 반면, 이 함수는 Flow를 구독하여 suspend action(side effect 처리)을 실행합니다. 두 함수는 상호보완적이므로 중복이 아닙니다.feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (3)
46-46: 0.82f 매직 넘버를 상수로 추출하는 것을 권장합니다.cutout 크기 비율
0.82f가 3회 반복됩니다 (lines 46, 60, 68). DRY 원칙을 위반하며, 향후 디자인 변경 시 일관성 유지가 어려울 수 있습니다.♻️ 상수 추출 제안
QRLayoutConst.kt또는 현재 파일 상단에 상수를 정의:private const val CUTOUT_WIDTH_RATIO = 0.82f그리고 사용처를 다음과 같이 변경:
- val cutoutSize = LocalConfiguration.current.screenWidthDp.dp * 0.82f + val cutoutSize = LocalConfiguration.current.screenWidthDp.dp * CUTOUT_WIDTH_RATIO // ... cutoutModifier - .fillMaxWidth(0.82f) + .fillMaxWidth(CUTOUT_WIDTH_RATIO) .aspectRatio(1f) // ... Box( modifier = Modifier - .fillMaxWidth(0.82f) + .fillMaxWidth(CUTOUT_WIDTH_RATIO)Also applies to: 60-60, 68-68
83-92: 접근성 향상을 위해 contentDescription 추가를 권장합니다.Lines 89, 134의 Icon들이
contentDescription = null로 설정되어 있습니다. 시각 장애인 사용자가 TalkBack 등의 보조 기술을 사용할 때 버튼의 용도를 파악할 수 없습니다.♿ 접근성 개선 제안
strings.xml에 추가:<string name="qr_scan_close_button">닫기</string> <string name="qr_scan_torch_on">손전등 켜기</string> <string name="qr_scan_torch_off">손전등 끄기</string>코드 변경:
Icon( imageVector = ImageVector.vectorResource(R.drawable.icon_close), - contentDescription = null, + contentDescription = stringResource(R.string.qr_scan_close_button), tint = Color.White, ) // ... Icon( imageVector = ImageVector.vectorResource(R.drawable.icon_qr_light), - contentDescription = null, + contentDescription = stringResource( + if (isTorchEnabled) R.string.qr_scan_torch_off + else R.string.qr_scan_torch_on + ), tint = if (isTorchEnabled) Color.Black else Color.White, )Also applies to: 120-137
84-84: [선택적] 기타 매직 넘버도 상수화 고려여러 디자인 관련 수치들이 하드코딩되어 있습니다:
- Line 84: 버튼 패딩
8.dp,7.dp- Line 93: 수직 간격
32.dp- Lines 113-115: 폰트 크기
24.sp, 줄 높이36.sp- Line 123: 토치 버튼 오프셋
41.dp- Line 124: 토치 버튼 크기
48.dp- Line 128: 비활성 상태 투명도
0.1f이들 값은 각각 한 번씩만 사용되므로 필수는 아니지만,
QRLayoutConst.kt나 별도 상수 객체로 관리하면 디자인 변경 시 유지보수가 용이합니다.특히 Line 123의
cutoutSize / 2 + 41.dp계산식은 토치 버튼 위치를 결정하는 중요한 로직이므로, 주석이나 네이밍으로 의도를 명확히 하면 좋습니다.Also applies to: 93-93, 113-115, 123-124, 128-128
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.kt (2)
16-16: WebView 참조 관리 패턴을 단순화하세요.
remember { mutableStateOf<WebView?>(null) }패턴은 불필요합니다.remember만으로도 이미 라이프사이클을 관리하므로, WebView를 MutableState로 감쌀 필요가 없습니다. MutableState는 주로 재구성을 트리거해야 하는 값에 사용되는데, WebView 참조는 factory에서 한 번만 생성되므로 해당하지 않습니다.♻️ 제안하는 리팩토링
- val webView = remember { mutableStateOf<WebView?>(null) } + var webView: WebView? = null DisposableEffect(Unit) { onDispose { - webView.value?.destroy() + webView?.destroy() } } AndroidView( factory = { context -> WebView(context).apply { - webView.value = this + webView = this settings.javaScriptEnabled = true또는 더 안전한 방법으로
remember를 직접 사용:+ val webViewRef = remember { mutableListOf<WebView?>(null) } + DisposableEffect(Unit) { onDispose { - webView.value?.destroy() + webViewRef[0]?.destroy() } }
24-45: WebView 에러 처리 및 로딩 상태 추가를 권장합니다.현재 구현에는 다음이 누락되어 있습니다:
- WebView 로드 실패 시 에러 처리
- 로딩 중 사용자 피드백 (로딩 인디케이터)
- 백 프레스 네비게이션 처리
사용자 경험 향상을 위해 이러한 기능 추가를 고려하세요.
💡 구현 예시
WebViewClient를 확장하여 에러 및 로딩 상태 처리:
@Composable internal fun PhotoWebViewContent( scannedUrl: String, onDetectImageUrl: (String) -> Unit, ) { var isLoading by remember { mutableStateOf(false) } var hasError by remember { mutableStateOf(false) } val webView = remember { mutableStateOf<WebView?>(null) } Box { AndroidView( factory = { context -> WebView(context).apply { webView.value = this settings.javaScriptEnabled = true settings.domStorageEnabled = true webViewClient = object : PhotoWebViewClient(onDetectImageUrl) { override fun onPageStarted(view: WebView?, url: String?, favicon: android.graphics.Bitmap?) { super.onPageStarted(view, url, favicon) isLoading = true hasError = false } override fun onPageFinished(view: WebView?, url: String?) { super.onPageFinished(view, url) isLoading = false } override fun onReceivedError(view: WebView?, request: WebResourceRequest?, error: WebResourceError?) { super.onReceivedError(view, request, error) isLoading = false hasError = true } } loadUrl(scannedUrl) } }, // ... update block ) if (isLoading) { CircularProgressIndicator(modifier = Modifier.align(Alignment.Center)) } if (hasError) { Text("페이지 로드 실패", modifier = Modifier.align(Alignment.Center)) } } }이러한 개선사항 구현을 도와드릴까요?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
build-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidApplicationConventionPlugin.ktcore/ui/src/main/java/com/neki/android/core/ui/compose/Flow.ktcore/ui/src/main/java/com/neki/android/core/ui/compose/Spacer.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/PhotoWebViewContent.ktfeature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/QRScanScreen.kt
- feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (4)
feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScanner.kt (1)
QRScanner(37-145)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/DimExceptContent.kt (1)
DimExceptContent(37-100)feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/ScanCornerFrame.kt (1)
ScanCornerFrame(24-103)core/ui/src/main/java/com/neki/android/core/ui/compose/Spacer.kt (2)
VerticalSpacer(27-32)VerticalSpacer(34-39)
⏰ 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: ci-build
🔇 Additional comments (4)
core/ui/src/main/java/com/neki/android/core/ui/compose/Spacer.kt (1)
13-32: 구현이 깔끔하고 정확합니다.Dp 기반 spacer 유틸리티들이 Compose 표준 패턴을 잘 따르고 있습니다. 기본값 0.dp는 호출 시 명시적으로 크기를 지정하도록 유도하는 합리적인 선택입니다.
build-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidApplicationConventionPlugin.kt (1)
20-27: minSdk 설정이 Android.kt 확장 파일에 올바르게 구성되어 있습니다.검증 결과,
minSdk설정이Android.kt의configureAndroid함수(17번 줄)에서BuildConst.MIN_SDK로 올바르게 구성되어 있으며, 18번 줄의configureAndroid(this)호출을 통해 적절하게 적용되고 있습니다. 이는 공유 설정을 확장 파일로 분리하는 적절한 리팩토링 패턴입니다.feature/photo-upload/impl/src/main/java/com/neki/android/feature/photo_upload/impl/qrscan/component/QRScannerContent.kt (2)
38-45: 함수 시그니처가 잘 설계되었습니다.파라미터 구성과 기본값 설정이 적절하며, 콜백 기반 설계로 재사용성이 좋습니다.
141-147: 프리뷰 구현이 적절합니다.Design-time preview가 올바르게 구성되어 있으며,
NekiTheme를 사용하여 테마 일관성을 유지하고 있습니다.
|
@ikseong00 변경사항 확인했습니다! |
2 participants
🔗 관련 이슈
📙 작업 설명
core:ui모듈 추가 (Spacer, Flow 확장함수)feature:photo-upload모듈 신규 생성📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
build-logic쪽AndroidExtenstion 에 minSDK 를 추가했습니다.detekt-config.yml파일에 임시로 UnusedParameter 를 제거했습니다.Q. Const 로 주요 값? 들을 상수화했는데 어떤가요 ??
Q. 텍스트로 표시하는 문자열 같은 경우에 1차 MVP 개발 단계에서도
strings.xml로 추출해서stringResource로 사용할까요 ??Summary by CodeRabbit
릴리스 노트
새로운 기능
UI/UX 개선
✏️ Tip: You can customize this high-level summary in your review settings.