Merge pull request #6152 from seadowg/entities-browser

Improve entity browser design
getodk · May 24, 2024 · 70a1968 · 70a1968
2 parents ffc5aa9 + 023639c
commit 70a1968
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 36 deletions.
diff --git a/entities/src/main/java/org/odk/collect/entities/EntitiesFragment.kt b/entities/src/main/java/org/odk/collect/entities/EntitiesFragment.kt
@@ -12,6 +12,7 @@ import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
 import androidx.recyclerview.widget.RecyclerView.ViewHolder
 import org.odk.collect.entities.databinding.ListLayoutBinding
+import org.odk.collect.lists.RecyclerViewUtils
 import org.odk.collect.lists.RecyclerViewUtils.matchParentWidth
 
 class EntitiesFragment(private val viewModelFactory: ViewModelProvider.Factory) : Fragment() {
@@ -29,6 +30,7 @@ class EntitiesFragment(private val viewModelFactory: ViewModelProvider.Factory)
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         val binding = ListLayoutBinding.bind(view)
         binding.list.layoutManager = LinearLayoutManager(requireContext())
+        binding.list.addItemDecoration(RecyclerViewUtils.verticalLineDivider(requireContext()))
 
         val list = EntitiesFragmentArgs.fromBundle(requireArguments()).list
         entitiesViewModel.getEntities(list).observe(viewLifecycleOwner) {

diff --git a/entities/src/main/java/org/odk/collect/entities/EntityItemView.kt b/entities/src/main/java/org/odk/collect/entities/EntityItemView.kt
@@ -14,7 +14,7 @@ class EntityItemView(context: Context) : FrameLayout(context) {
         binding.label.text = entity.label
         binding.properties.text = entity.properties
             .sortedBy { it.first }
-            .joinToString(separator = ", ") { "${it.first}: ${it.second}" }
+            .joinToString(separator = "\n") { "${it.first}: ${it.second}" }
         binding.offlinePill.isVisible = entity.state == Entity.State.OFFLINE
     }
 }
diff --git a/entities/src/main/java/org/odk/collect/entities/EntityListsFragment.kt b/entities/src/main/java/org/odk/collect/entities/EntityListsFragment.kt
@@ -22,6 +22,7 @@ import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import org.odk.collect.entities.databinding.AddEntitiesDialogLayoutBinding
 import org.odk.collect.entities.databinding.EntityListItemLayoutBinding
 import org.odk.collect.entities.databinding.ListLayoutBinding
+import org.odk.collect.lists.RecyclerViewUtils
 
 class EntityListsFragment(
     private val viewModelFactory: ViewModelProvider.Factory,
@@ -41,6 +42,7 @@ class EntityListsFragment(
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         val binding = ListLayoutBinding.bind(view)
         binding.list.layoutManager = LinearLayoutManager(requireContext())
+        binding.list.addItemDecoration(RecyclerViewUtils.verticalLineDivider(requireContext()))
 
         entitiesViewModel.lists.observe(viewLifecycleOwner) {
             binding.list.adapter = ListsAdapter(it, findNavController())
@@ -72,6 +74,7 @@ private class ListsMenuProvider(
                 val binding = AddEntitiesDialogLayoutBinding.inflate(LayoutInflater.from(context))
                 MaterialAlertDialogBuilder(context)
                     .setView(binding.root)
+                    .setTitle(org.odk.collect.strings.R.string.add_entity_list)
                     .setPositiveButton(org.odk.collect.strings.R.string.add) { _, _ ->
                         entitiesViewModel.addEntityList(binding.entityListName.text.toString())
                     }

diff --git a/entities/src/main/res/layout/entity_item_layout.xml b/entities/src/main/res/layout/entity_item_layout.xml
@@ -4,36 +4,46 @@
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
-    android:background="?android:attr/selectableItemBackground"
-    android:paddingHorizontal="@dimen/margin_standard"
-    android:paddingTop="@dimen/margin_standard">
+    android:background="?android:attr/selectableItemBackground">
+
+    <androidx.constraintlayout.widget.Guideline
+        android:id="@+id/guideline_start"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:orientation="vertical"
+        app:layout_constraintGuide_begin="@dimen/margin_standard" />
+
+    <org.odk.collect.material.MaterialPill
+        android:id="@+id/offline_pill"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="@dimen/margin_standard"
+        app:icon="@drawable/ic_baseline_wifi_off_24"
+        app:layout_constraintStart_toStartOf="@id/guideline_start"
+        app:layout_constraintTop_toTopOf="parent"
+        app:pillBackgroundColor="?colorSurfaceContainerHighest"
+        app:text="@string/offline" />
 
     <com.google.android.material.textview.MaterialTextView
         android:id="@+id/label"
         android:layout_width="0dp"
         android:layout_height="wrap_content"
+        android:layout_marginTop="@dimen/margin_small"
         android:textAppearance="?textAppearanceTitleMedium"
-        app:layout_constraintEnd_toStartOf="@id/offline_pill"
-        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintStart_toStartOf="@id/guideline_start"
+        app:layout_constraintTop_toBottomOf="@id/offline_pill"
         tools:text="Label" />
 
     <com.google.android.material.textview.MaterialTextView
         android:id="@+id/properties"
-        android:layout_width="match_parent"
+        android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/margin_extra_small"
+        android:layout_marginBottom="@dimen/margin_standard"
         android:textAppearance="?textAppearanceBodyMedium"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintStart_toStartOf="@id/guideline_start"
         app:layout_constraintTop_toBottomOf="@id/label"
-        tools:text="property1: value1, property2: value2" />
-
-    <org.odk.collect.material.MaterialPill
-        android:id="@+id/offline_pill"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-        app:icon="@drawable/ic_baseline_visibility_24"
-        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintTop_toTopOf="@id/label"
-        app:text="@string/offline" />
+        tools:text="property1: value1" />
 
 </androidx.constraintlayout.widget.ConstraintLayout>
diff --git a/entities/src/test/java/org/odk/collect/entities/EntityItemViewTest.kt b/entities/src/test/java/org/odk/collect/entities/EntityItemViewTest.kt
@@ -28,7 +28,7 @@ class EntityItemViewTest {
         )
 
         val propertiesView = view.binding.properties
-        assertThat(propertiesView.text, equalTo("length: 2:50, name: S.D.O.S"))
+        assertThat(propertiesView.text, equalTo("length: 2:50\nname: S.D.O.S"))
     }
 
     @Test

diff --git a/icons/src/main/res/drawable/ic_baseline_wifi_off_24.xml b/icons/src/main/res/drawable/ic_baseline_wifi_off_24.xml
@@ -0,0 +1,5 @@
+<vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="24dp" android:tint="#000000" android:viewportHeight="24" android:viewportWidth="24" android:width="24dp">
+
+    <path android:fillColor="@android:color/white" android:pathData="M22.99,9C19.15,5.16 13.8,3.76 8.84,4.78l2.52,2.52c3.47,-0.17 6.99,1.05 9.63,3.7l2,-2zM18.99,13c-1.29,-1.29 -2.84,-2.13 -4.49,-2.56l3.53,3.53 0.96,-0.97zM2,3.05L5.07,6.1C3.6,6.82 2.22,7.78 1,9l1.99,2c1.24,-1.24 2.67,-2.16 4.2,-2.77l2.24,2.24C7.81,10.89 6.27,11.73 5,13v0.01L6.99,15c1.36,-1.36 3.14,-2.04 4.92,-2.06L18.98,20l1.27,-1.26L3.29,1.79 2,3.05zM9,17l3,3 3,-3c-1.65,-1.66 -4.34,-1.66 -6,0z"/>
+
+</vector>
diff --git a/material/src/main/java/org/odk/collect/material/MaterialPill.kt b/material/src/main/java/org/odk/collect/material/MaterialPill.kt
@@ -2,6 +2,7 @@ package org.odk.collect.material
 
 import android.content.Context
 import android.content.res.ColorStateList
+import android.graphics.drawable.ColorDrawable
 import android.util.AttributeSet
 import android.view.LayoutInflater
 import android.widget.FrameLayout
@@ -29,21 +30,22 @@ open class MaterialPill(context: Context, attrs: AttributeSet?) :
             binding.text.text = text
         }
 
-    private val shapeAppearanceModel =
-        ShapeAppearanceModel.builder(context, getShapeAppearance(context), -1).build()
-
     val binding = PillBinding.inflate(LayoutInflater.from(context), this, true)
 
     init {
-        background = createMaterialShapeDrawable(getDefaultBackgroundColor(context))
-
         context.withStyledAttributes(attrs, R.styleable.MaterialPill) {
             text = getString(R.styleable.MaterialPill_text)
 
             val iconId = getResourceId(R.styleable.MaterialPill_icon, -1)
             if (iconId != -1) {
                 setIcon(iconId)
             }
+
+            val backgroundColor = getColor(
+                R.styleable.MaterialPill_pillBackgroundColor,
+                getDefaultBackgroundColor(context)
+            )
+            setPillBackgroundColor(backgroundColor)
         }
     }
 
@@ -61,26 +63,37 @@ open class MaterialPill(context: Context, attrs: AttributeSet?) :
     }
 
     fun setPillBackgroundColor(@ColorInt color: Int) {
-        background = createMaterialShapeDrawable(color)
+        if (isInEditMode) {
+            /**
+             * For some reason `ShapeAppearanceModel` can't be built in Android Studio's design
+             * preview (even when using a Material 3 theme). It could be that some of the
+             * attibutes used here are not available in the basic themes, but are set in the real
+             * ones we use. For now, just setting a "unshaped" background is an easier option than
+             * deep diving.
+             */
+            background = ColorDrawable(color)
+            return
+        }
+
+        val shapeAppearance = getThemeAttributeValue(
+            context,
+            com.google.android.material.R.attr.shapeAppearanceCornerSmall
+        )
+
+        val shapeAppearanceModel =
+            ShapeAppearanceModel.builder(context, shapeAppearance, -1).build()
+
+        background = MaterialShapeDrawable(shapeAppearanceModel).also {
+            it.fillColor = ColorStateList.valueOf(color)
+        }
     }
 
     fun setTextColor(@ColorInt color: Int) {
         binding.text.setTextColor(color)
     }
 
-    private fun getShapeAppearance(context: Context) = getThemeAttributeValue(
-        context,
-        com.google.android.material.R.attr.shapeAppearanceCornerSmall
-    )
-
     private fun getDefaultBackgroundColor(context: Context) = getThemeAttributeValue(
         context,
         com.google.android.material.R.attr.colorPrimaryContainer
     )
-
-    private fun createMaterialShapeDrawable(@ColorInt color: Int): MaterialShapeDrawable {
-        return MaterialShapeDrawable(shapeAppearanceModel).also {
-            it.fillColor = ColorStateList.valueOf(color)
-        }
-    }
 }
diff --git a/material/src/main/res/values/attrs.xml b/material/src/main/res/values/attrs.xml
@@ -16,5 +16,6 @@
     <declare-styleable name="MaterialPill">
         <attr name="icon" />
         <attr name="text" format="string" />
+        <attr name="pillBackgroundColor" format="reference"/>
     </declare-styleable>
 </resources>