Conversation
DongChyeon
force-pushed
the
feature/#125-schedule-ui
branch
from
8ca6cd2 to
52377ab
Compare
|
release/1.0.0 기준으로 파고 bottom-navigation 쪽 브랜치를 리베이스했더니 생긴 현상같은데.... 잠시만요! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 19
🛑 Comments failed to post (19)
feature/notice/src/main/java/com/yapp/feature/notice/notice/NoticeRoute.kt (1)
171-174: 🧹 Nitpick (assertive)
무한 스크롤 기능 구현
OnBottomReached확장 함수를 사용하여 사용자가 목록의 끝에 도달했을 때LoadMoreNoticeItem인텐트를 트리거하는 구현이 잘 되었습니다. 이는 사용자 경험을 향상시키는 좋은 구현입니다.다만, 페이지 로딩 상태를 나타내는 로딩 인디케이터 추가를 고려해 보는 것도 좋을 것 같습니다. 사용자가 스크롤 끝에 도달했을 때 추가 데이터가 로드 중임을 시각적으로 표시하면 더 나은 UX를 제공할 수 있습니다.
feature/notice/src/main/java/com/yapp/feature/notice/notice/NoticeViewModel.kt (2)
56-59: 🛠️ Refactor suggestion
데이터 결합 순서에 주의가 필요합니다.
copy(notices = noticeList.copy(notices = noticeList.notices + state.notices.notices), isNoticesLoading = false)현재 구현은 새로 로드된 항목을 기존 항목 앞에 추가하고 있습니다 (
noticeList.notices + state.notices.notices). 일반적인 페이징에서는 새 항목을 기존 목록의 끝에 추가하는 것이 자연스럽습니다. 의도적인 설계가 아니라면 순서를 확인해보세요.-reduce { copy(notices = noticeList.copy(notices = noticeList.notices + state.notices.notices), isNoticesLoading = false) } +reduce { copy(notices = noticeList.copy(notices = state.notices.notices + noticeList.notices), isNoticesLoading = false) }
46-60: 🛠️ Refactor suggestion
오류 처리 로직이 필요합니다.
API 호출이 실패했을 때의 처리 로직이 없습니다. 네트워크 오류 또는 서버 오류가 발생했을 때 사용자에게 적절한 피드백을 제공하고, 로딩 상태를 해제하는 로직을 추가하는 것이 좋습니다.
private fun loadNotices( state: NoticeState, reduce: (NoticeState.() -> NoticeState) -> Unit, ) = viewModelScope.launch { if (state.isNoticeEmpty) { reduce { copy(isNoticesLoading = true) } } try { getNoticeListRepository .getNoticeList(state.notices.lastNoticeId.ifEmpty { null }, 30, state.noticeType.apiValue) .collectLatest { noticeList -> reduce { copy(notices = noticeList.copy(notices = state.notices.notices + noticeList.notices), isNoticesLoading = false) } } } catch (e: Exception) { reduce { copy( isNoticesLoading = false, error = e.message ?: "알 수 없는 오류가 발생했습니다." ) } } }Committable suggestion skipped: line range outside the PR's diff.
core/data/build.gradle.kts (1)
18-24: 🧹 Nitpick (assertive)
local.properties직접 로드 방식 검토
매번 루트 프로젝트의local.properties파일을 로드하고 있습니다. 파일이 없거나 키가 누락될 경우 빌드 에러로 직결되므로, 존재 여부 체크 및 키 검증 로직을 추가하여 사용자 경험을 개선할 수 있습니다.val propertiesFile = project.rootProject.file("local.properties") +if (!propertiesFile.exists()) { + throw GradleException("local.properties 파일이 없습니다. 빌드 설정을 확인하세요.") +} localProperties.load(propertiesFile.bufferedReader()) val debugUrl = localProperties.getProperty("base.url.debug") + ?: throw GradleException("base.url.debug 설정이 누락되었습니다.")Committable suggestion skipped: line range outside the PR's diff.
core/ui/src/main/java/com/yapp/core/ui/extension/Modifier.kt (1)
31-51: 🧹 Nitpick (assertive)
점선 테두리를 그리는 확장 함수가 추가되었습니다.
dashedBorder확장 함수는 컴포저블에 점선 테두리를 쉽게 적용할 수 있게 해주는 유용한 기능입니다. 색상, 두께, 대시 길이, 간격, 모서리 반경 등 다양한 커스터마이징이 가능하도록 매개변수가 잘 설계되었습니다.코드 작성도 깔끔하고 기능적으로도 완성도가 높습니다. 다만, 아래와 같은 개선 사항을 고려해 볼 수 있습니다:
- 함수 문서화(KDoc) 추가하기 - 함수의 용도와 매개변수 설명을 추가하면 사용자가 더 쉽게 이해할 수 있습니다.
- 복잡한 배경이나 그라데이션 등이 있는 경우에 대한 처리 방법 고려하기
아래와 같이 KDoc을 추가하는 것을 고려해보세요:
+/** + * 컴포저블 주위에 사용자 정의 점선 테두리를 그리는 Modifier 확장 함수입니다. + * + * @param color 테두리의 색상 + * @param strokeWidth 테두리의 두께 + * @param dashLength 각 대시의 길이 + * @param gapLength 대시 사이의 간격 + * @param cornerRadius 테두리 모서리의 둥근 정도 + */ fun Modifier.dashedBorder( color: Color, strokeWidth: Dp = 1.dp, dashLength: Dp = 10.dp, gapLength: Dp = 5.dp, cornerRadius: Dp = 0.dp ): Modifier = drawBehind {📝 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./** * 컴포저블 주위에 사용자 정의 점선 테두리를 그리는 Modifier 확장 함수입니다. * * @param color 테두리의 색상 * @param strokeWidth 테두리의 두께 * @param dashLength 각 대시의 길이 * @param gapLength 대시 사이의 간격 * @param cornerRadius 테두리 모서리의 둥근 정도 */ fun Modifier.dashedBorder( color: Color, strokeWidth: Dp = 1.dp, dashLength: Dp = 10.dp, gapLength: Dp = 5.dp, cornerRadius: Dp = 0.dp ): Modifier = drawBehind { val stroke = Stroke( width = strokeWidth.toPx(), pathEffect = PathEffect.dashPathEffect( floatArrayOf(dashLength.toPx(), gapLength.toPx()), 0f ) ) drawRoundRect( color = color, size = size, cornerRadius = CornerRadius(cornerRadius.toPx()), style = stroke ) }feature/schedule/src/main/java/com/yapp/feature/schedule/component/IconWithText.kt (1)
14-36: 🧹 Nitpick (assertive)
아이콘과 텍스트를 함께 표시하는 재사용 가능한 컴포넌트 구현이 잘 되었습니다.
아이콘과 텍스트를 함께 표시하는 간결하고 재사용 가능한 컴포넌트가 잘 구현되었습니다. Row를 사용하여 아이템을 수평으로 배치하고, 적절한 간격과 정렬을 설정했습니다.
개선할 수 있는 부분:
- 더 다양한 상황에서 유연하게 사용할 수 있도록
modifier파라미터를 추가하는 것을 고려해 보세요.@Composable internal fun IconWithText( + modifier: Modifier = Modifier, iconResId: Int, text: String, contentDescription: String? = null ) { Row( + modifier = modifier, horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically ) { // 기존 코드 유지 } }📝 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.@Composable internal fun IconWithText( modifier: Modifier = Modifier, iconResId: Int, text: String, contentDescription: String? = null ) { Row( modifier = modifier, horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically ) { Icon( painter = painterResource(id = iconResId), contentDescription = contentDescription, tint = Color.Unspecified ) Text( text = text, style = YappTheme.typography.caption1Regular, color = YappTheme.colorScheme.labelAlternative ) } }feature/schedule/src/main/java/com/yapp/feature/schedule/component/TodaySessionSection.kt (3)
57-65: 🛠️ Refactor suggestion
문자열 하드코딩 대신 문자열 리소스를 사용하세요.
"D-"와 "D-day" 같은 문자열이 하드코딩되어 있습니다. 이러한 문자열은 strings.xml 파일에 정의하여 다국어 지원 및 일관된 용어 사용이 가능하도록 하는 것이 좋습니다.
YappChipSmall( text = if (remainingDays > 0) { - "D-${remainingDays}" + stringResource(R.string.d_day_format, remainingDays) } else { - "D-day" + stringResource(R.string.d_day) }, colorType = ChipColorType.Main, isFill = true )strings.xml에 다음 문자열을 추가하세요:
<string name="d_day_format">D-%d</string> <string name="d_day">D-day</string>
69-72: 🧹 Nitpick (assertive)
날짜 형식 문자열 연결 개선
날짜와 요일을 연결할 때 문자열 템플릿을 사용하는 것은 좋지만, 이 부분도 문자열 리소스를 활용하면 더 좋을 것 같습니다.
Text( - text = "$date ($dayOfWeek)", + text = stringResource(R.string.date_with_day_of_week, date, dayOfWeek), style = YappTheme.typography.caption1Medium, color = YappTheme.colorScheme.labelNeutral )strings.xml에 다음 문자열을 추가하세요:
<string name="date_with_day_of_week">%1$s (%2$s)</string>
38-43: 🛠️ Refactor suggestion
하드코딩된 색상 값을 테마에서 정의된 색상으로 대체하세요.
현재 dashed border 색상이 하드코딩되어 있습니다(
Color(0xFFFED9CB)). 테마에서 정의된 색상을 사용하면 앱 전체의 일관성을 유지하고 다크 모드 등 테마 변경 시 자동으로 적응할 수 있습니다..dashedBorder( - color = Color(0xFFFED9CB), + color = YappTheme.colorScheme.accentOrangeWeak, // 또는 적절한 테마 색상 dashLength = 2.dp, gapLength = 2.dp, cornerRadius = 10.dp, )📝 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..dashedBorder( color = YappTheme.colorScheme.accentOrangeWeak, dashLength = 2.dp, gapLength = 2.dp, cornerRadius = 10.dp, )feature/schedule/src/main/java/com/yapp/feature/schedule/component/AttendanceStatusChip.kt (2)
27-31: 🛠️ Refactor suggestion
하드코딩된 텍스트 대신 문자열 리소스를 사용하세요.
출석 상태에 따른 텍스트 값("예정", "출석", "지각")이 코드에 하드코딩되어 있습니다. 이러한 텍스트는 strings.xml 파일에 정의하여 다국어 지원 및 일관된 용어 사용이 가능하도록 하는 것이 좋습니다.
val text = when (status) { - AttendanceStatus.SCHEDULED -> "예정" - AttendanceStatus.ATTENDED -> "출석" - AttendanceStatus.LATE -> "지각" + AttendanceStatus.SCHEDULED -> stringResource(R.string.attendance_scheduled) + AttendanceStatus.ATTENDED -> stringResource(R.string.attendance_attended) + AttendanceStatus.LATE -> stringResource(R.string.attendance_late) }strings.xml에 다음 문자열을 추가하세요:
<string name="attendance_scheduled">예정</string> <string name="attendance_attended">출석</string> <string name="attendance_late">지각</string>
23-26: 🧹 Nitpick (assertive)
Modifier 파라미터 추가 제안
이 컴포넌트를 다양한 컨텍스트에서 더 유연하게 사용할 수 있도록
modifier파라미터를 추가하는 것이 좋습니다.@Composable internal fun AttendanceStatusChip( + modifier: Modifier = Modifier, status: AttendanceStatus, ) { // ... Box( - modifier = Modifier + modifier = modifier .background( color = backgroundColor, shape = RoundedCornerShape(6.dp) ) .padding( horizontal = 8.dp, vertical = 2.dp ) ) { // ... } }📝 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.@Composable internal fun AttendanceStatusChip( modifier: Modifier = Modifier, status: AttendanceStatus, ) { // ... Box( modifier = modifier .background( color = backgroundColor, shape = RoundedCornerShape(6.dp) ) .padding( horizontal = 8.dp, vertical = 2.dp ) ) { // ... } }feature/schedule/src/main/java/com/yapp/feature/schedule/component/AssignmentItem.kt (1)
38-84: 🧹 Nitpick (assertive)
접근성 개선을 위한 콘텐츠 설명 추가를 고려해보세요.
현재 컴포넌트에는 접근성을 위한 콘텐츠 설명이 없습니다. 스크린 리더 사용자를 위해 과제 항목의 중요한 정보를 포함한 설명을 추가하는 것이 좋을 것 같습니다.
Row( modifier = Modifier .fillMaxWidth() .clickable { onClick(id) } .graphicsLayer { if (isPast) alpha = 0.5f } + .semantics { + contentDescription = "$date $dayOfWeek $title $content ${if (isPast) "지난 과제" else if (isToday) "오늘 과제" else "예정 과제"}" + } .padding( horizontal = 20.dp, vertical = 16.dp ) )📝 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.Row( modifier = Modifier .fillMaxWidth() .clickable { onClick(id) } .graphicsLayer { if (isPast) alpha = 0.5f } .semantics { contentDescription = "$date $dayOfWeek $title $content ${if (isPast) "지난 과제" else if (isToday) "오늘 과제" else "예정 과제"}" } .padding( horizontal = 20.dp, vertical = 16.dp ) ) { Row( horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically, ) { Text( text = date, style = YappTheme.typography.body1NormalBold, color = dateColor ) Text( text = "(${dayOfWeek})", style = YappTheme.typography.caption1Medium, color = dateColor ) } Spacer(modifier = Modifier.width(20.dp)) Column(modifier = Modifier.weight(1f)) { Spacer(modifier = Modifier.height(2.dp)) Text( text = title, style = YappTheme.typography.label1NormalBold, color = YappTheme.colorScheme.labelNormal ) Spacer(modifier = Modifier.height(6.dp)) Text( text = content, style = YappTheme.typography.caption1Regular, color = YappTheme.colorScheme.labelAlternative ) } }feature/schedule/src/main/java/com/yapp/feature/schedule/component/SessionItem.kt (2)
84-96: 💡 Verification agent
🧩 Analysis chain
IconWithText 컴포넌트 사용이 적절합니다.
시간과 위치 정보를 아이콘과 함께 표시하는 접근 방식이 좋습니다. 다만, 접근성을 위해 contentDescription에 null 대신 의미 있는 설명을 추가하는 것이 좋겠습니다.
아이콘에 대한 접근성 설명을 추가하여 스크린 리더 사용자의 경험을 개선해 보세요. 다음 코드를 확인해주세요:
🏁 Script executed:
#!/bin/bash # IconWithText 컴포넌트가 어떻게 구현되어 있는지 확인 fd -e kt IconWithText -x cat {} \;Length of output: 1113
접근성 개선: IconWithText의 contentDescription 추가 필요
SessionItem.kt에서 시간·위치 아이콘에contentDescription = null을 사용하고 있는데, 스크린 리더 사용자 경험을 위해 의미 있는 설명을 지정해주세요.• 대상 위치
- feature/schedule/src/main/java/com/yapp/feature/schedule/component/SessionItem.kt (약 84–96라인)
• 수정 예시
IconWithText( iconResId = R.drawable.icon_time, text = time, contentDescription = "시간: $time" // 또는 stringResource(R.string.cd_session_time) 활용 ) Spacer(modifier = Modifier.height(4.dp)) IconWithText( iconResId = R.drawable.icon_location, text = location, contentDescription = "장소: $location" // 또는 stringResource(R.string.cd_session_location) 활용 )• 권장
- 문자열 리소스(
strings.xml)에 접근성 설명 추가 후stringResource()로 불러오기- 아이콘이 전달하는 의미를 간결하게 설명
위 변경으로 스크린 리더 사용자의 접근성을 개선할 수 있습니다.
23-37: 🧹 Nitpick (assertive)
파라미터가 많은 컴포넌트, 데이터 클래스 사용을 고려해 보세요.
현재 SessionItem 컴포넌트는 많은 파라미터를 받고 있습니다. 이는 사용성을 저하시킬 수 있습니다. 데이터 클래스를 사용하여 파라미터를 그룹화하는 것이 더 나은 접근 방식일 수 있습니다.
+data class SessionItemData( + val id: Long, + val title: String, + val assignmentTitle: String? = null, + val assignmentContent: String? = null, + val status: AttendanceStatus, + val date: String, + val dayOfWeek: String, + val isPast: Boolean = false, + val isToday: Boolean = false, + val location: String, + val time: String, +) @Composable internal fun SessionItem( - id: Long, - title: String, - assignmentTitle: String? = null, - assignmentContent: String? = null, - status: AttendanceStatus, - date: String, - dayOfWeek: String, - isPast: Boolean = false, - isToday: Boolean = false, - location: String, - time: String, + data: SessionItemData, onClick: (Long) -> Unit, ) { val dateColor = if (isToday) {이렇게 변경하면 사용하는 쪽에서도 더 깔끔하게 코드를 작성할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/ScheduleTabRow.kt (3)
104-116: 🧹 Nitpick (assertive)
프리뷰 함수 이름 불일치
프리뷰 함수 이름이
ExploreTabRowPreview로 되어 있는데, 컴포넌트 이름인ScheduleTabRow와 일치시키는 것이 더 일관성 있을 것 같습니다.-fun ExploreTabRowPreview() { +fun ScheduleTabRowPreview() {
58-58: 🧹 Nitpick (assertive)
코드 스타일 개선: 연산자 주변에 공백 추가 필요
selectedTabIndex== index에서 연산자==앞뒤로 공백이 일관되게 사용되어야 합니다.- selected = selectedTabIndex== index, + selected = selectedTabIndex == index,Committable suggestion skipped: line range outside the PR's diff.
74-90: 🧹 Nitpick (assertive)
탭 텍스트에 접근성 지원 개선 필요
탭 텍스트에 대한 접근성 지원이 부족합니다. 스크린 리더 사용자를 위해 추가 정보(예: 현재 선택 상태)를 제공하는 것이 좋습니다.
Text( modifier = Modifier.padding(6.dp), text = label, maxLines = 1, overflow = TextOverflow.Ellipsis, style = YappTheme.typography.body1ReadingRegular, color = textColor + .semantics { + contentDescription = "$label ${if (selected) "탭, 선택됨" else "탭"}" + } )Committable suggestion skipped: line range outside the PR's diff.
feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleScreen.kt (2)
243-276: 🛠️ Refactor suggestion
MonthHeader의 접근성 개선 필요
달력 네비게이션 아이콘에 contentDescription이 null로 설정되어 있습니다. 접근성을 위해 의미 있는 설명을 추가하는 것이 좋습니다.
Icon( modifier = Modifier.clickable { onPreviousMonthClick() }, painter = painterResource(id = R.drawable.icon_arrow_left), - contentDescription = null, + contentDescription = "이전 달로 이동", tint = Color.Unspecified ) // ... Icon( modifier = Modifier.clickable { onNextMonthClick() }, painter = painterResource(id = R.drawable.icon_arrow_right), - contentDescription = null, + contentDescription = "다음 달로 이동", tint = Color.Unspecified )📝 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.@Composable private fun MonthHeader( modifier: Modifier = Modifier, year: Int, month: Int, onPreviousMonthClick: () -> Unit, onNextMonthClick: () -> Unit, ) { Row( modifier = modifier, horizontalArrangement = Arrangement.spacedBy(8.dp), verticalAlignment = Alignment.CenterVertically ) { Icon( modifier = Modifier.clickable { onPreviousMonthClick() }, painter = painterResource(id = R.drawable.icon_arrow_left), contentDescription = "이전 달로 이동", tint = Color.Unspecified ) Text( text = "${year}년 ${month}월", style = YappTheme.typography.headline1Bold, color = YappTheme.colorScheme.labelNormal, ) Icon( modifier = Modifier.clickable { onNextMonthClick() }, painter = painterResource(id = R.drawable.icon_arrow_right), contentDescription = "다음 달로 이동", tint = Color.Unspecified ) } }
74-135: 🧹 Nitpick (assertive)
ScheduleAllScreen의 더미 데이터 관리 개선 필요
현재 for 루프를 사용하여 더미 데이터를 생성하고 있습니다. 실제 데이터로 교체할 때를 고려하여 리스트와 데이터 클래스를 사용하는 구조로 개선하면 좋을 것 같습니다.
+ // 실제 구현시 ViewModel에서 데이터를 받아오는 방식으로 변경 + data class SessionItemUiState( + val id: Long, + val title: String, + val assignmentTitle: String? = null, + val assignmentContent: String? = null, + val status: AttendanceStatus, + val date: String, + val dayOfWeek: String, + val isToday: Boolean, + val isPast: Boolean, + val location: String, + val time: String, + ) + + // 더미 데이터 리스트 (실제 구현시 제거) + val dummySessions = List(7) { index -> + SessionItemUiState( + id = index.toLong(), + title = "세션 제목", + assignmentTitle = if (index == 1) "과제 제목" else null, + assignmentContent = if (index == 1) "과제 내용" else null, + status = when (index % 3) { + 0 -> AttendanceStatus.SCHEDULED + 1 -> AttendanceStatus.ATTENDED + else -> AttendanceStatus.LATE + }, + date = "${9 - index}", + dayOfWeek = "금", + isToday = index == 0, + isPast = index > 0, + location = "공덕 창업허브", + time = "오후 2시 - 오후 6시" + ) + } LazyColumn { // ... - for (index in 0..6) { - item { - SessionItem( - id = index.toLong(), - title = "세션 제목", - assignmentTitle = if (index == 1) "과제 제목" else null, - assignmentContent = if (index == 1) "과제 내용" else null, - status = statuses[index % statuses.size], - date = "${9 - index}", - dayOfWeek = "금", - isToday = index == 0, - isPast = index > 0, - location = "공덕 창업허브", - time = "오후 2시 - 오후 6시", - onClick = {} - ) - // ... Spacer 코드 - } - } + items(dummySessions) { session -> + SessionItem( + id = session.id, + title = session.title, + assignmentTitle = session.assignmentTitle, + assignmentContent = session.assignmentContent, + status = session.status, + date = session.date, + dayOfWeek = session.dayOfWeek, + isToday = session.isToday, + isPast = session.isPast, + location = session.location, + time = session.time, + onClick = {} + ) + // ... Spacer 코드 + }📝 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.@Composable private fun ScheduleAllScreen() { // 실제 구현시 ViewModel에서 데이터를 받아오는 방식으로 변경 data class SessionItemUiState( val id: Long, val title: String, val assignmentTitle: String? = null, val assignmentContent: String? = null, val status: AttendanceStatus, val date: String, val dayOfWeek: String, val isToday: Boolean, val isPast: Boolean, val location: String, val time: String ) // 더미 데이터 리스트 (실제 구현시 제거) val dummySessions = List(7) { index -> SessionItemUiState( id = index.toLong(), title = "세션 제목", assignmentTitle = if (index == 1) "과제 제목" else null, assignmentContent = if (index == 1) "과제 내용" else null, status = when (index % 3) { 0 -> AttendanceStatus.SCHEDULED 1 -> AttendanceStatus.ATTENDED else -> AttendanceStatus.LATE }, date = "${9 - index}", dayOfWeek = "금", isToday = index == 0, isPast = index > 0, location = "공덕 창업허브", time = "오후 2시 - 오후 6시" ) } LazyColumn { item { Spacer(modifier = Modifier.height(20.dp)) MonthHeader( modifier = Modifier.padding(horizontal = 20.dp), year = 2024, month = 12, onPreviousMonthClick = {}, onNextMonthClick = {} ) Spacer(modifier = Modifier.height(8.dp)) } val statuses = listOf( AttendanceStatus.SCHEDULED, AttendanceStatus.ATTENDED, AttendanceStatus.LATE ) items(dummySessions) { session -> SessionItem( id = session.id, title = session.title, assignmentTitle = session.assignmentTitle, assignmentContent = session.assignmentContent, status = session.status, date = session.date, dayOfWeek = session.dayOfWeek, isToday = session.isToday, isPast = session.isPast, location = session.location, time = session.time, onClick = {} ) Spacer( modifier = Modifier .fillMaxWidth() .height(1.dp) .padding(horizontal = 20.dp) .background(YappTheme.colorScheme.lineNormalAlternative) ) } item { AssignmentItem( id = 0, title = "과제 제목", content = "과제 내용", date = "2", dayOfWeek = "금", isToday = false, isPast = true, onClick = {} ) } } }
DongChyeon
force-pushed
the
feature/#125-schedule-ui
branch
3 times, most recently
from
603c63e to
7bab9d7
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 변경 사항은 일정(Schedule) 기능의 UI를 전면적으로 도입합니다. 새로운 일정 화면과 관련된 컴포저블 함수, UI 컴포넌트(세션 아이템, 과제 아이템, 출석 상태 칩, 탭 행 등), 그리고 관련 아이콘 및 문자열 리소스가 추가되었습니다. 디자인 시스템의 색상 팔레트가 확장되었고, Modifier에 대시 테두리 확장 함수가 도입되었습니다. 네비게이션 그래프와 Gradle 설정도 일정 모듈을 반영하도록 수정되었습니다. 모든 변경은 UI 구조와 스타일에 집중되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ScheduleScreen
participant ScheduleTabRow
participant SessionItem
participant AssignmentItem
User->>ScheduleScreen: 화면 진입
ScheduleScreen->>ScheduleTabRow: 탭 렌더링 및 선택 상태 관리
ScheduleScreen->>SessionItem: 세션 리스트 렌더링
ScheduleScreen->>AssignmentItem: 과제 아이템 렌더링
User->>ScheduleTabRow: 탭 선택
ScheduleTabRow->>ScheduleScreen: 탭 인덱스 변경 콜백
ScheduleScreen->>ScheduleTabRow: UI 갱신
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@TaeseongYun 넵 다시 정리했습니다 |
There was a problem hiding this comment.
Actionable comments posted: 17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Color.kt(6 hunks)core/ui/src/main/java/com/yapp/core/ui/extension/Modifier.kt(2 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleScreen.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/AssignmentItem.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/AttendanceStatusChip.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/IconWithText.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/ScheduleTabRow.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/SessionItem.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/TodaySessionSection.kt(1 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/navigation/ScheduleNavigation.kt(2 hunks)feature/schedule/src/main/res/drawable/icon_arrow_left.xml(1 hunks)feature/schedule/src/main/res/drawable/icon_arrow_right.xml(1 hunks)feature/schedule/src/main/res/drawable/icon_location.xml(1 hunks)feature/schedule/src/main/res/drawable/icon_time.xml(1 hunks)feature/schedule/src/main/res/values/strings.xml(1 hunks)settings.gradle.kts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
feature/schedule/src/main/java/com/yapp/feature/schedule/navigation/ScheduleNavigation.kt (1)
feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleScreen.kt (1)
ScheduleRoute(37-42)
feature/schedule/src/main/java/com/yapp/feature/schedule/component/AttendanceStatusChip.kt (1)
core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Theme.kt (1)
YappTheme(9-24)
feature/schedule/src/main/java/com/yapp/feature/schedule/component/ScheduleTabRow.kt (1)
core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Theme.kt (1)
YappTheme(9-24)
feature/schedule/src/main/java/com/yapp/feature/schedule/component/AssignmentItem.kt (1)
core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Theme.kt (1)
YappTheme(9-24)
feature/schedule/src/main/java/com/yapp/feature/schedule/component/TodaySessionSection.kt (3)
core/designsystem/src/main/java/com/yapp/core/designsystem/component/chip/Chip.kt (1)
YappChipSmall(79-102)feature/schedule/src/main/java/com/yapp/feature/schedule/component/IconWithText.kt (1)
IconWithText(14-36)core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Theme.kt (1)
YappTheme(9-24)
🔇 Additional comments (26)
feature/schedule/src/main/res/drawable/icon_arrow_right.xml (1)
7-12: 아이콘 SVG 구조 적합
<clip-path>로 화살표 형태를 정의하고, 그 아래<path>로 배경을 마스킹하는 구조가 의도대로 동작합니다. 크기, 뷰포트, 불투명도 설정이 일관되고, 다른 화살표 아이콘과 스타일이 잘 매칭됩니다.feature/schedule/src/main/res/drawable/icon_arrow_left.xml (1)
7-12: 아이콘 SVG 구조 적합
우측 화살표 아이콘(icon_arrow_right.xml)과 대칭되는 좌측 화살표 구현이 일관된 방식으로 적용되었습니다. 크기, 뷰포트, 불투명도 설정이 동일하여 테마와 매칭이 잘 되고 있습니다.feature/schedule/src/main/res/drawable/icon_time.xml (1)
7-13: 시계 아이콘 경로 정의 적절
복잡한clip-path와 사각형 마스킹을 조합해 시계 형태가 명확하게 표현되었습니다. 뷰포트(16dp), 불투명도(fillAlpha="0.61") 설정 모두 다른 일정 관련 아이콘(location,arrow)과 잘 어우러집니다.feature/schedule/src/main/res/values/strings.xml (1)
1-10: 문자열 리소스 정의가 잘 구성되어 있습니다.문자열 리소스의 이름이 일관된 패턴(예:
schedule_tab_all)을 따르고 있어 코드의 가독성과 유지보수성이 좋습니다. 일정 화면에 필요한 모든 텍스트가 잘 정의되어 있습니다.feature/schedule/src/main/res/drawable/icon_location.xml (1)
1-15: 위치 아이콘이 적절히 정의되어 있습니다.벡터 드로어블이 16dp x 16dp로 적절한 크기로 정의되어 있고, 클립 패스와 경로를 사용하여 위치 핀 모양을 효과적으로 표현하고 있습니다. 반투명 배경 처리도 적절합니다.
core/ui/src/main/java/com/yapp/core/ui/extension/Modifier.kt (2)
5-9: 필요한 임포트가 추가되었습니다.대시 테두리 기능 구현에 필요한
CornerRadius,Color,PathEffect,Stroke등의 클래스가 적절히 임포트되었습니다.
29-51: 대시 테두리 기능이 잘 구현되어 있습니다.
dashedBorder확장 함수가 잘 구현되어 있으며, 색상, 선 두께, 대시 길이, 간격, 모서리 반경 등 다양한 사용자 정의 옵션을 제공합니다. Compose의drawBehind와PathEffect.dashPathEffect를 적절히 활용하여 점선 테두리를 그리는 접근 방식이 좋습니다.기본값들도 적절하게 설정되어 있어 사용하기 편리할 것 같습니다.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/IconWithText.kt (1)
1-13: 필요한 임포트와 패키지가 적절히 구성되어 있습니다.Compose UI 컴포넌트 구현에 필요한 임포트들이 잘 정의되어 있고, 패키지 구조도 기능별로 잘 구성되어 있습니다.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/AttendanceStatusChip.kt (1)
64-86: LGTM! 미리보기 구현이 잘 되었습니다.모든 상태를 한 번에 볼 수 있도록 미리보기를 구성한 것이 좋습니다. 이렇게 하면 디자인 변경 사항을 빠르게 확인할 수 있습니다.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/AssignmentItem.kt (2)
32-36: LGTM! 오늘 날짜 강조 처리가 잘 구현되었습니다.오늘 날짜를 primary 색상으로 강조하는 방식이 시각적으로 효과적입니다.
38-84: 과거 항목 처리와 레이아웃 구조가 잘 구현되었습니다.과거 항목을 alpha 값으로 흐리게 처리하고, 날짜와 내용을 효과적으로 구성한 레이아웃이 사용자 경험에 도움이 됩니다.
core/designsystem/src/main/java/com/yapp/core/designsystem/theme/Color.kt (3)
9-53: LGTM! 새로운 색상 추가가 잘 되었습니다.일정 기능 UI에 필요한 색상들이 기존 색상 시스템에 맞게 잘 추가되었습니다. 시각적 계층 구조와 사용성을 향상시키는 좋은 색상 선택입니다.
69-122: LGTM! 색상 스키마 정의가 잘 확장되었습니다.
YappColorScheme데이터 클래스에 새로운 색상 속성들이 적절히 추가되었습니다. 기존 구조를 잘 유지하면서 확장했습니다.
124-167: LGTM! 기본 색상 설정도 일관되게 업데이트되었습니다.
LocalColorScheme의 기본값도 새로운 색상 속성들로 잘 업데이트되었습니다.Color.Unspecified를 사용하여 일관성을 유지했습니다.feature/schedule/src/main/java/com/yapp/feature/schedule/component/ScheduleTabRow.kt (2)
28-65: ScheduleTabRow 구현이 잘 되었습니다!탭 선택 상태에 따라 스타일이 적절히 변경되고, 탭 목록을 받아 동적으로 생성하는 유연한 구현입니다. 색상과 타이포그래피에 디자인 시스템을 잘 활용했습니다.
67-102: ScheduleTab 컴포넌트가 잘 구현되었습니다.선택 상태에 따른 텍스트 색상 변경과 하단 인디케이터 표시가 잘 구현되었습니다. 레이아웃과 스타일링이 깔끔합니다.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/SessionItem.kt (4)
23-121: SessionItem 컴포넌트 구현이 잘 되었습니다.다양한 상태(오늘, 과거 세션)와 조건부 콘텐츠(과제 정보)를 잘 처리하고 있습니다.
graphicsLayer를 사용해 과거 항목에 투명도를 적용한 부분이 좋습니다. 레이아웃 구성과 요소 간 간격이 적절합니다.
123-182: Preview 구현이 다양한 상태를 보여주어 좋습니다.세 가지 다른 상태(예정됨, 참석함, 지각)와 과제 정보가 있는 경우와 없는 경우를 모두 보여주는 프리뷰를 구현하여 컴포넌트의 다양한 상태를 확인할 수 있어 좋습니다. 아이템 간 구분선 처리도 잘 되었습니다.
84-88:Details
✅ Verification successful
외부 컴포넌트 참조가 있습니다.
IconWithText컴포넌트를 사용하고 있지만 이 파일에서는 정의되어 있지 않습니다. 이 컴포넌트가 다른 파일에 정의되어 있는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash # IconWithText 컴포넌트가 정의된 파일 찾기 fd IconWithText.kt --type fLength of output: 112
IconWithText 컴포넌트 정의 확인 완료
동일 패키지(feature/schedule/src/main/java/com/yapp/feature/schedule/component/IconWithText.kt)에IconWithText가 이미 구현되어 있어 추가 작업이 필요 없습니다.
119-119:Details
❌ Incorrect review comment
외부 컴포넌트 참조가 있습니다.
AttendanceStatusChip컴포넌트를 사용하고 있지만 이 파일에서는 정의되어 있지 않습니다. 이 컴포넌트가 다른 파일에 정의되어 있는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash # AttendanceStatusChip 컴포넌트가 정의된 파일 찾기 fd AttendanceStatusChip.kt --type fLength of output: 128
AttendanceStatusChip 컴포넌트 위치 확인 완료
AttendanceStatusChip.kt파일이
feature/schedule/src/main/java/com/yapp/feature/schedule/component/
경로에 동일한 패키지로 존재하므로,SessionItem.kt에서 별도 import 없이 정상 참조됩니다.
해당 리뷰 코멘트는 불필요하니 무시해주세요.Likely an incorrect or invalid review comment.
feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleScreen.kt (4)
37-72: ScheduleScreen 구조가 잘 설계되었습니다.탭 기반 네비게이션이 잘 구현되어 있으며, 선택된 탭에 따라 적절한 화면이 표시됩니다. 레이아웃 구성이 깔끔합니다.
74-135: ScheduleAllScreen에서 하드코딩된 샘플 데이터를 사용하고 있습니다.실제 데이터로 교체할 계획이 있는지 확인해 주세요. 또한 이후에 ViewModel 또는 상태 관리 시스템을 통해 데이터를 가져오도록 수정될 예정인지 검토해 주세요.
비즈니스 로직 통합과 관련된 추가 PR 또는 이슈가 있는지 확인해 보세요.
223-241: ScheduleHeader 구현이 깔끔합니다.간결하고 읽기 쉬운 코드로 헤더를 구현했습니다. 디자인 시스템의 색상과 타이포그래피를 잘 활용했습니다.
156-167:Details
❌ Incorrect review comment
TodaySessionSection 컴포넌트 참조가 있습니다.
TodaySessionSection컴포넌트를 사용하고 있지만 이 파일에서는 정의되어 있지 않습니다. 이 컴포넌트가 다른 파일에 정의되어 있는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash # TodaySessionSection 컴포넌트가 정의된 파일 찾기 fd TodaySessionSection.kt --type fLength of output: 126
TodaySessionSection 컴포넌트 정의 위치 확인됨
해당 컴포넌트는
feature/schedule/src/main/java/com/yapp/feature/schedule/component/TodaySessionSection.kt
에서 정의되어 있어 참조에 문제가 없습니다.Likely an incorrect or invalid review comment.
feature/schedule/src/main/java/com/yapp/feature/schedule/component/TodaySessionSection.kt (2)
1-22: 패키지 구조와 임포트가 잘 정리되어 있습니다.필요한 모든 컴포넌트와 유틸리티가 명확하게 임포트되어 있습니다. 특히 디자인 시스템의 컴포넌트와 테마를 활용하는 것이 좋은 접근법입니다.
68-72: 날짜 표시 형식이 깔끔합니다.날짜와 요일을 함께 표시하는 방식이 사용자에게 명확한 정보를 제공합니다.
| import androidx.navigation.NavGraphBuilder | ||
| import androidx.navigation.NavOptions | ||
| import androidx.navigation.compose.composable | ||
| import com.yapp.feature.schedule.ScheduleRoute |
There was a problem hiding this comment.
🛠️ Refactor suggestion
불필요한 임포트 및 이름 충돌 위험
import com.yapp.feature.schedule.ScheduleRoute는 Composable 함수와 네비게이션용 data object ScheduleRoute가 같은 이름으로 중복됩니다. 이로 인해 참조가 애매해질 수 있으므로, 별도의 네비게이션 전용 타입 이름(예: ScheduleNavRoute) 또는 Composable 함수 이름 변경을 고려하세요.
| composable<ScheduleRoute> { | ||
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text("Schedule Screen") | ||
| } | ||
| ScheduleRoute() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Composable 호출과 네비게이션 라우트 구분 필요
composable<ScheduleRoute> { ScheduleRoute() }에서도 Composable 함수와 데이터 오브젝트가 혼용되어 가독성과 유지보수성이 떨어집니다.
data object ScheduleRoute→ScheduleNavRoute- Composable →
ScheduleRouteScreen등으로 명확히 분리 권장
| ":feature:home", | ||
| ":feature:notice", | ||
| ":feature:signup", | ||
| ":feature:login" | ||
| ":feature:login", | ||
| ":feature:schedule", | ||
| ":feature:profile", | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
기능 모듈 목록 정렬 권장
현재 :feature:home, :feature:notice, :feature:signup, :feature:login, :feature:schedule, :feature:profile 순으로 나열되어 있습니다. 가독성과 유지보수를 위해 알파벳 순서나 팀별 규칙에 맞춰 정렬하면 좋겠습니다.
| @Composable | ||
| internal fun IconWithText( | ||
| iconResId: Int, | ||
| text: String, | ||
| contentDescription: String? = null | ||
| ) { | ||
| Row( | ||
| horizontalArrangement = Arrangement.spacedBy(4.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(id = iconResId), | ||
| contentDescription = contentDescription, | ||
| tint = Color.Unspecified | ||
| ) | ||
|
|
||
| Text( | ||
| text = text, | ||
| style = YappTheme.typography.caption1Regular, | ||
| color = YappTheme.colorScheme.labelAlternative | ||
| ) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
아이콘과 텍스트를 함께 표시하는 컴포넌트가 잘 구현되어 있습니다.
IconWithText 컴포저블 함수가 내부 가시성(internal)으로 적절히 정의되어 있고, 아이콘과 텍스트를 수평으로 배치하는 레이아웃이 잘 구성되어 있습니다.
아이콘의 tint를 Color.Unspecified로 설정하여 원본 아이콘 색상을 유지한 것과 앱의 디자인 시스템 테마(YappTheme)를 사용하여 일관된 스타일을 적용한 점이 좋습니다.
다만, 재사용성을 높이기 위해 텍스트 스타일과 색상을 매개변수로 받을 수 있도록 확장하는 것을 고려해볼 수 있습니다.
-internal fun IconWithText(
- iconResId: Int,
- text: String,
- contentDescription: String? = null
-) {
+internal fun IconWithText(
+ iconResId: Int,
+ text: String,
+ contentDescription: String? = null,
+ textStyle: androidx.compose.ui.text.TextStyle = YappTheme.typography.caption1Regular,
+ textColor: Color = YappTheme.colorScheme.labelAlternative
+) {
Row(
horizontalArrangement = Arrangement.spacedBy(4.dp),
verticalAlignment = Alignment.CenterVertically
) {
Icon(
painter = painterResource(id = iconResId),
contentDescription = contentDescription,
tint = Color.Unspecified
)
Text(
text = text,
- style = YappTheme.typography.caption1Regular,
- color = YappTheme.colorScheme.labelAlternative
+ style = textStyle,
+ color = textColor
)
}
}📝 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.
| @Composable | |
| internal fun IconWithText( | |
| iconResId: Int, | |
| text: String, | |
| contentDescription: String? = null | |
| ) { | |
| Row( | |
| horizontalArrangement = Arrangement.spacedBy(4.dp), | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| Icon( | |
| painter = painterResource(id = iconResId), | |
| contentDescription = contentDescription, | |
| tint = Color.Unspecified | |
| ) | |
| Text( | |
| text = text, | |
| style = YappTheme.typography.caption1Regular, | |
| color = YappTheme.colorScheme.labelAlternative | |
| ) | |
| } | |
| } | |
| @Composable | |
| internal fun IconWithText( | |
| iconResId: Int, | |
| text: String, | |
| contentDescription: String? = null, | |
| textStyle: androidx.compose.ui.text.TextStyle = YappTheme.typography.caption1Regular, | |
| textColor: Color = YappTheme.colorScheme.labelAlternative | |
| ) { | |
| Row( | |
| horizontalArrangement = Arrangement.spacedBy(4.dp), | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| Icon( | |
| painter = painterResource(id = iconResId), | |
| contentDescription = contentDescription, | |
| tint = Color.Unspecified | |
| ) | |
| Text( | |
| text = text, | |
| style = textStyle, | |
| color = textColor | |
| ) | |
| } | |
| } |
| val backgroundColor = when (status) { | ||
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.yellow95 | ||
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlueWeak | ||
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50.copy(alpha = 0.05f) | ||
| } | ||
|
|
||
| val textColor = when (status) { | ||
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.secondaryNormal | ||
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlue | ||
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
색상 정의 방식이 효율적입니다.
각 상태에 따른 배경색과 텍스트 색상을 명확하게 구분하여 정의했습니다. 다만, LATE 상태의 배경색에서 alpha 값을 직접 수정하는 방식보다 테마 시스템에서 미리 정의된 색상을 사용하는 것이 더 일관성 있을 수 있습니다.
val backgroundColor = when (status) {
AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.yellow95
AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlueWeak
- AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50.copy(alpha = 0.05f)
+ AttendanceStatus.LATE -> YappTheme.colorScheme.fillAlternative // 또는 다른 적절한 배경색
}이 방식은 향후 디자인 시스템 변경 시 일관성을 유지하는 데 도움이 됩니다.
📝 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 backgroundColor = when (status) { | |
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.yellow95 | |
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlueWeak | |
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50.copy(alpha = 0.05f) | |
| } | |
| val textColor = when (status) { | |
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.secondaryNormal | |
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlue | |
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50 | |
| } | |
| val backgroundColor = when (status) { | |
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.yellow95 | |
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlueWeak | |
| AttendanceStatus.LATE -> YappTheme.colorScheme.fillAlternative // 또는 다른 적절한 배경색 | |
| } | |
| val textColor = when (status) { | |
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.secondaryNormal | |
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlue | |
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50 | |
| } |
| @Preview( | ||
| showBackground = true | ||
| ) | ||
| @Composable | ||
| private fun PreviewTodaySessionSection() { | ||
| YappTheme { | ||
| TodaySessionSection( | ||
| id = 1L, | ||
| title = "2차 데모데이", | ||
| date = "2023.10.01", | ||
| dayOfWeek = "일요일", | ||
| location = "서울시 강남구", | ||
| time = "오후 2시 - 오후 4시", | ||
| remainingDays = 3, | ||
| onClick = {} | ||
| ) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
프리뷰 함수가 적절히 구현되어 있습니다.
프리뷰 기능을 통해 개발 중 컴포넌트의 모습을 확인할 수 있어 좋습니다. 다양한 사례(긴 텍스트, 남은 일수 0 또는 음수)에 대한 프리뷰도 추가하면 좋을 것 같습니다.
다양한 사례를 보여주는 추가 프리뷰를 고려해보세요:
@Preview(showBackground = true)
@Composable
private fun PreviewTodaySessionSectionLongText() {
YappTheme {
TodaySessionSection(
id = 1L,
title = "매우 긴 세션 제목이 있는 경우 어떻게 표시되는지 확인하기 위한 예시",
date = "2023.10.01",
dayOfWeek = "일요일",
location = "서울특별시 강남구 테헤란로 매우 긴 주소",
time = "오전 10시 - 오후 6시 (8시간 이상의 긴 세션)",
remainingDays = 3,
onClick = {}
)
}
}
@Preview(showBackground = true)
@Composable
private fun PreviewTodaySessionSectionDDay() {
YappTheme {
TodaySessionSection(
id = 1L,
title = "오늘 진행되는 세션",
date = "2023.10.01",
dayOfWeek = "일요일",
location = "서울시 강남구",
time = "오후 2시 - 오후 4시",
remainingDays = 0,
onClick = {}
)
}
}| Spacer(modifier = Modifier.height(12.dp)) | ||
|
|
||
| IconWithText( | ||
| iconResId = R.drawable.icon_location, | ||
| text = location, | ||
| contentDescription = null, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성을 위한 contentDescription 개선이 필요합니다.
IconWithText 컴포넌트 사용 시 접근성을 위한 contentDescription이 null로 설정되어 있습니다. 스크린 리더 사용자를 위해 적절한 설명을 제공하는 것이 좋습니다.
다음과 같이 의미 있는 contentDescription을 추가하세요:
IconWithText(
iconResId = R.drawable.icon_location,
text = location,
- contentDescription = null,
+ contentDescription = "위치: $location",
)
Spacer(modifier = Modifier.height(4.dp))
IconWithText(
iconResId = R.drawable.icon_time,
text = time,
- contentDescription = null,
+ contentDescription = "시간: $time",
)Also applies to: 82-88
| Column( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .clickable { onClick(id) } | ||
| .dashedBorder( | ||
| color = Color(0xFFFED9CB), | ||
| dashLength = 2.dp, | ||
| gapLength = 2.dp, | ||
| cornerRadius = 10.dp, | ||
| ) | ||
| .padding(12.dp) | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 색상 값은 테마에서 참조하는 것이 좋습니다.
테두리 색상이 하드코딩되어 있습니다. 디자인 시스템의 색상 값을 사용하는 것이 테마 변경이나 다크 모드 지원 시 일관성을 유지하는 데 도움이 됩니다.
테마에서 색상을 참조하도록 수정하세요:
- .dashedBorder(
- color = Color(0xFFFED9CB),
- dashLength = 2.dp,
- gapLength = 2.dp,
- cornerRadius = 10.dp,
- )
+ .dashedBorder(
+ color = YappTheme.colorScheme.accentLightOrange,
+ dashLength = 2.dp,
+ gapLength = 2.dp,
+ cornerRadius = 10.dp,
+ )📝 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.
| Column( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .clickable { onClick(id) } | |
| .dashedBorder( | |
| color = Color(0xFFFED9CB), | |
| dashLength = 2.dp, | |
| gapLength = 2.dp, | |
| cornerRadius = 10.dp, | |
| ) | |
| .padding(12.dp) | |
| ) { | |
| Column( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .clickable { onClick(id) } | |
| .dashedBorder( | |
| color = YappTheme.colorScheme.accentLightOrange, | |
| dashLength = 2.dp, | |
| gapLength = 2.dp, | |
| cornerRadius = 10.dp, | |
| ) | |
| .padding(12.dp) | |
| ) { |
| @Composable | ||
| internal fun TodaySessionSection( | ||
| id: Long, | ||
| title: String, | ||
| date: String, | ||
| dayOfWeek: String, | ||
| location: String, | ||
| time: String, | ||
| remainingDays: Int, | ||
| onClick: (Long) -> Unit, | ||
| ) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
함수 파라미터가 명확히 정의되어 있습니다.
TodaySessionSection 컴포저블 함수의 파라미터들이 명확하게 정의되어 있어 사용하기 쉽습니다. 다만, 함수에 KDoc 주석을 추가하여 이 컴포넌트의 목적과 각 파라미터의 역할을 문서화하면 좋을 것 같습니다.
다음과 같이 KDoc 주석을 추가할 수 있습니다:
+/**
+ * 오늘의 세션 정보를 보여주는 카드형 UI 컴포넌트입니다.
+ * 점선 테두리로 강조되며 세션 제목, D-day 표시, 날짜, 위치, 시간 정보를 포함합니다.
+ *
+ * @param id 세션 ID
+ * @param title 세션 제목
+ * @param date 세션 날짜 (형식: YYYY.MM.DD)
+ * @param dayOfWeek 요일
+ * @param location 세션 장소
+ * @param time 세션 시간
+ * @param remainingDays 세션까지 남은 일수
+ * @param onClick 세션 클릭 시 호출되는 콜백 함수
+ */📝 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.
| @Composable | |
| internal fun TodaySessionSection( | |
| id: Long, | |
| title: String, | |
| date: String, | |
| dayOfWeek: String, | |
| location: String, | |
| time: String, | |
| remainingDays: Int, | |
| onClick: (Long) -> Unit, | |
| ) { | |
| /** | |
| * 오늘의 세션 정보를 보여주는 카드형 UI 컴포넌트입니다. | |
| * 점선 테두리로 강조되며 세션 제목, D-day 표시, 날짜, 위치, 시간 정보를 포함합니다. | |
| * | |
| * @param id 세션 ID | |
| * @param title 세션 제목 | |
| * @param date 세션 날짜 (형식: YYYY.MM.DD) | |
| * @param dayOfWeek 요일 | |
| * @param location 세션 장소 | |
| * @param time 세션 시간 | |
| * @param remainingDays 세션까지 남은 일수 | |
| * @param onClick 세션 클릭 시 호출되는 콜백 함수 | |
| */ | |
| @Composable | |
| internal fun TodaySessionSection( | |
| id: Long, | |
| title: String, | |
| date: String, | |
| dayOfWeek: String, | |
| location: String, | |
| time: String, | |
| remainingDays: Int, | |
| onClick: (Long) -> Unit, | |
| ) { | |
| // ... | |
| } |
| Row( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| horizontalArrangement = Arrangement.SpaceBetween | ||
| ) { | ||
| Text( | ||
| modifier = Modifier.padding(top = 2.dp), | ||
| text = title, | ||
| style = YappTheme.typography.body1ReadingBold, | ||
| color = YappTheme.colorScheme.labelNormal | ||
| ) | ||
|
|
||
| YappChipSmall( | ||
| text = if (remainingDays > 0) { | ||
| "D-${remainingDays}" | ||
| } else { | ||
| "D-day" | ||
| }, | ||
| colorType = ChipColorType.Main, | ||
| isFill = true | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
D-day 표시 로직이 잘 구현되어 있습니다.
D-day 칩 표시 로직이 남은 일수에 따라 적절히 구현되어 있습니다. 다만, 남은 일수가 음수일 경우(이미 지난 이벤트)를 좀 더 명확하게 처리할 수 있습니다.
남은 일수가 음수인 경우를 명시적으로 처리하는 것이 좋습니다:
text = if (remainingDays > 0) {
"D-${remainingDays}"
- } else {
+ } else if (remainingDays == 0) {
"D-day"
+ } else {
+ "종료됨" // 또는 "지남" 등 적절한 문구
},
TaeseongYun
left a comment
TaeseongYun
left a comment
There was a problem hiding this comment.
UI 가 쉽지 않았을거 같은데 고생하셨습니다.
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .clickable { onClick(id) } |
There was a problem hiding this comment.
R: 요거 중복클릭을 방지하기 위한 모디파이어가 있더라구요
yappClickable 를 사용하시면 됩니다.
다른 onClick 부분에서도 확인해보시고 적용 부탁드릴게요
| val text = when (status) { | ||
| AttendanceStatus.SCHEDULED -> "예정" | ||
| AttendanceStatus.ATTENDED -> "출석" | ||
| AttendanceStatus.LATE -> "지각" | ||
| } | ||
|
|
||
| val backgroundColor = when (status) { | ||
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.yellow95 | ||
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlueWeak | ||
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50.copy(alpha = 0.05f) | ||
| } | ||
|
|
||
| val textColor = when (status) { | ||
| AttendanceStatus.SCHEDULED -> YappTheme.colorScheme.secondaryNormal | ||
| AttendanceStatus.ATTENDED -> YappTheme.colorScheme.accentLightBlue | ||
| AttendanceStatus.LATE -> YappTheme.colorScheme.coolNeutral50 | ||
| } |
There was a problem hiding this comment.
R : 변경되지 않는 값들이라면 요런 상수형태의 값들을 아예 enum class 로 위임시키는건 어떨까요 ?
There was a problem hiding this comment.
넵! 그것도 좋아보이네요!
AttendanceStatus를 enum class로 만들고
core:model로 옮기는 것이 더 적절할까요?? 아니면 현재 파일에 그대로 두는 게 더 나을까요??
There was a problem hiding this comment.
여러군데에서 사용할거 같다면 core:model 로 옮기는게 좋을거 같고 아니라면 현 파일에 그대로 두는게 좋을거 같아요
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="16dp" | ||
| android:height="16dp" | ||
| android:viewportWidth="16" | ||
| android:viewportHeight="16"> | ||
| <group> | ||
| <clip-path | ||
| android:pathData="M3.786,3.367C4.885,2.059 6.443,1.4 7.998,1.4C9.554,1.4 11.105,2.052 12.211,3.367C14.121,5.638 13.824,8.988 11.77,11.05L11.769,11.051C10.604,12.217 9.262,13.634 8.699,14.228C8.661,14.269 8.626,14.305 8.595,14.338C8.272,14.683 7.725,14.683 7.401,14.338C7.371,14.305 7.336,14.269 7.297,14.228C6.735,13.634 5.393,12.217 4.227,11.051L4.227,11.05C2.173,8.989 1.875,5.632 3.786,3.367ZM7.998,2.6C6.781,2.6 5.565,3.114 4.704,4.139L4.704,4.14C3.228,5.889 3.424,8.544 5.076,10.203C6.149,11.275 7.365,12.554 7.998,13.223C8.632,12.554 9.848,11.275 10.92,10.203C12.572,8.544 12.769,5.895 11.292,4.139C10.425,3.108 9.216,2.6 7.998,2.6ZM7.998,6.24C7.636,6.24 7.345,6.531 7.345,6.893C7.345,7.255 7.636,7.547 7.998,7.547C8.36,7.547 8.652,7.255 8.652,6.893C8.652,6.531 8.36,6.24 7.998,6.24ZM6.145,6.893C6.145,5.869 6.974,5.04 7.998,5.04C9.023,5.04 9.852,5.869 9.852,6.893C9.852,7.918 9.023,8.747 7.998,8.747C6.974,8.747 6.145,7.918 6.145,6.893Z" | ||
| android:fillType="evenOdd"/> | ||
| <path | ||
| android:pathData="M0,0h16v16h-16z" | ||
| android:fillColor="#37383C" | ||
| android:fillAlpha="0.61"/> | ||
| </group> | ||
| </vector> |
There was a problem hiding this comment.
R: icon_location 과 time 둘 다 홈에서도 사용되는걸로 보여서 디자인모듈로 이전 부탁드릴게요 ~
There was a problem hiding this comment.
core:designsytem 모듈로 이전했습니다
|
@TaeseongYun 코멘트 남겨주신 사항 반영했습니다. |
TaeseongYun
left a comment
TaeseongYun
left a comment
There was a problem hiding this comment.
마지막 코멘트만 적용해주시고 머지해주시면 될듯합니다 ~
고생하셨습니다 !
DongChyeon
force-pushed
the
feature/#125-schedule-ui
branch
2 times, most recently
from
0df498e to
43e857b
Compare
DongChyeon
changed the base branch from
feature/#121-bottom-navigation
to
develop
DongChyeon
force-pushed
the
feature/#125-schedule-ui
branch
from
054a437 to
40ad273
Compare
DongChyeon
force-pushed
the
feature/#125-schedule-ui
branch
from
40ad273 to
44de13e
Compare
2 participants
💡 Issue
🌱 Key changes
✅ To Reviewers
PR 끊어가기 위해서 올립니다!
UI 부분만 봐주시면 되고, 비즈니스 로직 관련은 API 연동하면서 손 볼 예정입니다.
📸 스크린샷
UI.mp4
Summary by CodeRabbit
New Features
스타일
기타