Chore/#126 link#143
Conversation
# Conflicts: # feature/home/src/main/java/com/linku/home/ui/home/bar/LinkDetailTopBar.kt
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough코어 알람 모델과 저장소 계약이 바뀌고, data 계층에 알람 API·DTO·리포지토리·선호 저장소가 추가되었습니다. home/design에는 저장·상세·알람 화면과 관련 UI/리소스가 확장되었고, app/mypage는 알림 설정, FCM, 네비게이션 흐름을 새 계약에 맞게 연결했습니다. Changes링크 저장·상세 UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
…link # Conflicts: # feature/home/src/main/java/com/linku/home/ui/home/bar/LinkDetailTopBar.kt
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
feature/mypage/src/main/java/com/linku/mypage/component/notification/SystemAlarmTab.kt (1)
50-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value아이콘의
.offset(y = (-12).dp)하드코딩은 텍스트 줄 수/폰트 변경 시 정렬이 깨질 수 있습니다.상단 정렬이 목적이라면
Row의verticalAlignment = Alignment.Top이나 텍스트 첫 줄 기준 정렬을 사용하는 편이 폰트 스케일 변화에 더 견고합니다. 현 구현은 동작은 하지만 시각적 회귀 위험이 있습니다.🤖 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/mypage/src/main/java/com/linku/mypage/component/notification/SystemAlarmTab.kt` around lines 50 - 59, The icon positioning in SystemAlarmTab’s Image uses a hardcoded y offset, which can break alignment when text size, line count, or font scale changes. Replace the .offset(y = (-12).dp) approach with layout-based alignment in the surrounding Row or equivalent, using verticalAlignment = Alignment.Top or a first-line/text-baseline aligned arrangement so the icon stays anchored correctly without manual pixel tuning.feature/mypage/src/main/java/com/linku/mypage/screen/AlarmSettingScreen.kt (1)
67-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
sideEffect수집이 라이프사이클을 인지하지 못합니다.
LaunchedEffect(Unit)내부 수집은 화면이 백그라운드에 있을 때도 활성 상태를 유지합니다. Toast 정도라면 큰 문제는 아니지만,repeatOnLifecycle(STARTED)또는flowWithLifecycle로 감싸 정지 시 수집을 중단하는 편이 더 안전합니다.🤖 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/mypage/src/main/java/com/linku/mypage/screen/AlarmSettingScreen.kt` around lines 67 - 74, `AlarmSettingScreen`의 `LaunchedEffect(Unit)` 안에서 `viewModel.sideEffect.collect`가 라이프사이클을 무시한 채 계속 수집되고 있습니다. `LaunchedEffect` 대신 `repeatOnLifecycle(STARTED)` 또는 `flowWithLifecycle`을 사용해 `sideEffect` 수집이 화면이 STARTED일 때만 동작하고 정지 시 중단되도록 `NotificationEffect.ShowToast` 처리 로직을 옮기세요.design/src/main/java/com/linku/design/util/OnResumeEffect.kt (1)
20-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKDoc 사용 예시의 함수명이 실제와 다릅니다.
예시는
OnResume { ... }로 적혀 있으나 실제 함수명은OnResumeEffect입니다.📝 문서 수정 제안
- * OnResume { + * OnResumeEffect { * viewModel.sendIntent(NotificationIntent.RefreshSystemAlarm) * }🤖 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 `@design/src/main/java/com/linku/design/util/OnResumeEffect.kt` around lines 20 - 24, The KDoc example in OnResumeEffect is using the wrong function name, which makes the documentation inconsistent with the actual API. Update the usage example inside the OnResumeEffect documentation so it references OnResumeEffect instead of OnResume, keeping the sample intent call unchanged.data/src/main/java/com/linku/data/implementation/repository/AlarmRepositoryImpl.kt (1)
67-81: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueFCM 토큰 로깅에 토큰 값이 노출되지 않는지 확인.
현재 진입/성공/실패 로그에는 토큰 값이 포함되지 않아 양호합니다. 다만 디버그 로그가 릴리스 빌드에 남지 않도록
BuildConfig.DEBUG가드 사용을 고려하세요(onNewToken흐름에서는 이미 가드를 사용 중).🤖 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 `@data/src/main/java/com/linku/data/implementation/repository/AlarmRepositoryImpl.kt` around lines 67 - 81, The FCM token logs in registerFCMToken are already not exposing the token value, so keep them that way; update the logging in AlarmRepositoryImpl.registerFCMToken to be wrapped with BuildConfig.DEBUG so the debug/success/failure logs do not run in release builds. Mirror the existing onNewToken flow’s debug-only behavior and keep any future log statements free of the raw token value.
🤖 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 `@app/src/main/java/com/linku/LinkUFireBaseMessageService.kt`:
- Around line 69-74: `LinkUFireBaseMessageService`의 `PendingIntent.getActivity`
호출에 `packageManager.getLaunchIntentForPackage(packageName)`의 nullable 결과가 그대로
들어가 NPE 위험이 있습니다. `getLaunchIntentForPackage` 결과를 먼저 안전하게 처리하고, null이면
`pendingIntent` 생성을 건너뛰거나 대체 `Intent`를 사용하도록 `pendingIntent` 생성 로직을 수정하세요. 특히
`LinkUFireBaseMessageService`의 해당 알림/클릭 처리 경로에서 null-safe 가드가 있도록 정리해 주세요.
In `@app/src/main/java/com/linku/MainViewModel.kt`:
- Around line 147-170: `setNotificationEnabled` currently returns early when
`notificationPreference.getFcmToken()` is null, which can skip the first
registration after permission is granted. Update the
`MainViewModel.setNotificationEnabled` flow to fall back to fetching a fresh
token via `FirebaseMessaging.getInstance().token` when the cached token is
missing, then continue with `alarmRepository.registerFCMToken` and
`updateAlarmSetting(AlarmType.ALL)` using that token. Keep the existing
success/failure logging, but ensure the new token lookup path is handled before
exiting the coroutine.
In `@core/src/main/java/com/linku/core/di/SystemModule.kt`:
- Around line 35-37: `provideApplicationScope` is creating the shared
`CoroutineScope` on `Dispatchers.Main`, which can push
`LinkUFireBaseMessageService` work like
`alarmRepository.registerFCMToken(token)` onto the UI thread. Update
`SystemModule.provideApplicationScope` to use `Dispatchers.Default` or
`Dispatchers.IO` instead of `Dispatchers.Main`, keeping the `SupervisorJob`
behavior unchanged so the injected `externalScope` runs background work off the
main thread.
In
`@data/src/main/java/com/linku/data/implementation/repository/AlarmPagingSource.kt`:
- Around line 34-46: The paging logic in AlarmPagingSource currently uses
alarmList.nextCursor alone for nextKey, which can keep paging even on the last
page; update the load result logic to use AlarmList.hasNext as the gate for
whether another page should be requested. In the return block inside the paging
load flow, keep emitting LoadResult.Page from dto.toDomain(), but set nextKey
only when hasNext indicates more data and otherwise return null so paging stops
correctly. Use the AlarmPagingSource and AlarmList symbols to locate the change,
and rely on the hasNext value already mapped by AlarmMapper.
In `@data/src/main/java/com/linku/data/mapper/StringExt.kt`:
- Around line 27-34: `StringExt.kt`의 날짜 파싱 로직에서 `LocalDateTime.parse`가 UTC 오프셋이
포함된 문자열을 처리하지 못해 항상 실패하므로, `toHumanReadableDateTime`(또는 해당 파싱 확장 함수)에서 서버 타임스탬프를
`Instant.parse`로 직접 파싱하도록 바꾸세요. `runCatching` 안의 파싱 대상을 ISO-8601 UTC 문자열에 맞게
수정하고, 성공 시 기존처럼 `zoneId` 기준으로 변환해 출력하도록 유지하세요.
In `@feature/home/src/main/java/com/linku/home/HomeViewModel.kt`:
- Around line 279-281: `HomeViewModel.saveNewLink` is not passing the selected
`situationId` into the save flow, while `LinkuRepository.saveNewLink` and
`LinkuRepositoryImpl` currently do not support that field. Update the repository
interface, its implementation, and the `saveNewLink` call site so `situationId`
is accepted and forwarded end-to-end, or remove the UI selection until the model
supports it. Use the existing `saveNewLink`, `LinkuRepository`, and
`LinkuRepositoryImpl` symbols to keep the API and persistence layer aligned.
In
`@feature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmErrorContent.kt`:
- Line 37: The `AlarmErrorContent` message assignment is doing a hard `as
AppError` cast, which can crash when `errorState.error` is a raw `Throwable`
from `AlarmPagingSource` instead of an `AppError`. Update `AlarmErrorContent` to
use safe casting (`as?`) and handle the non-`AppError` case defensively, falling
back to a generic display message or other safe default so `ClassCastException`
cannot occur.
In `@feature/home/src/main/java/com/linku/home/viewmodel/AlarmViewModel.kt`:
- Around line 21-22: `AlarmViewModel`의 `_pushAlarmEnabled`가 생성 시점 값만 유지해서 실제 시스템
알림 상태와 달라질 수 있습니다. `pushAlarmEnabled`를 단순 초기화하지 말고, 권한 변경을 감지하거나 화면이 resumed 될 때
`notificationPreference.isMasterNotificationEnabled()`를 다시 조회해
`_pushAlarmEnabled`를 갱신하도록 `AlarmViewModel`에 업데이트 로직을 추가하세요.
In `@feature/mypage/src/main/java/com/linku/mypage/MyPageApp.kt`:
- Line 8: The MyPageApp.kt import is using the Navigation-specific hiltViewModel
entry point, so switch the import in MyPageApp to the pure Compose Hilt
ViewModel API and verify the feature/mypage module has the matching
androidx.hilt:hilt-lifecycle-viewmodel-compose dependency available. Update the
code at the hiltViewModel usage site to rely on the newer recommended package,
and keep the rest of the ViewModel wiring unchanged.
In `@feature/mypage/src/main/java/com/linku/mypage/NotificationViewModel.kt`:
- Around line 124-134: The onFailure handling in NotificationViewModel is
force-casting throwable to AppError, which can crash the coroutine if a
non-AppError arrives. Update the failure branch in the alarm toggle flow to use
a safe cast with a fallback message before sending NotificationEffect.ShowToast,
and apply the same defensive pattern in loadAlarmSetting as noted. Keep the
state rollback logic in _notificationState.update unchanged and ensure only a
user-friendly default message is shown when the throwable is not an AppError.
In `@gradle/libs.versions.toml`:
- Around line 153-160: Firebase BOM is being treated as a normal bundle
dependency, so version alignment will not work as intended. Remove
`firebase-bom` from the `firebase` bundle in `libs.versions.toml`, drop the
explicit version from `firebase-messaging-ktx`, and apply the BOM from the
consuming module with `implementation(platform(libs.firebase.bom))` while
keeping `firebase-messaging-ktx` as a versionless dependency.
---
Nitpick comments:
In
`@data/src/main/java/com/linku/data/implementation/repository/AlarmRepositoryImpl.kt`:
- Around line 67-81: The FCM token logs in registerFCMToken are already not
exposing the token value, so keep them that way; update the logging in
AlarmRepositoryImpl.registerFCMToken to be wrapped with BuildConfig.DEBUG so the
debug/success/failure logs do not run in release builds. Mirror the existing
onNewToken flow’s debug-only behavior and keep any future log statements free of
the raw token value.
In `@design/src/main/java/com/linku/design/util/OnResumeEffect.kt`:
- Around line 20-24: The KDoc example in OnResumeEffect is using the wrong
function name, which makes the documentation inconsistent with the actual API.
Update the usage example inside the OnResumeEffect documentation so it
references OnResumeEffect instead of OnResume, keeping the sample intent call
unchanged.
In
`@feature/mypage/src/main/java/com/linku/mypage/component/notification/SystemAlarmTab.kt`:
- Around line 50-59: The icon positioning in SystemAlarmTab’s Image uses a
hardcoded y offset, which can break alignment when text size, line count, or
font scale changes. Replace the .offset(y = (-12).dp) approach with layout-based
alignment in the surrounding Row or equivalent, using verticalAlignment =
Alignment.Top or a first-line/text-baseline aligned arrangement so the icon
stays anchored correctly without manual pixel tuning.
In `@feature/mypage/src/main/java/com/linku/mypage/screen/AlarmSettingScreen.kt`:
- Around line 67-74: `AlarmSettingScreen`의 `LaunchedEffect(Unit)` 안에서
`viewModel.sideEffect.collect`가 라이프사이클을 무시한 채 계속 수집되고 있습니다. `LaunchedEffect` 대신
`repeatOnLifecycle(STARTED)` 또는 `flowWithLifecycle`을 사용해 `sideEffect` 수집이 화면이
STARTED일 때만 동작하고 정지 시 중단되도록 `NotificationEffect.ShowToast` 처리 로직을 옮기세요.
🪄 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: 33c54216-db77-4989-98a9-a57c6aa75ad8
📒 Files selected for processing (56)
app/src/androidTest/java/com/linku/ExampleInstrumentedTest.ktapp/src/androidTest/java/com/linku/link/ExampleInstrumentedTest.ktapp/src/main/AndroidManifest.xmlapp/src/main/java/com/linku/LinkUFireBaseMessageService.ktapp/src/main/java/com/linku/MainApp.ktapp/src/main/java/com/linku/MainApplication.ktapp/src/main/java/com/linku/MainViewModel.ktapp/src/test/java/com/linku/link/ExampleUnitTest.ktcore/build.gradle.ktscore/src/main/java/com/linku/core/di/SystemModule.ktcore/src/main/java/com/linku/core/error/ApiError.ktcore/src/main/java/com/linku/core/model/alarm/Alarm.ktcore/src/main/java/com/linku/core/model/alarm/AlarmList.ktcore/src/main/java/com/linku/core/model/alarm/AlarmSetting.ktcore/src/main/java/com/linku/core/model/auth/Interest.ktcore/src/main/java/com/linku/core/model/auth/Purpose.ktcore/src/main/java/com/linku/core/repository/AlarmRepository.ktcore/src/main/java/com/linku/core/repository/NotificationPreference.ktcore/src/main/java/com/linku/core/system/NotificationController.ktcore/src/test/java/com/linku/core/ExampleUnitTest.ktdata/src/main/java/com/linku/data/api/alarm/AlarmApi.ktdata/src/main/java/com/linku/data/api/dto/server/alarm/AlarmSettingDTO.ktdata/src/main/java/com/linku/data/api/dto/server/alarm/AlarmSettingRequest.ktdata/src/main/java/com/linku/data/api/dto/server/alarm/AlarmsDTO.ktdata/src/main/java/com/linku/data/api/dto/server/alarm/FcmTokenRequest.ktdata/src/main/java/com/linku/data/api/mapToApiError.ktdata/src/main/java/com/linku/data/di/api/AlarmApiModule.ktdata/src/main/java/com/linku/data/di/preference/NotificationPreferenceModule.ktdata/src/main/java/com/linku/data/di/repository/AlarmRepositoryModule.ktdata/src/main/java/com/linku/data/implementation/preference/NotificationPreferenceImpl.ktdata/src/main/java/com/linku/data/implementation/repository/AlarmPagingSource.ktdata/src/main/java/com/linku/data/implementation/repository/AlarmRepositoryImpl.ktdata/src/main/java/com/linku/data/implementation/repository/FakeAlarmRepositoryImpl.ktdata/src/main/java/com/linku/data/mapper/AlarmMapper.ktdata/src/main/java/com/linku/data/mapper/StringExt.ktdata/src/main/java/com/linku/data/preference/NotificationPreference.ktdata/src/test/java/com/linku/data/ExampleUnitTest.ktdata/src/test/java/com/linku/data/mapper/StringExtTest.ktdesign/src/main/java/com/linku/design/util/OnResumeEffect.ktfeature/home/src/main/java/com/linku/home/HomeViewModel.ktfeature/home/src/main/java/com/linku/home/screen/AlarmScreen.ktfeature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmAppendStateFooter.ktfeature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmErrorContent.ktfeature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmItem.ktfeature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmLoadingContent.ktfeature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmNothingTab.ktfeature/home/src/main/java/com/linku/home/viewmodel/AlarmViewModel.ktfeature/mypage/src/main/java/com/linku/mypage/MyPageApp.ktfeature/mypage/src/main/java/com/linku/mypage/NotificationViewModel.ktfeature/mypage/src/main/java/com/linku/mypage/component/notification/SystemAlarmTab.ktfeature/mypage/src/main/java/com/linku/mypage/intent/NotificationIntent.ktfeature/mypage/src/main/java/com/linku/mypage/screen/AlarmSettingScreen.ktfeature/mypage/src/main/res/drawable/ic_info_blue.xmlfeature/mypage/src/main/res/drawable/ic_info_red.xmlfeature/mypage/src/main/res/drawable/ic_long_right.xmlgradle/libs.versions.toml
💤 Files with no reviewable changes (8)
- app/src/test/java/com/linku/link/ExampleUnitTest.kt
- core/src/main/java/com/linku/core/repository/NotificationPreference.kt
- app/src/androidTest/java/com/linku/ExampleInstrumentedTest.kt
- core/src/main/java/com/linku/core/model/alarm/AlarmList.kt
- data/src/main/java/com/linku/data/implementation/preference/NotificationPreferenceImpl.kt
- data/src/main/java/com/linku/data/implementation/repository/FakeAlarmRepositoryImpl.kt
- core/src/main/java/com/linku/core/system/NotificationController.kt
- data/src/test/java/com/linku/data/ExampleUnitTest.kt
✅ Files skipped from review due to trivial changes (9)
- data/src/main/java/com/linku/data/api/dto/server/alarm/AlarmSettingRequest.kt
- data/src/main/java/com/linku/data/api/dto/server/alarm/FcmTokenRequest.kt
- feature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmLoadingContent.kt
- feature/mypage/src/main/res/drawable/ic_long_right.xml
- feature/mypage/src/main/res/drawable/ic_info_blue.xml
- core/src/test/java/com/linku/core/ExampleUnitTest.kt
- data/src/main/java/com/linku/data/api/dto/server/alarm/AlarmSettingDTO.kt
- feature/home/src/main/java/com/linku/home/ui/alarm/component/AlarmItem.kt
- core/src/main/java/com/linku/core/error/ApiError.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/linku/MainApp.kt
| val pendingIntent = PendingIntent.getActivity( | ||
| this, | ||
| 0, | ||
| packageManager.getLaunchIntentForPackage(packageName), | ||
| PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Android PendingIntent.getActivity intent parameter nullable NonNull
💡 Result:
In the Android API, the intent parameter of the PendingIntent.getActivity method is marked as @NonNull [1][2]. While some older documentation or specific framework versions might occasionally display ambiguous nullability annotations in generated documentation [3][4], the official source code for PendingIntent.getActivity explicitly defines the intent parameter as @NonNull [1][2]. Attempting to pass a null Intent to this method will typically result in a NullPointerException, as the system requires a valid Intent to define the activity that the PendingIntent should launch [5].
Citations:
- 1: https://android.googlesource.com/platform/frameworks/base/+/HEAD/core/java/android/app/PendingIntent.java
- 2: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/app/PendingIntent.java
- 3: https://developer.android.google.cn/reference/kotlin/android/app/PendingIntent
- 4: https://developer.android.com/reference/kotlin/android/app/PendingIntent
- 5: https://developer.android.com/guide/components/intents-filters
getLaunchIntentForPackage의 nullable 반환값이 PendingIntent.getActivity에 전달되어 NPE 위험이 있습니다.
packageManager.getLaunchIntentForPackage(packageName)은 런처 활동을 찾지 못하면 null을 반환할 수 있으나, PendingIntent.getActivity의 intent 파라미터는 @NonNull으로 선언되어 있어 null 전달 시 예외가 발생합니다.
제안 수정
- val pendingIntent = PendingIntent.getActivity(
- this,
- 0,
- packageManager.getLaunchIntentForPackage(packageName),
- PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT
- )
+ val launchIntent = packageManager.getLaunchIntentForPackage(packageName)
+ val pendingIntent = launchIntent?.let {
+ PendingIntent.getActivity(
+ this,
+ 0,
+ it,
+ PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val pendingIntent = PendingIntent.getActivity( | |
| this, | |
| 0, | |
| packageManager.getLaunchIntentForPackage(packageName), | |
| PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | |
| ) | |
| val launchIntent = packageManager.getLaunchIntentForPackage(packageName) | |
| val pendingIntent = launchIntent?.let { | |
| PendingIntent.getActivity( | |
| this, | |
| 0, | |
| it, | |
| PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | |
| ) | |
| } |
🤖 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 `@app/src/main/java/com/linku/LinkUFireBaseMessageService.kt` around lines 69 -
74, `LinkUFireBaseMessageService`의 `PendingIntent.getActivity` 호출에
`packageManager.getLaunchIntentForPackage(packageName)`의 nullable 결과가 그대로 들어가
NPE 위험이 있습니다. `getLaunchIntentForPackage` 결과를 먼저 안전하게 처리하고, null이면
`pendingIntent` 생성을 건너뛰거나 대체 `Intent`를 사용하도록 `pendingIntent` 생성 로직을 수정하세요. 특히
`LinkUFireBaseMessageService`의 해당 알림/클릭 처리 경로에서 null-safe 가드가 있도록 정리해 주세요.
| fun setNotificationEnabled(isGranted: Boolean) { | ||
| if (!isGranted) return | ||
|
|
||
| // 알림 허용 여부 저장 | ||
| // 로그인 성공 후 시스템 권한 요청 결과를 로컬에 반영 | ||
| fun setNotificationEnabled(enabled: Boolean) { | ||
| notificationController.setNotificationEnabled(enabled) | ||
| } | ||
| viewModelScope.launch { | ||
| val token = notificationPreference.getFcmToken() | ||
|
|
||
| if (token == null) { | ||
| Log.d("FCM", "token 없음 → skip") | ||
| return@launch | ||
| } | ||
|
|
||
| // 토큰 등록이 성공했으면 전체 푸시알림 활성화 | ||
| val registerResult = alarmRepository.registerFCMToken(token) | ||
|
|
||
| if (registerResult.isSuccess) { | ||
| alarmRepository.updateAlarmSetting(AlarmType.ALL) | ||
| .onFailure { e -> | ||
| Log.e("FCM", "알람 설정 실패: ${e.message}", e) | ||
| } | ||
| } else { | ||
| Log.e("FCM", "register 실패: ${registerResult.exceptionOrNull()?.message}") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
토큰 미존재 시 단순 skip이라 최초 권한 허용 시점에 등록이 누락될 수 있습니다.
notificationPreference.getFcmToken()이 null이면 등록 없이 종료합니다. 사용자가 권한을 처음 허용하는 시점에 아직 FCM 토큰이 로컬에 저장되지 않았다면(예: onNewToken이 아직 호출되지 않은 신규 설치), 전체 알림 활성화가 한 번 누락될 수 있습니다. 이 경우 FirebaseMessaging.getInstance().token을 조회해 폴백 등록을 수행하는 것을 검토해 주세요.
🤖 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 `@app/src/main/java/com/linku/MainViewModel.kt` around lines 147 - 170,
`setNotificationEnabled` currently returns early when
`notificationPreference.getFcmToken()` is null, which can skip the first
registration after permission is granted. Update the
`MainViewModel.setNotificationEnabled` flow to fall back to fetching a fresh
token via `FirebaseMessaging.getInstance().token` when the cached token is
missing, then continue with `alarmRepository.registerFCMToken` and
`updateAlarmSetting(AlarmType.ALL)` using that token. Keep the existing
success/failure logging, but ensure the new token lookup path is handled before
exiting the coroutine.
| fun provideApplicationScope(): CoroutineScope { | ||
| return CoroutineScope(SupervisorJob() + Dispatchers.Main) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Android recommended CoroutineScope for application-wide external scope SupervisorJob dispatcher
💡 Result:
For application-wide work in Android that must outlive specific screen or ViewModel lifecycles, the recommended approach is to create and inject a custom CoroutineScope rather than using GlobalScope [1][2][3]. The standard implementation for an application-scoped CoroutineScope consists of the following components [2][3]: 1. SupervisorJob: Using a SupervisorJob is critical because it ensures that a failure in one child coroutine does not cancel the entire scope or other unrelated child coroutines [4][3]. 2. CoroutineDispatcher: You should inject an appropriate dispatcher (e.g., Dispatchers.Default or Dispatchers.Main, depending on the requirements) rather than hardcoding it [1][2]. 3. Dependency Injection: The recommended best practice is to define this scope as a Singleton (e.g., using Hilt/Dagger) and inject it into the classes that require it [2][3]. This makes the code testable and avoids the issues associated with global state [2][5]. Example implementation using Hilt [2]: @Module @InstallIn(SingletonComponent::class) object CoroutinesScopesModule { @Singleton @Provides fun providesApplicationScope( @DefaultDispatcher defaultDispatcher: CoroutineDispatcher): CoroutineScope = CoroutineScope(SupervisorJob + defaultDispatcher) } Why avoid GlobalScope? GlobalScope is considered a "delicate API" that bypasses structured concurrency [6]. Coroutines launched in GlobalScope are not bound to any job, making them difficult to manage, test, and cancel [5][6]. They can easily lead to resource leaks because they do not automatically shut down when the application or specific features are no longer active [6]. Important Note: If the task you are performing is intended to be managed by the system (e.g., background synchronization, data uploads), consider using WorkManager instead of a custom application-wide CoroutineScope, as it is designed to handle such work reliably even if the application process is killed [3].
Citations:
- 1: https://developer.android.com/kotlin/coroutines/coroutines-best-practices
- 2: https://manuelvivo.dev/coroutinescope-hilt
- 3: https://mukuljangra.com/guide/kotlin-coroutines-best-practises/
- 4: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-supervisor-job.html
- 5: https://stackoverflow.com/questions/61255807/coroutine-scope-on-application-class-android
- 6: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-global-scope/
🏁 Script executed:
find -name "LinkUFireBaseMessageService.kt" -o -name "LinkUFireBaseMessageService.java"Repository: LinkYou-2025/LinkU_Android
Length of output: 225
🏁 Script executed:
cat -n ./app/src/main/java/com/linku/LinkUFireBaseMessageService.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 3800
provideApplicationScope 에서 Dispatchers.Main 대신 Dispatchers.Default(또는 IO) 를 사용해야 합니다.
LinkUFireBaseMessageService에 주입된 externalScope를 통해 alarmRepository.registerFCMToken(token) (네트워크 요청) 이 호출되고 있습니다. 현재 이 스코프가 Dispatchers.Main 으로 설정되어 있어, 백그라운드 IO 작업이 메인 스레드에서 실행되어 UI 프리징을 유발할 위험이 있습니다.
♻️ 제안 변경
- fun provideApplicationScope(): CoroutineScope {
- return CoroutineScope(SupervisorJob() + Dispatchers.Main)
- }
+ fun provideApplicationScope(): CoroutineScope {
+ return CoroutineScope(SupervisorJob() + Dispatchers.Default)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun provideApplicationScope(): CoroutineScope { | |
| return CoroutineScope(SupervisorJob() + Dispatchers.Main) | |
| } | |
| fun provideApplicationScope(): CoroutineScope { | |
| return CoroutineScope(SupervisorJob() + Dispatchers.Default) | |
| } |
🤖 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 `@core/src/main/java/com/linku/core/di/SystemModule.kt` around lines 35 - 37,
`provideApplicationScope` is creating the shared `CoroutineScope` on
`Dispatchers.Main`, which can push `LinkUFireBaseMessageService` work like
`alarmRepository.registerFCMToken(token)` onto the UI thread. Update
`SystemModule.provideApplicationScope` to use `Dispatchers.Default` or
`Dispatchers.IO` instead of `Dispatchers.Main`, keeping the `SupervisorJob`
behavior unchanged so the injected `externalScope` runs background work off the
main thread.
| // 파싱 | ||
| val parsed = runCatching { | ||
| LocalDateTime.parse(this) | ||
| .atZone(zoneId) | ||
| .toInstant() | ||
| }.getOrElse { | ||
| return "알 수 없음" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 테스트가 어떤 입력 포맷을 검증하는지 확인
fd 'StringExtTest.kt' data/src/test --exec cat {}Repository: LinkYou-2025/LinkU_Android
Length of output: 1336
LocalDateTime.parse는 'Z' 오프셋이 포함된 UTC 타임스탬프를 파싱할 수 않아 항상 "알 수 없음"이 반환됩니다.
테스트가 "2026-01-01T00:09:30Z" 형태의 ISO-8601 UTC 문자열을 사용하는데, LocalDateTime.parse는 오프셋 정보를 포함하지 않는 ISO_LOCAL_DATE_TIME 패턴만 지원합니다. 이로 인해 파싱 중 DateTimeParseException이 발생하여 runCatching이 실패하고, 유효한 모든 서버 타임스탬프가 "알 수 없음"으로 표시됩니다.
서버에서 보내는 UTC 타임스탬프는 Instant.parse로 직접 처리해야 합니다.
🐛 제안 수정
-fun String.toRelativeTime(
- now: Instant = Instant.now(),
- zoneId: ZoneId = ZoneId.systemDefault()
-): String {
- val parsed = runCatching {
- LocalDateTime.parse(this)
- .atZone(zoneId)
- .toInstant()
- }.getOrElse {
- return "알 수 없음"
- }
+fun String.toRelativeTime(
+ now: Instant = Instant.now()
+): String {
+ val parsed = runCatching {
+ Instant.parse(this)
+ }.getOrElse {
+ return "알 수 없음"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 파싱 | |
| val parsed = runCatching { | |
| LocalDateTime.parse(this) | |
| .atZone(zoneId) | |
| .toInstant() | |
| }.getOrElse { | |
| return "알 수 없음" | |
| } | |
| fun String.toRelativeTime( | |
| now: Instant = Instant.now() | |
| ): String { | |
| // 파싱 | |
| val parsed = runCatching { | |
| Instant.parse(this) | |
| }.getOrElse { | |
| return "알 수 없음" | |
| } |
🤖 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 `@data/src/main/java/com/linku/data/mapper/StringExt.kt` around lines 27 - 34,
`StringExt.kt`의 날짜 파싱 로직에서 `LocalDateTime.parse`가 UTC 오프셋이 포함된 문자열을 처리하지 못해 항상
실패하므로, `toHumanReadableDateTime`(또는 해당 파싱 확장 함수)에서 서버 타임스탬프를 `Instant.parse`로 직접
파싱하도록 바꾸세요. `runCatching` 안의 파싱 대상을 ISO-8601 UTC 문자열에 맞게 수정하고, 성공 시 기존처럼
`zoneId` 기준으로 변환해 출력하도록 유지하세요.
| alarmPagingItems: LazyPagingItems<AlarmSummary>, | ||
| errorState: LoadState.Error | ||
| ) { | ||
| val message = (errorState.error as AppError).displayMessage |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# AlarmPagingSource의 에러 처리 경로 확인
fd -t f 'AlarmPagingSource.kt' --exec cat -n {}
# mapToApiError / AppError 변환 지점 확인
rg -nP -C3 'LoadResult\.Error|catch\s*\(' data/src/main/java/com/linku/data/implementation/repository/AlarmPagingSource.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 2222
as AppError 강제 캐스팅으로 인한 크래시 위험
AlarmPagingSource 가 네트워크 호출 중 CancellationException 등의 예외를 AppError 로 변환하지 않고 원본 Throwable (LoadResult.Error) 로 전달할 수 있습니다. 이때 AlarmErrorContent 가 as AppError 강제 캐스팅을 수행하면 타입 불일치로 인해 ClassCastException 발생하여 앱이 크래시됩니다. as? 안전 캐스팅을 사용하여 예외 처리를 방어적으로 변경해야 합니다.
🛡️ 안전 캐스팅 제안
- val message = (errorState.error as AppError).displayMessage
+ val message = (errorState.error as? AppError)?.displayMessage
+ ?: NetworkError.Unknown().displayMessage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val message = (errorState.error as AppError).displayMessage | |
| val message = (errorState.error as? AppError)?.displayMessage | |
| ?: NetworkError.Unknown().displayMessage |
🤖 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/home/src/main/java/com/linku/home/ui/alarm/component/AlarmErrorContent.kt`
at line 37, The `AlarmErrorContent` message assignment is doing a hard `as
AppError` cast, which can crash when `errorState.error` is a raw `Throwable`
from `AlarmPagingSource` instead of an `AppError`. Update `AlarmErrorContent` to
use safe casting (`as?`) and handle the non-`AppError` case defensively, falling
back to a generic display message or other safe default so `ClassCastException`
cannot occur.
| private val _pushAlarmEnabled = MutableStateFlow(notificationPreference.isMasterNotificationEnabled()) | ||
| val pushAlarmEnabled = _pushAlarmEnabled.asStateFlow() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '_pushAlarmEnabled|isMasterNotificationEnabled|pushAlarmEnabled' feature/home/src/main/java/com/linku/homeRepository: LinkYou-2025/LinkU_Android
Length of output: 2420
pushAlarmEnabled 값이 갱신되지 않아 실제 시스템 알림 설정과 불일치할 수 있습니다.
AlarmViewModel의 _pushAlarmEnabled는 생성 시점의 값으로만 초기화되며, 이후 알림 권한 설정 변경이나 화면 재진입 시 이를 갱신하는 로직이 없습니다. 시스템 권한이 변경된 후 앱으로 복귀하면 UI 가 과거의 잘못된 상태를 계속 노출하게 됩니다.
private val _pushAlarmEnabled = MutableStateFlow(notificationPreference.isMasterNotificationEnabled())
val pushAlarmEnabled = _pushAlarmEnabled.asStateFlow()권한 변경 이벤트를 감지하거나, 화면이 다시 resumed 될 때 최신 상태를 재조회하여 _pushAlarmEnabled 를 업데이트하는 로직을 추가해 주세요.
🤖 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/home/src/main/java/com/linku/home/viewmodel/AlarmViewModel.kt` around
lines 21 - 22, `AlarmViewModel`의 `_pushAlarmEnabled`가 생성 시점 값만 유지해서 실제 시스템 알림
상태와 달라질 수 있습니다. `pushAlarmEnabled`를 단순 초기화하지 말고, 권한 변경을 감지하거나 화면이 resumed 될 때
`notificationPreference.isMasterNotificationEnabled()`를 다시 조회해
`_pushAlarmEnabled`를 갱신하도록 `AlarmViewModel`에 업데이트 로직을 추가하세요.
| onFailure = { throwable -> | ||
| _notificationState.update { currentState -> | ||
| currentState.copy( | ||
| alarmToggleUiState = previous.alarmToggleUiState | ||
| ) | ||
| } | ||
|
|
||
| val message = (throwable as AppError).displayMessage | ||
| _sideEffect.send(NotificationEffect.ShowToast(message)) | ||
| } | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
throwable as AppError 강제 캐스팅은 ClassCastException으로 코루틴을 크래시시킬 수 있습니다.
onFailure로 전달되는 예외가 항상 AppError라는 보장이 없습니다(예: 매핑 단계 외의 예기치 못한 예외). 캐스팅이 실패하면 토스트 발행 대신 크래시가 발생합니다. as? + 폴백 메시지로 방어하세요. 동일한 패턴이 L156의 loadAlarmSetting에도 존재합니다.
🛡️ 제안
- val message = (throwable as AppError).displayMessage
- _sideEffect.send(NotificationEffect.ShowToast(message))
+ val message = (throwable as? AppError)?.displayMessage
+ ?: throwable.message.orEmpty()
+ _sideEffect.send(NotificationEffect.ShowToast(message))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onFailure = { throwable -> | |
| _notificationState.update { currentState -> | |
| currentState.copy( | |
| alarmToggleUiState = previous.alarmToggleUiState | |
| ) | |
| } | |
| val message = (throwable as AppError).displayMessage | |
| _sideEffect.send(NotificationEffect.ShowToast(message)) | |
| } | |
| ) | |
| onFailure = { throwable -> | |
| _notificationState.update { currentState -> | |
| currentState.copy( | |
| alarmToggleUiState = previous.alarmToggleUiState | |
| ) | |
| } | |
| val message = (throwable as? AppError)?.displayMessage | |
| ?: throwable.message.orEmpty() | |
| _sideEffect.send(NotificationEffect.ShowToast(message)) | |
| } |
🤖 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/mypage/src/main/java/com/linku/mypage/NotificationViewModel.kt`
around lines 124 - 134, The onFailure handling in NotificationViewModel is
force-casting throwable to AppError, which can crash the coroutine if a
non-AppError arrives. Update the failure branch in the alarm toggle flow to use
a safe cast with a fallback message before sending NotificationEffect.ShowToast,
and apply the same defensive pattern in loadAlarmSetting as noted. Keep the
state rollback logic in _notificationState.update unchanged and ensure only a
user-friendly default message is shown when the throwable is not an AppError.
| firebase-bom = { module = "com.google.firebase:firebase-bom", version.ref = "firebaseBom" } | ||
| firebase-messaging-ktx = { group = "com.google.firebase", name = "firebase-messaging-ktx", version.ref = "firebaseMessagingKtx" } | ||
|
|
||
| [bundles] | ||
| firebase = [ | ||
| "firebase-bom", | ||
| "firebase-messaging-ktx" | ||
| ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Firebase BOM 구성이 의도대로 동작하지 않습니다.
firebase-bom을 [bundles]에 넣어 implementation(libs.bundles.firebase)로 소비하면 BOM이 일반 의존성으로 추가될 뿐 platform(...)으로 적용되지 않아 버전 정렬 기능이 동작하지 않습니다. 또한 BOM을 쓰는 경우 firebase-messaging-ktx에는 버전을 명시하지 않는 것이 일반적인데, 여기서는 version.ref = "firebaseMessagingKtx"로 버전을 직접 지정하고 있어 BOM 사용 의도와 충돌합니다.
권장: BOM은 모듈의 build.gradle.kts에서 implementation(platform(libs.firebase.bom))로 적용하고, firebase-messaging-ktx에서는 버전 참조를 제거하세요.
♻️ 제안
firebase-bom = { module = "com.google.firebase:firebase-bom", version.ref = "firebaseBom" }
-firebase-messaging-ktx = { group = "com.google.firebase", name = "firebase-messaging-ktx", version.ref = "firebaseMessagingKtx" }
+firebase-messaging-ktx = { group = "com.google.firebase", name = "firebase-messaging-ktx" }
[bundles]
-firebase = [
- "firebase-bom",
- "firebase-messaging-ktx"
-]
+firebase = [
+ "firebase-messaging-ktx"
+]#!/bin/bash
# [versions]에 firebaseBom / firebaseMessagingKtx 존재 여부와 platform() 적용 여부 확인
fd 'libs.versions.toml' --exec sed -n '1,60p' {}
rg -nP 'platform\(\s*libs\.firebase' -g '*.gradle.kts'
rg -nP '^\s*\[bundles\]' gradle/libs.versions.toml🤖 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 `@gradle/libs.versions.toml` around lines 153 - 160, Firebase BOM is being
treated as a normal bundle dependency, so version alignment will not work as
intended. Remove `firebase-bom` from the `firebase` bundle in
`libs.versions.toml`, drop the explicit version from `firebase-messaging-ktx`,
and apply the BOM from the consuming module with
`implementation(platform(libs.firebase.bom))` while keeping
`firebase-messaging-ktx` as a versionless dependency.
| val EmotionType.imgRes: Int | ||
| @DrawableRes get() = when (this) { | ||
| EmotionType.JOY -> R.drawable.ic_joy | ||
| EmotionType.CALM -> R.drawable.ic_calm | ||
| EmotionType.EXCITE -> R.drawable.ic_excite | ||
| EmotionType.SAD -> R.drawable.ic_sad | ||
| EmotionType.IRRITATION -> R.drawable.ic_irritation | ||
| EmotionType.ANGER -> R.drawable.ic_anger | ||
| } No newline at end of file |
There was a problem hiding this comment.
지금 이 방식을 조금 수정해 보는 게 어떨까? 지금 구현 방식대로라면
Image(
painter = painterResource(id = emotion.imgRes), // 우으..여기서 페인터리소스 함수 호출해서 id값 넣는 거 시러..
contentDescription = emotion.tagName,
modifier = Modifier.size(20.dp)
)이런 식으로 매핑할 때 painterResource(id = emotion.imgRes) 이렇게 다 써줘야 하거든.(id값으로 매핑하는 거니까...) 이거보다 난
val EmotionType.imgRes: Painter // 애초부터 Painter로 매핑
@Composable get() = painterResource(
when (this) {
EmotionType.JOY -> R.drawable.ic_joy
EmotionType.CALM -> R.drawable.ic_calm
EmotionType.EXCITE -> R.drawable.ic_excite
EmotionType.SAD -> R.drawable.ic_sad
EmotionType.IRRITATION -> R.drawable.ic_irritation
EmotionType.ANGER -> R.drawable.ic_anger
}
)이런 식으로 하는 게 더 좋다고 봐. 그러면
Image(
painter = emotion.imgRes, // 와! 깔끔!
contentDescription = emotion.tagName,
modifier = Modifier.size(20.dp)
)과 같이 쓸 수 있거든(지민이한테 배운 거임 나도)
| onEmotionSelect: (Long?) -> Unit, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| val emotions = EmotionType.entries.toList() |
There was a problem hiding this comment.
toList()는 없애도 될 것 같아. EnumEntries의 내부 구현은 다음과 같거든.
sealed interface EnumEntries<E : Enum<E>> : List<E>이미 EnumEntries는 List의 구현체야. 그래서 toList() 없어도 컴파일에 문제 없어.
| .then( | ||
| if (selected) { | ||
| Modifier.border( | ||
| width = 1.dp, | ||
| brush = Basic.maincolor, | ||
| shape = RoundedCornerShape(20.dp) | ||
| ) | ||
| } else { | ||
| Modifier.border( | ||
| width = 1.dp, | ||
| color = colors.gray[200], | ||
| shape = RoundedCornerShape(20.dp) | ||
| ) | ||
| } | ||
| ) |
There was a problem hiding this comment.
여기서 분기에 따라 바뀌는 건 두번째 인자밖에 없어. 그렇다면 다음과 같이 바꾸는 건 어떨까?
.border(
width = 1.dp,
brush = if (selected) Basic.maincolor else SolidColor(colors.gray[200]), // 와! 코드가 확 줄어드러욧!
shape = shape
)이게 더 나을 것 같앙
| Text( | ||
| text = category.name, | ||
| fontSize = 15.sp, | ||
| fontWeight = if (category.name == selectedCategory) { | ||
| FontWeight.Medium | ||
| } else { | ||
| FontWeight.Normal | ||
| }, | ||
| color = if (category.name == selectedCategory) { | ||
| colors.blue[200] | ||
| } else { | ||
| colors.gray[800] | ||
| } |
There was a problem hiding this comment.
category.name == selectedCategory 가 중복되는데, 다음과 같이 분리해보면 어떨까?
val isSelected = category.id == selectedCategoryId
Text(
fontWeight = if (isSelected) FontWeight.Medium else FontWeight.Normal,
color = if (isSelected) colors.blue[200] else colors.gray[800]
)| LinkDetailDropdownItem( | ||
| iconRes = R.drawable.ic_link_edit, | ||
| text = "링크 수정하기", | ||
| onClick = { onEditClick() } | ||
| ) | ||
|
|
||
| LinkDetailDropdownItem( | ||
| iconRes = R.drawable.ic_link_delete, | ||
| text = "링크 삭제하기", | ||
| onClick = { onDeleteClick() } | ||
| ) | ||
|
|
||
| LinkDetailDropdownItem( | ||
| iconRes = R.drawable.ic_link_share, | ||
| text = "링크 공유하기", | ||
| onClick = { onShareClick() } | ||
| ) | ||
|
|
||
| LinkDetailDropdownItem( | ||
| iconRes = R.drawable.ic_link_go_gray, | ||
| text = "링크 보러가기", | ||
| onClick = { onGoClick() } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Composable | ||
| private fun LinkDetailDropdownItem( | ||
| @DrawableRes iconRes: Int, | ||
| text: String, | ||
| onClick: () -> Unit | ||
| ) { | ||
| val colors = MaterialTheme.linkuColors | ||
|
|
||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .noRippleClickable { onClick() } | ||
| ) { | ||
| Row( | ||
| modifier = Modifier.padding(vertical = 7.dp), | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(14.dp) | ||
| ) { | ||
| Image( | ||
| painter = painterResource(iconRes), | ||
| contentDescription = null, | ||
| modifier = Modifier.size(18.dp) | ||
| ) | ||
|
|
||
| Text( | ||
| text = text, | ||
| fontSize = 16.sp, | ||
| fontWeight = FontWeight.Normal, | ||
| color = colors.black | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
요거 뭔가 XML View 스멜이 나서 (R.drawable.ic_link_go_gray 하드코딩 부분이) 더 컴포즈스럽게 바꿔 볼 수 있지 않을까? 우선 요런 식으로 간단하게 ui에만 쓸 정적 데이터 모델을 만들고,
enum class LinkDetailAction(
@DrawableRes val iconRes: Int,
val label: String
) {
EDIT(R.drawable.ic_link_edit, "링크 수정하기"),
DELETE(R.drawable.ic_link_delete, "링크 삭제하기"),
SHARE(R.drawable.ic_link_share, "링크 공유하기"),
GO(R.drawable.ic_link_go_gray, "링크 보러가기")
}그 다음 다음과 같은 식으로 LinkDetailCustomDropdown의 구현을 간단하게 바꿔버리는 거지.
@Composable
fun LinkDetailCustomDropdown(
onAction: (LinkDetailAction) -> Unit, // 콜백 4개 → 1개로 통합
modifier: Modifier = Modifier
) {
Column(...) {
LinkDetailAction.entries.forEach { action -> // 항목 추가/삭제 시 enum만 수정하면 됨
LinkDetailDropdownItem(
action = action,
onClick = { onAction(action) } // 하드코딩 제거 → enum이 캡슐화
)
}
}
}정적인 메뉴 바 같은 거 만들 때 내가 자주 쓰는 스타일이긴 한데, 여기에도 적용해 볼 수도 있지 않을까 해서 남겨봐~~
4 participants
📝 설명
✔️ PR 유형
어떤 변경 사항이 있나요?
📎 관련 이슈 번호
Summary by CodeRabbit
릴리스 노트