Conversation
| @Composable | ||
| fun ProductGridItem( | ||
| product: ProductData, | ||
| showWishButton: Boolean = true, | ||
| onWishClick: () -> Unit = {} | ||
| ) { |
jeongkyueun
left a comment
jeongkyueun
left a comment
There was a problem hiding this comment.
LazyVerticalGrid로 마이그레이션하면서 기존 Adapter 구조보다 훨씬 코드가 직관적이고 깔끔해진 것 같습니다! 잘 구현하셨고 고생 많으셨습니다!
kimdoyeon1234
left a comment
kimdoyeon1234
left a comment
There was a problem hiding this comment.
중첩 네비게이션(BuyGraph)까지 구현하신 점, ViewModel에서 상태를 안전하게 관리하신 점, SharedPreferences로 위시리스트를 영속적으로 저장하신 점 모두 훌륭합니다!
다만 HomeScreen에서 verticalScroll이 붙은 Column 안에 LazyRow를 중첩하고 계신데, LazyRow의 높이 측정이 제대로 되지 않아 레이아웃이 깨질 수 있습니다! LazyColumn으로 변경하고 item { } / items { } DSL로 구성해주세요!
그리고 selectedTabIndex 상태가 MainScreen에서 관리되고 있는데, 탭 상태는 BuyScreen 내부나 ViewModel에서 관리하는 게 더 적절합니다!
바텀 네비게이션 라벨도 하드코딩된 문자열 대신 strings.xml로 분리해주시면 좋겠습니다!
전반적으로 완성도가 매우 높고 챌린저 미션 그 이상을 구현하셨습니다! 수고하셨습니다
| fun HomeScreen(viewModel: MainViewModel) { | ||
| val products = viewModel.productList | ||
| val scrollState = rememberScrollState() | ||
|
|
||
| Column( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .padding(16.dp) | ||
| .verticalScroll(scrollState) | ||
| ) { | ||
| Text( | ||
| text = stringResource(id = R.string.Title), | ||
| fontSize = 40.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| modifier = Modifier.padding(top = 24.dp, start = 20.dp) | ||
| ) | ||
| Text( | ||
| text = stringResource(id = R.string.date), | ||
| fontSize = 20.sp, | ||
| modifier = Modifier.padding(start = 20.dp, bottom = 30.dp) | ||
| ) | ||
| Image( | ||
| painter = painterResource(id = R.drawable.img_home), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .align(Alignment.CenterHorizontally), | ||
| contentScale = ContentScale.Crop | ||
| ) | ||
|
|
||
| Text( | ||
| text = stringResource(id = R.string.newEn), | ||
| fontSize = 16.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| modifier = Modifier.padding(start = 20.dp, top = 20.dp) | ||
| ) | ||
| Text( | ||
| text = stringResource(id = R.string.newKr), | ||
| fontSize = 24.sp, | ||
| modifier = Modifier.padding(start = 20.dp, bottom = 12.dp) | ||
| ) | ||
|
|
||
| LazyRow( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| contentPadding = PaddingValues(horizontal = 16.dp), | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp) | ||
| ) { | ||
| items( | ||
| items = products, | ||
| key = { item -> item.id }, | ||
| contentType = { "ProductHorizontalItem" } | ||
| ) { product -> | ||
| HorizontalProductItem( | ||
| product = product, | ||
| onWishClick = { viewModel.toggleWishStatus(product.id) } | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
스크롤 방향이 달라서 크래시는 발생하지 않지만, verticalScroll이 붙은 Column 안에 Lazy 계열 컴포저블을 중첩하면 LazyRow의 높이를 측정할 수 없어서 레이아웃이 깨질 수 있습니다! Column 대신 LazyColumn으로 바꾸고 item { } / items { } DSL로 모든 콘텐츠를 구성하는 걸 추천드립니다!
There was a problem hiding this comment.
현재 selectedTabIndex를 MainScreen에서 remember로 관리하고 계신데, 탭 상태는 BuyScreen에 속하는 UI 상태이므로 BuyScreen 내부나 ViewModel에서 관리하는 게 더 적절합니다! MainScreen이 탭 상태까지 알고 있을 필요가 없습니다!
4 participants
📌 PR 제목
🔗 관련 이슈
Closes #55
✨ 변경 사항
items()함수를 사용하여 동일한 리스트 데이터 표시key를 지정하여 상태 안정성 확보🔍 테스트
📸 스크린샷 (선택)
🚨 추가 이슈