Conversation
… 여정 지도 화면을 위한 UI 모델 및 상태 클래스 추가
…' into feat/#101-today-journey-map-view
vvan2
left a comment
There was a problem hiding this comment.
고생했으요!! 역시 대 승 재 .. 날이 갈수록 너무 잘해지네요(원래도 잘했지만)
이것이 38기 OB인가요!
| .border( | ||
| width = 1.dp, | ||
| color = KieroTheme.colors.main, | ||
| shape = RoundedCornerShape(100.dp) |
| color = KieroTheme.colors.main, | ||
| shape = RoundedCornerShape(100.dp) | ||
| ) | ||
| .padding(horizontal = 8.dp, vertical = 2.dp) |
| import com.kiero.R | ||
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Keep |
There was a problem hiding this comment.
p3) 안녕하세요, baselineprofile 세팅하다가 해당 파일에서 직렬화 오류나서 달아놓은 거긴한데요, 테스트 한번 해주실래요..? 없어도 잘되나
There was a problem hiding this comment.
그냥 디버그 빌드는 잘되서 제거했는디 navigation 인자로 쓰여서 그런가?
다시 그대로 유지하는게 좋을 것 같네요
| val thicknessPx = thickness.toPx() | ||
| val thumbHeightPx = thumbHeight.toPx() | ||
| val paddingPx = verticalPadding.toPx() | ||
| val cornerRadius = CornerRadius(thicknessPx / 2f) |
There was a problem hiding this comment.
p2) thumbheight와 padding 관련 px 연산은 상관없지만, cornerRadius 는 drawWithContent 외부에서 연산하게해서 객체 생성을 줄이는 방법도 있을것 같습니당
There was a problem hiding this comment.
저두 같은 의견입니다!
그래서
return drawWithCache {
val thicknessPx = thickness.toPx()
val thumbHeightPx = thumbHeight.toPx()
val paddingPx = verticalPadding.toPx()
...
을 사용해보시는 것도 좋을 것 같습니다!
주완님이 더 위에 남기신 detrived도 좋지만
drawWithCache 후 onDrawWithContent안에 val canScroll = state.canScrollForward || state.canScrollBackward
을 옮겨 리컴포지션을 방지해도 좋을 것 같습니다!
There was a problem hiding this comment.
drawWithContent는 매 프레임마다 실행되어서, 그때마다 size가 바뀌지 않는 한 항상 동일한 값임에도 불구하고ㅠtoPx() 연산과 CornerRadius 객체 생성이 반복이 되는 문제가 있었네요
drawWithCache 후 onDrawWithContent를 이용해서 size 변경 시에만 실행되도록 변경하겠습니다!!
| val scrollbarOffsetX = size.width - thicknessPx | ||
| val scrollbarOffsetY = paddingPx + (scrollProportion * maxScrollOffsetY) | ||
|
|
||
| if (animationAlpha > 0f) { |
There was a problem hiding this comment.
p2) 스크롤바가 보이고 사라질 때에 대해서 alpha 값이 0이면 계산을 최소화 할 수 있게 drawWithContent 생성 전에 ealry return 으로 빼주면 좋을 것 같습니다.
| ) { | ||
| val borderColor = when (status) { |
There was a problem hiding this comment.
p2) SSE 적용되는 화면이기도 하고, 리스트 내부 요소들이(상태가) 자주 바뀌니까 remember 붙혀줘서 status 변할 때마다 매번 계산되지않게 해주는게 좋을것 같슴다
val borderColor = remember(status) {
when (status) {
...
| KidMapScheduleStatus.PENDING -> KieroTheme.colors.white | ||
| } | ||
|
|
||
| val statusLabel: Pair<String, Color>? = when (status) { |
There was a problem hiding this comment.
p3) borderColor 랑 묶어서 data class 로 관리하는게 편해 보이긴 하는데, label 요소는 기능상 nullable 하니까 분리해준거면 상관없을 것 같기도 하고...
근데 또 Pair사용하는 것보다는 data class 로 사용하는 게 좀 더 좋을 때가 많아서 그냥 borderColor 랑 묶어주는게 좋은 거 같기도 하고..
대승재 판단에 맡긴다
| } | ||
|
|
||
| Text( | ||
| text = statusLabel?.first ?: "획득!", |
There was a problem hiding this comment.
p2) 그렇게 되면 여기서도 명시적으로 확인할 수 있으니까 키하하
|
|
||
| Text( | ||
| text = statusLabel?.first ?: "획득!", | ||
| color = (statusLabel?.second ?: KieroTheme.colors.main).copy( |
There was a problem hiding this comment.
p2) 이것도 위에서 한번에 Transparent 로 해주면 alpha값 안건들여도 될듯요!
|
|
||
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable |
There was a problem hiding this comment.
p2) 왜 직렬화 했긔, 얘도 navigation에서 사용하나? 못찾겠어서 없으면 ㅂ 빼도 될듯
sonms
left a comment
There was a problem hiding this comment.
고생하셨습니다 !!!!! 코리 한 번 확인 부탁드리겠습니다!
| val thicknessPx = thickness.toPx() | ||
| val thumbHeightPx = thumbHeight.toPx() | ||
| val paddingPx = verticalPadding.toPx() | ||
| val cornerRadius = CornerRadius(thicknessPx / 2f) |
There was a problem hiding this comment.
저두 같은 의견입니다!
그래서
return drawWithCache {
val thicknessPx = thickness.toPx()
val thumbHeightPx = thumbHeight.toPx()
val paddingPx = verticalPadding.toPx()
...
을 사용해보시는 것도 좋을 것 같습니다!
주완님이 더 위에 남기신 detrived도 좋지만
drawWithCache 후 onDrawWithContent안에 val canScroll = state.canScrollForward || state.canScrollBackward
을 옮겨 리컴포지션을 방지해도 좋을 것 같습니다!
| @Composable | ||
| fun KieroSnackbar( | ||
| message: String, | ||
| modifier: Modifier = Modifier, | ||
| contentPadding: Dp = 16.dp, | ||
| ) { | ||
| Box( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .clip(RoundedCornerShape(12.dp)) | ||
| .background(KieroTheme.colors.schedule1) | ||
| .clip(RoundedCornerShape(999.dp)) |
|
|
||
| data class KidFireUiModel( | ||
| val earnedStones: List<StoneUiType> = listOf(), | ||
| val earnedStones: List<KidJourneyStoneType> = listOf(), |
There was a problem hiding this comment.
p2: List는 가변 콜렉션 (UnStable) 으로 불필요한 리컴포지션이 발생할 수 있습니다!
ImmutableList와 PersistentList의 차이를 확인하시고 어떤게 좋을지 고민하셔서 선택해주세요 😊
| fun ScheduleFireModel.toUiModel(): KidFireUiModel { | ||
| return KidFireUiModel( | ||
| earnedStones = this.gotStones.map { StoneUiType.valueOf(it) }, | ||
| earnedStones = this.gotStones.map { KidJourneyStoneType.valueOf(it) }, | ||
| earnedCoin = this.earnedCoinAmount | ||
| ) | ||
| } No newline at end of file |
There was a problem hiding this comment.
p2: 음 자세한 사항 확인은 못했긴한데..
서버에서 내려준 문자열it이 Enum 에 정해진 이름과 정확히 일치하지않으면 크래시가 발생할 위험이 있어요
그래서
- 변환 시도 후 에러는 null로 반환
fun ScheduleFireModel.toUiModel(): KidFireUiModel {
return KidFireUiModel(
earnedStones = this.gotStones.mapNotNull { stoneString ->
runCatching { KidJourneyStoneType.valueOf(stoneString) }.getOrNull()
},
earnedCoin = this.earnedCoinAmount
)
}- 그냥 있는거 찾기
fun ScheduleFireModel.toUiModel(): KidFireUiModel {
return KidFireUiModel(
earnedStones = this.gotStones.mapNotNull { stoneString ->
KidJourneyStoneType.entries.find { it.name == stoneString }
},
earnedCoin = this.earnedCoinAmount
)
}확인 후 편하신거 사용하시면 좋을 것 같습니다!
| shape = CircleShape, | ||
| shadow = Shadow( | ||
| radius = 8.dp, | ||
| spread = 0.dp, | ||
| color = KieroTheme.colors.main, | ||
| offset = DpOffset(x = 0.dp, y = 0.dp) |
There was a problem hiding this comment.
p3: 제가 알기론 기본값이 존재하는걸로 아는데 0을 넣으신 이유가 있으실까요?
괜찮으시다면 제거하여 가독성을 높여 필요한 사용 부분만 명확하게 해주시면 더욱 좋을 것 같습니다!
| package com.kiero.presentation.kid.journey.map.model | ||
|
|
||
| import com.kiero.presentation.kid.journey.model.KidJourneyStoneType | ||
|
|
There was a problem hiding this comment.
p2: @immutable 을 명시적으로 붙여주면 좋을 것 같아요
물론 지금은 val이기도 하고 원시타입 및 enum은 클래스긴 하지만 상태가 변할 수 없어서 stable이라 괜찮긴하지만 명시적 그리고
val 은 재할당 불가를 의미할 뿐 내부에 가변 상태를 가질 수 있기 때문입니다잉
| val schedules: ImmutableList<KidMapItemUiModel> = persistentListOf() | ||
| ) { | ||
| companion object { | ||
| fun FAKE() = KidMapState( |
There was a problem hiding this comment.
p2: 우선 함수는 카멜케이스죵? 소문자 부탁드리고
함수로 사용할 때는 매번 새로운 객체가 필요할 때에요
불변 객체 하나를 돌려 쓸 때에는 val fake로 만들어서 매번 생성할 필요없이 재사용 가능해요!
지금은 val fake가 좋아보입니다!
|
|
||
| private val _state = MutableStateFlow<UiState<KidMapState>>( | ||
| UiState.Success( | ||
| KidMapState.FAKE().copy(date = map.date) |
There was a problem hiding this comment.
KidMapState.FAKE().copy(date = map.date)
-> 불필요한 객체를 생성하고 있죵?
=> 원본 가짜 객체 생성 후 다시 copy하여 필요한 객체가 재생성
KidMapState.FAKE.copy(date = map.date)
-> 변수를 바로 참조 가능해요!
=> 이미 만들어져있는 변수에 진짜 필요한 객체 1번만 생성
There was a problem hiding this comment.
p2: 이거 음 백터로 뽑지말고 충분히 native로 만들 수 있을 것 같은데 어떻게 생각하시나염?
There was a problem hiding this comment.
구현전에 아요꺼 구경했는디
벡터로 뽑았길래.. 키하하
… 내 `FAKE` 데이터를 함수에서 프로퍼티로 변경


ISSUE
❗ WORK DESCRIPTION
자녀 플로우의 오늘의 여정 지도 컴포와 뷰 구현하였습니다.
추가로 스낵바와 버튼, 아이콘 등의 디자인 변경사항 반영하였습니다.
📢 TO REVIEWERS
ModifierExt.kt에 LazyColumn을 위한 자체 수직 스크롤바 확장 함수 구현했습니다. https://stackoverflow.com/questions/66341823/jetpack-compose-scrollbars
기본으로 스크롤바 만들어져 있는 건 없는 것 같아서 요거 보면서 참고 했어여
-> KidJourneyScreen의 지도 이동 플로팅 버튼, 타임라인 진행 노드(KidMapTimelineNode)의 별 모양, 스톤 뱃지(KidMapStoneBadge) 테두리, KidMapEmptyView 아이콘 등
원에 그냥 색 넣어뒀으요..
📸 SCREENSHOT
Screen_recording_20260305_182045.mp4