[feat] 공부 타이머 페이지 구현(#3)#4
Conversation
띄울 컴포넌트 RecordItemCard 추가
- ./gradlew ktlintformat 실행 - Wildcard import 제거
- kotlin.test (1.9.0) - junit.jupiter.api 추가 (6.0.1) - junit.jupiter.params 추가 (6.0.1) - junit.jupiter.engine 추가 (6.0.1) - mockk 추가 (1.14.6)
- TimerScreen 기본 UI 렌더링 테스트 추가 - RecordTabNavigationTest 추가 (기록 탭 전환 및 카드 표시 검증) - TimerRunningScreenTest 추가 (타이머 실행 화면 UI 요소 검증) - 테스트용 ViewModel 세팅 및 mock 데이터 구성 - 테스트 안정화를 위한 구조 조정 (testTag, 파라미터 정리 등)
MyKnow
left a comment
MyKnow
left a comment
There was a problem hiding this comment.
고생 많으셨습니다. 코드에 굉장한 노고가 보입니다.
PR이나 Commit하기 전에는 "./gradlew ktlintFormat"을 "꼭" 수행해주셨으면 합니다. 코드 컨벤션은 협업에서 중요한 규칙입니다.
리팩토링 관련해서 다양한 리뷰를 작성했는데, 요점은 아래와 같습니다.
- ViewModel의 역할 명확히 인지하고 Repository 패턴 적용
- Domain 모델과 UI 모델 구분
- 유지보수를 위한 디자인 시스템 사용
- UI는 최대한 잘게 쪼개서 재사용성을 늘리기
확인된 버그는 다음과 같습니다.
- androidTest failed
- 타이머 수정 > 시간 설정 시 앱 강제 종료
- 네비게이션 바 > 아직 구현이 안 된 "검색탭" 클릭 시 앱 강제 종료
시간이 촉박하기에 ktlintFormat을 수행하고 wildcard를 수작업으로 제거하신 후 Merge하시면 됩니다. 리팩토링은 추후에 진행하시면 됩니다.
| // ✅ 기록 데이터 (내부 mutable, 외부는 읽기 전용으로 노출) | ||
| private val _records = mutableStateListOf( | ||
| RecordData( | ||
| id = "0", | ||
| title = "전체 시간", | ||
| color = Color(0xFFFFFFFF), | ||
| totalTime = "02:10", | ||
| dayCounts = mapOf(2 to 1, 3 to 3, 4 to 5) | ||
| ), | ||
| RecordData( | ||
| id = "1", | ||
| title = "자바스크립트", | ||
| color = Color(0xFFFF8686), | ||
| totalTime = "01:20", | ||
| dayCounts = mapOf(1 to 2, 2 to 5, 3 to 6) | ||
| ), | ||
| RecordData( | ||
| id = "2", | ||
| title = "파이썬", | ||
| color = Color(0xFFFFE382), | ||
| totalTime = "00:45", | ||
| dayCounts = mapOf(1 to 1, 3 to 1, 4 to 1) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
현재 ViewModel 안에 실제 데이터를 하드코딩 하고 있는 것으로 보이는데, 이는 "상태관리"가 역할인 ViewModel에 "데이터 소스" 역할을 동시에 부여하기에 SRP를 위반하고 있다고 생각됩니다.
| fun getDailyGroupedRecords(day: Int): Map<String, List<DailyFocus>> { | ||
| val result = mutableMapOf<String, MutableList<DailyFocus>>() | ||
|
|
||
| records.filter { it.id != "0" }.forEach { record -> | ||
| val count = record.dayCounts[day] ?: return@forEach | ||
|
|
||
| // 예시: count 만큼 더미 데이터 생성 (지금은 더미 → 나중에 실제 데이터 연결) | ||
| repeat(count) { | ||
| val item = DailyFocus( | ||
| timerTitle = record.title, | ||
| color = record.color, | ||
| start = "13:00:00", | ||
| end = "13:10:00", | ||
| duration = "00:10:00" | ||
| ) | ||
| result.getOrPut(record.title) { mutableListOf() }.add(item) | ||
| } | ||
| } | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| fun getRecordDetailByDay(day: Int): List<RecordDetailItem> { | ||
|
|
||
| // TODO: 실제 기록 DB 연동 시 교체 가능 | ||
| val example = listOf( | ||
| RecordDetailItem( | ||
| title = "자바스크립트", | ||
| color = Color(0xFFFF8686), | ||
| totalTimeFormatted = "00:20:00", | ||
| records = listOf( | ||
| RecordDetailRowData("13:00:00", "13:10:00", "00:10:00"), | ||
| RecordDetailRowData("14:00:00", "14:10:00", "00:10:00") | ||
| ) | ||
| ), | ||
| RecordDetailItem( | ||
| title = "파이썬", | ||
| color = Color(0xFFFFE382), | ||
| totalTimeFormatted = "00:10:00", | ||
| records = listOf( | ||
| RecordDetailRowData("13:00:00", "13:10:00", "00:10:00") | ||
| ) | ||
| ) | ||
| ) | ||
|
|
||
| return example | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
UI Mock을 위해 ViewModel이 Mock data와 강하게 결합되어 있으면 유지보수가 어렵고, 추후 실제 DB 연결 시 전체 구조를 다시 작성해야 합니다.
| import androidx.compose.runtime.mutableStateListOf | ||
| import androidx.compose.runtime.mutableStateOf | ||
| import androidx.compose.runtime.setValue | ||
| import androidx.compose.ui.graphics.Color |
There was a problem hiding this comment.
Color 같은 Compose UI 타입이 ViewModel 내부 모델에 포함되어 있어, ViewModel이 UI 레이어에 직접적으로 의존하고 있습니다.
이는 MVVM 계층 분리를 완전히 깨는 구조입니다.
| package com.sprinter.studyspot.timer.model | ||
|
|
||
| import androidx.compose.ui.graphics.Color | ||
|
|
||
| data class RecordDetailItem( | ||
| val title: String, // 타이머명 | ||
| val color: Color, // 컬러 | ||
| val totalTimeFormatted: String, // "00:20:00" | ||
| val records: List<RecordDetailRowData> // 시작/종료/집중 세부 기록 | ||
| ) |
There was a problem hiding this comment.
TimeViewModel에서 사용하는 모델이라면 Domain 모델을 기준으로 구성되어야 합니다.
그러나 RecordDetailItem은 Compose Color와 formatted string을 가지고 있어
UI 전용 모델입니다. ViewModel이 UI 타입에 직접 의존하게 되어 MVVM 구조가 깨집니다.
이 모델은 UI 변환 모델(UI Model)로 이동하고,
TimeViewModel은 Domain 모델만 보관한 뒤 UI 모델은 화면에서 매핑하도록
구조를 분리하는 것이 좋을 것으로 보입니다.
| package com.sprinter.studyspot.timer.model | ||
|
|
||
| data class RecordDetailRowData( | ||
| val start: String, // ex: "13:00:00" | ||
| val end: String, // ex: "13:10:00" | ||
| val duration: String // ex: "00:10:00" | ||
| ) |
There was a problem hiding this comment.
RecordDetailRowData는 필드들이 모두 String으로 구성되어 있어,Domain 모델이 아니라 UI 모델입니다. Domain 계층에서는 시간을 문자열로 보관하면 계산, 정렬, 비교가 불가능해지며 Repository/DB/네트워크 계층과도 호환되지 않습니다.
이 모델은 UI 표시용 모델(UI Model)로 이동해야 하고, Domain에서는 raw 시간값(epoch, seconds 등)으로 구성된 모델을 사용한 뒤 ViewModel에서 UI 모델로 매핑하는 구조가 맞습니다.
| @Composable | ||
| private fun BottomNavItem( | ||
| icon: Int, | ||
| label: String, | ||
| isSelected: Boolean, | ||
| onClick: () -> Unit, | ||
| ) { | ||
| Column( | ||
| modifier = | ||
| Modifier | ||
| .width(60.dp) | ||
| .clickable(onClick = onClick), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(id = icon), | ||
| contentDescription = label, | ||
| modifier = Modifier.size(20.dp), | ||
| tint = Color.Black, | ||
| ) | ||
| Spacer(modifier = Modifier.height(4.dp)) | ||
| Text( | ||
| text = label, | ||
| fontSize = 12.sp, | ||
| color = Color.Black, | ||
| fontWeight = FontWeight.Medium, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Icon tint가 Color.Black으로 고정되어 있어 다크 모드, Theme, ColorScheme 적용이 불가능한 구조입니다.
isSelected 여부와 MaterialTheme.colorScheme을 기반으로 동적으로 적용하는 방식이 필요할 것으로 보입니다.
| package com.sprinter.studyspot.timer.ui.component | ||
|
|
There was a problem hiding this comment.
DailyFocus 모델은 현재 TimeViewModel에서만 사용되고 있고, Color와 포맷된 문자열을 포함하고 있어서 사실상 UI 모델입니다. ViewModel이 UI 타입에 의존하게 되어 MVVM 구조가 깨지고 있습니다.
리팩토링을 추천드립니다.
- 이름을 DailyFocusUiModel로 변경하고 ui.model 패키지로 이동
- ViewModel은 Domain 모델(raw 시간, 색상 값 String 등)만 가지고, UI 표현용 데이터는 UI에서 매핑하도록 구조를 분리하는 것이 적절합니다.
| package com.sprinter.studyspot.timer.ui.component | ||
|
|
||
| import androidx.compose.ui.graphics.Color | ||
| import java.util.UUID | ||
|
|
||
| data class TimerItemData( | ||
| val id: String = UUID.randomUUID().toString(), // ✅ 고유 식별자 추가 | ||
| val title: String, | ||
| val color: Color, | ||
| val hour: Int, | ||
| val minute: Int, | ||
| val second: Int, | ||
| ) |
There was a problem hiding this comment.
- TimerItemData는 UI 타입(Color)을 포함하고 있어 UI 모델로 한정됩니다. ViewModel이나 Domain 계층에서 직접 사용하면 MVVM 구조가 깨집니다.
- 시간 단위를 hour, minute, second로 나눈 것은 직관적이지만 계산이나 정렬 로직이 필요할 경우 totalSeconds 등 단일 값으로 처리하는 것이 더 효율적일 것으로 생각됩니다.
- id를 UUID 기본값으로 두는 것은 편리하지만 단위 테스트에서는 예측 불가능성을 유발할 수 있어 필요시 생성자를 통한 주입 방식을 고려하는 것이 좋아보입니다.
- 전체적으로 UI 모델임을 명확히 하기 위해 이름을 TimerItemUiModel로 변경하면 혼동을 줄일 수 있습니다.
| @@ -0,0 +1,190 @@ | |||
| // timer/ui/component/SwipeableStudyItem.kt | |||
There was a problem hiding this comment.
하드코딩된 색상/크기/spacing은 추후 유지보수에 취약합니다. 별도의 Theme 관리 파일로 분리하는 것이 좋을 것 같습니다.
| @@ -0,0 +1,64 @@ | |||
| package com.sprinter.studyspot.timer.ui.component | |||
There was a problem hiding this comment.
하드코딩된 색상(Color.Black, Color.Gray)과 spacing 값(28.dp, 34.dp 등)이 많아 다크 모드와 Theme 대응이 어렵습니다. 또한 Magic Number를 관리하는 파일 생성을 추천드립니다.
2 participants
🧩 관련 Issue
Closes #3
🎯 작업 개요
타이머 페이지, 타이머 수정 페이지, 타이머 진행 창, 타이머 기록 페이지, 타이머 기록 상세 페이지 구현
🔍 상세 내용
홈 화면에서 타이머 페이지가 첫 화면으로 뜨게 되고 탭 바에서 "타이머" 선택 시 뜨는 화면에서 + 아이콘을 통해서 타이머를 추가하고 오른쪽으로 슬라이드하여 타이머를 수정 및 삭제할 수 있습니다.
타이머 색상, 이름, 시간 설정을 진행 후 타이머 박스를 눌러 타이머를 진행시킵니다.
홈 화면의 탭 바에서 "기록"을 선택 시 전체 시간 및 내가 설정한 타이머에 대한 일주일 치 기록과 총 시간이 보여지게 되고 카드를 클릭하게 되면 1달치의 기록이 보여지게 됩니다. 한 블럭을 클릭하게 되면 그 날짜에 해당하는 타이머 기록들이 보여지게 됩니다.
✅ 체크리스트
💬 참고 사항