[오둥이] 장바구니 미션 제출합니다. #4
Conversation
readme config: package 이름과 어플리케이션 id 일치 feat: 장바구니 TopAppbar build: Coil 추가 config: Internet Perssion 추가 feat: Product 모델 feat: ProductPreviewProvider 구현 feat: 가격 포메팅 StringRes refactor: ProductScreen 및 컴포넌트화
build: navigation 추가 ui: init ProductDetailScreen ui: init CartScreen build: kotlin-serialization, navigation 의존성 feat: Navigation 적용 feat: 장바구니 화면의 빈 껍데기를 연결한다. feat: 상품 목록에서 상품을 누르면 상품 상세 화면으로 이동한다. feat: 뒤로 가기 버튼이나 아이콘을 누르면 직전 화면으로 돌아간다. ui: Product StringRes화 build: navigation-compose-test gradle 추가 refactor: 데이터 ProductRepository 도입 test: NavigationTest test: ProductScreenTest ui: ShoppingButton refactor: Product property 네이밍 변경 refactor: ProductRepository Interface화, FakeProductRepository 적용 feat: 상품 상세 화면을 구현, 장바구니 담기 버튼을 누르면 장바구니 화면으로 이동
refactor: ProductScreenTest product/ 패키지 이동 feat: ProductRepository 반환괎 Flow로 마이그레이션 feat: CartProduct, CartRepository feat: Counter Component refactor: ProductImage 컴포넌트 분리 refactor: ProductItem 컴포넌트 분리 feat: CartProductItem 컴포넌트 feat: 장바구니 화면 구현 feat: 상품을 장바구니에 담는 기능을 구현한다. refactor: presentation 패키지에 이동 feat: 주문 완료 시 장바구니가 비워진다. test: CartScreenTest test: FakeRepository 적용
feat: + 아이콘을 누르면 장바구니에 상품이 추가됨과 동시에 수량 조절 버튼이 노출된다. feat: 상품 목록에서 장바구니에 담을/담긴 상품의 수량을 조절할 수 있다.
| companion object { | ||
| private var cartRepository: CartRepository? = null | ||
|
|
||
| fun get(): CartRepository { | ||
| return cartRepository ?: DefaultCartRepository(ProductRepository.get()).also { | ||
| cartRepository = it | ||
| } | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| fun set(repository: CartRepository) { | ||
| cartRepository = repository | ||
| } | ||
| } |
| if (product != null) { | ||
| ShoppingButton( | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| text = stringResource(id = R.string.product_detail_add_cart_button), | ||
| onClick = onCartAdd | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
해당 코드와 아래 코드 모두 Product가 null일 경우 에러 뷰가 표시되도록 설정된 것으로 보입니다.
루트 컴포저블에서 최초로 Product가 null인지 확인하여 에러 뷰를 표시하는 대신, 각 컴포저블에서 개별적으로 분기 처리를 한 이유가 궁금합니다. 이 분기 처리가 재사용되는 컴포저블에서 이루어진 것이라면 납득할 수 있지만, 현재는 재사용되지 않는 뷰에서 분기처리를 하고 있어 의문이 듭니다.
There was a problem hiding this comment.
질문의 의도가 헷갈려서 2가지 관점으로 답변드리도록 하겠습니다!
- Screen 단이 아닌 NavGraphBuilder 의 composable{} 안에서 분기처리하는 것이 더 적절하다.
저는 Screen 이 UI의 상태에 따라 적절한 뷰를 보여주는 것이 Screen 컴포넌트의 책임이라 생각합니다.
따라서, 상태(e. loading, error, empty) 에 따라 적절한 뷰를 보여주는 것을 선호합니다. 😉
- ProductDetailScreen 이 Passive하지 않다.
현재 UI 가 Product 가 Null 일 때 Error 인지 판단하�고 있어 Passive 하지 않습니다.
저 또한, 공감하는 부분입니다.
UiState 로 스테이트홀더를 부분 적용하고, UI는 UIState 를 관찰하여 상태에 따라 적절한 화면을 보여주는 것이 적절할 것 같습니다! 💪
| @Composable | ||
| private fun ProductImage( | ||
| imageUrl: String | ||
| ) { | ||
| AsyncImage( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .aspectRatio(1f), | ||
| model = imageUrl, | ||
| contentDescription = stringResource(id = R.string.product_content_description) | ||
| ) | ||
| } |
There was a problem hiding this comment.
이미 ProductImage라는 재사용 가능한 컴포넌트를 이미 만드셨는데, 따로 생성하신 이유가 있나요?!
There was a problem hiding this comment.
아니요.. 리팩토링하면서 놓쳤던거 같습니다! 삭제하도록 하겠습니다 🚀
| @Composable | ||
| private fun ProductPrice( | ||
| price: Int | ||
| ) { |
There was a problem hiding this comment.
크으 이부분도 레아한테 리뷰 받은 부분인데 너무 잘해주셨네요!
| fun ProductScreen( | ||
| products: List<ProductUiModel>, | ||
| onItemClick: (id: Long) -> Unit, | ||
| onCartClick: () -> Unit, | ||
| onProductPlus: (id: Long) -> Unit, | ||
| onProductMinus: (id: Long) -> Unit, | ||
| ) { |
| shape = RoundedCornerShape(16.dp) | ||
| ), | ||
| imageUrl = product.imageUrl, | ||
| contentDescription = "상품 이미지", |
There was a problem hiding this comment.
contentDescription을 string resources에 따로 지정 안하신 이유가 있을까요?
| Box(modifier = modifier.pointerInput(Unit) { | ||
| detectTapGestures {} | ||
| }) { |
There was a problem hiding this comment.
부모의 터치 이벤트 전파를 방지하기 위해서 작성한 코드입니다.
다음과 같이 작성하는 것이 좀 더 이해하기 쉽겠네요 ㅎㅎ
Modifier.clickable(enabled=false, onClick={})그리고, ProductItemCounter 보다는 사용하는 클라이언트 코드에서 결정할 수 있도록 수정하겠습니다!
반영 커밋: 872813d
| @Composable | ||
| fun ProductImage( | ||
| imageUrl: String, | ||
| modifier: Modifier = Modifier, | ||
| contentDescription: String? = null | ||
| ) { | ||
| AsyncImage( | ||
| model = imageUrl, | ||
| contentDescription = contentDescription, | ||
| modifier = modifier | ||
| ) | ||
| } |
There was a problem hiding this comment.
저는 레아한테 아래 리뷰를 받았는데요. 오둥이는 너무 잘해주셨네요 👏
장바구니 앱에서 활용되는 AsyncImage 관련 코드를 공통된 컴포넌트로 분리해보면 어떨까요?
여러 가지 이점이 있겠지만, 대표적으로는 나중에 Coil에서 Glide등으로 마이그레이션 하는 의사결정을 하게 된다면 변경사항을 최소화할 수 있습니다.
There was a problem hiding this comment.
얼마전에 Picasso 가 deprecated 됐더라구요..
진짜 무섭습니다 🥶
레벨 1때 배웠던 것처럼 서드파티 라이브러리의 경우 Adapter 패턴을 사용하여 감싸, 유연하게 대처할 수 있도록 설계하는 것이 중요하다는 생각이 들었습니다 😁
| ) { | ||
| IconButton( | ||
| onClick = onMinus, | ||
| modifier = Modifier |
There was a problem hiding this comment.
modifier = Modifier 이렇게 구현하신 이유가 따로 있을까요?
| if (product != null) { | ||
| ShoppingButton( | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| text = stringResource(id = R.string.product_detail_add_cart_button), | ||
| onClick = onCartAdd | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
질문의 의도가 헷갈려서 2가지 관점으로 답변드리도록 하겠습니다!
- Screen 단이 아닌 NavGraphBuilder 의 composable{} 안에서 분기처리하는 것이 더 적절하다.
저는 Screen 이 UI의 상태에 따라 적절한 뷰를 보여주는 것이 Screen 컴포넌트의 책임이라 생각합니다.
따라서, 상태(e. loading, error, empty) 에 따라 적절한 뷰를 보여주는 것을 선호합니다. 😉
- ProductDetailScreen 이 Passive하지 않다.
현재 UI 가 Product 가 Null 일 때 Error 인지 판단하�고 있어 Passive 하지 않습니다.
저 또한, 공감하는 부분입니다.
UiState 로 스테이트홀더를 부분 적용하고, UI는 UIState 를 관찰하여 상태에 따라 적절한 화면을 보여주는 것이 적절할 것 같습니다! 💪
| @Composable | ||
| private fun ProductImage( | ||
| imageUrl: String | ||
| ) { | ||
| AsyncImage( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .aspectRatio(1f), | ||
| model = imageUrl, | ||
| contentDescription = stringResource(id = R.string.product_content_description) | ||
| ) | ||
| } |
There was a problem hiding this comment.
아니요.. 리팩토링하면서 놓쳤던거 같습니다! 삭제하도록 하겠습니다 🚀
| @Composable | ||
| private fun ProductPrice( | ||
| price: Int | ||
| ) { |
| shape = RoundedCornerShape(16.dp) | ||
| ), | ||
| imageUrl = product.imageUrl, | ||
| contentDescription = "상품 이미지", |
| Box(modifier = modifier.pointerInput(Unit) { | ||
| detectTapGestures {} | ||
| }) { |
There was a problem hiding this comment.
부모의 터치 이벤트 전파를 방지하기 위해서 작성한 코드입니다.
다음과 같이 작성하는 것이 좀 더 이해하기 쉽겠네요 ㅎㅎ
Modifier.clickable(enabled=false, onClick={})그리고, ProductItemCounter 보다는 사용하는 클라이언트 코드에서 결정할 수 있도록 수정하겠습니다!
반영 커밋: 872813d
| ) { | ||
| IconButton( | ||
| onClick = onMinus, | ||
| modifier = Modifier |
| @Composable | ||
| fun ProductImage( | ||
| imageUrl: String, | ||
| modifier: Modifier = Modifier, | ||
| contentDescription: String? = null | ||
| ) { | ||
| AsyncImage( | ||
| model = imageUrl, | ||
| contentDescription = contentDescription, | ||
| modifier = modifier | ||
| ) | ||
| } |
There was a problem hiding this comment.
얼마전에 Picasso 가 deprecated 됐더라구요..
진짜 무섭습니다 🥶
레벨 1때 배웠던 것처럼 서드파티 라이브러리의 경우 Adapter 패턴을 사용하여 감싸, 유연하게 대처할 수 있도록 설계하는 것이 중요하다는 생각이 들었습니다 😁
4단계까지 구현하고 제출합니다~
다른 일정 때문에 많이 늦어졌지만.. 고민해나가면서 미션을 수행했다는 것만으로도 많은것을 배웠습니다!
재사용할만한 컴포넌트를 2가지로 구분해서 위치시켰습니다!
ui/componentcart/component,product/component나머지의 경우 모두 private function 으로 두었습니다.
시연영상
2024-11-03.8.42.49.mov