[FEAT] 이번달 챙길사람 기능 #59
Conversation
- 홈 화면의 '이달의 리마인더' 섹션에 '전체보기' 버튼을 추가하고, 클릭 시 해당 화면으로 이동하도록 구현했습니다. - `ic_front_8` 벡터 드로어블을 추가하여 '전체보기' 버튼 아이콘으로 사용했습니다. - 전체보기 화면으로 이동하는 `navigateToMonthlyReminderAll` 네비게이션 로직과 `MonthlyReminderAllScreen` 컴포저블을 추가했습니다.
- 월별 전체 리마인더 화면에서 사용될 `MonthlyReminderFriendCard` 컴포저블을 추가했습니다. - 이 카드는 친구의 이름, 리마인더 내용, D-day, 그리고 '챙김 기록하기' 버튼으로 구성됩니다.
- 월별 리마인더에 표시할 내용이 없을 때 보여줄 `MonthlyReminderEmpty` 컴포저블을 추가했습니다. - 해당 컴포저블은 안내 이미지와 "이번달은 챙길 사람이 없네요."라는 텍스트로 구성됩니다.
- 월별 리마인더의 완료된 항목을 표시하는 `MonthlyReminderComplete` 컴포저블을 새로 추가했습니다. - 이 컴포저블은 아이콘, 이름, 날짜를 포함하며, 완료된 상태의 UI를 나타냅니다.
- UI 상태(로딩, 데이터 있음, 데이터 없음)에 따라 각각 다른 화면을 표시합니다. - 뒤로가기 버튼을 통해 이전 화면으로 이동할 수 있습니다.
- `MonthlyReminderAllViewModel`을 추가하여 월별 리마인더 목록을 가져오고 UI 상태를 관리합니다. - API 응답을 `MonthlyReminderUIModel`로 변환하여 UI에 필요한 데이터를 제공합니다. - `MonthlyReminderAllUIState` sealed interface를 `Loading`, `Empty`, `Success` 상태로 정의하여 UI 상태를 관리하도록 수정했습니다.
- `MonthlyReminderComplete` 컴포저블이 `MonthlyReminderUIModel` 데이터를 받아 실제 데이터를 표시하도록 수정했습니다. - 날짜 형식을 'yy.MM.dd'로 포맷하는 `formatDate` 함수를 추가했습니다. - 이름이 길어질 경우 말줄임표(...)로 처리되도록 `TextOverflow.Ellipsis` 속성을 적용했습니다.
- 연락을 기록했을 때 표시되는 성공 다이얼로그 UI를 구현했습니다. - 다이얼로그는 1.5초 후 자동으로 사라집니다.
- 월별 리마인더 목록에서 완료 처리한 친구를 연락 완료 목록으로 즉시 이동시켜 UI에 반영하도록 수정했습니다. - 기존의 네트워크 통신을 통한 목록 갱신 방식 대신, 로컬에서 목록을 직접 업데이트하여 불필요한 API 호출을 제거하고 응답성을 개선했습니다.
- 불필요한 Spacer를 제거하여 레이아웃을 개선했습니다. - LazyColumn의 컨텐츠 패딩을 적용하여 상하단 여백을 조정했습니다.
- 월별 리마인더 전체보기 화면의 카드 사이에 16dp의 간격을 추가했습니다.
- 월별 리마인더 목록을 `nextContactAt`(다음 연락일) 기준으로 오름차순 정렬하여 표시하도록 수정했습니다.
Summary of ChangesHello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements the '이번달 챙길사람' feature, which allows users to view and manage a list of contacts they need to check in with each month. It includes UI components, data fetching from the network, and navigation setup to access the new screen from the home screen. The feature displays both a list of people to contact and a list of those already contacted. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Summary of ChangesHello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 사용자가 이번 달에 챙겨야 할 친구들을 한눈에 확인하고, 챙김 활동을 기록할 수 있는 '이번달 챙길사람' 기능을 도입합니다. 홈 화면에서 해당 기능으로의 접근성을 높이고, 월별 챙김 목록과 완료된 챙김 목록을 효율적으로 관리할 수 있도록 UI와 백엔드 연동 로직을 구현했습니다. 이를 통해 사용자 경험을 개선하고 친구 관계 관리를 더욱 용이하게 합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| init { | ||
| fetchMonthlyFriends() | ||
| fetchMonthlyCompleteFriends() | ||
| } | ||
|
|
||
| private fun fetchMonthlyFriends() { | ||
| friendRepository | ||
| .fetchMonthlyFriends() | ||
| .onEach { monthlyFriends -> | ||
| val uiModels = convertToUIModels(monthlyFriends).sortedBy { it.nextContactAt } | ||
| _monthlyReminders.value = uiModels | ||
| updateUIState() | ||
| }.catch { exception -> | ||
| viewModelScope.launch { | ||
| _uiEvent.send(MonthlyReminderAllUIEvent.NetworkError) | ||
| } | ||
| }.launchIn(viewModelScope) | ||
| } | ||
|
|
||
| private fun fetchMonthlyCompleteFriends() { | ||
| friendRepository | ||
| .fetchMonthlyCompleteFriends() | ||
| .onEach { monthlyFriends -> | ||
| val uiModels = convertToUIModels(monthlyFriends) | ||
| _completedReminders.value = uiModels | ||
| updateUIState() | ||
| }.catch { exception -> | ||
| viewModelScope.launch { | ||
| _uiEvent.send(MonthlyReminderAllUIEvent.NetworkError) | ||
| } | ||
| }.launchIn(viewModelScope) | ||
| } |
There was a problem hiding this comment.
init 블록에서 fetchMonthlyFriends와 fetchMonthlyCompleteFriends를 각각 호출하여 UI 상태를 개별적으로 업데이트하고 있습니다. 이 방식은 데이터 로딩 시점에 따라 UI가 깜빡이는 현상(flicker)을 유발할 수 있습니다. 두 API 호출을 병렬로 실행하고, 두 호출이 모두 완료된 후에 한 번만 UI 상태를 업데이트하여 사용자 경험을 개선하는 것이 좋습니다.
Near/app/src/main/java/com/alarmy/near/presentation/feature/home/HomeScreen.kt
Outdated
Show resolved
Hide resolved
| is MonthlyReminderAllUIEvent.NetworkError -> { | ||
| onShowErrorSnackBar( | ||
| IllegalStateException( | ||
| context.getString(R.string.monthly_reminder_all_network_error), | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
네트워크 오류 발생 시 스낵바에 메시지를 표시하기 위해 새로운 IllegalStateException을 생성하고 있습니다. 이 방식은 디버깅에 유용한 원래 예외의 스택 트레이스와 타입 정보를 유실시킵니다. ViewModel에서 발생한 원본 Throwable을 이벤트로 전달하고, UI 단에서는 이를 로깅한 후 사용자에게 친화적인 메시지를 보여주는 것이 더 나은 방법입니다.
이를 위해 MonthlyReminderAllUIEvent.NetworkError를 Throwable을 포함하는 data class로 변경하고, ViewModel에서 원본 예외를 전달하도록 수정하는 것을 권장합니다.
| fun onRecordFriendShip(friendId: String) { | ||
| friendRepository | ||
| .recordContact(friendId) | ||
| .onEach { _ -> | ||
| viewModelScope.launch { | ||
| _uiEvent.send(MonthlyReminderAllUIEvent.RecordFriendShipSuccess) | ||
| } | ||
| val recordedFriend = _monthlyReminders.value.find { it.friendId == friendId } | ||
| if (recordedFriend != null) { | ||
| _monthlyReminders.value = | ||
| _monthlyReminders.value.filter { it.friendId != friendId } | ||
| _completedReminders.value = listOf(recordedFriend) + _completedReminders.value | ||
| updateUIState() | ||
| } | ||
| }.catch { exception -> | ||
| viewModelScope.launch { | ||
| _uiEvent.send(MonthlyReminderAllUIEvent.NetworkError) | ||
| } | ||
| }.launchIn(viewModelScope) | ||
| } |
There was a problem hiding this comment.
NetworkError 이벤트를 보내는 catch 블록이 fetchMonthlyFriends, fetchMonthlyCompleteFriends, onRecordFriendShip 세 곳에서 중복되고 있습니다. 코드의 가독성과 유지보수성을 높이기 위해 이 로직을 Flow에 대한 확장 함수로 추출하여 공통으로 사용하는 것을 권장합니다.
| Text( | ||
| stringResource(R.string.friend_profile_info_contact_success_text), | ||
| style = NearTheme.typography.B1_16_BOLD, | ||
| color = Color(0xff222222), |
There was a problem hiding this comment.
Color(0xff222222)와 같이 색상 값이 하드코딩되어 있습니다. 앱 전체의 시각적 일관성을 유지하고 향후 테마 변경을 용이하게 하려면 NearTheme.colors에 정의된 색상을 사용하는 것이 좋습니다. 예를 들어 NearTheme.colors.BLACK_1A1A1A와 같은 테마 색상으로 교체해주세요.
| color = Color(0xff222222), | |
| color = NearTheme.colors.BLACK_1A1A1A, |
There was a problem hiding this comment.
Code Review
이번 PR은 '이번달 챙길 사람' 기능을 추가하는군요. 새로운 화면과 관련 로직들이 잘 구현되었습니다. 몇 가지 개선점을 제안합니다. 홈 화면의 '전체보기' 버튼에 하드코딩된 문자열을 리소스로 추출하고, 새로 추가된 화면에서 LazyColumn의 패딩 처리 방식을 개선하는 것이 좋겠습니다. 또한, ViewModel에서 도메인 모델의 역할 분리, UI 컴포넌트에서의 매직 스트링 및 매직 넘버 사용을 개선하는 부분에 대한 의견을 남겼습니다. 특히 'D-DAY' 문자열 비교 로직은 버그를 유발할 수 있으니 꼭 확인해주세요. 추가로, 새로 생성된 mothlyreminderall 패키지명에 오타가 있는 것 같습니다. monthlyreminderall로 수정하는 것을 고려해보세요. 전반적으로 새로운 기능 구현을 위해 수고 많으셨습니다!
| if (reminder.daysUntilNextContact == "D-DAY") { | ||
| NearTheme.typography.B2_14_BOLD to NearTheme.colors.BLUE01_5AA2E9 | ||
| } else { | ||
| NearTheme.typography.B2_14_MEDIUM to NearTheme.colors.GRAY01_888888 | ||
| } |
There was a problem hiding this comment.
"D-DAY" 문자열을 직접 비교하는 부분에서 버그가 발생할 수 있습니다. MonthlyFriend 모델에서는 "D-day"(소문자 'd')를 반환하지만, 여기서는 "D-DAY"(대문자)와 비교하고 있어 if문이 항상 false를 반환하게 됩니다.
이러한 '매직 스트링' 비교는 오류에 취약합니다. MonthlyReminderUIModel에 isDday: Boolean과 같은 필드를 추가하고 ViewModel에서 값을 설정한 뒤, UI에서는 이 boolean 값을 확인하는 방식으로 리팩토링하는 것을 강력히 권장합니다.
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| Text( | ||
| text = "전체보기", |
There was a problem hiding this comment.
"전체보기" 문자열이 하드코딩되어 있습니다. strings.xml에 리소스로 추출하고 stringResource를 사용하여 참조하는 것이 좋습니다. 이렇게 하면 나중에 텍스트를 변경하거나 다른 언어를 지원할 때 더 쉽게 관리할 수 있습니다.
| text = "전체보기", | |
| text = stringResource(R.string.home_monthly_reminder_view_all), |
| } | ||
| } | ||
|
|
||
| Spacer(modifier = Modifier.size(80.dp)) |
There was a problem hiding this comment.
LazyColumn 외부에 Spacer를 사용하여 하단 여백을 주는 방식은 리스트가 길어져 스크롤이 생길 경우 의도대로 동작하지 않을 수 있습니다. LazyColumn의 contentPadding 속성을 사용하는 것이 더 바람직합니다.
예시:
LazyColumn(
modifier = Modifier
.fillMaxSize()
.padding(horizontal = 20.dp),
contentPadding = PaddingValues(bottom = 80.dp)
) {
// ...
}이렇게 수정하고 이 Spacer는 제거하는 것을 권장합니다.
| imageRes = typeInfo.imageRes, | ||
| descriptionRes = typeInfo.descriptionRes, | ||
| nextContactAt = friend.nextContactAt, | ||
| daysUntilNextContact = friend.daysUntilNextContact(today), |
There was a problem hiding this comment.
daysUntilNextContact를 계산하는 로직이 도메인 모델인 MonthlyFriend 내부에 있습니다. "D-day", "D-X"와 같은 문자열 형식은 표현(Presentation)과 관련된 로직이므로, 도메인 모델이 아닌 ViewModel에서 처리하는 것이 좋습니다. 이렇게 하면 도메인 모델이 특정 UI 구현에 종속되지 않아 재사용성과 테스트 용이성이 향상됩니다. MonthlyFriend에서는 날짜 정보만 제공하고, ViewModel에서 UI에 표시할 문자열로 가공하는 구조를 권장합니다.
...m/alarmy/near/presentation/feature/mothlyreminderall/components/MonthlyReminderFriendCard.kt
Outdated
Show resolved
Hide resolved
...ava/com/alarmy/near/presentation/feature/mothlyreminderall/components/RecordSuccessDialog.kt
Outdated
Show resolved
Hide resolved
- `combine`을 사용하여 `fetchMonthlyFriends`와 `fetchMonthlyCompleteFriends` API 호출을 병렬로 처리하도록 수정했습니다. - 두 API의 응답을 모두 받은 후에 한 번에 UI 상태를 업데이트하여 데이터 로딩 중 발생할 수 있는 화면 깜빡임 현상을 개선했습니다. - API 응답을 함께 관리하기 위한 `MonthlyReminderCombinedData` data class를 추가했습니다.
- 반복적으로 사용되는 Flow의 `catch` 블록 내 에러 처리 로직을 `handleError` 확장 함수로 분리하여 공통화했습니다. - `MonthlyReminderAllViewModel` 내 `fetchMonthlyReminders`와 `onRecordFriendShip` 함수의 에러 처리 로직에 `handleError`를 적용하여 코드 중복을 제거하고 가독성을 개선했습니다.
- `RecordSuccessDialog`에서 사용하던 하드코딩된 색상 값을 `NearTheme`의 색상으로 변경했습니다. - `NearColor`에 `BLACK_222222` 색상을 추가했습니다.
- 연락 주기까지 남은 날짜를 표시하는 문자열을 "D-day"에서 "D-DAY"로 변경했습니다.
- `MonthlyReminderUIModel`에 연락일이 오늘인지 여부를 나타내는 `isToday` 프로퍼티를 추가했습니다. - `isToday` 값에 따라 D-day 텍스트 스타일(파란색, 굵은 글씨)이 적용되도록 `MonthlyReminderFriendCard`를 수정했습니다. - 월간 리마인더 목록에서 중복된 친구 데이터가 표시되지 않도록 `distinctBy`를 사용하여 필터링 로직을 추가했습니다.
rhkrwngud445
left a comment
rhkrwngud445
left a comment
There was a problem hiding this comment.
고생하셨습니다!
확장함수와 Preview 등 DX에 신경 많이 써주셨네요👍 감사합니다!
코멘트 조금 남겼는데, 확인 부탁드립니다!
| @Preview | ||
| @Composable | ||
| private fun MonthlyReminderAllScreenEmptyPreview() { | ||
| NearTheme { | ||
| MonthlyReminderAllScreen( | ||
| uiState = MonthlyReminderAllUIState.Empty, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Preview | ||
| @Composable | ||
| private fun MonthlyReminderAllScreenLoadingPreview() { | ||
| NearTheme { | ||
| MonthlyReminderAllScreen( | ||
| uiState = MonthlyReminderAllUIState.Loading, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
상세한 프리뷰 좋네요👍
PreviewParameter를 통해 하나의 프리뷰 주석으로 해결할 수도 있습니다!
괜찮으시다면 한 번 적용해보아도 좋을 것 같습니다!
There was a problem hiding this comment.
오..! PreviewParameter라는 것이 있군요! 덕분에 알았습니다
확인해보고 적용해보겠습니다!!
| private val _monthlyReminders = MutableStateFlow<List<MonthlyReminderUIModel>>(emptyList()) | ||
| private val _completedReminders = MutableStateFlow<List<MonthlyReminderUIModel>>(emptyList()) |
There was a problem hiding this comment.
내부 데이터 캐싱을 위해 사용되나 보네요! 외부 노출이 안된다면
언더스코어는 제외해도 될 것 같습니다!
There was a problem hiding this comment.
내부에서 사용되는 것들은 언더스코어로 다 작성을 하였는데
외부에서는 알 필요가 없는 프로퍼티는 생략하는 것이 좋겠네요 감사합니다!
| LaunchedEffect(viewModel.uiEvent) { | ||
| launch { | ||
| viewModel.uiEvent.collect { event -> | ||
| when (event) { | ||
| is MonthlyReminderAllUIEvent.ShowError -> { | ||
| onShowErrorSnackBar(event.throwable) | ||
| } | ||
|
|
||
| is MonthlyReminderAllUIEvent.RecordFriendShipSuccess -> { |
There was a problem hiding this comment.
launch 블록은 제외해도 될 것 같은데, 지석님 의견은 어떠실까요?!
추가적으로 LaunchedEffect에서 uiEvent를 인자로 쓰면 어떤 일이 일어날까요?🤔
There was a problem hiding this comment.
좋은 의견 감사합니다! 말씀하신 부 동의합니다!
확인해보니 이미 LaunchEffect 내부 람다에서 CoroutineScope를 제공하고 있어 불필요하네요!!
viewModel.uiEvent를 key로 사용하면, 아마 uiEvent 객체 자체를 key로 하기 때문에 viewmodel의 소멸 / 생성할 때 마다 LaunchEffect가 시작된다고 알고 있습니다.. 특별한 케이스에 대해서는 생각이 나는 것이 잘 없네요,,! 😢 고려해야할 부분이 있을까요/?
There was a problem hiding this comment.
제가 내부 값인 Event 타입과 헷갈렸군요😅 지석님께서 말씀주신대로 viewModel의 Flow 맵핑이 되어있어 실제 동작에서는 문제가 없는 것이 맞습니다!
Flow가 불변이라서 해당 값으로 지정주신 것이면, Unit, true 등으로 최초 컴포지션 시 1회만 실행됨을 명시적으로 표현할 수 있을 것 같아요!
There was a problem hiding this comment.
네! Unit으로 명확하게 해두는 것이 좋을 것 같네요! 확인해주셔서 감사합니다 :)
| }.launchIn(viewModelScope) | ||
| } | ||
|
|
||
| private fun updateUIState() { |
There was a problem hiding this comment.
"UI 상태를 업데이트한다" 라는 의미를 가지기에는 추가적인 로직이 포함되어 있는 것 같습니다!
해당 함수명만 보고 내부 기능을 유추할 수 있게 수정해보면 어떨까요?
함수명이 길어도 함수 내부를 보는 것보다 이점이 있을 것 같아요!
There was a problem hiding this comment.
명확하게 네이밍하는것이 중요한데 해당 부분에서 어떻게 네이밍을 해야할지 고민하다가 수정을 놓친 것 같습니다! 캐치 감사드립니다!
`LaunchedEffect` 내에서 `collect`를 사용할 때 불필요한 `launch` 블록을 제거하여 코드를 간소화했습니다.
- `_monthlyReminders`와 `_completedReminders`에 대한 불필요한 `_` 접두사를 제거하여 코드를 간소화했습니다.
- `updateUIState` 함수의 역할을 더 명확하게 나타내기 위해 `combineRemindersToUIState`로 이름을 변경했습니다.
작업 내용
홈 화면에서 이번달 챙길사람 화면으로 이동합니다.
이번달 챙길사람 UI 구성
챙김 기록하기 기능
확인 방법
참고 사항
만약 이번달 챙길 사람은 없고, 챙김 완료한 사람이 있는 경우는
"이번달은 챙길 사람이 없네요"를 표기해야하는지
챙김 완료만 보여주는지 확인 후 수정
현재는 이번달 챙김 리스트와 챙김 완료가 모두 비어야 빈 화면을 표시합니다.
챙김 완료시 표시되는 날짜는
챙긴 날짜를 표시하는 것인지,
기념일을 표시하는 것인지 확인 후 수정
현재는 기념일을 표시합니다
관련 이슈