[Custom Fields] Read-only mode for JSON values #12708

hichamboushaba · 2024-09-26T17:13:39Z

Description

This PR adds support for displaying JSON values in read-only mode, it depends on the FluxC work done in wordpress-mobile/WordPress-FluxC-Android#3102

Steps to reproduce

Add a JSON custom field to one of your orders using the API:

POST {site}/wc/v3/orders/{orderId}

{
    "meta_data": [
        {
            "key": "json_object",
            "value": {
                "Key": "Value"
            }
        },
        {
            "key": "json_array",
            "value": [
                {
                    "Key": "Value"
                }
            ]
        }
    ]
}

Open the order in the app.
Tap on View custom fields.

Testing information

Confirm the JSON fields are listed.
Test tapping on the JSON field and checking the popup.

The tests that have been performed

Images/gif

Object	Array

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

wpmobilebot · 2024-09-26T17:21:51Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`8452fc7`
	Direct Download	`woocommerce-wear-prototype-build-pr12708-8452fc7.apk`

wpmobilebot · 2024-09-26T17:34:30Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`8452fc7`
	Direct Download	`woocommerce-prototype-build-pr12708-8452fc7.apk`

hichamboushaba · 2024-09-27T08:52:27Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsScreen.kt

+            val jsonFormatted = remember(customField.value) {
+                runCatching {
+                    if (customField.value.trimStart().startsWith("[")) {
+                        JSONArray(customField.value).toString(4)
+                    } else {
+                        JSONObject(customField.value).toString(4)
+                    }
+                }.getOrDefault(customField.value)
+            }
+


I didn't add this formatting logic to the ViewModel because it's UI only, and uses an Android specific class org.json.*, let me know if you have some concerns with this approach.

hichamboushaba · 2024-09-27T08:54:13Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsScreen.kt

+                OutlinedTextField(
+                    value = customField.key,
+                    onValueChange = {},
+                    label = { Text(text = stringResource(id = R.string.custom_fields_editor_key_label)) },
+                    readOnly = true,
+                    modifier = Modifier
+                )


I used read-only text fields for displaying the custom field's content because it has built-in support for selection and allowing copy/paste.

Good call! Just a minor nitpick. From a UX point of view, having the edit text fields focusable (highlighted with the purple border) when they are not editable seems confusing.
I was wondering if we should use enabled = false instead of readOnly = true. This still allows for copy pasting (although usability is worse for selecting the text), but makes it way more apparent that the content is not editable.
Wdyt?

Good point here @JorgeMucientes, I made some updates to disable the focus of the fields, I didn't use enabled = false reduced the experience a lot, please check the commit dbcf0be and let me know what you think.

With this commit, and the other UI updates suggested below, the result looks like this now:

Screen_recording_20241001_130945.mp4

wpmobilebot · 2024-09-27T10:52:02Z

Project dependencies changes

The following changes in project dependencies were detected (configuration vanillaReleaseRuntimeClasspath):

list

Upgraded Dependencies
org.wordpress.fluxc.plugins:woocommerce:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056, (changed from trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0)
org.wordpress.fluxc:fluxc-annotations:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056, (changed from trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0)
org.wordpress:fluxc:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056, (changed from trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0)

tree

-+--- org.wordpress:fluxc:trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0
-|    +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.9.25
-|    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.8.0 (*)
-|    |    +--- com.google.crypto.tink:tink-android:1.5.0
-|    |    \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0 (*)
-|    +--- androidx.room:room-runtime:2.6.1 (*)
-|    +--- androidx.room:room-ktx:2.6.1
-|    |    +--- androidx.room:room-common:2.6.1 (*)
-|    |    +--- androidx.room:room-runtime:2.6.1 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 1.9.25 (*)
-|    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
-|    |    +--- androidx.room:room-common:2.6.1 (c)
-|    |    \--- androidx.room:room-runtime:2.6.1 (c)
-|    +--- com.google.dagger:dagger:2.51.1
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- org.greenrobot:eventbus-java:3.3.1
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.8.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.7.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.7.0 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
++--- org.wordpress:fluxc:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056
+|    +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.9.25
+|    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
+|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.8.0 (*)
+|    |    +--- com.google.crypto.tink:tink-android:1.5.0
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0 (*)
+|    +--- androidx.room:room-runtime:2.6.1 (*)
+|    +--- androidx.room:room-ktx:2.6.1
+|    |    +--- androidx.room:room-common:2.6.1 (*)
+|    |    +--- androidx.room:room-runtime:2.6.1 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 1.9.25 (*)
+|    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
+|    |    +--- androidx.room:room-common:2.6.1 (c)
+|    |    \--- androidx.room:room-runtime:2.6.1 (c)
+|    +--- com.google.dagger:dagger:2.51.1
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- org.greenrobot:eventbus-java:3.3.1
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.8.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.7.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.7.0 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0
-     +--- org.wordpress:fluxc:trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0 (*)
-     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-     +--- com.google.dagger:dagger:2.51.1 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-     +--- androidx.room:room-runtime:2.6.1 (*)
-     +--- org.wordpress:wellsql:2.0.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:trunk-06e2a179771b9d8c726a15843d56fc897b6d5ca0
-     +--- androidx.room:room-ktx:2.6.1 (*)
-     \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056
+     +--- org.wordpress:fluxc:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056 (*)
+     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+     +--- com.google.dagger:dagger:2.51.1 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+     +--- androidx.room:room-runtime:2.6.1 (*)
+     +--- org.wordpress:wellsql:2.0.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-373bc6d30f8d9da6b7750f3ddc38929611b50056
+     +--- androidx.room:room-ktx:2.6.1 (*)
+     \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 (*)

