diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapper.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapper.kt new file mode 100644 index 000000000000..ea03e7a9363e --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapper.kt @@ -0,0 +1,215 @@ +package com.woocommerce.android.ui.products.details + +import com.woocommerce.android.R +import com.woocommerce.android.ui.products.details.ProductDetailViewModel.ProductDetailViewState.AuxiliaryState +import com.woocommerce.android.ui.products.details.ProductDetailViewModel.ProductDetailViewState.AuxiliaryState.Error +import com.woocommerce.android.ui.products.details.ProductDetailViewModel.ProductDetailViewState.AuxiliaryState.Loading +import com.woocommerce.android.ui.products.details.ProductDetailViewModel.ProductDetailViewState.AuxiliaryState.None +import com.woocommerce.android.ui.products.models.ProductProperty +import com.woocommerce.android.ui.products.models.ProductPropertyCard + +class ProductDetailUiMapper { + fun mapScreenState( + auxiliaryState: AuxiliaryState, + hasProduct: Boolean, + cards: List, + showAddMore: Boolean, + showLinkedProductPromo: Boolean, + ): ProductDetailScreenState = when (auxiliaryState) { + Loading -> ProductDetailScreenState.Loading + is Error -> if (auxiliaryState.message == R.string.product_detail_product_not_selected) { + ProductDetailScreenState.Empty(auxiliaryState.message) + } else { + ProductDetailScreenState.Error(auxiliaryState.message) + } + None -> if (hasProduct) { + ProductDetailScreenState.Content( + cards = cards, + showAddMore = showAddMore, + showLinkedProductPromo = showLinkedProductPromo, + ) + } else { + ProductDetailScreenState.Empty() + } + } + + fun map(cards: List): List { + val cardKeyOccurrences = mutableMapOf() + return cards.map { card -> + val rows = mapRows(card.properties) + val cardBaseKey = when (card.type) { + ProductPropertyCard.Type.PRIMARY -> PRIMARY_CARD_KEY + ProductPropertyCard.Type.SECONDARY -> if (card.properties.any { it.isBlazeProperty() }) { + BLAZE_CARD_KEY + } else { + SECONDARY_CARD_KEY + } + } + val cardKey = cardBaseKey.withOccurrence(cardKeyOccurrences) + + ProductDetailCardUiModel( + key = cardKey, + style = when (card.type) { + ProductPropertyCard.Type.PRIMARY -> ProductDetailCardStyle.PRIMARY + ProductPropertyCard.Type.SECONDARY -> ProductDetailCardStyle.SECONDARY + }, + caption = card.caption, + rows = rows, + ) + } + } + + private fun mapRows(properties: List): List { + val keyOccurrences = mutableMapOf() + return properties.mapIndexed { index, property -> + val key = property.semanticKey().withOccurrence(keyOccurrences) + property.toUiModel(key).let { row -> + if (row is ProductDetailRowUiModel.Rating) { + row.copy(showDivider = index != properties.lastIndex) + } else { + row + } + } + } + } + + private fun ProductProperty.toUiModel(key: String): ProductDetailRowUiModel = when (this) { + ProductProperty.Divider -> ProductDetailRowUiModel.Divider(key) + is ProductProperty.Property -> toPropertyUiModel(key) + is ProductProperty.ComplexProperty -> toComplexPropertyUiModel(key) + is ProductProperty.RatingBar -> toRatingUiModel(key) + is ProductProperty.Editable -> toEditableUiModel(key) + is ProductProperty.PropertyGroup -> toPropertyGroupUiModel(key) + is ProductProperty.Link -> toLinkUiModel(key) + is ProductProperty.Button -> toButtonUiModel(key) + is ProductProperty.Switch -> toSwitchUiModel(key) + is ProductProperty.Warning -> ProductDetailRowUiModel.Warning(key, content) + } + + private fun ProductProperty.Property.toPropertyUiModel(key: String) = + ProductDetailRowUiModel.Property( + key = key, + title = title, + value = value, + showDivider = isDividerVisible, + ) + + private fun ProductProperty.ComplexProperty.toComplexPropertyUiModel(key: String) = + ProductDetailRowUiModel.ComplexProperty( + key = key, + title = title, + value = value, + icon = icon, + showTitle = showTitle, + maxLines = maxLines, + showDivider = isDividerVisible, + onClick = onClick, + ) + + private fun ProductProperty.RatingBar.toRatingUiModel(key: String) = + ProductDetailRowUiModel.Rating( + key = key, + title = title, + value = value, + rating = rating, + icon = icon, + showDivider = false, + onClick = onClick, + ) + + private fun ProductProperty.Editable.toEditableUiModel(key: String) = + ProductDetailRowUiModel.Editable( + key = key, + hint = hint, + text = text, + shouldFocus = shouldFocus, + isReadOnly = isReadOnly, + badgeText = badgeText, + badgeTone = badgeColor?.let { + if (it == R.color.product_status_badge_pending) { + ProductDetailBadgeTone.WARNING + } else { + ProductDetailBadgeTone.NEUTRAL + } + }, + onTextChanged = onTextChanged, + ) + + private fun ProductProperty.PropertyGroup.toPropertyGroupUiModel(key: String) = + ProductDetailRowUiModel.PropertyGroup( + key = key, + title = title, + properties = properties.entries.map { ProductDetailPropertyValueUiModel(it.key, it.value) }, + icon = icon, + showTitle = showTitle, + showDivider = isDividerVisible, + isHighlighted = isHighlighted, + propertyFormat = propertyFormat, + onClick = onClick, + ) + + private fun ProductProperty.Link.toLinkUiModel(key: String) = + ProductDetailRowUiModel.Link( + key = key, + title = title, + icon = icon, + showDivider = isDividerVisible, + onClick = onClick, + ) + + private fun ProductProperty.Button.toButtonUiModel(key: String) = + ProductDetailRowUiModel.Button( + key = key, + text = text, + icon = icon, + showDivider = isDividerVisible, + tooltip = tooltip?.let { + ProductDetailTooltipUiModel( + title = it.title, + text = it.text, + dismissButtonText = it.dismissButtonText, + onDismiss = it.onDismiss, + ) + }, + link = link?.let { ProductDetailButtonLinkUiModel(it.text, it.onClick) }, + onClick = onClick, + ) + + private fun ProductProperty.Switch.toSwitchUiModel(key: String) = + ProductDetailRowUiModel.Switch( + key = key, + title = title, + isOn = isOn, + icon = icon, + onStateChanged = onStateChanged, + ) + + private fun ProductProperty.semanticKey(): String = when (this) { + ProductProperty.Divider -> "divider" + is ProductProperty.Property -> "property_$title" + is ProductProperty.ComplexProperty -> "complex_${title ?: NO_RESOURCE}_${icon ?: NO_RESOURCE}" + is ProductProperty.RatingBar -> "rating_$title" + is ProductProperty.Editable -> "editable_$hint" + is ProductProperty.PropertyGroup -> "group_$title" + is ProductProperty.Link -> "link_$title" + is ProductProperty.Button -> "button_$text" + is ProductProperty.Switch -> "switch_$title" + is ProductProperty.Warning -> "warning" + } + + private fun ProductProperty.isBlazeProperty() = + this is ProductProperty.Link && title == R.string.product_details_blaze_card + + private fun String.withOccurrence(occurrences: MutableMap): String { + val occurrence = occurrences.getOrDefault(this, 0) + occurrences[this] = occurrence + 1 + return if (occurrence == 0) this else "${this}_$occurrence" + } + + private companion object { + const val PRIMARY_CARD_KEY = "primary" + const val SECONDARY_CARD_KEY = "secondary" + const val BLAZE_CARD_KEY = "blaze" + const val NO_RESOURCE = 0 + } +} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiModel.kt new file mode 100644 index 000000000000..b3d5459f8022 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiModel.kt @@ -0,0 +1,158 @@ +package com.woocommerce.android.ui.products.details + +import androidx.annotation.DrawableRes +import androidx.annotation.StringRes +import androidx.compose.runtime.Immutable + +@Immutable +sealed interface ProductDetailScreenState { + data object Loading : ProductDetailScreenState + + data class Empty(@StringRes val message: Int? = null) : ProductDetailScreenState + + data class Error(@StringRes val message: Int) : ProductDetailScreenState + + data class Content( + val cards: List, + val showAddMore: Boolean, + val showLinkedProductPromo: Boolean, + ) : ProductDetailScreenState +} + +@Immutable +sealed interface ProductDetailImageUiState { + data object Loading : ProductDetailImageUiState + data object Gallery : ProductDetailImageUiState + data object AddImage : ProductDetailImageUiState + data object Unavailable : ProductDetailImageUiState + data object Hidden : ProductDetailImageUiState +} + +@Immutable +data class ProductDetailCardUiModel( + val key: String, + val style: ProductDetailCardStyle, + val caption: String, + val rows: List, +) + +enum class ProductDetailCardStyle { + PRIMARY, + SECONDARY, +} + +enum class ProductDetailBadgeTone { + NEUTRAL, + WARNING, +} + +@Immutable +sealed interface ProductDetailRowUiModel { + val key: String + + data class Divider( + override val key: String, + ) : ProductDetailRowUiModel + + data class Property( + override val key: String, + @StringRes val title: Int, + val value: String, + val showDivider: Boolean, + ) : ProductDetailRowUiModel + + data class ComplexProperty( + override val key: String, + @StringRes val title: Int?, + val value: String, + @DrawableRes val icon: Int?, + val showTitle: Boolean, + val maxLines: Int, + val showDivider: Boolean, + val onClick: (() -> Unit)?, + ) : ProductDetailRowUiModel + + data class Rating( + override val key: String, + @StringRes val title: Int, + val value: String, + val rating: Float, + @DrawableRes val icon: Int, + val showDivider: Boolean, + val onClick: (() -> Unit)?, + ) : ProductDetailRowUiModel + + data class Editable( + override val key: String, + @StringRes val hint: Int, + val text: String, + val shouldFocus: Boolean, + val isReadOnly: Boolean, + @StringRes val badgeText: Int?, + val badgeTone: ProductDetailBadgeTone?, + val onTextChanged: ((String) -> Unit)?, + ) : ProductDetailRowUiModel + + data class PropertyGroup( + override val key: String, + @StringRes val title: Int, + val properties: List, + @DrawableRes val icon: Int?, + val showTitle: Boolean, + val showDivider: Boolean, + val isHighlighted: Boolean, + @StringRes val propertyFormat: Int, + val onClick: (() -> Unit)?, + ) : ProductDetailRowUiModel + + data class Link( + override val key: String, + @StringRes val title: Int, + @DrawableRes val icon: Int?, + val showDivider: Boolean, + val onClick: (() -> Unit)?, + ) : ProductDetailRowUiModel + + data class Button( + override val key: String, + @StringRes val text: Int, + @DrawableRes val icon: Int?, + val showDivider: Boolean, + val tooltip: ProductDetailTooltipUiModel?, + val link: ProductDetailButtonLinkUiModel?, + val onClick: () -> Unit, + ) : ProductDetailRowUiModel + + data class Switch( + override val key: String, + @StringRes val title: Int, + val isOn: Boolean, + @DrawableRes val icon: Int?, + val onStateChanged: ((Boolean) -> Unit)?, + ) : ProductDetailRowUiModel + + data class Warning( + override val key: String, + val content: String, + ) : ProductDetailRowUiModel +} + +@Immutable +data class ProductDetailPropertyValueUiModel( + val label: String, + val value: String, +) + +@Immutable +data class ProductDetailTooltipUiModel( + @StringRes val title: Int, + @StringRes val text: Int, + @StringRes val dismissButtonText: Int, + val onDismiss: () -> Unit, +) + +@Immutable +data class ProductDetailButtonLinkUiModel( + @StringRes val text: Int, + val onClick: () -> Unit, +) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt index dbeebe5ad385..9953ea3ac12b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt @@ -473,11 +473,7 @@ class ProductDetailViewModel @Inject constructor( } else { when (val mode = navArgs.mode) { is ProductDetailFragment.Mode.ShowProduct -> { - productRepository.getProductAggregate( - viewState.productDraft?.remoteId ?: mode.remoteProductId - )?.let { - storedProductAggregate.value = it - } + restoreStoredProduct(viewState.productDraft?.remoteId ?: mode.remoteProductId) } ProductDetailFragment.Mode.Loading -> { @@ -491,12 +487,22 @@ class ProductDetailViewModel @Inject constructor( ) ) - is ProductDetailFragment.Mode.AddNewProduct -> Unit + is ProductDetailFragment.Mode.AddNewProduct -> { + viewState.productDraft?.remoteId + ?.takeIf { it != DEFAULT_ADD_NEW_PRODUCT_ID } + ?.let { restoreStoredProduct(it) } + } } } } } + private suspend fun restoreStoredProduct(remoteProductId: Long) { + productRepository.getProductAggregate(remoteProductId)?.let { + storedProductAggregate.value = it + } + } + fun getProduct() = viewState fun getRemoteProductId() = viewState.productDraft?.remoteId ?: DEFAULT_ADD_NEW_PRODUCT_ID @@ -1468,8 +1474,14 @@ class ProductDetailViewModel @Inject constructor( fun refreshProduct() { launch { - val mode = navArgs.mode as ProductDetailFragment.Mode.ShowProduct - fetchProduct(viewState.productDraft?.remoteId ?: mode.remoteProductId) + val remoteProductId = when (val mode = navArgs.mode) { + is ProductDetailFragment.Mode.ShowProduct -> viewState.productDraft?.remoteId ?: mode.remoteProductId + ProductDetailFragment.Mode.AddNewProduct -> + viewState.productDraft?.remoteId?.takeIf { it != DEFAULT_ADD_NEW_PRODUCT_ID } + ProductDetailFragment.Mode.Empty, + ProductDetailFragment.Mode.Loading -> null + } + remoteProductId?.let { fetchProduct(it) } } } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt index e4ae75059d94..cc012c0e6da1 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt @@ -31,6 +31,7 @@ import org.wordpress.android.fluxc.model.SiteModel @ExperimentalCoroutinesApi class ProductDetailCardBuilderTest : BaseUnitTest() { private lateinit var sut: ProductDetailCardBuilder + private lateinit var viewModel: ProductDetailViewModel private lateinit var productStub: Product private val isBlazeEnabled: IsBlazeEnabled = mock { on { invoke() } doReturn false @@ -45,7 +46,7 @@ class ProductDetailCardBuilderTest : BaseUnitTest() { @Before fun setUp() { - val viewModel: ProductDetailViewModel = mock { + viewModel = mock { on { getShippingClassByRemoteShippingClassId(any()) } doReturn "" } @@ -312,4 +313,45 @@ class ProductDetailCardBuilderTest : BaseUnitTest() { resourceProvider.getString(R.string.subscription_one_time_shipping) ) } + + @Test + fun `given every product detail type, when mapping builder output, then card and row order is preserved`() = testBlocking { + doReturn(0).whenever(viewModel).getBundledProductsSize(any()) + doReturn(emptyList()).whenever(viewModel).getComponents(any()) + val productTypes = listOf( + ProductType.SIMPLE, + ProductType.VARIABLE, + ProductType.GROUPED, + ProductType.EXTERNAL, + ProductType.SUBSCRIPTION, + ProductType.VARIABLE_SUBSCRIPTION, + ProductType.BUNDLE, + ProductType.COMPOSITE, + ProductType.OTHER, + ) + val mapper = ProductDetailUiMapper() + + productTypes.forEach { productType -> + val type = productType.value.ifEmpty { "unsupported" } + val aggregate = ProductAggregate( + product = ProductTestUtils.generateProduct().copy(type = type), + subscription = ProductHelper.getDefaultSubscriptionDetails(), + ) + + val cards = sut.buildPropertyCards(aggregate, "") + val mappedCards = mapper.map(cards) + + Assertions.assertThat(mappedCards.map { it.style }).containsExactlyElementsOf( + cards.map { + when (it.type) { + ProductPropertyCard.Type.PRIMARY -> ProductDetailCardStyle.PRIMARY + ProductPropertyCard.Type.SECONDARY -> ProductDetailCardStyle.SECONDARY + } + } + ) + Assertions.assertThat(mappedCards.map { it.rows.size }).containsExactlyElementsOf( + cards.map { it.properties.size } + ) + } + } } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapperTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapperTest.kt new file mode 100644 index 000000000000..8b3046c5c7ab --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailUiMapperTest.kt @@ -0,0 +1,268 @@ +package com.woocommerce.android.ui.products.details + +import com.woocommerce.android.R +import com.woocommerce.android.ui.products.details.ProductDetailViewModel.ProductDetailViewState.AuxiliaryState +import com.woocommerce.android.ui.products.models.ProductProperty +import com.woocommerce.android.ui.products.models.ProductPropertyCard +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test + +class ProductDetailUiMapperTest { + private val mapper = ProductDetailUiMapper() + + @Test + fun `given offline cache miss, when None is mapped without a product, then terminal empty state is returned`() { + val result = mapper.mapScreenState( + auxiliaryState = AuxiliaryState.None, + hasProduct = false, + cards = emptyList(), + showAddMore = false, + showLinkedProductPromo = false, + ) + + assertThat(result).isEqualTo(ProductDetailScreenState.Empty()) + } + + @Test + fun `when cards are mapped, then card and property order is preserved`() { + val cards = listOf( + ProductPropertyCard( + type = ProductPropertyCard.Type.PRIMARY, + properties = listOf( + ProductProperty.Editable(R.string.product_detail_title_hint, "Title"), + ProductProperty.ComplexProperty(value = "Description"), + ), + ), + ProductPropertyCard( + type = ProductPropertyCard.Type.SECONDARY, + caption = "Details", + properties = allPropertyVariants(), + ), + ) + + val result = mapper.map(cards) + + assertThat(result.map { it.style }).containsExactly( + ProductDetailCardStyle.PRIMARY, + ProductDetailCardStyle.SECONDARY, + ) + assertThat(result[1].caption).isEqualTo("Details") + assertThat(result[1].rows.map { it::class.java }).containsExactly( + ProductDetailRowUiModel.Divider::class.java, + ProductDetailRowUiModel.Property::class.java, + ProductDetailRowUiModel.ComplexProperty::class.java, + ProductDetailRowUiModel.Rating::class.java, + ProductDetailRowUiModel.Editable::class.java, + ProductDetailRowUiModel.PropertyGroup::class.java, + ProductDetailRowUiModel.Link::class.java, + ProductDetailRowUiModel.Button::class.java, + ProductDetailRowUiModel.Switch::class.java, + ProductDetailRowUiModel.Warning::class.java, + ) + } + + @Test + fun `when properties are mapped, then values callbacks and ordered groups are preserved`() { + val callbackResults = mutableListOf() + val properties = allPropertyVariants(onCallback = callbackResults::add) + + val rows = mapper.map( + listOf(ProductPropertyCard(ProductPropertyCard.Type.SECONDARY, properties = properties)) + ).single().rows + + val group = rows.filterIsInstance().single() + assertThat(group.properties).containsExactly( + ProductDetailPropertyValueUiModel("First", "1"), + ProductDetailPropertyValueUiModel("Second", "2"), + ) + val editable = rows.filterIsInstance().single() + assertThat(editable.shouldFocus).isTrue() + assertThat(editable.isReadOnly).isTrue() + editable.onTextChanged?.invoke("updated") + rows.filterIsInstance().single().onClick?.invoke() + rows.filterIsInstance().single().onClick?.invoke() + group.onClick?.invoke() + rows.filterIsInstance().single().onClick?.invoke() + val button = rows.filterIsInstance().single() + button.tooltip?.onDismiss?.invoke() + button.link?.onClick?.invoke() + rows.filterIsInstance().single().onStateChanged?.invoke(false) + button.onClick() + + assertThat(callbackResults).containsExactly( + "updated", + "complex", + "rating", + "group", + "link", + "tooltip", + "buttonLink", + "switch", + "button", + ) + } + + @Test + fun `when mutable editable flags change after mapping, then mapped values remain a snapshot`() { + val editable = ProductProperty.Editable( + hint = R.string.product_detail_title_hint, + shouldFocus = true, + isReadOnly = true, + ) + + val mapped = mapper.map( + listOf( + ProductPropertyCard( + ProductPropertyCard.Type.PRIMARY, + properties = listOf(editable), + ) + ) + ).single().rows.single() as ProductDetailRowUiModel.Editable + editable.shouldFocus = false + editable.isReadOnly = false + + assertThat(mapped.shouldFocus).isTrue() + assertThat(mapped.isReadOnly).isTrue() + } + + @Test + fun `when rating position is mapped, then its legacy divider is hidden only at the end of a card`() { + val rating = ProductProperty.RatingBar( + title = R.string.product_reviews, + value = "4 reviews", + rating = 4.5f, + icon = R.drawable.ic_reviews, + ) + val followedRating = mapper.map( + listOf( + ProductPropertyCard( + ProductPropertyCard.Type.SECONDARY, + properties = listOf(rating, ProductProperty.Warning("Warning")), + ) + ) + ).single().rows.first() as ProductDetailRowUiModel.Rating + val finalRating = mapper.map( + listOf(ProductPropertyCard(ProductPropertyCard.Type.SECONDARY, properties = listOf(rating))) + ).single().rows.single() as ProductDetailRowUiModel.Rating + + assertThat(followedRating.showDivider).isTrue() + assertThat(finalRating.showDivider).isFalse() + } + + @Test + fun `when semantic rows repeat, then keys are stable and unique without list indexes`() { + val properties = listOf( + ProductProperty.Property(R.string.product_price, "10"), + ProductProperty.Property(R.string.product_price, "20"), + ) + + val first = mapper.map( + listOf(ProductPropertyCard(ProductPropertyCard.Type.SECONDARY, properties = properties)) + ) + val second = mapper.map( + listOf(ProductPropertyCard(ProductPropertyCard.Type.SECONDARY, properties = properties)) + ) + + assertThat(first.single().rows.map { it.key }).containsExactly( + "property_${R.string.product_price}", + "property_${R.string.product_price}_1", + ) + assertThat(second.single().rows.map { it.key }).isEqualTo(first.single().rows.map { it.key }) + } + + @Test + fun `given Add is persisted, when cards are remapped, then semantic keys stay stable and callbacks are current`() { + var callback = "" + val initialCards = listOf( + ProductPropertyCard( + ProductPropertyCard.Type.PRIMARY, + properties = listOf( + ProductProperty.Editable( + hint = R.string.product_detail_title_hint, + onTextChanged = { callback = "initial:$it" }, + ) + ), + ) + ) + val persistedCards = listOf( + ProductPropertyCard( + ProductPropertyCard.Type.PRIMARY, + properties = listOf( + ProductProperty.Editable( + hint = R.string.product_detail_title_hint, + onTextChanged = { callback = "persisted:$it" }, + ) + ), + ) + ) + + val initial = mapper.map(initialCards) + val persisted = mapper.map(persistedCards) + val editable = persisted.single().rows.single() as ProductDetailRowUiModel.Editable + editable.onTextChanged?.invoke("Title") + + assertThat(persisted.map { it.key }).isEqualTo(initial.map { it.key }) + assertThat(persisted.single().rows.map { it.key }).isEqualTo(initial.single().rows.map { it.key }) + assertThat(callback).isEqualTo("persisted:Title") + } + + private fun allPropertyVariants( + onCallback: (String) -> Unit = {}, + ) = listOf( + ProductProperty.Divider, + ProductProperty.Property(R.string.product_price, "10"), + ProductProperty.ComplexProperty( + title = R.string.product_description, + value = "Description", + icon = R.drawable.ic_gridicons_product, + onClick = { onCallback("complex") }, + ), + ProductProperty.RatingBar( + title = R.string.product_reviews, + value = "4 reviews", + rating = 4.5f, + icon = R.drawable.ic_reviews, + onClick = { onCallback("rating") }, + ), + ProductProperty.Editable( + hint = R.string.product_detail_title_hint, + text = "Title", + shouldFocus = true, + isReadOnly = true, + badgeText = R.string.product_status_private, + badgeColor = R.color.product_status_badge_pending, + onTextChanged = onCallback, + ), + ProductProperty.PropertyGroup( + title = R.string.product_inventory, + properties = linkedMapOf("First" to "1", "Second" to "2"), + icon = R.drawable.ic_gridicons_list_checkmark, + isHighlighted = true, + onClick = { onCallback("group") }, + ), + ProductProperty.Link( + title = R.string.product_detail_add_more, + icon = R.drawable.ic_add, + onClick = { onCallback("link") }, + ), + ProductProperty.Button( + text = R.string.set_up_now, + icon = R.drawable.ic_add, + tooltip = ProductProperty.Button.Tooltip( + title = R.string.tip, + text = R.string.promo_linked_products_banner_message, + dismissButtonText = R.string.dismiss, + onDismiss = { onCallback("tooltip") }, + ), + link = ProductProperty.Button.Link(R.string.learn_more) { onCallback("buttonLink") }, + onClick = { onCallback("button") }, + ), + ProductProperty.Switch( + title = R.string.product_reviews, + isOn = true, + icon = R.drawable.ic_reviews, + onStateChanged = { onCallback("switch") }, + ), + ProductProperty.Warning("Warning"), + ) +} diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt index b1fc684af0cb..a0dc07ea5c53 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt @@ -396,6 +396,20 @@ class ProductDetailViewModelTest : BaseUnitTest() { Assertions.assertThat(snackbar).isEqualTo(MultiLiveEvent.Event.ShowSnackbar(R.string.offline_error)) } + @Test + fun `given offline cache miss, when product loads, then None is emitted without a product`() = testBlocking { + doReturn(null).whenever(productRepository).getProductAggregate(PRODUCT_REMOTE_ID) + doReturn(false).whenever(networkStatus).isConnected() + + viewModel.start() + + verify(productRepository, times(1)).getProductAggregate(PRODUCT_REMOTE_ID) + verify(productRepository, never()).fetchAndGetProductAggregate(any()) + Assertions.assertThat(viewModel.getProduct().productDraft).isNull() + Assertions.assertThat(viewModel.getProduct().auxiliaryState) + .isEqualTo(ProductDetailViewModel.ProductDetailViewState.AuxiliaryState.None) + } + @Test fun `Shows and hides product detail skeleton correctly`() = testBlocking { doReturn(null).whenever(productRepository).getProductAggregate(any()) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel_AddFlowTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel_AddFlowTest.kt index e59b2a540a1b..f4980b46ecd3 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel_AddFlowTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel_AddFlowTest.kt @@ -12,6 +12,7 @@ import com.woocommerce.android.model.ProductAggregate import com.woocommerce.android.tools.NetworkStatus import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.blaze.IsBlazeEnabled +import com.woocommerce.android.ui.customfields.CustomFieldsRepository import com.woocommerce.android.ui.media.MediaFileUploadHandler import com.woocommerce.android.ui.products.DuplicateProduct import com.woocommerce.android.ui.products.ParameterRepository @@ -63,7 +64,9 @@ class ProductDetailViewModel_AddFlowTest : BaseUnitTest() { private val wooCommerceStore: WooCommerceStore = mock() private val networkStatus: NetworkStatus = mock() - private val productRepository: ProductDetailRepository = mock() + private val productRepository: ProductDetailRepository = mock { + on { getCachedVariationCount(any()) } doReturn 0 + } private val productCategoriesRepository: ProductCategoriesRepository = mock() private val productTagsRepository: ProductTagsRepository = mock() private val mediaFilesRepository: MediaFilesRepository = mock() @@ -88,6 +91,9 @@ class ProductDetailViewModel_AddFlowTest : BaseUnitTest() { private val isBlazeEnabled: IsBlazeEnabled = mock { on { invoke() } doReturn false } + private val customFieldsRepository: CustomFieldsRepository = mock { + on { hasDisplayableCustomFields(any()) } doReturn false + } private var savedState: SavedStateHandle = ProductDetailFragmentArgs( mode = ProductDetailFragment.Mode.AddNewProduct @@ -198,7 +204,7 @@ class ProductDetailViewModel_AddFlowTest : BaseUnitTest() { isProductCurrentlyPromoted = mock(), isWindowClassLargeThanCompact = mock(), determineProductPasswordApi = mock(), - customFieldsRepository = mock(), + customFieldsRepository = customFieldsRepository, canAutoAuthenticateInWebView = mock(), ) ) @@ -421,6 +427,50 @@ class ProductDetailViewModel_AddFlowTest : BaseUnitTest() { verify(mediaFileUploadHandler).assignUploadsToCreatedProduct(PRODUCT_REMOTE_ID) } + @Test + fun `given an added product has a remote id, when reviews return, then refresh the persisted product`() = testBlocking { + doReturn(Pair(true, PRODUCT_REMOTE_ID)).whenever(productRepository).addProduct(any()) + doReturn(ProductAggregate(product)).whenever(productRepository).getProductAggregate(PRODUCT_REMOTE_ID) + doReturn(ProductAggregate(product)).whenever(productRepository).fetchAndGetProductAggregate(PRODUCT_REMOTE_ID) + viewModel.productDetailViewStateData.observeForever { _, _ -> } + + viewModel.onSaveAsDraftButtonClicked() + clearInvocations(productRepository) + viewModel.refreshProduct() + + verify(productRepository).fetchAndGetProductAggregate(PRODUCT_REMOTE_ID) + } + + @Test + fun `given edited persisted Add state is process-restored, when backed out, then discard is protected`() = + testBlocking { + val storedAggregate = ProductAggregate(product) + val restoredDraft = storedAggregate.copy(product = product.copy(name = "Restored edit")) + savedState = ProductDetailFragmentArgs( + mode = ProductDetailFragment.Mode.AddNewProduct + ).toSavedStateHandle().apply { + set( + ProductDetailViewModel.ProductDetailViewState::class.java.name, + ProductDetailViewModel.ProductDetailViewState( + productAggregateDraft = restoredDraft, + auxiliaryState = ProductDetailViewModel.ProductDetailViewState.AuxiliaryState.None, + areImagesAvailable = true, + ) + ) + } + doReturn(storedAggregate).whenever(productRepository).getProductAggregate(PRODUCT_REMOTE_ID) + setup() + + var hasChanges: Boolean? = null + viewModel.productDetailViewStateData.observeForever { _, _ -> } + viewModel.hasChanges.observeForever { hasChanges = it } + + viewModel.onBackButtonClickedProductDetail() + + Assertions.assertThat(hasChanges).isTrue() + Assertions.assertThat(viewModel.event.value).isInstanceOf(MultiLiveEvent.Event.ShowDialog::class.java) + } + @Test fun `given a product is under creation, when displaying discard changes dialog, then stop observing uploads`() = testBlocking { diff --git a/libs/store-design-system/src/debug/res/menu/woo_design_system_toolbar_test_menu.xml b/libs/store-design-system/src/debug/res/menu/woo_design_system_toolbar_test_menu.xml index 4230612c7e65..b2040983f9e6 100644 --- a/libs/store-design-system/src/debug/res/menu/woo_design_system_toolbar_test_menu.xml +++ b/libs/store-design-system/src/debug/res/menu/woo_design_system_toolbar_test_menu.xml @@ -8,4 +8,9 @@ android:title="Open" tools:ignore="HardcodedText" app:showAsAction="always" /> + diff --git a/libs/store-design-system/src/main/kotlin/com/woocommerce/android/ui/compose/designsystem/component/WooDesignSystemToolbar.kt b/libs/store-design-system/src/main/kotlin/com/woocommerce/android/ui/compose/designsystem/component/WooDesignSystemToolbar.kt index bde4a1387a8e..13c8ed08e8e6 100644 --- a/libs/store-design-system/src/main/kotlin/com/woocommerce/android/ui/compose/designsystem/component/WooDesignSystemToolbar.kt +++ b/libs/store-design-system/src/main/kotlin/com/woocommerce/android/ui/compose/designsystem/component/WooDesignSystemToolbar.kt @@ -25,6 +25,8 @@ import androidx.core.content.ContextCompat import androidx.core.view.children import com.google.android.material.appbar.MaterialToolbar import com.woocommerce.android.ui.compose.designsystem.R +import kotlin.math.ceil +import kotlin.math.roundToInt class WooDesignSystemToolbar @JvmOverloads constructor( context: Context, @@ -50,7 +52,7 @@ class WooDesignSystemToolbar @JvmOverloads constructor( decorateNavigationButton() decorateRenderedMenuActions() super.onMeasure(widthMeasureSpec, heightMeasureSpec) - if (decorateNavigationButton() || decorateRenderedMenuActions()) { + if (decorateTitle() || decorateNavigationButton() || decorateRenderedMenuActions()) { super.onMeasure(widthMeasureSpec, heightMeasureSpec) } } @@ -77,17 +79,15 @@ class WooDesignSystemToolbar @JvmOverloads constructor( private fun decorateNavigationButton(): Boolean { val navigationButton = children.filterIsInstance().firstOrNull() ?: return false - var changed = navigationButton.applyToolbarIconTouchTarget() - if (navigationButton.scaleType != ImageView.ScaleType.FIT_CENTER) { - navigationButton.scaleType = ImageView.ScaleType.FIT_CENTER - changed = true - } - if (navigationButton.getTag(R.id.woo_ds_toolbar_action_view) != true) { - navigationButton.background = context.toolbarIconButtonBackground(icon = null) - navigationButton.setTag(R.id.woo_ds_toolbar_action_view, true) - changed = true - } - return changed + return navigationButton.applyOutlinedToolbarImageButtonStyle() + } + + private fun decorateTitle(): Boolean { + val titleView = children.filterIsInstance().firstOrNull { it.text == title } ?: return false + if (!titleView.includeFontPadding) return false + + titleView.includeFontPadding = false + return true } private fun decorateRenderedMenuActions(): Boolean { @@ -97,6 +97,12 @@ class WooDesignSystemToolbar @JvmOverloads constructor( .filterIsInstance() .flatMap { actionMenuView -> actionMenuView.children.asIterable() } .forEach { child -> + val layoutParams = child.layoutParams as? ActionMenuView.LayoutParams + if (layoutParams?.isOverflowButton == true) { + changed = child.applyOutlinedToolbarImageButtonStyle() || changed + return@forEach + } + val item = menu.findItem(child.id) ?: return@forEach if (item.actionView === child) { return@forEach @@ -111,6 +117,20 @@ class WooDesignSystemToolbar @JvmOverloads constructor( return changed } + private fun View.applyOutlinedToolbarImageButtonStyle(): Boolean { + var changed = applyToolbarIconTouchTarget() + if (this is ImageView && scaleType != ImageView.ScaleType.FIT_CENTER) { + scaleType = ImageView.ScaleType.FIT_CENTER + changed = true + } + if (getTag(R.id.woo_ds_toolbar_action_view) != true) { + background = context.toolbarIconButtonBackground(icon = null) + setTag(R.id.woo_ds_toolbar_action_view, true) + changed = true + } + return changed + } + private fun View.applyOutlinedToolbarActionStyle(icon: Drawable?, iconSize: Int): Boolean { if (getTag(R.id.woo_ds_toolbar_action_original_state) == null) { setTag(R.id.woo_ds_toolbar_action_original_state, captureToolbarActionViewState()) @@ -156,50 +176,72 @@ class WooDesignSystemToolbar @JvmOverloads constructor( this !is TextView || text.isNullOrEmpty() private fun applyToolbarControlEdgeInsets() { - val edgeInset = context.dimensionPixelSize(R.dimen.woo_ds_toolbar_edge_padding) + val controlEdgeInset = ( + resources.getDimension(R.dimen.woo_ds_toolbar_edge_padding) - + resources.getDimension(R.dimen.woo_ds_toolbar_icon_border_inset) + ).roundToInt() val isRtl = layoutDirection == View.LAYOUT_DIRECTION_RTL val toolbarWidth = width + val toolbarHeight = height children.filterIsInstance().firstOrNull()?.layoutWithStartInset( - edgeInset = edgeInset, + edgeInset = controlEdgeInset, toolbarWidth = toolbarWidth, + toolbarHeight = toolbarHeight, isRtl = isRtl, ) - children.filterIsInstance().firstOrNull()?.layoutWithEndInset( - edgeInset = edgeInset, - toolbarWidth = toolbarWidth, - isRtl = isRtl, - ) + children.filterIsInstance().firstOrNull()?.let { actionMenuView -> + actionMenuView.layoutWithEndInset( + edgeInset = controlEdgeInset, + toolbarWidth = toolbarWidth, + toolbarHeight = toolbarHeight, + isRtl = isRtl, + ) + actionMenuView.centerOutlinedActionsVertically(toolbarHeight) + } } } private fun View.layoutWithStartInset( edgeInset: Int, toolbarWidth: Int, + toolbarHeight: Int, isRtl: Boolean, ) { val childWidth = measuredWidth + val childTop = ((toolbarHeight - measuredHeight) / 2f).roundToInt() val childLeft = if (isRtl) { toolbarWidth - edgeInset - childWidth } else { edgeInset } - layout(childLeft, top, childLeft + childWidth, bottom) + layout(childLeft, childTop, childLeft + childWidth, childTop + measuredHeight) } private fun View.layoutWithEndInset( edgeInset: Int, toolbarWidth: Int, + toolbarHeight: Int, isRtl: Boolean, ) { val childWidth = measuredWidth + val childTop = ((toolbarHeight - measuredHeight) / 2f).roundToInt() val childLeft = if (isRtl) { edgeInset } else { toolbarWidth - edgeInset - childWidth } - layout(childLeft, top, childLeft + childWidth, bottom) + layout(childLeft, childTop, childLeft + childWidth, childTop + measuredHeight) +} + +private fun ActionMenuView.centerOutlinedActionsVertically(toolbarHeight: Int) { + children + .filter { child -> child.getTag(R.id.woo_ds_toolbar_action_view) == true } + .forEach { child -> + val childTop = ((toolbarHeight - child.measuredHeight) / 2f).roundToInt() - top + child.layout(child.left, childTop, child.left + child.measuredWidth, childTop + child.measuredHeight) + } } private fun View.applyToolbarIconTouchTarget(): Boolean { @@ -215,11 +257,14 @@ private fun View.applyToolbarIconTouchTarget(): Boolean { minimumHeight = touchTarget changed = true } - if (layoutParams?.width != touchTarget || layoutParams?.height != touchTarget) { - layoutParams = (layoutParams ?: ViewGroup.LayoutParams(touchTarget, touchTarget)).apply { - width = touchTarget - height = touchTarget - } + val currentLayoutParams = layoutParams + if (currentLayoutParams == null) { + layoutParams = ViewGroup.LayoutParams(touchTarget, touchTarget) + changed = true + } else if (currentLayoutParams.width != touchTarget || currentLayoutParams.height != touchTarget) { + currentLayoutParams.width = touchTarget + currentLayoutParams.height = touchTarget + requestLayout() changed = true } if (!hasUniformPadding(iconPadding)) { @@ -289,20 +334,22 @@ private class CenteredToolbarIconButtonDrawable( icon: Drawable?, private val spec: CenteredToolbarIconButtonSpec, ) : Drawable() { + private val borderStrokeWidth = ceil(spec.strokeWidth) private val icon = icon?.newMutableDrawable()?.apply { setTintList(spec.iconTint) } private val paint = Paint(Paint.ANTI_ALIAS_FLAG).apply { style = Paint.Style.STROKE - strokeWidth = spec.strokeWidth + strokeWidth = borderStrokeWidth color = spec.color } private val rect = RectF() override fun draw(canvas: Canvas) { centeredSquareIn(bounds, spec.boxSize, rect) - rect.inset(spec.strokeWidth / 2, spec.strokeWidth / 2) - canvas.drawRoundRect(rect, spec.cornerRadius, spec.cornerRadius, paint) + rect.inset(borderStrokeWidth / 2, borderStrokeWidth / 2) + val strokeCornerRadius = spec.cornerRadius - borderStrokeWidth / 2 + canvas.drawRoundRect(rect, strokeCornerRadius, strokeCornerRadius, paint) icon?.let { drawable -> val iconLeft = bounds.left + (bounds.width() - spec.iconSize) / 2 @@ -377,8 +424,8 @@ private class CenteredToolbarIconButtonMaskDrawable( private fun centeredSquareIn(bounds: Rect, boxSize: Float, out: RectF) { val size = boxSize.coerceAtMost(bounds.width().toFloat()).coerceAtMost(bounds.height().toFloat()) - val left = bounds.left + (bounds.width() - size) / 2 - val top = bounds.top + (bounds.height() - size) / 2 + val left = (bounds.left + (bounds.width() - size) / 2).roundToInt().toFloat() + val top = (bounds.top + (bounds.height() - size) / 2).roundToInt().toFloat() out.set(left, top, left + size, top + size) } diff --git a/libs/store-design-system/src/main/res/values/dimens.xml b/libs/store-design-system/src/main/res/values/dimens.xml index 42f0bf86cd96..da6cf5711019 100644 --- a/libs/store-design-system/src/main/res/values/dimens.xml +++ b/libs/store-design-system/src/main/res/values/dimens.xml @@ -9,6 +9,6 @@ 18dp 15dp 4dp - 0.5dp + 1dp 12dp diff --git a/libs/store-design-system/src/main/res/values/styles.xml b/libs/store-design-system/src/main/res/values/styles.xml index 3db458f8e8bc..7048fc4d9925 100644 --- a/libs/store-design-system/src/main/res/values/styles.xml +++ b/libs/store-design-system/src/main/res/values/styles.xml @@ -20,6 +20,7 @@ @style/Widget.Woo.DesignSystem.Toolbar @style/Widget.Woo.DesignSystem.Toolbar.NavigationButton @style/Widget.Woo.DesignSystem.Toolbar.ActionButton + @style/Widget.Woo.DesignSystem.Toolbar.OverflowButton @style/TextAppearance.Woo.DesignSystem.ToolbarTextAction @color/woo_ds_toolbar_text_action_tint @color/woo_ds_color_surface_on_default @@ -33,6 +34,12 @@ fitCenter + +