Conversation
Walkthrough디자인 시스템 색상 확장, 다운타임 유효성 타입·검증 로직, Compose 기반 다운타임 바텀시트(숫자 피커 포함), 툴팁 화살표 벡터 리소스, 고정 DP→SP 폰트 유틸이 추가되었습니다. (총 변경: UI 컴포저블·로직·테마·리소스·유틸) Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant UI as DowntimeBottomSheet
participant Picker as NumberPicker
participant Logic as DowntimeDayLogic
participant Sheet as SheetState
User->>UI: 바텀시트 열기 요청
UI->>Sheet: show()
User->>Picker: 일수 선택(스크롤)
Picker->>UI: 선택된 일수 전달
UI->>Logic: validate(endDay, selectedDays)
Logic-->>UI: DowntimeValidationType(VALID/EXCEEDS_GOAL)
UI->>User: 가이드 문구/진행바 갱신
User->>UI: 확인/취소 클릭
UI->>Caller: onConfirmClick/onAddWithoutDowntimeClick
UI->>Sheet: dismiss()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
🤖 Fix all issues with AI agents
In @app/src/main/java/com/cherrish/android/core/designsystem/theme/Color.kt:
- Around line 225-229: Update the preview Text label that currently shows
"Shadow" to the correct descriptive name "BottomSheetScrimColor": locate the
Text composable where style = CherrishTheme.typography.body1M14 and color =
CherrishTheme.colors.bottomSheetScrimColor, and change its text argument from
"Shadow" to "BottomSheetScrimColor".
In
@app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt:
- Around line 480-481: The preview contains inconsistent date formatting: in
DowntimeBottomSheet.kt the properties downtimeStartDay and downtimeEndDay are
set as "2023.08.01" and "2023.08.1" respectively; update downtimeEndDay to match
the same zero-padded format ("2023.08.01") so both downtimeStartDay and
downtimeEndDay use a consistent "yyyy.MM.dd" representation.
- Around line 91-94: The inner Column in DowntimeBottomSheet is reusing the
external parameter named modifier (passed into ModalBottomSheet), causing
duplicate modifier application; update the Column to use a fresh Compose
Modifier (e.g., Modifier.fillMaxWidth().padding(top = 32.dp, bottom = 20.dp))
instead of the external modifier parameter, leaving the outer ModalBottomSheet
to receive the original modifier.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt (1)
7-12: 테스트 용이성 및 설계 개선이 필요합니다.
LocalDate.now()를 생성자에서 직접 호출하면 단위 테스트가 어려워집니다.startDate를 생성자 파라미터로 받거나Clock을 주입하는 것을 권장합니다.endDate = endDay는 불필요한 중복입니다.endDay를 직접 사용할 수 있습니다.♻️ 리팩토링 제안
class DowntimeDayLogic( + private val startDate: LocalDate = LocalDate.now(), private val endDay: LocalDate, private val downtimeDay: Int ) { - private val startDate: LocalDate = LocalDate.now() - private val endDate: LocalDate = endDay - val downtimeValidationType: DowntimeValidationType get() { - val diffDays = ChronoUnit.DAYS.between(startDate, endDate).toInt() + val diffDays = ChronoUnit.DAYS.between(startDate, endDay).toInt()app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (2)
383-384: remember와 toPersistentList() 사용 개선
remember블록 외부에서toPersistentList()를 호출하면 매 recomposition마다 새로운ImmutableList가 생성됩니다. 전체 변환을remember블록 내부로 이동하세요.♻️ 수정 제안
val paddedList: ImmutableList<Int?> = - remember(list) { listOf(null) + list + listOf(null) }.toPersistentList() + remember(list) { (listOf(null) + list + listOf(null)).toPersistentList() }
372-378: NumberPicker의 접근 제어자를 확인하세요.
NumberPicker가public으로 선언되어 있지만, 이 파일의 다른 helper composable들은 모두private입니다. 이 컴포넌트가 다른 곳에서 재사용될 예정이 아니라면private으로 변경하거나, 재사용 컴포넌트라면 별도의 공통 컴포넌트 위치로 이동하는 것을 고려하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Color.ktapp/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.ktapp/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/model/DowntimeValidationType.ktapp/src/main/res/drawable/ic_tooltip_arrow.xml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/presentation/calendar/model/DowntimeValidationType.ktapp/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.ktapp/src/main/java/com/cherrish/android/core/designsystem/theme/Color.kt
🧠 Learnings (1)
📚 Learning: 2026-01-12T19:49:27.085Z
Learnt from: nhyeonii
Repo: TEAM-Cherrish/Cherrish-Android PR: 41
File: app/src/main/java/com/cherrish/android/presentation/challenge/ChallengeScreen.kt:30-39
Timestamp: 2026-01-12T19:49:27.085Z
Learning: When a Jetpack Compose screen composable receives a Scaffold paddingValues: PaddingValues parameter (commonly from Scaffold), apply it to the root container's modifier first (e.g., .padding(paddingValues)) before applying any additional padding. This ensures content respects system bars (status/navigation) and avoids layout overlap. This pattern should be followed across presentation screens under app/src/main/java/com/cherrish/android/presentation/ to maintain consistent insets handling.
Applied to files:
app/src/main/java/com/cherrish/android/presentation/calendar/model/DowntimeValidationType.ktapp/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (1)
CherrishButton(27-79)app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
🔇 Additional comments (6)
app/src/main/res/drawable/ic_tooltip_arrow.xml (1)
1-9: LGTM!툴팁 화살표 벡터 드로어블이 올바르게 정의되어 있습니다. 크기와 뷰포트가 적절하게 설정되었습니다.
app/src/main/java/com/cherrish/android/core/designsystem/theme/Color.kt (1)
38-40: LGTM!색상 상수가 올바르게 정의되었습니다.
shadow는copy(alpha)를 사용하여 투명도를 적용했고,bottomSheetScrimColor는 hex 값에 알파를 포함하여 정의했습니다.app/src/main/java/com/cherrish/android/presentation/calendar/model/DowntimeValidationType.kt (1)
1-6: LGTM!다운타임 유효성 검증 타입 열거형이 간결하고 명확하게 정의되었습니다.
app/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt (1)
14-23: 엣지 케이스 및 코드 간소화를 확인하세요.
endDay가startDate보다 과거인 경우diffDays가 음수가 되어 항상VALID를 반환합니다. 이것이 의도된 동작인지 확인이 필요합니다.- if-else 구문을 간소화할 수 있습니다.
♻️ 간소화 제안
val downtimeValidationType: DowntimeValidationType - get() { - val diffDays = ChronoUnit.DAYS.between(startDate, endDate).toInt() - - if (diffDays <= downtimeDay) { - return DowntimeValidationType.VALID - } else { - return DowntimeValidationType.EXCEEDS_GOAL - } - } + get() = if (ChronoUnit.DAYS.between(startDate, endDay) <= downtimeDay) { + DowntimeValidationType.VALID + } else { + DowntimeValidationType.EXCEEDS_GOAL + }app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (2)
164-168: 0으로 나누기 가능성을 확인하세요.
downtimeDay와spareTimeDay가 모두 0인 경우 0으로 나누기 오류가 발생합니다. 이 경우에 대한 방어 로직이 필요할 수 있습니다.🔧 방어 로직 제안
val downtimeWeight by remember(downtimeDay, spareTimeDay) { derivedStateOf { - downtimeDay.toFloat() / (downtimeDay + spareTimeDay) + val total = downtimeDay + spareTimeDay + if (total > 0) downtimeDay.toFloat() / total else 0f } }
55-71: 전반적인 구조가 잘 설계되었습니다.
DowntimeBottomSheet가 적절하게 분리된 하위 컴포넌트들로 구성되어 있고, 테마 색상과 타이포그래피를 일관되게 사용하고 있습니다.derivedStateOf와remember를 적절히 활용하여 recomposition을 최적화했습니다.
| downtimeStartDay = "2023.08.01", | ||
| downtimeEndDay = "2023.08.1", |
There was a problem hiding this comment.
Preview 날짜 형식 불일치
downtimeStartDay는 "2023.08.01"이고 downtimeEndDay는 "2023.08.1"입니다. 날짜 형식이 일관되지 않습니다.
🔧 수정 제안
downtimeStartDay = "2023.08.01",
- downtimeEndDay = "2023.08.1",
+ downtimeEndDay = "2023.08.11",📝 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.
| downtimeStartDay = "2023.08.01", | |
| downtimeEndDay = "2023.08.1", | |
| downtimeStartDay = "2023.08.01", | |
| downtimeEndDay = "2023.08.11", |
🤖 Prompt for AI Agents
In
@app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt
around lines 480 - 481, The preview contains inconsistent date formatting: in
DowntimeBottomSheet.kt the properties downtimeStartDay and downtimeEndDay are
set as "2023.08.01" and "2023.08.1" respectively; update downtimeEndDay to match
the same zero-padded format ("2023.08.01") so both downtimeStartDay and
downtimeEndDay use a consistent "yyyy.MM.dd" representation.
nhyeonii
left a comment
nhyeonii
left a comment
There was a problem hiding this comment.
바텀시트 데이트 피커랑 여러 요소들 있어서 어려웠을텐데 너무 잘 작업 되어이ㅛ는거 가타요 최고쵝고 !! 리뷰 한번 확인 부탁드립니당 넘 고생햇더요 !!!
| shape = RoundedCornerShape(topStart = 20.dp, topEnd = 20.dp), | ||
| containerColor = CherrishTheme.colors.gray0, | ||
| scrimColor = scrimColor, | ||
| dragHandle = { DowntimeDragHandle() }, |
There was a problem hiding this comment.
P2 : 요거 상위로 올리지 않은 이육 잇을까요이 ?!?!
There was a problem hiding this comment.
이거 공통 바텀시트가 아닌 해당 화면에서만 사용하는 바텀시트이다 보니 dragHandle을 상위로 올리지 않아도 된다고 생각했습니다!
| shape = RoundedCornerShape(8.dp), | ||
| color = CherrishTheme.colors.shadow, | ||
| blur = 10.dp, | ||
| offsetX = 0.dp, | ||
| offsetY = 0.dp | ||
| ) | ||
| .clip(shape = RoundedCornerShape(8.dp)) | ||
| .background(CherrishTheme.colors.gray0) | ||
| .border( | ||
| width = 1.dp, | ||
| color = CherrishTheme.colors.gray200, | ||
| shape = RoundedCornerShape(8.dp) | ||
| ) |
There was a problem hiding this comment.
P1 : 요거 shape 10.dp 가틍유 ~~ !! 얘 글고 텍스트 말고 칼럼에 모디파이ㅓㅇ 관련 속성 주는건 오떤가요??
There was a problem hiding this comment.
이거 두개를 붙이거여서 Column을 쓰게 된다면 이 네모바를 위해서만 column을 써야되가지고 Text로 구현했습니다!
| color = CherrishTheme.colors.gray200, | ||
| shape = RoundedCornerShape(8.dp) | ||
| ) | ||
| .padding(vertical = 12.dp), |
There was a problem hiding this comment.
P1 : 여기 패딩 한번만 확인해주시면 감사하겟숩니닿ㅎㅎ
There was a problem hiding this comment.
이거 수직 기준으로 패딩 먹인거여서 12.dp 줬습니다!
| contentDescription = null, | ||
| tint = CherrishTheme.colors.gray0, | ||
| modifier = Modifier | ||
| .offset(y = -5.dp) |
There was a problem hiding this comment.
| .offset(y = -5.dp) | |
| .offset(y = (-5).dp) |
P1 : 음수 괄호에 넣어쥬쟈 ~~
| @Composable | ||
| private fun DowntimeProgressBar( | ||
| downtimeWeight: Float, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| BoxWithConstraints( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .height(8.dp) | ||
| ) { | ||
| val totalWidth = maxWidth | ||
|
|
||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .clip(shape = RoundedCornerShape(24.dp)) | ||
| .background(color = CherrishTheme.colors.gray400) | ||
| ) | ||
|
|
||
| Box( | ||
| modifier = Modifier | ||
| .width(totalWidth * downtimeWeight) | ||
| .fillMaxHeight() | ||
| .clip(shape = RoundedCornerShape(24.dp)) | ||
| .background(color = CherrishTheme.colors.red600) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
| @Composable | |
| private fun DowntimeProgressBar( | |
| downtimeWeight: Float, | |
| modifier: Modifier = Modifier | |
| ) { | |
| BoxWithConstraints( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .height(8.dp) | |
| ) { | |
| val totalWidth = maxWidth | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .clip(shape = RoundedCornerShape(24.dp)) | |
| .background(color = CherrishTheme.colors.gray400) | |
| ) | |
| Box( | |
| modifier = Modifier | |
| .width(totalWidth * downtimeWeight) | |
| .fillMaxHeight() | |
| .clip(shape = RoundedCornerShape(24.dp)) | |
| .background(color = CherrishTheme.colors.red600) | |
| ) | |
| } | |
| } | |
| @Composable | |
| private fun DowntimeProgressBar( | |
| downtimeWeight: Float, | |
| modifier: Modifier = Modifier | |
| ) { | |
| Box( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .height(8.dp) | |
| .clip(shape = RoundedCornerShape(24.dp)) | |
| .background(color = CherrishTheme.colors.gray400) | |
| ) { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxWidth(fraction = downtimeWeight) | |
| .fillMaxHeight() | |
| .clip(shape = RoundedCornerShape(24.dp)) | |
| .background(color = CherrishTheme.colors.red600) | |
| ) | |
| } | |
| } |
P1 : 이거 요렇게 박스 두개로도 구현할 수 잇을거 같아용 !! BoxWithConstraints 사용보다 조금 더 이해가 쉬울 수 있을 거 같은데 어떻게 생각하시나여 ~ ?!!/
| ) | ||
|
|
||
| Text( | ||
| text = "보통 3-5일", |
There was a problem hiding this comment.
P1 : 요거 다운타임 시술 마다 달라서 서버에서 받아와야하는 로직으로 가야할 거가트요 !!
|
|
||
| NumberPicker( | ||
| list = (1..30).toPersistentList(), | ||
| fontSize = CherrishTheme.typography.headlineSB20.fontSize, |
There was a problem hiding this comment.
P3 : 요 headlineSB20은 오디서 사용댈까요...?!?!
| style = CherrishTheme.typography.body1M14, | ||
| color = CherrishTheme.colors.shadow | ||
| ) | ||
| Text( |
There was a problem hiding this comment.
P3: 사소한데 Shadow 라벨이 위와 똑같으니 bottomSheetScrimColor 이걸로 바꾸는 건 어떨까요 ??
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt`:
- Around line 358-363: The NumberPicker currently uses a hardcoded range
(1..30); update DowntimeBottomSheet to build the NumberPicker list from the
passed parameters by replacing (1..30).toPersistentList() with a range based on
minDowntimeDays and maxDowntimeDays (e.g.,
(minDowntimeDays..maxDowntimeDays).toPersistentList()), and handle edge cases
where minDowntimeDays > maxDowntimeDays (either clamp, swap, or default to a
safe range) so NumberPicker receives a valid non-empty list; keep the existing
state and flingBehavior parameters unchanged.
♻️ Duplicate comments (1)
app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (1)
488-489: Preview 날짜 형식 불일치
downtimeStartDay는 "2023.08.01"이고downtimeEndDay는 "2023.08.1"입니다. 날짜 형식을 일관되게 맞춰주세요.🔧 수정 제안
downtimeStartDay = "2023.08.01", - downtimeEndDay = "2023.08.1", + downtimeEndDay = "2023.08.11",
🧹 Nitpick comments (6)
app/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt (1)
7-12: 테스트 용이성을 위해startDate주입 고려
LocalDate.now()가 클래스 내부에 하드코딩되어 있어 단위 테스트가 어렵습니다. 테스트 시 날짜를 제어할 수 있도록startDate를 생성자 매개변수로 받는 것을 권장합니다.또한
endDate = endDay는 중복 할당이므로endDay를 직접 사용할 수 있습니다.♻️ 리팩토링 제안
class DowntimeDayLogic( + private val startDay: LocalDate = LocalDate.now(), private val endDay: LocalDate, private val downtimeDay: Int ) { - private val startDate: LocalDate = LocalDate.now() - private val endDate: LocalDate = endDay - val downtimeValidationType: DowntimeValidationType get() { - val diffDays = ChronoUnit.DAYS.between(startDate, endDate).toInt() + val diffDays = ChronoUnit.DAYS.between(startDay, endDay).toInt()app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (5)
74-78:showBottomSheet가false일 때 불필요한 객체 생성
flingBehavior가if (showBottomSheet)블록 외부에서 생성되어, 바텀시트가 표시되지 않을 때도 매 리컴포지션마다 생성됩니다. 조건 블록 내부로 이동하면 불필요한 연산을 피할 수 있습니다.♻️ 리팩토링 제안
- val flingBehavior = rememberSnapFlingBehavior( - lazyListState = state - ) - if (showBottomSheet) { + val flingBehavior = rememberSnapFlingBehavior( + lazyListState = state + ) + ModalBottomSheet(
380-381:NumberPicker가 public으로 선언됨다른 내부 컴포저블들과 달리
NumberPicker만private키워드 없이 public으로 선언되어 있습니다. 의도적으로 다른 곳에서 재사용하려는 것이 아니라면private으로 변경하는 것이 좋습니다.♻️ 수정 제안
`@Composable` -fun NumberPicker( +private fun NumberPicker(
393-393:dpBasedSp변환을remember로 감싸기
fontSize.toDp().toSp()변환이 매 리컴포지션마다 수행됩니다.fontSize가 변경되지 않는 한 동일한 값을 반환하므로remember로 감싸는 것이 좋습니다.♻️ 리팩토링 제안
- val dpBasedSp = with(LocalDensity.current) { fontSize.toDp().toSp() } + val density = LocalDensity.current + val dpBasedSp = remember(fontSize, density) { + with(density) { fontSize.toDp().toSp() } + }
168-177:totalDay변수 재사용Line 174에서
downtimeDay + spareTimeDay를 다시 계산하는 대신 이미 선언된totalDay변수를 사용하면 더 일관성 있습니다.♻️ 리팩토링 제안
if (totalDay == 0) { 0f } else { - downtimeDay.toFloat() / (downtimeDay + spareTimeDay) + downtimeDay.toFloat() / totalDay }
316-316: 코드 포맷팅: 등호 앞 공백 누락
minDowntimeDays=부분에서=앞에 공백이 누락되었습니다.♻️ 수정 제안
- minDowntimeDays= minDowntimeDays, + minDowntimeDays = minDowntimeDays,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Color.ktapp/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.ktapp/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/cherrish/android/core/designsystem/theme/Color.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (1)
CherrishButton(27-79)app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
🔇 Additional comments (2)
app/src/main/java/com/cherrish/android/presentation/calendar/DowntimeDayLogic.kt (1)
14-23: 엣지 케이스 처리 확인 필요
endDay가startDate보다 이전인 경우diffDays가 음수가 되어 항상EXCEEDS_GOAL을 반환합니다. 이것이 의도된 동작인지 확인하고, 필요하다면 음수 또는 0일 경우에 대한 방어 로직 추가를 고려해주세요.app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (1)
395-397: 스크롤 중 선택 인덱스 정확도 확인
firstVisibleItemIndex + 1으로 선택된 항목을 결정하는데, 스크롤 오프셋을 고려하지 않습니다. 스크롤 중에 시각적으로 중앙에 있는 항목과selectedIndex가 일치하지 않을 수 있습니다.firstVisibleItemScrollOffset을 활용하여 더 정확한 선택 로직을 구현하는 것을 고려해주세요.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| NumberPicker( | ||
| list = (1..30).toPersistentList(), | ||
| fontSize = CherrishTheme.typography.headlineSB20.fontSize, | ||
| state = state, | ||
| flingBehavior = flingBehavior | ||
| ) |
There was a problem hiding this comment.
NumberPicker 범위가 하드코딩됨
minDowntimeDays와 maxDowntimeDays가 매개변수로 전달되지만, NumberPicker의 리스트는 (1..30)으로 하드코딩되어 있습니다. 매개변수 값에 따라 범위를 동적으로 설정해야 할 것 같습니다.
🔧 수정 제안
NumberPicker(
- list = (1..30).toPersistentList(),
+ list = (minDowntimeDays..maxDowntimeDays).toPersistentList(),
fontSize = CherrishTheme.typography.headlineSB20.fontSize,
state = state,
flingBehavior = flingBehavior
)만약 1-30 범위가 의도된 것이라면, minDowntimeDays와 maxDowntimeDays 매개변수의 용도를 명확히 해주세요 (현재는 가이드 텍스트에만 사용됨).
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt`
around lines 358 - 363, The NumberPicker currently uses a hardcoded range
(1..30); update DowntimeBottomSheet to build the NumberPicker list from the
passed parameters by replacing (1..30).toPersistentList() with a range based on
minDowntimeDays and maxDowntimeDays (e.g.,
(minDowntimeDays..maxDowntimeDays).toPersistentList()), and handle edge cases
where minDowntimeDays > maxDowntimeDays (either clamp, swap, or default to a
safe range) so NumberPicker receives a valid non-empty list; keep the existing
state and flingBehavior parameters unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/cherrish/android/core/util/FontScaleUnits.kt`:
- Around line 7-12: The current rememberFixedDpFontSize uses
fontSize.toDp().toSp() which round-trips through sp and still applies fontScale;
instead treat the numeric value as Dp and convert that to Sp so fontScale is
ignored: inside rememberFixedDpFontSize with LocalDensity.current, use
fontSize.value as a raw Dp (e.g. fontSize.value.dp) and then convert that Dp to
Sp via the density (with(density) { fontSize.value.dp.toSp() }), replacing the
fontSize.toDp().toSp() path so the result is fixed in DP.
♻️ Duplicate comments (2)
app/src/main/java/com/cherrish/android/presentation/calendar/component/DowntimeBottomSheet.kt (2)
354-356: NumberPicker 범위가 매개변수와 불일치합니다.min/max가 안내 텍스트에만 쓰이고 리스트는 (1..30)로 고정됩니다. 이전 리뷰와 동일하게 매개변수 범위를 사용하고, 역전/빈 범위 방어가 필요합니다.
🔧 수정 제안
NumberPicker( - list = (1..30).toPersistentList(), + list = (minDowntimeDays..maxDowntimeDays).toPersistentList(), state = state, flingBehavior = flingBehavior )
487-488: Preview 날짜 포맷 일관성이 필요합니다.downtimeEndDay가 0 패딩 없이 표기되어 yyyy.MM.dd 포맷과 불일치합니다. 이전 리뷰와 동일하게 두 자리 포맷으로 맞춰주세요.
🔧 수정 제안
- downtimeEndDay = "2023.08.1", + downtimeEndDay = "2023.08.01",
| @Composable | ||
| fun rememberFixedDpFontSize(fontSize: TextUnit): TextUnit { | ||
| val density = LocalDensity.current | ||
| return with(density) { | ||
| fontSize.toDp().toSp() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jetpack Compose TextUnit toDp toSp conversion fontScale density
💡 Result:
Short answer
- For SP TextUnit (TextUnitType.Sp) Compose does:
- TextUnit.toDp() multiplies the SP value by fontScale (unless non-linear font scaling is active, in which case a converter may be used). [1][2]
- Dp.toPx() then multiplies that Dp by density.
So (linear case) px = sp * fontScale * density, and dp = sp * fontScale. [1][3]
Notes
- TextUnit.toPx() calls TextUnit.toDp().toPx() and will throw if the TextUnit is not SP. [1][3]
- Compose may use a non-linear FontScale converter for accessibility sizes; in that case TextUnit.toDp() uses the converter instead of simple multiplication. [2][3]
Sources
[1] Android Compose Density API (Density, toPx/toDp behavior).
[2] Android Compose FontScaling (TextUnit.toDp, fontScale, non-linear scaling).
[3] Compose source/docs showing TextUnit.toPx delegating to toDp and Dp.toPx.
🏁 Script executed:
# Check the context and usage of the function in the codebase
fd -t f "FontScaleUnits.kt" -x cat -n {}
# Look for similar patterns or usages of rememberFixedDpFontSize
rg "rememberFixedDpFontSize" --type kt -B 2 -A 5
# Check for other related utility functions
rg "LocalDensity" --type kt -B 2 -A 3 app/src/main/java/com/cherrish/android/core/util/Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 586
🏁 Script executed:
# Check if there are any existing uses of rememberFixedDpFontSize
rg "rememberFixedDpFontSize" --type-add 'kt:*.kt' -B 2 -A 5
# Check for other similar patterns in the codebase
rg "toDp\(\)\.toSp\(\)" --type-add 'kt:*.kt'Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 3468
현재 구현은 fontScale 고정을 달성하지 못합니다.
함수 이름이 "rememberFixedDpFontSize"로 크기를 DP에 고정하려는 의도를 나타내지만, fontSize.toDp().toSp()는 왕복 변환으로 원래 값이 그대로 돌아옵니다. 구체적으로, toDp()는 sp 값에 fontScale을 곱하고, toSp()는 다시 나누므로 (sp × fontScale) ÷ fontScale = sp가 되어 fontScale의 영향을 여전히 받습니다.
고정 DP 의미를 올바르게 구현하려면 수치 값을 Dp로 직접 취급하여 변환해야 합니다.
🔧 수정 제안
import androidx.compose.ui.unit.TextUnit
+import androidx.compose.ui.unit.dp
`@Composable`
fun rememberFixedDpFontSize(fontSize: TextUnit): TextUnit {
val density = LocalDensity.current
return with(density) {
- fontSize.toDp().toSp()
+ if (fontSize.isSpecified) {
+ fontSize.value.dp.toSp()
+ } else {
+ TextUnit.Unspecified
+ }
}
}🤖 Prompt for AI Agents
In `@app/src/main/java/com/cherrish/android/core/util/FontScaleUnits.kt` around
lines 7 - 12, The current rememberFixedDpFontSize uses fontSize.toDp().toSp()
which round-trips through sp and still applies fontScale; instead treat the
numeric value as Dp and convert that to Sp so fontScale is ignored: inside
rememberFixedDpFontSize with LocalDensity.current, use fontSize.value as a raw
Dp (e.g. fontSize.value.dp) and then convert that Dp to Sp via the density
(with(density) { fontSize.value.dp.toSp() }), replacing the
fontSize.toDp().toSp() path so the result is fixed in DP.
4 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
2026-01-14.5.00.31.mov
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
스타일
유틸리티
✏️ Tip: You can customize this high-level summary in your review settings.