From 850b702092038366ff8dd1b72fbc1eeca7b0885f Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Tue, 22 Oct 2024 16:57:03 +0200 Subject: [PATCH] change: Use checkboxes to show whether an account is a member of a list (#1036) User reports suggested that the "x" / "+" icons used to add / remove accounts from lists could be difficult to visually distinguish. Replace with checkboxes. Checked indicates the account is in the list, unchecked indicates it isn't. Clicking the checkbox to change state changes the presence in the list. Fixes #812 --- .../feature/lists/AccountsInListFragment.kt | 49 +++++++++--------- .../feature/lists/ListsForAccountFragment.kt | 34 +++++++------ .../main/res/layout/item_account_in_list.xml | 50 +++++++------------ .../layout/item_add_or_remove_from_list.xml | 37 ++++---------- 4 files changed, 73 insertions(+), 97 deletions(-) diff --git a/feature/lists/src/main/kotlin/app/pachli/feature/lists/AccountsInListFragment.kt b/feature/lists/src/main/kotlin/app/pachli/feature/lists/AccountsInListFragment.kt index 71de07831..f6c6e9f0f 100644 --- a/feature/lists/src/main/kotlin/app/pachli/feature/lists/AccountsInListFragment.kt +++ b/feature/lists/src/main/kotlin/app/pachli/feature/lists/AccountsInListFragment.kt @@ -65,8 +65,8 @@ import timber.log.Timber private typealias AccountInfo = Pair /** - * Display the members of a given list with UI to add/remove existing accounts - * and search for followers to add them to the list. + * Display the members of a given list with a checkbox to add/remove existing, + * accounts and search for followed accounts to add them to the list. */ @AndroidEntryPoint class AccountsInListFragment : DialogFragment() { @@ -229,11 +229,12 @@ class AccountsInListFragment : DialogFragment() { val binding = ItemAccountInListBinding.inflate(LayoutInflater.from(parent.context), parent, false) val holder = BindingHolder(binding) - binding.addButton.hide() - binding.removeButton.setOnClickListener { - onRemoveFromList(getItem(holder.bindingAdapterPosition).id) + binding.checkBox.setOnCheckedChangeListener { _, isChecked -> + if (!isChecked) { + onRemoveFromList(getItem(holder.bindingAdapterPosition).id) + } } - binding.removeButton.contentDescription = + binding.checkBox.contentDescription = binding.root.context.getString(R.string.action_remove_from_list) return holder @@ -241,9 +242,10 @@ class AccountsInListFragment : DialogFragment() { override fun onBindViewHolder(holder: BindingHolder, position: Int) { val account = getItem(position) - holder.binding.displayNameTextView.text = account.name.emojify(account.emojis, holder.binding.displayNameTextView, animateEmojis) - holder.binding.usernameTextView.text = account.username + holder.binding.displayName.text = account.name.emojify(account.emojis, holder.binding.displayName, animateEmojis) + holder.binding.username.text = binding.root.context.getString(DR.string.post_username_format, account.username) holder.binding.avatarBadge.visible(account.bot) + holder.binding.checkBox.isChecked = true loadAvatar(account.avatar, holder.binding.avatar, radius, animateAvatar) } } @@ -265,13 +267,13 @@ class AccountsInListFragment : DialogFragment() { val binding = ItemAccountInListBinding.inflate(LayoutInflater.from(parent.context), parent, false) val holder = BindingHolder(binding) - binding.addButton.hide() - binding.removeButton.setOnClickListener { + binding.checkBox.setOnCheckedChangeListener { buttonView, isChecked -> val (account, inAList) = getItem(holder.bindingAdapterPosition) - if (inAList) { - onRemoveFromList(account.id) - } else { + + if (isChecked) { onAddToList(account) + } else { + onRemoveFromList(account.id) } } @@ -281,26 +283,23 @@ class AccountsInListFragment : DialogFragment() { override fun onBindViewHolder(holder: BindingHolder, position: Int) { val (account, inAList) = getItem(position) - holder.binding.displayNameTextView.text = account.name.emojify(account.emojis, holder.binding.displayNameTextView, animateEmojis) - holder.binding.usernameTextView.text = account.username + holder.binding.displayName.text = account.name.emojify(account.emojis, holder.binding.displayName, animateEmojis) + holder.binding.username.text = binding.root.context.getString(DR.string.post_username_format, account.username) loadAvatar(account.avatar, holder.binding.avatar, radius, animateAvatar) holder.binding.avatarBadge.visible(account.bot) - holder.binding.removeButton.apply { - contentDescription = if (inAList) { - setImageResource(app.pachli.core.ui.R.drawable.ic_clear_24dp) - getString(R.string.action_remove_from_list) - } else { - setImageResource(app.pachli.core.ui.R.drawable.ic_plus_24dp) - getString(R.string.action_add_to_list) - } + with(holder.binding.checkBox) { + contentDescription = getString( + if (inAList) R.string.action_remove_from_list else R.string.action_add_to_list, + ) + isChecked = inAList } } } companion object { - private const val ARG_LIST_ID = "listId" - private const val ARG_LIST_NAME = "listName" + private const val ARG_LIST_ID = "app.pachli.ARG_LIST_ID" + private const val ARG_LIST_NAME = "app.pachli.ARG_LIST_NAME" @JvmStatic fun newInstance(listId: String, listName: String): AccountsInListFragment { diff --git a/feature/lists/src/main/kotlin/app/pachli/feature/lists/ListsForAccountFragment.kt b/feature/lists/src/main/kotlin/app/pachli/feature/lists/ListsForAccountFragment.kt index 349ac787a..0cf0920fe 100644 --- a/feature/lists/src/main/kotlin/app/pachli/feature/lists/ListsForAccountFragment.kt +++ b/feature/lists/src/main/kotlin/app/pachli/feature/lists/ListsForAccountFragment.kt @@ -31,7 +31,6 @@ import androidx.recyclerview.widget.ListAdapter import app.pachli.core.common.extensions.hide import app.pachli.core.common.extensions.show import app.pachli.core.common.extensions.viewBinding -import app.pachli.core.common.extensions.visible import app.pachli.core.designsystem.R as DR import app.pachli.core.ui.BackgroundMessage import app.pachli.core.ui.BindingHolder @@ -49,7 +48,7 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch /** - * Shows all the user's lists with a button to allow them to add/remove the given + * Shows all the user's lists with a checkbox to allow them to add/remove the given * account from each list. */ @AndroidEntryPoint @@ -189,23 +188,30 @@ class ListsForAccountFragment : DialogFragment() { ): BindingHolder { val binding = ItemAddOrRemoveFromListBinding.inflate(LayoutInflater.from(parent.context), parent, false) - return BindingHolder(binding) + val holder = BindingHolder(binding) + + binding.checkBox.setOnCheckedChangeListener { _, isChecked -> + val item = getItem(holder.bindingAdapterPosition) + if (isChecked == item.isMember) return@setOnCheckedChangeListener + + if (isChecked) { + viewModel.addAccountToList(item.list.id) + } else { + viewModel.deleteAccountFromList(item.list.id) + } + } + return holder } override fun onBindViewHolder(holder: BindingHolder, position: Int) { val item = getItem(position) holder.binding.listNameView.text = item.list.title - holder.binding.addButton.apply { - visible(!item.isMember) - setOnClickListener { - viewModel.addAccountToList(item.list.id) - } - } - holder.binding.removeButton.apply { - visible(item.isMember) - setOnClickListener { - viewModel.deleteAccountFromList(item.list.id) - } + + with(holder.binding.checkBox) { + contentDescription = getString( + if (item.isMember) R.string.action_remove_from_list else R.string.action_add_to_list, + ) + isChecked = item.isMember } } } diff --git a/feature/lists/src/main/res/layout/item_account_in_list.xml b/feature/lists/src/main/res/layout/item_account_in_list.xml index 44eb4207c..fc4ed6026 100644 --- a/feature/lists/src/main/res/layout/item_account_in_list.xml +++ b/feature/lists/src/main/res/layout/item_account_in_list.xml @@ -23,6 +23,7 @@ android:layout_height="wrap_content" android:paddingStart="?android:attr/listPreferredItemPaddingStart" android:paddingEnd="?android:attr/listPreferredItemPaddingEnd" + android:paddingTop="8dp" android:paddingBottom="8dp"> - - - + app:layout_constraintTop_toTopOf="@+id/avatar" /> + diff --git a/feature/lists/src/main/res/layout/item_add_or_remove_from_list.xml b/feature/lists/src/main/res/layout/item_add_or_remove_from_list.xml index 5b72fba55..7264277bd 100644 --- a/feature/lists/src/main/res/layout/item_add_or_remove_from_list.xml +++ b/feature/lists/src/main/res/layout/item_add_or_remove_from_list.xml @@ -6,8 +6,10 @@ android:layout_height="wrap_content" android:gravity="center_vertical" android:orientation="horizontal" - android:paddingHorizontal="16dp" - android:paddingVertical="8dp"> + android:paddingStart="?android:attr/listPreferredItemPaddingStart" + android:paddingEnd="?android:attr/listPreferredItemPaddingEnd" + android:paddingTop="8dp" + android:paddingBottom="8dp"> - - - - +