ANDROID-14134 Set assets clickable #324

jeprubio · 2023-12-22T13:19:18Z

🥅 What's the goal?

Until now the assets in ListRow didn't have a possible interaction different from the rest of the row but this should be allowed as it has been defined in the specs: https://www.figma.com/file/Be8QB9onmHunKCCAkIBAVr/%F0%9F%94%B8-Lists-Specs?type=design&node-id=0%3A608&mode=design&t=lG2wUE8oNXAI5z4d-1

🚧 How do we do it?

Added setAssetOnClickListener for traditional views.
Made the modifier accessible for the different ListRowIcon so they can be made clickable.
Added tests for the traditional views and compose elements.
Added a row example in the catalog at the end.
Also added some section titles in the compose catalog as the traditional views already had.

☑️ Checks

I updated the documentation, including readmes and wikis. If this is a breaking change, update UPGRADING.md to inform users how to proceed. If no updates are necessary, indicate so.
Tested with dark mode.
Tested with API 23.

🧪 How can I test this?

In the catalog.
Classic views:

Compose:

🖼️ Screenshots/Videos
Mistica App QR or download link
Reviewed by Mistica design team
...

AnaMontes11 · 2023-12-22T13:57:57Z

library/src/main/java/com/telefonica/mistica/compose/list/ListRowIcon.kt

