From f477c63ab442c10e815cd4541343c4d412d7a1ff Mon Sep 17 00:00:00 2001
From: Nik Clayton <nik@ngo.org.uk>
Date: Tue, 25 Jun 2024 11:53:47 +0200
Subject: [PATCH] fix: Improve readability of status media labels (#778)

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.
---
 .../pachli/adapter/StatusBaseViewHolder.kt    |  2 +-
 .../main/res/layout/item_media_preview.xml    | 13 +--
 .../TextViewWithVerticallyAlignedDrawable.kt  | 84 +++++++++++++++++++
 3 files changed, 93 insertions(+), 6 deletions(-)
 create mode 100644 core/ui/src/main/kotlin/app/pachli/core/ui/TextViewWithVerticallyAlignedDrawable.kt

diff --git a/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.kt b/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.kt
index 640b3eb90f..963dc47491 100644
--- a/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.kt
+++ b/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.kt
@@ -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
diff --git a/app/src/main/res/layout/item_media_preview.xml b/app/src/main/res/layout/item_media_preview.xml
index e148ca8a19..81548c3713 100644
--- a/app/src/main/res/layout/item_media_preview.xml
+++ b/app/src/main/res/layout/item_media_preview.xml
@@ -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"
@@ -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"
@@ -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"
@@ -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"
diff --git a/core/ui/src/main/kotlin/app/pachli/core/ui/TextViewWithVerticallyAlignedDrawable.kt b/core/ui/src/main/kotlin/app/pachli/core/ui/TextViewWithVerticallyAlignedDrawable.kt
new file mode 100644
index 0000000000..3155d3af9f
--- /dev/null
+++ b/core/ui/src/main/kotlin/app/pachli/core/ui/TextViewWithVerticallyAlignedDrawable.kt
@@ -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)
+    }
+}