커스텀 테마 리팩토링 #382
커스텀 테마 리팩토링 #382
Conversation
| override suspend fun fetchThemes() { | ||
| api._getThemes().let { themes -> | ||
| _customThemes.value = themes.filter { it.isCustom }.map { it.toTableTheme() as CustomTheme } | ||
| _customThemes.value = themes.filter { it.isCustom == true }.map { it.toTableTheme() as CustomTheme } |
There was a problem hiding this comment.
이건 보다가 궁금한건데 기존 로직 (it.isCustom)에서 바뀐 로직 (it.isCuston == true)으로 바꾼 이유가 있어?
내가 찾은 바로는 어차피 kotlin/jvm 컴파일러는 '== true' 부분 제거해서 처리한다고 하긴 하는데
그러면 원래대로 하는게 낫지 않나 싶어서
There was a problem hiding this comment.
아 저거 isCustom을 nullable로 바꿔서 그래
원래는 non-nullable이고 기본값을 넣어놨었던 걸 DTO답게 nullable으로 바꿨어
| val themeConfigViewModel = hiltViewModel<ThemeConfigViewModel>(parentEntry) | ||
| ThemeConfigRoute( | ||
| themeConfigViewModel = themeConfigViewModel, | ||
| onNavigateBack = { navController.popBackStack() }, | ||
| onNavigateToDetail = { theme -> | ||
| when (theme) { | ||
| is CustomTheme -> navController.navigate("${NavigationDestination.ThemeDetail}?themeId=${theme.id}") | ||
| is BuiltInTheme -> navController.navigate("${NavigationDestination.ThemeDetail}?theme=${theme.code}") | ||
| } | ||
| }, | ||
| onClickAddTheme = { | ||
| navController.navigate(NavigationDestination.ThemeDetail) | ||
| }, |
There was a problem hiding this comment.
여기는 parentEntry 넣어주는 이유가 있는거지?
나도 전에 비슷한 고민 한적이 있는데, 일단 이런 구조가 괜찮은지 + 그걸 대체하는게 좋은건지 등...
추가로 기억해뒀다가 Navigation 부분도 잘 바꿔봐야겠다 지금은 바꾸기 어렵겠지만...
There was a problem hiding this comment.
밑에 보니까 바로 알겠네 BottomSheet에서 변경되는거 반영되려고 하는것처럼 보이네
There was a problem hiding this comment.
오잉 그래도 Home을 받을 필요는 없을 거 같은데 왜 그렇게 해놨지 확인해볼게
There was a problem hiding this comment.
ㄹㅇ 왜그랬지 일단 지웠어ㅋㅋㅋ
ThemeConfigViewModel의 모든 상태가 싱글턴인 Repository의 상태라서 아무 문제 없을듯
물론 Repository가 상태를 가지는 게 그거대로 이상하지만 일단 패스...
9b1a005
| themeConfigViewModel: ThemeConfigViewModel = hiltViewModel(), | ||
| timetableViewModel: TimetableViewModel = hiltViewModel(), |
There was a problem hiding this comment.
근데 혹시 이렇게 ViewModel 통째로 주는거 말고
필요한 값이나 함수만 전달하는건 별로인가?
There was a problem hiding this comment.
그게 맞긴 한데 ChangeThemeBottomSheet가 depth가 너무 깊어서 저 부분에선 그냥 뷰모델 받았던 듯
| val apiOnError = LocalApiOnError.current | ||
| val apiOnProgress = LocalApiOnProgress.current |
There was a problem hiding this comment.
혹시 하는 김에 이 부분 개선도 시도해볼만 할까?
조금 큰 작업일것 같긴 한데, 위에 보니까 어차피 Flow로 내려주는 김에 .asResult() 이런거 만들어서 쓸 수 있을 것 같기도 해서.
There was a problem hiding this comment.
아하 NIA의 .asResult()
나는 좋아 ApiOnError ApiOnProgress 없애고 싶어
NIA .asResult()보니까 Loading, Success, Fail 상태 갱신을 Flow로 간단하게 할 수 있네
There was a problem hiding this comment.
별도로 분리해서 하는 게 좋긴 할듯...!
|
ㅋㅋㅋㅋㅋ신경 못 쓰고 있었네요 미안... |
커스텀테마 너무 못만들어서 도저히 테마마켓을 붙일 수가 없었음.....
그래도 1년전보다는 낫지 않을까.....!
커밋관리 실패해버렸어 미안.....