Skip to content

Commit

Permalink
change: Use checkboxes to show whether an account is a member of a list
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nikclayton committed Oct 22, 2024
1 parent 2234c4c commit e4f06a3
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ import timber.log.Timber
private typealias AccountInfo = Pair<TimelineAccount, Boolean>

/**
* 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() {
Expand Down Expand Up @@ -229,21 +229,23 @@ 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
}

override fun onBindViewHolder(holder: BindingHolder<ItemAccountInListBinding>, 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)
}
}
Expand All @@ -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)
}
}

Expand All @@ -281,26 +283,23 @@ class AccountsInListFragment : DialogFragment() {
override fun onBindViewHolder(holder: BindingHolder<ItemAccountInListBinding>, 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -189,23 +188,30 @@ class ListsForAccountFragment : DialogFragment() {
): BindingHolder<ItemAddOrRemoveFromListBinding> {
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<ItemAddOrRemoveFromListBinding>, 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
}
}
}
Expand Down
50 changes: 18 additions & 32 deletions feature/lists/src/main/res/layout/item_account_in_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">

<ImageView
Expand All @@ -34,6 +35,7 @@
android:contentDescription="@string/action_view_profile"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
tools:src="@drawable/avatar_default" />

<ImageView
Expand All @@ -46,60 +48,44 @@
app:layout_constraintEnd_toEndOf="@id/avatar" />

<TextView
android:id="@+id/displayNameTextView"
android:id="@+id/displayName"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="14dp"
android:layout_marginTop="6dp"
android:ellipsize="end"
android:maxLines="1"
android:textColor="?android:textColorPrimary"
android:textSize="?attr/status_text_medium"
android:textStyle="normal|bold"
app:layout_constraintEnd_toStartOf="@id/removeButton"
app:layout_constraintBottom_toTopOf="@id/username"
app:layout_constraintEnd_toStartOf="@+id/checkBox"
app:layout_constraintStart_toEndOf="@+id/avatar"
app:layout_constraintTop_toTopOf="@id/avatar"
app:layout_constraintVertical_chainStyle="packed"
tools:text="Display name" />

<TextView
android:id="@+id/usernameTextView"
android:id="@+id/username"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="14dp"
android:ellipsize="end"
android:maxLines="1"
android:textColor="?android:textColorSecondary"
android:textSize="?attr/status_text_medium"
app:layout_constraintEnd_toStartOf="@+id/removeButton"
app:layout_constraintEnd_toEndOf="@+id/displayName"
app:layout_constraintStart_toEndOf="@id/avatar"
app:layout_constraintTop_toBottomOf="@id/displayNameTextView"
app:layout_constraintTop_toBottomOf="@id/displayName"
tools:text="\@username" />

<ImageButton
android:id="@+id/removeButton"
style="@style/AppImageButton"
android:layout_width="52dp"
android:layout_height="48dp"
android:layout_marginStart="12dp"
android:background="?attr/selectableItemBackgroundBorderless"
android:contentDescription="@string/action_remove_from_list"
android:padding="4dp"
app:layout_constraintBottom_toBottomOf="@id/avatar"
app:layout_constraintEnd_toStartOf="@id/addButton"
app:layout_constraintStart_toEndOf="@id/displayNameTextView"
app:layout_constraintTop_toTopOf="@id/avatar"
app:srcCompat="@drawable/ic_clear_24dp" />

<ImageButton
android:id="@+id/addButton"
style="@style/AppImageButton"
android:layout_width="52dp"
android:layout_height="48dp"
android:layout_marginEnd="4dp"
android:background="?attr/selectableItemBackgroundBorderless"
android:contentDescription="@string/action_add_to_list"
android:padding="4dp"
<CheckBox
android:id="@+id/checkBox"
android:layout_width="wrap_content"
android:layout_height="0dp"
android:minWidth="48dp"
android:minHeight="48dp"
app:layout_constraintBottom_toBottomOf="@+id/avatar"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="@+id/removeButton"
app:srcCompat="@drawable/ic_check_24dp" />
app:layout_constraintTop_toTopOf="@+id/avatar" />

</androidx.constraintlayout.widget.ConstraintLayout>
37 changes: 11 additions & 26 deletions feature/lists/src/main/res/layout/item_add_or_remove_from_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">

<TextView
android:id="@+id/listNameView"
Expand All @@ -18,33 +20,16 @@
android:ellipsize="end"
android:gravity="center_vertical"
android:maxLines="1"
android:textColor="?android:attr/textColorSecondary"
android:textColor="?android:attr/textColorPrimary"
android:textSize="?attr/status_text_medium"
app:drawableStartCompat="@drawable/ic_list"
app:drawableTint="?android:attr/textColorSecondary"
tools:text="Example list" />

<ImageButton
android:id="@+id/addButton"
style="@style/AppImageButton"
android:layout_width="32dp"
android:layout_height="32dp"
android:layout_marginStart="12dp"
android:background="?attr/selectableItemBackgroundBorderless"
android:contentDescription="@string/action_add_to_list"
android:padding="4dp"
android:src="@drawable/ic_plus_24dp" />

<ImageButton
android:id="@+id/removeButton"
style="@style/AppImageButton"
android:layout_width="32dp"
android:layout_height="32dp"
android:layout_marginStart="12dp"
android:background="?attr/selectableItemBackgroundBorderless"
android:contentDescription="@string/action_remove_from_list"
android:padding="4dp"
android:src="@drawable/ic_clear_24dp"
android:visibility="gone" />

<CheckBox
android:id="@+id/checkBox"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:minWidth="48dp"
android:minHeight="48dp" />
</LinearLayout>

0 comments on commit e4f06a3

Please sign in to comment.