Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a product "cover" tag over the cover image #13344

Merged
merged 9 commits into from
Jan 21, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ object MediaPickerUtil {

private fun handleMediaLibraryPickerResult(data: Bundle): List<Product.Image> {
return data.parcelableArrayList<MediaItem.Identifier.RemoteMedia>(MediaPickerConstants.EXTRA_REMOTE_MEDIA)
?.map { Product.Image(it.id, it.name, it.url, DateTimeUtils.dateFromIso8601(it.date)) }
?.map { Product.Image(it.id, it.name, it.url, DateTimeUtils.dateFromIso8601(it.date), false) }
?: emptyList()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ data class Product(
val id: Long,
val name: String?,
val source: String,
val dateCreated: Date?
val dateCreated: Date?,
val isCoverImage: Boolean
) : Parcelable

fun isSameProduct(product: Product): Boolean {
Expand Down Expand Up @@ -539,10 +540,11 @@ fun WCProductModel.toAppModel(): Product {
numVariations = this.getNumVariations(),
images = this.getImageListOrEmpty().map {
Product.Image(
it.id,
it.name,
it.src,
DateTimeUtils.dateFromIso8601(this.dateCreated) ?: Date()
id = it.id,
name = it.name,
source = it.src,
dateCreated = DateTimeUtils.dateFromIso8601(this.dateCreated) ?: Date(),
isCoverImage = it.src == this.getFirstImageUrl()
)
},
attributes = this.getAttributeList().map { it.toAppModel() },
Expand Down Expand Up @@ -597,7 +599,8 @@ fun MediaModel.toAppModel(): Product.Image {
id = this.mediaId,
name = this.fileName.orEmpty(),
source = this.url,
dateCreated = DateTimeUtils.dateFromIso8601(this.uploadDate)
dateCreated = DateTimeUtils.dateFromIso8601(this.uploadDate),
isCoverImage = false
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,11 @@ fun WCProductVariationModel.toAppModel(): ProductVariation {
globalUniqueId = this.globalUniqueId,
image = this.getImageModel()?.let {
Product.Image(
it.id,
it.name,
it.src,
DateTimeUtils.dateFromIso8601(this.dateCreated) ?: Date()
id = it.id,
name = it.name,
source = it.src,
dateCreated = DateTimeUtils.dateFromIso8601(this.dateCreated) ?: Date(),
isCoverImage = false
)
},
price = this.price.toBigDecimalOrNull(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ class SubscriptionProductVariation(
globalUniqueId = model.globalUniqueId,
image = model.getImageModel()?.let {
Product.Image(
it.id,
it.name,
it.src,
DateTimeUtils.dateFromIso8601(model.dateCreated) ?: Date()
id = it.id,
name = it.name,
source = it.src,
dateCreated = DateTimeUtils.dateFromIso8601(model.dateCreated) ?: Date(),
isCoverImage = false
)
},
price = model.price.toBigDecimalOrNull(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ class ProductImagesViewModel @Inject constructor(
}

fun onValidateButtonClicked() {
viewState = viewState.copy(productImagesState = Browsing)
viewState = viewState.copy(
images = images.updateProductCoverImageToFirstItem(),
productImagesState = Browsing
)
}

fun onNavigateBackButtonClicked() {
Expand All @@ -159,6 +162,7 @@ class ProductImagesViewModel @Inject constructor(
images = productImagesState.initialState
)
}

Browsing -> {
val hasChange = !images.areSameImagesAs(originalImages)
analyticsTracker.track(
Expand Down Expand Up @@ -229,7 +233,11 @@ class ProductImagesViewModel @Inject constructor(
fun onGalleryImageDragStarted() {
when (viewState.productImagesState) {
is Dragging -> { /* no-op*/ }
Browsing -> viewState = viewState.copy(productImagesState = Dragging(images))

Browsing -> viewState = viewState.copy(
images = images.uncheckProductCoverImage(),
productImagesState = Dragging(images)
)
}
}

Expand All @@ -249,6 +257,11 @@ class ProductImagesViewModel @Inject constructor(
}
}

private fun List<Product.Image>.updateProductCoverImageToFirstItem() =
this.mapIndexed { index, image -> image.copy(isCoverImage = index == 0) }

private fun List<Product.Image>.uncheckProductCoverImage() = this.map { it.copy(isCoverImage = false) }

@Parcelize
data class ViewState(
val showSourceChooser: Boolean? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,16 @@ class WCProductImageGalleryView @JvmOverloads constructor(
// use a negative id so we can check it in isPlaceholder() below
val id = (-index - 1).toLong()
// set the image src to this uri so we can preview it while uploading
placeholders.add(0, Product.Image(id, "", imageUriList[index].toString(), Date()))
placeholders.add(
0,
Product.Image(
id = id,
name = "",
source = imageUriList[index].toString(),
dateCreated = Date(),
isCoverImage = false
)
)
}

adapter.setPlaceholderImages(placeholders)
Expand Down Expand Up @@ -243,13 +252,14 @@ class WCProductImageGalleryView @JvmOverloads constructor(
imageList.addAll(images)

// restore the "Add image" icon (never shown when list is empty)
if (showAddImageIcon && imageList.size > 0) {
if (showAddImageIcon && imageList.isNotEmpty()) {
imageList.add(
Product.Image(
id = ADD_IMAGE_ITEM_ID,
name = "",
source = "",
dateCreated = Date()
dateCreated = Date(),
isCoverImage = false
)
)
}
Expand Down Expand Up @@ -290,7 +300,7 @@ class WCProductImageGalleryView @JvmOverloads constructor(
}

for (index in images.indices) {
if (images[index].id != actualImages[index].id) {
if (images[index] != actualImages[index]) {
return false
}
}
Expand Down Expand Up @@ -423,11 +433,13 @@ class WCProductImageGalleryView @JvmOverloads constructor(
viewBinding.uploadProgess.visibility = View.VISIBLE
viewBinding.addImageContainer.visibility = View.GONE
}

VIEW_TYPE_ADD_IMAGE -> {
viewBinding.productImage.visibility = View.GONE
viewBinding.uploadProgess.visibility = View.GONE
viewBinding.addImageContainer.visibility = View.VISIBLE
}

else -> {
viewBinding.productImage.visibility = View.VISIBLE
viewBinding.productImage.alpha = 1.0F
Expand All @@ -439,6 +451,10 @@ class WCProductImageGalleryView @JvmOverloads constructor(
viewBinding.deleteImageButton.setOnClickListener {
listener.onGalleryImageDeleteIconClicked(image)
}
viewBinding.coverTag.visibility = when {
image.isCoverImage -> View.VISIBLE
else -> View.GONE
}
}

private fun setMargins() {
Expand Down
7 changes: 5 additions & 2 deletions WooCommerce/src/main/res/drawable/bg_rounded_box.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android" android:id="@+id/listview_background_shape">
<stroke android:width="@dimen/minor_10" android:color="@color/divider_color" />
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/listview_background_shape">
<stroke
android:width="@dimen/minor_10"
android:color="@color/divider_color" />
<corners android:radius="@dimen/minor_75" />
</shape>
18 changes: 18 additions & 0 deletions WooCommerce/src/main/res/layout/image_gallery_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,22 @@
android:visibility="gone"
tools:visibility="visible" />

<TextView
android:id="@+id/coverTag"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="top|start"
android:layout_margin="@dimen/minor_50"
android:background="@drawable/bg_rounded_box"
android:backgroundTint="@color/color_primary"
android:contentDescription="@string/product_cover_photo_tag"
android:paddingHorizontal="@dimen/minor_75"
android:paddingVertical="@dimen/minor_50"
android:text="@string/product_cover_photo_tag"
android:textAppearance="?attr/textAppearanceCaption"
android:textColor="@color/white"
android:textStyle="bold"
android:visibility="gone"
tools:visibility="visible" />

</FrameLayout>
1 change: 1 addition & 0 deletions WooCommerce/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2397,6 +2397,7 @@
<string name="product_add_photo">Add photo</string>
<string name="product_replace_photo">Replace photo</string>
<string name="product_remove_photo">Remove photo</string>
<string name="product_cover_photo_tag">Cover</string>
<string name="product_image_add">Add a product image</string>
<string name="product_image_error_removing">Error removing product image</string>
<string name="product_image_service_error_uploading">Error uploading product image</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ object ProductTestUtils {
id = imageId,
name = "Image $imageId",
source = "Image $imageId source",
dateCreated = Date.from(Instant.EPOCH)
dateCreated = Date.from(Instant.EPOCH),
isCoverImage = false
)

fun generateProductImagesList() =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class AiProductPreviewViewModelTest : BaseUnitTest() {
companion object {
private const val PRODUCT_FEATURES = "product_features"
private val SAMPLE_PRODUCT = AIProductModel.buildDefault("default name", "default description")
private val SAMPLE_UPLOADED_IMAGE = Product.Image(0, "image", "url", Date())
private val SAMPLE_UPLOADED_IMAGE = Product.Image(0, "image", "url", Date(), false)
}

private val buildProductPreviewProperties: BuildProductPreviewProperties = mock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ import org.mockito.kotlin.verify

@ExperimentalCoroutinesApi
class ProductImagesViewModelTest : BaseUnitTest() {
companion object {
private val DEFAULT_PRODUCT_IMAGES = ProductTestUtils.generateProductImagesList()
.mapIndexed { index, image ->
image.copy(isCoverImage = index == 0)
}
}

lateinit var viewModel: ProductImagesViewModel

private val networkStatus: NetworkStatus = mock()
Expand All @@ -34,7 +41,7 @@ class ProductImagesViewModelTest : BaseUnitTest() {
requestCode = 123
).toSavedStateHandle()

private fun initialize(productImages: List<Product.Image> = ProductTestUtils.generateProductImagesList()) {
private fun initialize(productImages: List<Product.Image> = DEFAULT_PRODUCT_IMAGES) {
viewModel = ProductImagesViewModel(
networkStatus,
mediaFileUploadHandler,
Expand Down Expand Up @@ -73,7 +80,7 @@ class ProductImagesViewModelTest : BaseUnitTest() {
fun `Trigger exitWithResult event on back button clicked when in browsing state`() {
initialize()

val images = ProductTestUtils.generateProductImagesList()
val images = DEFAULT_PRODUCT_IMAGES
viewModel.onDeleteImageConfirmed(images[0])
viewModel.onNavigateBackButtonClicked()

Expand Down Expand Up @@ -157,6 +164,17 @@ class ProductImagesViewModelTest : BaseUnitTest() {
}
}

@Test
fun `When switching to drag mode, clear product image cover`() {
initialize()

viewModel.onGalleryImageDragStarted()

observeState { state ->
assertThat(state.images?.none { it.isCoverImage }).isTrue()
}
}

@Test
fun `Validate drag and drop process on validation button clicked`() {
val images = ProductTestUtils.generateProductImagesList()
Expand All @@ -175,6 +193,7 @@ class ProductImagesViewModelTest : BaseUnitTest() {
observeState { state ->
assertThat(state.images).doesNotContain(imageToRemove)
assertThat(state.images).contains(imageToReorder, Index.atIndex(2))
assertThat(state.images?.first()?.isCoverImage).isTrue()
}
}

Expand Down
Loading