Skip to content

Commit

Permalink
fix: Improve readability of status media labels (#778)
Browse files Browse the repository at this point in the history
Previous code did not provide whitespace between different media labels
when media is not loaded.

In addition, the icon for the media was centre-aligned vertically with
the text, making it difficult to scan and determine when one media label
ends and another one starts.

Fix this by adding an 8dp margin between the media labels, and using a
TextView subclass that vertically aligns the media icon with the first
line of text.

Set the compound drawables with relative alignment, so they behave
appropriately in RTL layouts.

Fixes #751.
  • Loading branch information
nikclayton authored Jun 25, 2024
1 parent 1d87ac8 commit f477c63
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(i

// Set the icon next to the label.
val drawableId = attachments[0].iconResource()
mediaLabel.setCompoundDrawablesWithIntrinsicBounds(drawableId, 0, 0, 0)
mediaLabel.setCompoundDrawablesRelativeWithIntrinsicBounds(drawableId, 0, 0, 0)
setAttachmentClickListener(viewData, mediaLabel, listener, i, attachment, false)
} else {
mediaLabel.visibility = View.GONE
Expand Down
13 changes: 8 additions & 5 deletions app/src/main/res/layout/item_media_preview.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<TextView
<app.pachli.core.ui.TextViewWithVerticallyAlignedDrawable
android:id="@+id/status_media_label_0"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand All @@ -59,14 +59,15 @@
android:maxLines="10"
android:textSize="?attr/status_text_medium"
android:visibility="gone"
app:drawableTint="?android:attr/textColorTertiary"
app:drawableTint="?android:attr/colorControlNormal"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<TextView
<app.pachli.core.ui.TextViewWithVerticallyAlignedDrawable
android:id="@+id/status_media_label_1"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:background="?attr/selectableItemBackground"
android:drawablePadding="4dp"
android:ellipsize="end"
Expand All @@ -79,10 +80,11 @@
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/status_media_label_0" />

<TextView
<app.pachli.core.ui.TextViewWithVerticallyAlignedDrawable
android:id="@+id/status_media_label_2"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:background="?attr/selectableItemBackground"
android:drawablePadding="4dp"
android:ellipsize="end"
Expand All @@ -95,10 +97,11 @@
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/status_media_label_1" />

<TextView
<app.pachli.core.ui.TextViewWithVerticallyAlignedDrawable
android:id="@+id/status_media_label_3"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:background="?attr/selectableItemBackground"
android:drawablePadding="4dp"
android:ellipsize="end"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright 2024 Pachli Association
*
* This file is a part of Pachli.
*
* 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.
*
* Pachli 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 Pachli; if not,
* see <http://www.gnu.org/licenses>.
*/

package app.pachli.core.ui

import android.content.Context
import android.graphics.Canvas
import android.graphics.Rect
import android.graphics.drawable.Drawable
import android.util.AttributeSet
import android.widget.TextView

/**
* [TextView] that vertically aligns any start/end (or left/right) compound drawables
* with the first line of text.
*
* If the drawable height is <= to the line height it is centred within the line.
* Otherwise the top of the drawable is aligned with the top of the text.
*
* Necessary because the gravity of compound drawables can't be set.
*/
class TextViewWithVerticallyAlignedDrawable @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null,
defStyleAttr: Int = android.R.attr.textViewStyle,
) : androidx.appcompat.widget.AppCompatTextView(context, attrs, defStyleAttr) {
private var boundsRect: Rect = Rect()

override fun onDraw(canvas: Canvas) {
val centreTextView = height / 2

// Get the height of the first line
getLineBounds(0, boundsRect)
val lineHeight = boundsRect.height() + lineSpacingExtra.toInt()

// If the drawable fits within the height of the first line, centre-align it
// within that line. Otherwise align it with the top of the first line
(compoundDrawablesRelative[0] ?: compoundDrawables[0])?.let {
alignDrawableWithFirstLine(it, centreTextView, lineHeight)
}

(compoundDrawablesRelative[2] ?: compoundDrawables[2])?.let {
alignDrawableWithFirstLine(it, centreTextView, lineHeight)
}

super.onDraw(canvas)
}

/**
* Computes and sets a new top bound for [drawable] that vertically aligns it with
* the first line of text.
*
* @param drawable the drawable to align
* @param centreTextView the Y coordinate of the centre of the text view
* @param lineHeight the height of the first line of text
*/
private fun alignDrawableWithFirstLine(drawable: Drawable, centreTextView: Int, lineHeight: Int) {
// Difference between the height of the drawable and the height of the line.
// Positive if the line is taller than the drawable, negative otherwise
val heightDiff = lineHeight - drawable.intrinsicHeight

// The drawable's "natural" Y position, vertically centred
val naturalY = centreTextView - (drawable.intrinsicHeight / 2)

// Offset the drawable to the top of the view plus heightDiff. Coerce heightDiff to at
// least 0 so if the drawable is taller than the line it is not clipped off the top.
val offsetY = -naturalY + heightDiff.coerceAtLeast(0)
drawable.setBounds(0, offsetY, drawable.intrinsicWidth, drawable.intrinsicHeight + offsetY)
}
}

0 comments on commit f477c63

Please sign in to comment.