Step2 - GitHub(데이터 레이어) #35
Conversation
| modifier = Modifier.fillMaxSize(), | ||
| color = MaterialTheme.colorScheme.background | ||
| ) { | ||
| RepositoryListScreenStateful(viewModel = viewModel) |
There was a problem hiding this comment.
인지하고 작성하신 것으로 추측은 됩니다.
일반적으로 화면 레벨의 컴포저블을 작성할 때에는 XXXScreen을 사용하고는 합니다.
RepositoryListScreen(viewModel: ViewModel) { ... }
RepositoryListScreen(stateless: ...) { ... }동일한 이름의 컴포저블이 여러 시그니처를 가지는 것 처럼 유사한 방법입니다.
There was a problem hiding this comment.
앞선 과정의 힌트 코드에서 말씀하신 방식을 경험했던 것 같은데도 Stateful과 같은 사족을 더해버렸네요. 동일한 이름으로 변경하는 것이 좋을 것 같습니다!
| var repositories by remember { mutableStateOf<List<NextStepRepositoryEntity>>(emptyList()) } | ||
|
|
||
| LaunchedEffect(Unit) { | ||
| repositories = viewModel.getRepositories("next-step") | ||
| } |
There was a problem hiding this comment.
지금 구조라면, 데이터를 뷰모델을 통해 받아올 필요가 있나요?
레포지터리에서 직접 데이터를 받아와도 동일한 것은 아닌지 생각이 들었습니다.
뷰모델을 Stateless로 만든 이유가 있을까요?
There was a problem hiding this comment.
구조적으로나 현재 요구 사항을 만족하는 정도에선 말씀하신 것처럼 레포지터리에서 바로 데이터를 가져와도 결과는 같습니다. 하지만 미션의 프로그래밍 요구 사항인 'UI 레이어는 데이터 레이어를 의존하지만, 데이터 레이어는 UI 레이어를 의존해선 안 된다.'를 만족하기 위해서 뷰모델을 추가하였습니다.
뷰모델을 Stateless로 만든 이유는 일단 현재는 뷰모델을 사용하는 곳이 여러 곳이 아니라서 이렇게 했던 것 같습니다. 그런데 피드백 주셔서 다시보니 컴포저블 함수에서 UI 처리에만 집중할 수 있도록 해주는 것이 좀 더 좋겠다는 생각이 드네요.
| modifier = Modifier | ||
| .background(MaterialTheme.colorScheme.surface) |
There was a problem hiding this comment.
배경색이 필요한 것은 TopAppBar 인가요? 아니면 Text 인가요?
There was a problem hiding this comment.
제가 생각한 것은 TopAppBar인데 Text에만 적용하고 있네요. 저의 명백한 실수네요. TopAppBar에 적용하도록 변경해 보겠습니다!
| ) { innerPadding -> | ||
| LazyColumn( | ||
| modifier = Modifier | ||
| .padding(innerPadding) |
There was a problem hiding this comment.
LazyColumn에 있는 contentPadding과는 어떤 차이가 있을까요?
There was a problem hiding this comment.
padding은 LazyColumn 외부 패딩을 설정하는 것으로 알고 있었습니다. contentPadding는 내부 아이템 패딩을 지정할 수 있는 것으로 알고 있습니다. 리스트를 다루기에 contentPadding가 더 적절하겠네요. 꼼꼼한 리뷰 감사합니다!
| modifier = Modifier | ||
| .padding(innerPadding) | ||
| .fillMaxSize() | ||
| .background(MaterialTheme.colorScheme.surface) |
There was a problem hiding this comment.
LazyColumn에 배경색을 지정하고 싶은 니즈라고 봐야 할까요? 아니면 xml에서 windowBackground 같은 역할을 하고 싶은 것일까요?
Scaffold에서 제공하는 containerColor를 활용할 수 있습니다.
There was a problem hiding this comment.
와 후자와 같은 역할을 생각했는데 알려주신 containerColor를 사용하는 것이 적절하겠네요. 감사합니다!
| RepositoryItem(repository = repository) | ||
| HorizontalDivider( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| thickness = 1.dp, |
| items(repositories) { repository -> | ||
| RepositoryItem(repository = repository) | ||
| HorizontalDivider( | ||
| modifier = Modifier.fillMaxWidth(), |
There was a problem hiding this comment.
마찬가지로 HorizontalDivider 내부에서 처리하고 있는 코드를 살펴보면 좋을 것 같습니다.
There was a problem hiding this comment.
LazyColumn에서 가로 넓이를 최대로 설정하고 있어서 modifier도 생략해도 되겠네요. 색상은 기본값으로 정확히 어떤 값을 주고 있는지 찾다 찾다가.. 파악을 못했는데 육안으로는 피그마 가이드와 비슷한 것 같습니다.
| HorizontalDivider( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| thickness = 1.dp, | ||
| color = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.2f) |
There was a problem hiding this comment.
마이너한 의견입니다.
테마를 사용한다면 HorizontalDivider에서 사용하는 기본 색상을 일괄적으로 적용이 가능한데요!
내부 코드에서 색상을 어떻게 지정하는지 찾아보셔도 좋습니다.
There was a problem hiding this comment.
앗 이곳에도 색상에 대한 의견이 있었네요. 정확한 헥사값을 피그마와 비교하고 싶었는데 그 값을 찾지 못했고 기본값을 할당해 주고 있는 것을 보았습니다!
| @Preview(showBackground = true) | ||
| @Composable | ||
| fun StatelessRepositoryListScreenPreview() { |
There was a problem hiding this comment.
저는 개인적으로 Preview가 이처럼 길어지는 것을 피하고자 이런식으로 작성하고는 합니다.
Junit5의 DisplayName와 비슷하다라고 할까요?
@Preview(name = "XX 케이스")
private fun Preview1() { ... }There was a problem hiding this comment.
저도 함수명이 조금 거슬렸는데 꿀팁 너무 감사합니다. 👍
| @Preview(showBackground = true) | ||
| @Composable | ||
| fun StatelessRepositoryListScreenPreview() { |
There was a problem hiding this comment.
하나 더, 미리보기 코드는 파일의 최하단에 작성 해주는 것이 좋습니다.
왜..라고 한다면 관습적이라고 해야 할까요?
만약 현재 파일과 관계가 없는 별도의 컴포저블이라면 파일을 분리하는 것도 방법입니다.
There was a problem hiding this comment.
아하 이것도 좋은 정보네요! 저는 프리뷰와 관련된 컴포저블 바로 아래에 두면 좋다는 것을 어디선가 들은 것 같아서 일단 그 규칙을 따라 왔었는데요. 말씀해 주신 방법도 편리할 것 같습니다.
영직님 안녕하세요. 😃
강의 기간 내 마무리 하지 못하고 이렇게 늦게 PR을 보내어 면목도 없고 죄송한 마음이 큽니다.
끝까지 응원해 주셔서 감사합니다.
이번 단계까지는 컴포즈보다는 아키텍처에 조금 더 집중되어 있다는 느낌을 받았습니다.
또한 좋았던 점은 컴포넌트에 ViewModel을 주입 받으니 프리뷰 표시를 위해서 자동적으로 Stateless/Stateful로 나누게 되었다는 점입니다. 물론 그렇지 않아도 신경써야 하는 부분이겠지만 자연스럽게 그렇게 유도된 점이 좋았습니다.
리뷰는 시간 나실 때 천천히 봐주세요. 잘 부탁드리겠습니다.
스크린샷