JorgeMucientes

Nice work @hichamboushaba looks good and works well!
I just left a minor suggestion. Let me know what you think about it.

JorgeMucientes · 2024-10-01T10:45:43Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsScreen.kt

+                OutlinedTextField(
+                    value = customField.key,
+                    onValueChange = {},
+                    label = { Text(text = stringResource(id = R.string.custom_fields_editor_key_label)) },
+                    readOnly = true,
+                    modifier = Modifier
+                )


Good call! Just a minor nitpick. From a UX point of view, having the edit text fields focusable (highlighted with the purple border) when they are not editable seems confusing.
I was wondering if we should use enabled = false instead of readOnly = true. This still allows for copy pasting (although usability is worse for selecting the text), but makes it way more apparent that the content is not editable.
Wdyt?

keunes · 2024-10-01T11:25:36Z

That's great, thanks so much @hichamboushaba!

In terms of UX it'd be nice if the data could be 'transformed' into regular text, e.g.

Json_object `As modal title`

Key: Value
Key2: Value 2

Json_array `As modal title`

Key: Value

Having it as regular modal text would also address the point that @JorgeMucientes raised, while keeping exact text selection possible.

But no idea if that'd be feasible or unintended consequences. We'll already be very happy when this lands as is 🙂

keunes · 2024-10-01T11:30:33Z

Regarding the 'Close' button; minor point, but I'd personally change it to a simple 'OK' in the bottom-right, more in line with e.g. this modal:

(Currently, the 'Close' button attracts more attention than the content itself.)

hichamboushaba · 2024-10-01T11:37:43Z

In terms of UX it'd be nice if the data could be 'transformed' into regular text, e.g.

Thanks @keunes for the suggestion, but sadly it won't be as simple as this, as the JSON could have multiple nested levels for the fields, for example:

{ 
  "obj" : {
       "key" : "value"
  }
}

I'm sorry, but displaying the information as a list won't work in this case because we need to maintain the nesting level for the JSON values. We'll have to continue showing the JSON as raw text for now.

Having it as regular modal text would also address the point that @JorgeMucientes raised, while keeping exact text selection possible.

I don't think these points are related, Jorge's concern above is just about the "focus" indicator, when the fields are selected, they will be focused and be shown with a purple Color, and this could be confusing given the fields are not editable, I'll adjust this to remove this part later.

(Currently, the 'Close' button attracts more attention than the content itself.)

Thanks for bringing this up, I'll see what I can do for this.

JorgeMucientes · 2024-10-01T14:37:17Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsScreen.kt

+    // We use this to disable focus on the text fields used to show the key and value as it's not needed for our case
+    val inactiveInteractionSource = remember {
+        object : MutableInteractionSource {
+            override val interactions: Flow<Interaction> = emptyFlow()
+            override suspend fun emit(interaction: Interaction) {}
+            override fun tryEmit(interaction: Interaction): Boolean = false
+        }
+    }


# Conflicts: # build.gradle

codecov-commenter · 2024-10-01T16:02:35Z

Codecov Report

Attention: Patch coverage is 76.74419% with 10 lines in your changes missing coverage. Please review.

Project coverage is 40.69%. Comparing base (392343e) to head (8452fc7).
Report is 15 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...roid/ui/customfields/list/CustomFieldsViewModel.kt	84.00%	0 Missing and 4 partials ⚠️
...om/woocommerce/android/viewmodel/SavedStateFlow.kt	20.00%	2 Missing and 2 partials ⚠️
...merce/android/ui/customfields/CustomFieldModels.kt	84.61%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #12708      +/-   ##
============================================
+ Coverage     40.66%   40.69%   +0.02%     
- Complexity     5727     5730       +3     
============================================
  Files          1232     1232              
  Lines         69408    69445      +37     
  Branches       9621     9630       +9     
============================================
+ Hits          28228    28260      +32     
+ Misses        38578    38577       -1     
- Partials       2602     2608       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hichamboushaba commented Sep 27, 2024

View reviewed changes

hichamboushaba requested review from irfano and JorgeMucientes September 27, 2024 08:52