@@ -110,7 +116,7 @@ sealed class ListRowIcon(val contentDescription: String?) {
            Image(
                painter = painter,
                contentDescription = contentDescription,
-                modifier = Modifier
+                modifier = modifier
                    .size(40.dp)


For accessibility reasons, we might consider expanding the clickable area of the asset to 48px. We have updated our specs.

OK, I'm fine with that if the specs have also been updated. Done.

This is just exposing the modifier and not making it clickable with just that. Also doing that includes breaking changes in the lib. In my opinion we should ensure what we want to make clickable or not on the designs and then use this modifier to make it clickable or not depending on those designs. I'm going to revert this change.

AnaMontes11

Great job!

jeprubio · 2023-12-26T10:16:20Z

library/src/main/java/com/telefonica/mistica/list/ListRowView.kt

+    fun setAssetOnClickListener(clickListener: OnClickListener) {
+        assetImageLayout.setOnClickListener(clickListener)
+    }


This is the code added for classic views in order to be able to set the click listener.

jeprubio · 2023-12-26T10:17:50Z

library/src/main/java/com/telefonica/mistica/compose/list/ListRowIcon.kt

@@ -28,31 +28,36 @@ sealed class ListRowIcon(val contentDescription: String?) {
    data class NormalIcon(
        val painter: Painter? = null,
        private val description: String? = null,
+        val modifier: Modifier = Modifier,


In compose we expose the modifier which is a common used practice which we were not following until now: https://chrisbanes.me/posts/always-provide-a-modifier/

jeprubio · 2023-12-26T10:19:07Z

library/src/test/java/com/telefonica/mistica/list/ListRowViewTest.kt

+    @Test
+    fun `check ListRowView xml onClick`() {
+        rule.scenario.onActivity { activity ->
+            var clicks = 0
+            val wrapper: FrameLayout = activity.findViewById(R.id.dummy_activity_wrapper)
+            activity.layoutInflater.inflate(R.layout.test_list_row_view, wrapper, true)
+
+            with (activity.findViewById<ListRowView>(R.id.list_row_view_32)) {
+                setOnClickListener { clicks++ }
+                performClick()
+            }
+
+            assertEquals(1, clicks)
+        }
+    }


Tests that the asset is clickable on xml

jeprubio · 2023-12-26T10:21:54Z

library/src/test/java/com/telefonica/mistica/compose/list/ListRowItemKtTest.kt

+    @Test
+    fun `check ListRowItem with clickable asset`() {
+        var clicked = 0
+        val onAssetClick: () -> Unit = {
+            clicked++
+        }
+        `when ListRowItem with asset`(
+            dimensions = ImageDimensions(width = 44, height = 44),
+            onAssetClick = onAssetClick,
+        )
+        composeTestRule.onNode(hasTestTag(LIST_ROW_ITEM_ASSET_TAG)).performClick()
+
+        assertEquals(1, clicked)
+    }


Tests that the asset is clickable on compose

jdelga

Cool! Good job!

jdelga · 2023-12-26T10:55:33Z

catalog/src/main/java/com/telefonica/mistica/catalog/ui/compose/components/Lists.kt

@@ -298,10 +302,14 @@ const val IMAGE_URL = "https://www.fotoaparat.cz/imgs/a/26/2639/0n1wjdf0-cr-em13
 @Composable
 fun Lists() {
    val samples = samples()
+    val context = LocalContext.current


This is used only once, there is no need to declare it as a property

but can't ve moved to the same line it's used as that would not compile:

jdelga · 2023-12-26T10:58:31Z

library/src/test/java/com/telefonica/mistica/compose/list/ListRowItemKtTest.kt

@@ -35,14 +43,30 @@ internal class ListRowItemKtTest: ScreenshotsTest() {
        `then screenshot is OK`()
    }

+    @Test
+    fun `check ListRowItem with clickable asset`() {
+        var clicked = 0


This is acting as a boolean, not as an integer

Not exactly as I want to know that it has been invoked only once. A boolean would only ensure it has been invoked but it could be multiple times. By doing it this way the condition is stronger.

The condition is exactly the same, you are checking if it has been clicked 1 time instead of 0.

jdelga · 2023-12-26T11:01:05Z

library/src/test/java/com/telefonica/mistica/list/ListRowViewTest.kt

+    @Test
+    fun `check ListRowView xml onClick`() {
+        rule.scenario.onActivity { activity ->
+            var clicks = 0


Same here, it is used as a boolean

Same answer here 😄

dagonco · 2023-12-26T11:13:07Z

...g/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/ListsCatalogFragment.kt

@@ -553,6 +556,31 @@ class ListsCatalogFragment : Fragment() {

    class ListViewHolder(val rowView: ListRowView) : RecyclerView.ViewHolder(rowView)

+    class ClickableListAdapter(
+        @ListRowView.BackgroundType private val backgroundType: Int,


This parameter are not being used!

True, it's not needed at least for now, removed from here the catalog. Thanks.

dagonco · 2023-12-26T11:24:14Z

...g/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/ListsCatalogFragment.kt

+        override fun onBindViewHolder(holder: ListViewHolder, position: Int) {
+            with(holder.rowView) {
+                setTitle("Clickable Asset")
+                setAssetOnClickListener {


From the way I have understood Figma, the list row should be clickable. cc: @AnaMontes11 is that true?

Otherwise, probably is a good idea to have all or atleast some of the list row variants with this new modification.
This is the intention of the catalog. A sample of the capabilities that each Mística's component has.

The row can be clickable, that's for sure, and that remains the same as it was.

Added an extra row which is clickable the row and the asset to each of the catalogs if it's that what you mean.

When clicking on the row:

And on the asset:

github-actions · 2023-12-26T11:40:41Z

📱 New catalog for testing generated: Download

dagonco

Niiice!

set assets clickable

f765cc6

jeprubio requested review from a team, dpastor, jdelga and AnaMontes11 and removed request for a team December 22, 2023 13:22

AnaMontes11 reviewed Dec 22, 2023

View reviewed changes

ANDROID-14134 Update icon size

435b934

AnaMontes11 approved these changes Dec 22, 2023

View reviewed changes

jeprubio requested review from dagonco and removed request for dpastor December 26, 2023 08:06

ANDROID-14134 Revert update icon size

da4e9d6

Telefonica deleted a comment from github-actions bot Dec 26, 2023

jeprubio commented Dec 26, 2023

View reviewed changes

ANDROID-14134 remove space

ba3ca7f

jeprubio commented Dec 26, 2023

View reviewed changes

Telefonica deleted a comment from github-actions bot Dec 26, 2023

jdelga approved these changes Dec 26, 2023

View reviewed changes

dagonco reviewed Dec 26, 2023

View reviewed changes

ANDROID-14134 remove unused parameter

c599cb4

Telefonica deleted a comment from github-actions bot Dec 26, 2023

dagonco reviewed Dec 26, 2023

View reviewed changes

ANDROID-14134 add clickable row examples

b56750c

Telefonica deleted a comment from github-actions bot Dec 26, 2023

dagonco approved these changes Dec 26, 2023

View reviewed changes

jeprubio merged commit b760ce2 into main Dec 26, 2023
5 checks passed

jeprubio deleted the feature/ANDROID-14134-SetAssetsClickable branch December 26, 2023 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ANDROID-14134 Set assets clickable #324

ANDROID-14134 Set assets clickable #324

jeprubio commented Dec 22, 2023 •

edited

Loading

AnaMontes11 Dec 22, 2023

jeprubio Dec 22, 2023 •

edited

Loading

jeprubio Dec 26, 2023

AnaMontes11 left a comment

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023

jeprubio Dec 26, 2023

jdelga left a comment

jdelga Dec 26, 2023

jeprubio Dec 26, 2023

jdelga Dec 26, 2023

jeprubio Dec 26, 2023 •

edited

Loading

jdelga Dec 26, 2023

jdelga Dec 26, 2023

jeprubio Dec 26, 2023 •

edited

Loading

dagonco Dec 26, 2023

jeprubio Dec 26, 2023 •

edited

Loading

dagonco Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

github-actions bot commented Dec 26, 2023

dagonco left a comment

ANDROID-14134 Set assets clickable #324

ANDROID-14134 Set assets clickable #324

Conversation

jeprubio commented Dec 22, 2023 • edited Loading

🥅 What's the goal?

🚧 How do we do it?

☑️ Checks

🧪 How can I test this?

Choose a reason for hiding this comment

jeprubio Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnaMontes11 left a comment

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdelga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

dagonco Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

jeprubio Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 26, 2023

dagonco left a comment

Choose a reason for hiding this comment

jeprubio commented Dec 22, 2023 •

edited

Loading

jeprubio Dec 22, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading

dagonco Dec 26, 2023 •

edited

Loading

jeprubio Dec 26, 2023 •

edited

Loading