[Fix] 푸시 알림 설정 변경 사항 반영 안되는 거 수정(공통), 안드로이드 스타일 복구(폰트, 그림자) #12#13
Merged
[Fix] 푸시 알림 설정 변경 사항 반영 안되는 거 수정(공통), 안드로이드 스타일 복구(폰트, 그림자) #12#13
Conversation
Walkthrough푸시 알림 토글이 변경될 때 ViewModel이 서버로 즉시 반영하도록 로직을 재구성했고, Modifier.figmaShadow를 expect/actual로 멀티플랫폼화하여 Android와 iOS에 각각 그림자 구현을 추가했습니다. 테마는 CompositionLocal로 래핑되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Settings Screen)
participant VM as SettingsEditMyDataViewModel
participant Repo as UserRepository
participant API as Server API
UI->>VM: toggle notification (type, checked)
VM->>VM: compute updated PushNotification & UI state
VM->>Repo: updateUserData(updated UserInfo)
Repo->>API: PATCH /user (new PushNotification)
API-->>Repo: 200 OK
Repo-->>VM: persist & return updated UserInfo
VM-->>UI: update state (master/complete/abnormal/missed, isUpdateSuccess)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
![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
🧹 Nitpick comments (4)
composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/notification/screen/SettingsNotificationScreen.kt (1)
48-50: LaunchedEffect와 myDataInfo 동기화 사이에 불필요한 리컴포지션 가능성
updateNotificationSettingByType호출 시updateUserData가 성공하면myDataInfo가 업데이트됩니다. 이때 이LaunchedEffect가 다시 트리거되어initializeNotificationSettings를 호출하게 되는데, 결과적으로 동일한 값으로 상태를 다시 설정하게 되어 불필요한 리컴포지션이 발생할 수 있습니다.현재 로직이 기능적으로 깨지진 않지만, 초기 로딩 시에만 초기화하도록 분리하면 더 깔끔해질 수 있습니다.
♻️ 선택적 개선안: 초기 로딩 시에만 초기화
- LaunchedEffect(uiState.myDataInfo) { - uiState.myDataInfo?.let { viewModel.initializeNotificationSettings(it) } - } + // 초기 로딩 완료 후 한 번만 초기화 + LaunchedEffect(uiState.myDataInfo?.memberId) { + uiState.myDataInfo?.let { viewModel.initializeNotificationSettings(it) } + }또는 ViewModel 내부에서 초기화 여부를 플래그로 관리하는 방법도 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/notification/screen/SettingsNotificationScreen.kt` around lines 48 - 50, The LaunchedEffect tied to uiState.myDataInfo causes redundant re-initialization when updateNotificationSettingByType -> updateUserData updates myDataInfo; change the initialization to run only once on first load (e.g., switch LaunchedEffect(uiState.myDataInfo) to LaunchedEffect(Unit) or guard with a one-time flag) and ensure you still call viewModel.initializeNotificationSettings(myDataInfo) only when myDataInfo is non-null and not yet initialized; alternatively implement an "initialized" boolean in the ViewModel (checked in initializeNotificationSettings) to avoid repeated work when updateUserData updates myDataInfo.composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt (2)
94-96: Magic string 대신 enum 또는 sealed class 사용 권장
type: String파라미터에 "master", "complete", "abnormal", "missed" 같은 magic string을 사용하고 있습니다. 오타나 잘못된 값 전달 시 컴파일 타임에 잡지 못하고else -> return으로 조용히 무시됩니다.♻️ NotificationType sealed class/enum 사용 예시
enum class NotificationType { MASTER, COMPLETE, ABNORMAL, MISSED } fun updateNotificationSettingByType( type: NotificationType, checked: Boolean, ) { // when (type) 에서 else 분기 불필요 }Screen에서 호출:
viewModel.updateNotificationSettingByType( type = NotificationType.MASTER, checked = isChecked, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt` around lines 94 - 96, updateNotificationSettingByType에서 사용하는 magic string 대신 enum 또는 sealed class를 도입해 타입 안정성을 확보하세요: 정의한 NotificationType (예: MASTER, COMPLETE, ABNORMAL, MISSED)을 만들고 SettingsEditMyDataViewModel.updateNotificationSettingByType의 시그니처를 type: NotificationType으로 변경한 다음 when 타입별 분기를 명시적으로 처리해 else 분기를 제거하세요; 또한 이 변경에 맞춰 화면/호출부에서 문자열 대신 NotificationType 상수로 호출하고(필요하면 직렬화/저장 시 변환 유틸을 추가) 기존 when 분기(또는 저장/로딩 로직)를 NotificationType으로 대체하여 컴파일 타임에 누락/오타를 잡도록 하세요.
98-98: myDataInfo가 null일 때 조용히 리턴하면 사용자가 이유를 알 수 없음
myDataInfo가 null이면 함수가 아무 동작 없이 리턴되어 스위치 토글이 무시됩니다. 사용자 입장에서는 스위치가 왜 동작하지 않는지 알 수 없습니다.실제로 이 상황이 발생하기 어려울 수 있지만, 방어적으로 로그를 남기거나 에러 메시지를 설정하는 것도 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt` at line 98, The code silently returns when uiState.value.myDataInfo is null (currentInfo assignment) causing toggles to be ignored; update the handler in SettingsEditMyDataViewModel to handle the null case by logging the condition (use your logger) and/or updating the UI state with an error/feedback flag (e.g., set an errorMessage or showToast field on uiState) instead of an early return so the user sees why the toggle didn't apply; keep referencing uiState.value.myDataInfo and currentInfo to locate and modify the null-check path.composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt (1)
43-56: 불필요한 코드로 보입니다 - 제거를 고려해 주세요.
paint.color = 0(완전 투명)으로 설정된 상태에서clearShadowLayer()후drawRoundRect를 호출하면 화면에 아무것도 렌더링되지 않습니다. 이 코드 블록은 제거해도 동작에 영향이 없을 것으로 보입니다.♻️ 불필요한 코드 제거 제안
} - paint.clearShadowLayer() - - drawIntoCanvas { canvas -> - val nc = canvas.nativeCanvas - nc.drawRoundRect( - 0f, - 0f, - size.width, - size.height, - radiusPx, - radiusPx, - paint, - ) - } - drawContent() } }혹시 이 코드가 특정 목적(예: 그림자 클리핑, 배경 마스킹 등)을 위해 의도적으로 추가된 것인지 확인 부탁드립니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt` around lines 43 - 56, The drawIntoCanvas block that calls paint.clearShadowLayer() and then nc.drawRoundRect(...) with paint.color = 0 is dead/unnecessary (it renders nothing); remove the entire drawIntoCanvas { ... } block (the calls to paint.clearShadowLayer(), drawIntoCanvas, and nc.drawRoundRect) unless you intentionally rely on it for a side effect like shadow clipping—if that is the case, add a comment explaining the side effect and keep only the minimal operations required for that behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt`:
- Around line 138-156: The code performs an optimistic UI update via
_uiState.update then calls updateUserData without rollback; capture the previous
UI state and previous currentInfo before calling _uiState.update (e.g., store
prevState and prevInfo), perform the optimistic update, then call updateUserData
and handle failure (catch exception or use onFailure callback) to restore
_uiState to prevState and, if needed, reset currentInfo to prevInfo and show an
error/toast; implement this rollback inside SettingsEditMyDataViewModel around
the _uiState.update and updateUserData calls so failures revert the
PushNotification changes and UI flags back to their previous values.
In
`@composeApp/src/iosMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.ios.kt`:
- Around line 17-24: The iOS implementation currently ignores
ShadowLayer.blurRadius (so group.layers loop only draws hard-edged rounded rects
via drawRoundRect), causing no blur; update the loop in Effect.ios.kt to apply a
shadow/blur when rendering each ShadowLayer by using the Compose Multiplatform
shadow APIs or drawIntoCanvas to configure a Paint with the layer's blurRadius,
color and offset before drawing; specifically, for each layer in group.layers
create/configure a Paint (or framework paint via paint.asFrameworkPaint()) to
set the shadow/mask filter using ShadowLayer.blurRadius and layer.color/offset,
then draw the rounded rect with that paint (replacing the plain drawRoundRect
call) so iOS renders the same blurred shadows as Android.
---
Nitpick comments:
In
`@composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt`:
- Around line 43-56: The drawIntoCanvas block that calls
paint.clearShadowLayer() and then nc.drawRoundRect(...) with paint.color = 0 is
dead/unnecessary (it renders nothing); remove the entire drawIntoCanvas { ... }
block (the calls to paint.clearShadowLayer(), drawIntoCanvas, and
nc.drawRoundRect) unless you intentionally rely on it for a side effect like
shadow clipping—if that is the case, add a comment explaining the side effect
and keep only the minimal operations required for that behavior.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt`:
- Around line 94-96: updateNotificationSettingByType에서 사용하는 magic string 대신 enum
또는 sealed class를 도입해 타입 안정성을 확보하세요: 정의한 NotificationType (예: MASTER, COMPLETE,
ABNORMAL, MISSED)을 만들고
SettingsEditMyDataViewModel.updateNotificationSettingByType의 시그니처를 type:
NotificationType으로 변경한 다음 when 타입별 분기를 명시적으로 처리해 else 분기를 제거하세요; 또한 이 변경에 맞춰
화면/호출부에서 문자열 대신 NotificationType 상수로 호출하고(필요하면 직렬화/저장 시 변환 유틸을 추가) 기존 when 분기(또는
저장/로딩 로직)를 NotificationType으로 대체하여 컴파일 타임에 누락/오타를 잡도록 하세요.
- Line 98: The code silently returns when uiState.value.myDataInfo is null
(currentInfo assignment) causing toggles to be ignored; update the handler in
SettingsEditMyDataViewModel to handle the null case by logging the condition
(use your logger) and/or updating the UI state with an error/feedback flag
(e.g., set an errorMessage or showToast field on uiState) instead of an early
return so the user sees why the toggle didn't apply; keep referencing
uiState.value.myDataInfo and currentInfo to locate and modify the null-check
path.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/notification/screen/SettingsNotificationScreen.kt`:
- Around line 48-50: The LaunchedEffect tied to uiState.myDataInfo causes
redundant re-initialization when updateNotificationSettingByType ->
updateUserData updates myDataInfo; change the initialization to run only once on
first load (e.g., switch LaunchedEffect(uiState.myDataInfo) to
LaunchedEffect(Unit) or guard with a one-time flag) and ensure you still call
viewModel.initializeNotificationSettings(myDataInfo) only when myDataInfo is
non-null and not yet initialized; alternatively implement an "initialized"
boolean in the ViewModel (checked in initializeNotificationSettings) to avoid
repeated work when updateUserData updates myDataInfo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dff7311-2600-43b8-82bc-28684916cc71
📒 Files selected for processing (6)
composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/notification/screen/SettingsNotificationScreen.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/theme/Theme.ktcomposeApp/src/iosMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.ios.kt
Comment on lines
138
to
+156
| _uiState.update { | ||
| it.copy( | ||
| missedChecked = value, | ||
| masterChecked = if (!value) false else it.masterChecked, | ||
| masterChecked = newMasterChecked, | ||
| completeChecked = newCompleteChecked, | ||
| abnormalChecked = newAbnormalChecked, | ||
| missedChecked = newMissedChecked, | ||
| ) | ||
| } | ||
|
|
||
| val updatedUserInfo = currentInfo.copy( | ||
| pushNotification = PushNotification( | ||
| all = if (newMasterChecked) "ON" else "OFF", | ||
| carecallCompleted = if (newCompleteChecked) "ON" else "OFF", | ||
| healthAlert = if (newAbnormalChecked) "ON" else "OFF", | ||
| carecallMissed = if (newMissedChecked) "ON" else "OFF", | ||
| ), | ||
| ) | ||
|
|
||
| updateUserData(updatedUserInfo) |
There was a problem hiding this comment.
낙관적 업데이트(Optimistic Update) 시 에러 발생 시 롤백 로직 부재
UI 상태를 먼저 업데이트(Line 138-145)한 후 서버 호출(Line 156)을 하는 낙관적 업데이트 패턴을 사용하고 있는데, updateUserData가 실패하면 UI는 이미 변경된 상태로 남아있고 서버와 불일치하게 됩니다.
사용자 경험을 위해 에러 발생 시 이전 상태로 롤백하거나, 실패 토스트 메시지와 함께 상태를 복원하는 로직을 추가하면 좋을 것 같습니다.
🛡️ 롤백 로직 추가 예시
+ // 롤백을 위해 이전 상태 저장
+ val previousState = _uiState.value.copy()
+
_uiState.update {
it.copy(
masterChecked = newMasterChecked,
completeChecked = newCompleteChecked,
abnormalChecked = newAbnormalChecked,
missedChecked = newMissedChecked,
)
}
val updatedUserInfo = currentInfo.copy(
pushNotification = PushNotification(
all = if (newMasterChecked) "ON" else "OFF",
carecallCompleted = if (newCompleteChecked) "ON" else "OFF",
healthAlert = if (newAbnormalChecked) "ON" else "OFF",
carecallMissed = if (newMissedChecked) "ON" else "OFF",
),
)
- updateUserData(updatedUserInfo)
+ updateUserData(updatedUserInfo) {
+ // onComplete - 성공 시 아무것도 안 함
+ }
+ // 또는 실패 시 롤백하는 별도 콜백 파라미터 추가 고려updateUserData의 onFailure 핸들러에서 previousState로 복원하는 방식도 고려할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/feature/settings/mydata/viewmodel/SettingsEditMyDataViewModel.kt`
around lines 138 - 156, The code performs an optimistic UI update via
_uiState.update then calls updateUserData without rollback; capture the previous
UI state and previous currentInfo before calling _uiState.update (e.g., store
prevState and prevInfo), perform the optimistic update, then call updateUserData
and handle failure (catch exception or use onFailure callback) to restore
_uiState to prevState and, if needed, reset currentInfo to prevInfo and show an
error/toast; implement this rollback inside SettingsEditMyDataViewModel around
the _uiState.update and updateUserData calls so failures revert the
PushNotification changes and UI flags back to their previous values.
Contributor
Author
|
@ProtossManse 리뷰 부탁드려요 ~ |
Contributor
|
안드로이드에서도 그림자가 제대로 적용이 안된 것 같습니다! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt`:
- Around line 8-19: The current Modifier.figmaShadow implementation ignores each
ShadowLayer's offsetX/offsetY so shadows don't match Figma; update
Modifier.figmaShadow (the fold over group.layers) to apply layer offsets before
applying shadow (e.g., compose acc.offset(x = layer.offsetX, y = layer.offsetY)
then .shadow(...)) so the ShadowGroup layers' offsets are respected when
rendering; ensure you reference layer.offsetX and layer.offsetY from the
ShadowLayer and preserve existing cornerRadius, blurRadius, color, and clip
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2e6606c-d365-4600-abc6-35a143de8cd0
📒 Files selected for processing (1)
composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt
Comment on lines
+8
to
+19
| actual fun Modifier.figmaShadow( | ||
| group: ShadowGroup, | ||
| cornerRadius: Dp, | ||
| ): Modifier = group.layers.fold(this) { acc, layer -> | ||
| acc.shadow( | ||
| elevation = layer.blurRadius, | ||
| shape = RoundedCornerShape(cornerRadius), | ||
| ambientColor = layer.color, | ||
| spotColor = layer.color, | ||
| clip = false | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: ShadowLayer 및 ShadowGroup 정의 확인
# ShadowLayer 클래스/데이터 클래스 정의 검색
echo "=== ShadowLayer 정의 ==="
rg -n -A 10 'data class ShadowLayer|class ShadowLayer'
echo ""
echo "=== ShadowGroup 정의 ==="
rg -n -A 10 'data class ShadowGroup|class ShadowGroup'Repository: Medicare-Call/Medicare-Call-KMP
Length of output: 4049
offset 속성 처리 필요해요 🎨
코드 구조는 깔끔하고 fold 패턴으로 여러 shadow layer를 쌓아가는 방식도 좋습니다. 근데 예상대로 ShadowLayer에는 offsetX와 offsetY 속성이 있는데, 현재 구현에서는 이게 무시되고 있어요.
Modifier.shadow()는 x/y offset을 직접 지원하지 않기 때문에, offset 값들을 반영하려면 Modifier.offset()을 조합하거나 다른 접근이 필요합니다. Figma 디자인에서 offset이 중요하다면 다음처럼 처리할 수 있어요:
actual fun Modifier.figmaShadow(
group: ShadowGroup,
cornerRadius: Dp,
): Modifier = group.layers.fold(this) { acc, layer ->
acc.offset(x = layer.offsetX, y = layer.offsetY)
.shadow(
elevation = layer.blurRadius,
shape = RoundedCornerShape(cornerRadius),
ambientColor = layer.color,
spotColor = layer.color,
clip = false
)
}offset 없이도 디자인이 괜찮다면 현재대로 두셔도 괜찮지만, Figma와의 정확한 일치가 필요하다면 위처럼 처리해주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/androidMain/kotlin/com/konkuk/medicarecall/ui/theme/Effect.android.kt`
around lines 8 - 19, The current Modifier.figmaShadow implementation ignores
each ShadowLayer's offsetX/offsetY so shadows don't match Figma; update
Modifier.figmaShadow (the fold over group.layers) to apply layer offsets before
applying shadow (e.g., compose acc.offset(x = layer.offsetX, y = layer.offsetY)
then .shadow(...)) so the ShadowGroup layers' offsets are respected when
rendering; ensure you reference layer.offsetX and layer.offsetY from the
ShadowLayer and preserve existing cornerRadius, blurRadius, color, and clip
behavior.
Contributor
Author
엄... 적용돼 있는 거 아닌가요 ....? |
This was referenced Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
새로운 기능
개선 사항
기타