알림 페이지 리팩토링 #379
알림 페이지 리팩토링 #379
Conversation
| fun getNotificationTime(context: Context, info: Notification): String { | ||
| try { |
There was a problem hiding this comment.
이거 알림 시간 받아서, 현재 시간이랑 비교한 다음에 어떻게 텍스트로 띄울지 변환하는 부분인데
- 일단 context를 제거하고 싶은 마음이 굴뚝 같았으나 어쩔 수 없었음...
- SimpleDateFormat 쓰지 말고 다르게 하라는데 이것보다는 다른 부분이 중요하다고 생각해서 일단 보류
고치려면 고칠 수 있는 부분이라고 생각되지만 일단 PR 올리고 얘기해본 뒤에 고치기로 생각해서 그대로인 상태
| @@ -258,7 +258,7 @@ class RootActivity : AppCompatActivity() { | |||
|
|
|||
| composable2(NavigationDestination.ImportantNotice) { ImportantNoticePage() } | |||
|
|
|||
| composable2(NavigationDestination.Notification) { NotificationPage() } | |||
| composable2(NavigationDestination.Notification) { NotificationRoute() } | |||
There was a problem hiding this comment.
설명에 써놓은 대로, Navigation은 그대로 뒀기에 Page()가 아니라 Route()를 호출하는 언뜻 보면 이상한 상황 발생
|
|
||
| if (viewModel.isRefactoring()) { | ||
| NotificationPage( |
There was a problem hiding this comment.
동엽이형 말로부터 영감을 받아 refactor 상태를 알 수 있게 따로 함수로 둠
보니까 컴포저블에 의존성 주입은 안되는듯... 애초에 클래스가 아니라 함수기도 하고 (내가 방법을 못찾은 거일지도)
| @Composable | ||
| fun NotificationPage( | ||
| modifier: Modifier = Modifier, | ||
| uiState: NotificationUiState, | ||
| onBackClick: () -> Unit, | ||
| ) { | ||
| Column( | ||
| modifier = Modifier | ||
| .background(SNUTTColors.White900) |
There was a problem hiding this comment.
이제 여기에는 context나 navController 이런거 필요 없음
|
|
||
| @Composable | ||
| fun NotificationItem(notification: Notification, onClick: () -> Unit) { | ||
| val context = LocalContext.current | ||
| Column( | ||
| modifier = Modifier |
There was a problem hiding this comment.
NotificationItem과, NotificationIcon은 딱히 달라진거 없고 그냥 어떤 타입을 받느냐 (네트워크/도메인 모델 분리해서) 그것만 달라지고, 함수 분리만 조금 한 정도
There was a problem hiding this comment.
그래서 같은 이름을 갖는 함수가 2개씩 있음
NotificationPage, NotificationItem 등등..
| when (snuttAppState) { | ||
| SNUTTAppState.NORMAL -> { | ||
| notificationRepository.getNotificationResultStream() | ||
| .cachedIn(viewModelScope) | ||
| .collect { | ||
| _notificationResult.emit(it) | ||
| } | ||
| } | ||
| SNUTTAppState.REFACTOR -> { | ||
| notificationRepository.getNotificationListStream() | ||
| .cachedIn(viewModelScope) | ||
| .collect { | ||
| _notificationList.emit(it) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
여기도 마찬가지, 경우에 따라 다른 타입의 PagingData를 받아야 하므로 거의 비슷한 부분이 중복으로 2개
|
벌써기대되는군 |
| @Provides | ||
| @Singleton | ||
| fun provideSNUTTAppState(): SNUTTAppState { | ||
| return SNUTTAppState.REFACTOR | ||
| } |
There was a problem hiding this comment.
이것도 상관없긴 한데 flavor dimension 새로 하나 만들어서 BuildConfig로 관리해도 좋을듯ㅋㅋ
| fun NotificationRoute( | ||
| modifier: Modifier = Modifier, | ||
| viewModel: NotificationsViewModel = hiltViewModel() | ||
| ) { | ||
| val navController = LocalNavController.current | ||
| if (viewModel.isRefactoring()) { | ||
| NotificationPage( | ||
| onBackClick = { | ||
| if (navController.currentDestination?.route == NavigationDestination.Notification) { | ||
| navController.popBackStack() | ||
| } | ||
| }, | ||
| modifier = modifier, | ||
| ) | ||
| } else { | ||
| NotificationPage() | ||
| } | ||
| } |
There was a problem hiding this comment.
새로 생기는 UI 코드들도, flavor 다르게 빌드하고 아예 다른 폴더에 격리시켜버리는 건 어떄
뭐가 정답인지 잘 모르지만, 오답지라도 만들면서 고쳐나가기 위한 PR
바뀐 점
특이점