hichamboushaba marked this pull request as ready for review September 27, 2024 08:52

hichamboushaba commented Sep 27, 2024

View reviewed changes

hichamboushaba added this to the 20.7 milestone Sep 27, 2024

hichamboushaba added 8 commits September 27, 2024 11:35

Bump fluxc

f5f29ea

Expose whether a custom field is json or not in the UI model

0b8c68c

Add logic for showing a custom field on a popup

cb89d47

Update SavedStateFlow implementation to allow primitive types

341eb47

Add UI for the popup

f98c8aa

Fix detekt issues

ed234c1

Add unit tests

1563eca

Catch any parsing errors when formatting the value

b585623

hichamboushaba force-pushed the issue/12699-json-fields-read-only branch from 1c7051e to b585623 Compare September 27, 2024 10:36

JorgeMucientes self-assigned this Sep 30, 2024

Fix issue where inserted values shown in the popup

2c7bd7b

hichamboushaba mentioned this pull request Oct 1, 2024

Orders and Products Custom Fields #12207

Open

JorgeMucientes approved these changes Oct 1, 2024

View reviewed changes

Disable focus on the JSON custom field text fields

dbcf0be

hichamboushaba added 2 commits October 1, 2024 13:10

Some UI updates to the JSON viewer

c5eb9e5

Bump fluxc

f3b515e

hichamboushaba mentioned this pull request Oct 1, 2024

Update FluxC changeset to include fix #12721

Merged

JorgeMucientes reviewed Oct 1, 2024

View reviewed changes

Merge branch 'refs/heads/trunk' into issue/12699-json-fields-read-only

5b2cae4

# Conflicts: # build.gradle

hichamboushaba force-pushed the issue/12699-json-fields-read-only branch from 89541d1 to 5b2cae4 Compare October 1, 2024 15:19

hichamboushaba enabled auto-merge October 1, 2024 15:19

hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 1, 2024

Fix detekt issue

8452fc7

hichamboushaba merged commit 14b24c0 into trunk Oct 1, 2024
14 of 15 checks passed

hichamboushaba deleted the issue/12699-json-fields-read-only branch October 1, 2024 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Custom Fields] Read-only mode for JSON values #12708

[Custom Fields] Read-only mode for JSON values #12708

hichamboushaba commented Sep 26, 2024 •

edited

Loading

wpmobilebot commented Sep 26, 2024 •

edited

Loading

wpmobilebot commented Sep 26, 2024 •

edited

Loading

hichamboushaba Sep 27, 2024

hichamboushaba Sep 27, 2024

JorgeMucientes Oct 1, 2024

hichamboushaba Oct 1, 2024

wpmobilebot commented Sep 27, 2024 •

edited

Loading

JorgeMucientes left a comment

JorgeMucientes Oct 1, 2024

keunes commented Oct 1, 2024

keunes commented Oct 1, 2024 •

edited

Loading

hichamboushaba commented Oct 1, 2024 •

edited

Loading

JorgeMucientes Oct 1, 2024

codecov-commenter commented Oct 1, 2024

[Custom Fields] Read-only mode for JSON values #12708

[Custom Fields] Read-only mode for JSON values #12708

Conversation

hichamboushaba commented Sep 26, 2024 • edited Loading

Description

Steps to reproduce

Testing information

The tests that have been performed

Images/gif

Reviewer (or Author, in the case of optional code reviews):

wpmobilebot commented Sep 26, 2024 • edited Loading

wpmobilebot commented Sep 26, 2024 • edited Loading

hichamboushaba Sep 27, 2024

Choose a reason for hiding this comment

hichamboushaba Sep 27, 2024

Choose a reason for hiding this comment

JorgeMucientes Oct 1, 2024

Choose a reason for hiding this comment

hichamboushaba Oct 1, 2024

Choose a reason for hiding this comment

wpmobilebot commented Sep 27, 2024 • edited Loading

Project dependencies changes

JorgeMucientes left a comment

Choose a reason for hiding this comment

JorgeMucientes Oct 1, 2024

Choose a reason for hiding this comment

keunes commented Oct 1, 2024

Json_object As modal title

Json_array As modal title

keunes commented Oct 1, 2024 • edited Loading

hichamboushaba commented Oct 1, 2024 • edited Loading

JorgeMucientes Oct 1, 2024

Choose a reason for hiding this comment

codecov-commenter commented Oct 1, 2024

Codecov Report

hichamboushaba commented Sep 26, 2024 •

edited

Loading

wpmobilebot commented Sep 26, 2024 •

edited

Loading

wpmobilebot commented Sep 26, 2024 •

edited

Loading

wpmobilebot commented Sep 27, 2024 •

edited

Loading

Json_object `As modal title`

Json_array `As modal title`

keunes commented Oct 1, 2024 •

edited

Loading

hichamboushaba commented Oct 1, 2024 •

edited

Loading