Adjust list UX for platform consistency (#3142)

Most lists in the app use (explicitly or implicitly) platform metrics for dimensions, text size, colour, and so on, possibly via styles.

A few don't, inadvertently using the user's setting for status text size

Fix these, and simplify code where possible.

- Use android attributes for padding and height, for consistent UX.

- Remove explicit usage of app:tabTextAppearance, rely on the style.

- Remove ListSelectionAdapter and item_picker_list.xml, and adjust TabPreferenceActivity to use an ArrayAdapter with simple_list_item_1.xml

- Simplify item_followed_hashtag.xml, consistent with item_list.xml.

Fixes https://github.com/tuskyapp/Tusky/issues/3131

app/src/main/java/com/keylesspalace/tusky/TabPreferenceActivity.kt

@@ -21,6 +21,7 @@ import android.os.Bundle
 import android.util.Log
 import android.view.Gravity
 import android.view.View
+import android.widget.ArrayAdapter
 import android.widget.FrameLayout
 import android.widget.LinearLayout
 import android.widget.ProgressBar
@@ -42,12 +43,12 @@ import com.google.android.material.snackbar.Snackbar
 import com.google.android.material.transition.MaterialArcMotion
 import com.google.android.material.transition.MaterialContainerTransform
 import com.keylesspalace.tusky.adapter.ItemInteractionListener
-import com.keylesspalace.tusky.adapter.ListSelectionAdapter
 import com.keylesspalace.tusky.adapter.TabAdapter
 import com.keylesspalace.tusky.appstore.EventHub
 import com.keylesspalace.tusky.appstore.MainTabsChangedEvent
 import com.keylesspalace.tusky.databinding.ActivityTabPreferenceBinding
 import com.keylesspalace.tusky.di.Injectable
+import com.keylesspalace.tusky.entity.MastoList
 import com.keylesspalace.tusky.network.MastodonApi
 import com.keylesspalace.tusky.util.getDimension
 import com.keylesspalace.tusky.util.hide
@@ -272,7 +273,7 @@ class TabPreferenceActivity : BaseActivity(), Injectable, ItemInteractionListene
     }
 
     private fun showSelectListDialog() {
-        val adapter = ListSelectionAdapter(this)
+        val adapter = ArrayAdapter<MastoList>(this, android.R.layout.simple_list_item_1)
 
         val statusLayout = LinearLayout(this)
         statusLayout.gravity = Gravity.CENTER
@@ -298,12 +299,13 @@ class TabPreferenceActivity : BaseActivity(), Injectable, ItemInteractionListene
             .setNegativeButton(android.R.string.cancel, null)
             .setView(statusLayout)
             .setAdapter(adapter) { _, position ->
-                val list = adapter.getItem(position)
+                adapter.getItem(position)?.let { item ->
-                val newTab = createTabDataFromId(LIST, listOf(list!!.id, list.title))
+                    val newTab = createTabDataFromId(LIST, listOf(item.id, item.title))
-                currentTabs.add(newTab)
+                    currentTabs.add(newTab)
-                currentTabsAdapter.notifyItemInserted(currentTabs.size - 1)
+                    currentTabsAdapter.notifyItemInserted(currentTabs.size - 1)
-                updateAvailableTabs()
+                    updateAvailableTabs()
-                saveTabs()
+                    saveTabs()
+                }
             }
 
         val showProgressBarJob = getProgressBarJob(progress, 500)

app/src/main/java/com/keylesspalace/tusky/adapter/ListSelectionAdapter.kt

@@ -1,41 +0,0 @@
-/* Copyright 2019 kyori19
- *
- * This file is a part of Tusky.
- *
- * This program is free software; you can redistribute it and/or modify it under the terms of the
- * GNU General Public License as published by the Free Software Foundation; either version 3 of the
- * License, or (at your option) any later version.
- *
- * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
- * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
- * Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along with Tusky; if not,
- * see <http://www.gnu.org/licenses>. */
-
-package com.keylesspalace.tusky.adapter
-
-import android.content.Context
-import android.view.LayoutInflater
-import android.view.View
-import android.view.ViewGroup
-import android.widget.ArrayAdapter
-import com.keylesspalace.tusky.R
-import com.keylesspalace.tusky.databinding.ItemPickerListBinding
-import com.keylesspalace.tusky.entity.MastoList
-
-class ListSelectionAdapter(context: Context) : ArrayAdapter<MastoList>(context, R.layout.item_picker_list) {
-    override fun getView(position: Int, convertView: View?, parent: ViewGroup): View {
-        val binding = if (convertView == null) {
-            ItemPickerListBinding.inflate(LayoutInflater.from(context), parent, false)
-        } else {
-            ItemPickerListBinding.bind(convertView)
-        }
-
-        getItem(position)?.let { list ->
-            binding.root.text = list.title
-        }
-
-        return binding.root
-    }
-}

app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchAccountsFragment.kt

@@ -15,15 +15,28 @@
 
 package com.keylesspalace.tusky.components.search.fragments
 
+import android.os.Bundle
+import android.view.View
 import androidx.paging.PagingData
 import androidx.paging.PagingDataAdapter
 import androidx.preference.PreferenceManager
+import androidx.recyclerview.widget.DividerItemDecoration
 import com.keylesspalace.tusky.components.search.adapter.SearchAccountsAdapter
 import com.keylesspalace.tusky.entity.TimelineAccount
 import com.keylesspalace.tusky.settings.PrefKeys
 import kotlinx.coroutines.flow.Flow
 
 class SearchAccountsFragment : SearchFragment<TimelineAccount>() {
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+        binding.searchRecyclerView.addItemDecoration(
+            DividerItemDecoration(
+                binding.searchRecyclerView.context,
+                DividerItemDecoration.VERTICAL
+            )
+        )
+    }
+
     override fun createAdapter(): PagingDataAdapter<TimelineAccount, *> {
         val preferences = PreferenceManager.getDefaultSharedPreferences(binding.searchRecyclerView.context)
 

app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchFragment.kt

@@ -13,7 +13,6 @@ import androidx.lifecycle.lifecycleScope
 import androidx.paging.LoadState
 import androidx.paging.PagingData
 import androidx.paging.PagingDataAdapter
-import androidx.recyclerview.widget.DividerItemDecoration
 import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.SimpleItemAnimator
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
@@ -129,7 +128,6 @@ abstract class SearchFragment<T : Any> :
     }
 
     private fun initAdapter() {
-        binding.searchRecyclerView.addItemDecoration(DividerItemDecoration(binding.searchRecyclerView.context, DividerItemDecoration.VERTICAL))
         binding.searchRecyclerView.layoutManager = LinearLayoutManager(binding.searchRecyclerView.context)
         adapter = createAdapter()
         binding.searchRecyclerView.adapter = adapter

app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchHashtagsFragment.kt

@@ -15,8 +15,11 @@
 
 package com.keylesspalace.tusky.components.search.fragments
 
+import android.os.Bundle
+import android.view.View
 import androidx.paging.PagingData
 import androidx.paging.PagingDataAdapter
+import androidx.recyclerview.widget.DividerItemDecoration
 import com.keylesspalace.tusky.components.search.adapter.SearchHashtagsAdapter
 import com.keylesspalace.tusky.entity.HashTag
 import kotlinx.coroutines.flow.Flow
@@ -26,6 +29,16 @@ class SearchHashtagsFragment : SearchFragment<HashTag>() {
     override val data: Flow<PagingData<HashTag>>
         get() = viewModel.hashtagsFlow
 
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+        binding.searchRecyclerView.addItemDecoration(
+            DividerItemDecoration(
+                binding.searchRecyclerView.context,
+                DividerItemDecoration.VERTICAL
+            )
+        )
+    }
+
     override fun createAdapter(): PagingDataAdapter<HashTag, *> = SearchHashtagsAdapter(this)
 
     companion object {

app/src/main/res/layout/activity_account.xml

@@ -443,8 +443,7 @@
                 android:layout_height="wrap_content"
                 android:background="?attr/colorSurface"
                 app:tabGravity="center"
-                app:tabMode="scrollable"
+                app:tabMode="scrollable" />
-                app:tabTextAppearance="@style/TuskyTabAppearance" />
 
         </com.google.android.material.appbar.AppBarLayout>
 

app/src/main/res/layout/activity_followed_tags.xml

@@ -25,6 +25,7 @@
         android:layout_height="wrap_content"
         android:layout_gravity="center"
         app:layout_behavior="@string/appbar_scrolling_view_behavior"
+        tools:visibility="gone"
         />
 
     <ProgressBar
@@ -33,6 +34,7 @@
         android:layout_height="wrap_content"
         android:layout_gravity="center"
         app:layout_behavior="@string/appbar_scrolling_view_behavior"
+        tools:visibility="gone"
         />
 
     <com.google.android.material.floatingactionbutton.FloatingActionButton

app/src/main/res/layout/activity_search.xml

@@ -28,8 +28,7 @@
             android:layout_height="wrap_content"
             app:tabGravity="fill"
             app:tabMaxWidth="0dp"
-            app:tabMode="fixed"
+            app:tabMode="fixed" />
-            app:tabTextAppearance="@style/TuskyTabAppearance" />
 
     </com.google.android.material.appbar.AppBarLayout>
 

app/src/main/res/layout/item_followed_hashtag.xml

@@ -1,41 +1,36 @@
 <?xml version="1.0" encoding="utf-8"?>
-<androidx.constraintlayout.widget.ConstraintLayout
+<LinearLayout
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto"
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
-    android:layout_height="72dp"
+    android:layout_height="wrap_content"
+    android:minHeight="?android:attr/listPreferredItemHeightSmall"
     android:gravity="center_vertical"
-    android:paddingLeft="16dp"
+    android:paddingStart="?android:attr/listPreferredItemPaddingStart"
-    android:paddingRight="16dp"
+    android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
     >
 
+    <TextView
+        android:id="@+id/followed_tag"
+        android:layout_width="0dp"
+        android:layout_weight="1"
+        android:layout_height="wrap_content"
+        android:gravity="center_vertical"
+        android:ellipsize="end"
+        android:maxLines="1"
+        android:textAppearance="?android:attr/textAppearanceListItemSmall"
+        tools:text="hashtag" />
+
     <ImageButton
         android:id="@+id/followed_tag_unfollow"
         style="@style/TuskyImageButton"
         android:layout_width="32dp"
         android:layout_height="32dp"
-        app:layout_constraintTop_toTopOf="parent"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintRight_toRightOf="parent"
         android:background="?attr/selectableItemBackgroundBorderless"
         android:contentDescription="@string/action_unfollow"
         android:padding="4dp"
         app:srcCompat="@drawable/ic_person_remove_24dp"
         />
 
-    <TextView
+</LinearLayout>
-        android:id="@+id/followed_tag"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        app:layout_constraintTop_toTopOf="parent"
-        app:layout_constraintLeft_toLeftOf="parent"
-        app:layout_constraintRight_toRightOf="parent"
-        android:gravity="center_vertical"
-        android:ellipsize="end"
-        android:maxLines="1"
-        android:textColor="?android:textColorSecondary"
-        android:textSize="?attr/status_text_medium"
-        tools:text="#hashtag" />
-
-</androidx.constraintlayout.widget.ConstraintLayout>

app/src/main/res/layout/item_hashtag.xml

@@ -1,7 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
+
+<!-- Copied from android.R.layout.simple_list_item_1, because view binding does not work with
+     android.R.layout.* -->
+
 <TextView xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:id="@android:id/text1"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
-    android:padding="16dp"
+    android:textAppearance="?android:attr/textAppearanceListItemSmall"
+    android:gravity="center_vertical"
+    android:paddingStart="?android:attr/listPreferredItemPaddingStart"
+    android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
+    android:minHeight="?android:attr/listPreferredItemHeightSmall"
     android:background="?attr/selectableItemBackground"
-    android:textSize="?attr/status_text_medium" />
+    tools:ignore="SelectableText" />

app/src/main/res/layout/item_list.xml

@@ -4,30 +4,29 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
     android:gravity="center_vertical"
-    android:orientation="horizontal">
+    android:orientation="horizontal"
+    android:minHeight="?android:attr/listPreferredItemHeightSmall"
+    android:paddingStart="?android:attr/listPreferredItemPaddingStart"
+    android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
+    android:background="?attr/selectableItemBackground"
+    tools:ignore="Overdraw">
 
     <TextView
         android:id="@+id/list_name_textview"
         android:layout_width="0dp"
-        android:layout_height="60dp"
+        android:layout_height="wrap_content"
         android:layout_weight="1"
-        android:background="?selectableItemBackground"
         android:drawablePadding="8dp"
         android:gravity="center_vertical"
-        android:paddingLeft="16dp"
+        android:textAppearance="?android:attr/textAppearanceListItemSmall"
-        android:paddingRight="16dp"
-        android:textSize="?attr/status_text_medium"
         tools:text="Example list" />
 
     <ImageButton
         android:id="@+id/editListButton"
         style="@style/TuskyImageButton"
-        android:layout_width="36dp"
+        android:layout_width="32dp"
-        android:layout_height="wrap_content"
+        android:layout_height="32dp"
-        android:layout_margin="8dp"
         android:background="?selectableItemBackgroundBorderless"
         android:contentDescription="@string/action_more"
-        android:paddingLeft="8dp"
-        android:paddingRight="8dp"
         android:src="@drawable/ic_more_horiz_24dp" />
 </LinearLayout>

app/src/main/res/layout/item_picker_list.xml

@@ -1,8 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<TextView xmlns:android="http://schemas.android.com/apk/res/android"
-    android:layout_width="match_parent"
-    android:layout_height="wrap_content"
-    android:padding="16dp"
-    android:drawablePadding="8dp"
-    android:textColor="@color/textColorSecondary"
-    android:textSize="?attr/status_text_medium" />

app/src/main/res/layout/item_tab_preference.xml

@@ -6,9 +6,10 @@
     android:layout_height="wrap_content"
     android:background="?android:colorBackground"
     android:orientation="horizontal"
-    android:paddingStart="16dp"
     android:paddingTop="8dp"
-    android:paddingEnd="16dp">
+    android:paddingStart="?android:attr/listPreferredItemPaddingStart"
+    android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
+    android:minHeight="?android:attr/listPreferredItemHeightSmall">
 
     <ImageView
         android:id="@+id/imageView"
@@ -19,7 +20,8 @@
         android:paddingBottom="8dp"
         android:src="@drawable/ic_drag_indicator_24dp"
         app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent" />
+        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintBottom_toBottomOf="@id/textView"/>
 
     <TextView
         android:id="@+id/textView"
@@ -30,8 +32,7 @@
         android:drawablePadding="12dp"
         android:paddingTop="8dp"
         android:paddingBottom="8dp"
-        android:textColor="?android:attr/textColorSecondary"
+        android:textAppearance="?android:attr/textAppearanceListItemSmall"
-        android:textSize="?attr/status_text_large"
         app:drawableTint="?android:attr/textColorSecondary"
         app:layout_constraintBottom_toTopOf="@id/chipGroup"
         app:layout_constraintEnd_toEndOf="parent"

app/src/main/res/layout/item_tab_preference_small.xml

@@ -11,8 +11,7 @@
     android:lines="1"
     android:paddingStart="8dp"
     android:paddingEnd="8dp"
-    android:textColor="?android:attr/textColorSecondary"
+    android:textAppearance="?android:attr/textAppearanceListItemSmall"
-    android:textSize="?attr/status_text_large"
     app:drawableStartCompat="@drawable/ic_home_24dp"
     app:drawableTint="?android:attr/textColorSecondary" />
 

app/src/main/res/values/styles.xml

@@ -106,7 +106,7 @@
     </style>
 
     <style name="TuskyTabAppearance" parent="Widget.MaterialComponents.TabLayout">
-        <item name="android:textSize">?attr/status_text_medium</item>
+        <item name="tabTextAppearance">?android:attr/textAppearanceButton</item>
         <item name="android:textAllCaps">true</item>
         <item name="tabIndicatorHeight">3dp</item>
     </style>