Merge pull request #7647 from thunderbird/fix_RecipientLayoutCreator

Fix `RecipientLayoutCreator` to retain contact name colors
This commit is contained in:
cketti 2024-02-16 17:54:59 +01:00 committed by GitHub
commit 60cb3dea2a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 104 additions and 36 deletions

View file

@ -1,5 +1,8 @@
package com.fsck.k9.ui.messageview
import android.text.SpannableStringBuilder
import androidx.core.text.toSpanned
private const val LIST_SEPARATOR = ", "
/**
@ -11,16 +14,25 @@ private const val LIST_SEPARATOR = ", "
* to me, Alice, Bob, Charly, Dora +11
*
* If there's not enough room to display the first recipient name, we return it anyway and expect the component that is
* actually rendering the text to ellipsize [RecipientLayoutData.recipientNames], but not
* actually rendering the text to ellipsize [RecipientLayoutData.recipientList], but not
* [RecipientLayoutData.additionalRecipients].
*/
internal class RecipientLayoutCreator(
private val textMeasure: TextMeasure,
private val maxNumberOfRecipientNames: Int,
private val recipientsFormat: String,
recipientsFormat: String,
private val additionalRecipientSpacing: Int,
private val additionalRecipientsPrefix: String,
) {
private val recipientsPrefix: String
private val recipientsSuffix: String
init {
require(recipientsFormat.contains("%s")) { "recipientFormat must contain '%s'" }
recipientsPrefix = recipientsFormat.substringBefore("%s")
recipientsSuffix = recipientsFormat.substringAfter("%s")
}
fun createRecipientLayout(
recipientNames: List<CharSequence>,
totalNumberOfRecipients: Int,
@ -30,7 +42,7 @@ internal class RecipientLayoutCreator(
if (recipientNames.size == 1) {
return RecipientLayoutData(
recipientNames = recipientsFormat.format(recipientNames.first()),
recipientList = createRecipientList(recipientNames),
additionalRecipients = null,
)
}
@ -39,11 +51,7 @@ internal class RecipientLayoutCreator(
val maxRecipientNames = recipientNames.size.coerceAtMost(maxNumberOfRecipientNames)
for (numberOfDisplayRecipients in maxRecipientNames downTo 2) {
val recipientNamesString = recipientNames.asSequence()
.take(numberOfDisplayRecipients)
.joinToString(separator = LIST_SEPARATOR)
val displayRecipients = recipientsFormat.format(recipientNamesString)
val recipientList = createRecipientList(recipientNames.take(numberOfDisplayRecipients))
additionalRecipientsBuilder.setLength(0)
val numberOfAdditionalRecipients = totalNumberOfRecipients - numberOfDisplayRecipients
@ -52,36 +60,47 @@ internal class RecipientLayoutCreator(
additionalRecipientsBuilder.append(numberOfAdditionalRecipients)
}
if (doesTextFitAvailableWidth(displayRecipients, additionalRecipientsBuilder, availableWidth)) {
if (doesTextFitAvailableWidth(recipientList, additionalRecipientsBuilder, availableWidth)) {
return RecipientLayoutData(
recipientNames = displayRecipients,
recipientList = recipientList,
additionalRecipients = additionalRecipientsBuilder.toStringOrNull(),
)
}
}
return RecipientLayoutData(
recipientNames = recipientsFormat.format(recipientNames.first()),
recipientList = createRecipientList(recipientNames.take(1)),
additionalRecipients = "$additionalRecipientsPrefix${totalNumberOfRecipients - 1}",
)
}
private fun doesTextFitAvailableWidth(
displayRecipients: CharSequence,
recipientList: CharSequence,
additionalRecipients: CharSequence,
availableWidth: Int,
): Boolean {
val recipientNamesWidth = textMeasure.measureRecipientNames(displayRecipients)
if (recipientNamesWidth > availableWidth) {
val recipientListWidth = textMeasure.measureRecipientNames(recipientList)
if (recipientListWidth > availableWidth) {
return false
} else if (additionalRecipients.isEmpty()) {
return true
}
val totalWidth = recipientNamesWidth + additionalRecipientSpacing +
textMeasure.measureRecipientCount(additionalRecipients)
return if (additionalRecipients.isEmpty()) {
true
} else {
val totalWidth = recipientListWidth + additionalRecipientSpacing +
textMeasure.measureRecipientCount(additionalRecipients)
return totalWidth <= availableWidth
totalWidth <= availableWidth
}
}
private fun createRecipientList(recipientNames: List<CharSequence>): CharSequence {
return recipientNames.joinTo(
buffer = SpannableStringBuilder(),
separator = LIST_SEPARATOR,
prefix = recipientsPrefix,
postfix = recipientsSuffix,
).toSpanned()
}
}
@ -90,7 +109,7 @@ private fun StringBuilder.toStringOrNull(): String? {
}
internal data class RecipientLayoutData(
val recipientNames: CharSequence,
val recipientList: CharSequence,
val additionalRecipients: String?,
)

View file

@ -125,7 +125,7 @@ class RecipientNamesView(context: Context, attrs: AttributeSet?) : ViewGroup(con
availableWidth,
)
recipientNameTextView.text = recipientLayoutData.recipientNames
recipientNameTextView.text = recipientLayoutData.recipientList
val additionalRecipientsVisible = recipientLayoutData.additionalRecipients != null
val remainingWidth: Int
if (additionalRecipientsVisible) {

View file

@ -1,11 +1,19 @@
package com.fsck.k9.ui.messageview
import android.text.Spannable
import android.text.SpannableString
import android.text.Spanned
import android.text.style.ForegroundColorSpan
import app.k9mail.core.android.testing.RobolectricTest
import assertk.Assert
import assertk.assertThat
import assertk.assertions.isEqualTo
import assertk.assertions.isInstanceOf
import assertk.assertions.isNull
import org.junit.Test
private const val COLOR = 0xFF0000
class RecipientLayoutCreatorTest : RobolectricTest() {
private val textMeasure = object : TextMeasure {
override fun measureRecipientNames(text: CharSequence): Int {
@ -46,7 +54,19 @@ class RecipientLayoutCreatorTest : RobolectricTest() {
availableWidth = 10,
)
assertThat(result.recipientNames.toString()).isEqualTo("to me")
assertThat(result.recipientList).isEqualToSpanned("to me")
assertThat(result.additionalRecipients).isNull()
}
@Test
fun `single colored recipient name`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf(coloredString("Alice")),
totalNumberOfRecipients = 1,
availableWidth = 10,
)
assertThat(result.recipientList).isEqualToSpanned("to <color>Alice</color>")
assertThat(result.additionalRecipients).isNull()
}
@ -58,55 +78,55 @@ class RecipientLayoutCreatorTest : RobolectricTest() {
availableWidth = 1,
)
assertThat(result.recipientNames.toString()).isEqualTo("to me")
assertThat(result.recipientList).isEqualToSpanned("to me")
assertThat(result.additionalRecipients).isNull()
}
@Test
fun `two recipient names where first one doesn't fit available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("Alice", "Bob"),
recipientNames = listOf(coloredString("Alice"), coloredString("Bob")),
totalNumberOfRecipients = 2,
availableWidth = 5,
)
assertThat(result.recipientNames.toString()).isEqualTo("to Alice")
assertThat(result.recipientList).isEqualToSpanned("to <color>Alice</color>")
assertThat(result.additionalRecipients).isEqualTo("+1")
}
@Test
fun `two recipient names where both fit available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("Alice", "Bob"),
recipientNames = listOf("Alice", coloredString("Bob")),
totalNumberOfRecipients = 2,
availableWidth = 13,
)
assertThat(result.recipientNames.toString()).isEqualTo("to Alice, Bob")
assertThat(result.recipientList).isEqualToSpanned("to Alice, <color>Bob</color>")
assertThat(result.additionalRecipients).isNull()
}
@Test
fun `three recipient names where only first one fits available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("Alice", "Bob", "Charly"),
recipientNames = listOf("Alice", coloredString("Bob"), "Charly"),
totalNumberOfRecipients = 3,
availableWidth = 13,
)
assertThat(result.recipientNames.toString()).isEqualTo("to Alice")
assertThat(result.recipientList).isEqualToSpanned("to Alice")
assertThat(result.additionalRecipients).isEqualTo("+2")
}
@Test
fun `three recipient names where only first two fit available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("Alice", "Bob", "Charly"),
recipientNames = listOf("Alice", coloredString("Bob"), "Charly"),
totalNumberOfRecipients = 3,
availableWidth = 16,
)
assertThat(result.recipientNames.toString()).isEqualTo("to Alice, Bob")
assertThat(result.recipientList).isEqualToSpanned("to Alice, <color>Bob</color>")
assertThat(result.additionalRecipients).isEqualTo("+1")
}
@ -118,31 +138,60 @@ class RecipientLayoutCreatorTest : RobolectricTest() {
availableWidth = 100,
)
assertThat(result.recipientNames.toString()).isEqualTo("to Alice, Bob, Charly")
assertThat(result.recipientList).isEqualToSpanned("to Alice, Bob, Charly")
assertThat(result.additionalRecipients).isNull()
}
@Test
fun `five recipient names and additional recipients where only two fit available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("One", "Two", "Three", "Four", "Five"),
recipientNames = listOf(coloredString("One"), coloredString("Two"), "Three", "Four", "Five"),
totalNumberOfRecipients = 10,
availableWidth = 20,
)
assertThat(result.recipientNames.toString()).isEqualTo("to One, Two")
assertThat(result.recipientList).isEqualToSpanned("to <color>One</color>, <color>Two</color>")
assertThat(result.additionalRecipients).isEqualTo("+8")
}
@Test
fun `five recipient names and additional recipients where all five fit available width`() {
val result = recipientLayoutCreator.createRecipientLayout(
recipientNames = listOf("One", "Two", "Three", "Four", "Five"),
recipientNames = listOf("One", "Two", "Three", "Four", coloredString("Five")),
totalNumberOfRecipients = 10,
availableWidth = 100,
)
assertThat(result.recipientNames.toString()).isEqualTo("to One, Two, Three, Four, Five")
assertThat(result.recipientList).isEqualToSpanned("to One, Two, Three, Four, <color>Five</color>")
assertThat(result.additionalRecipients).isEqualTo("+5")
}
private fun coloredString(text: String): Spannable {
return SpannableString(text).apply {
setSpan(ForegroundColorSpan(COLOR), 0, text.length, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
}
}
private fun Assert<CharSequence>.isEqualToSpanned(expected: String) = given { charSequence ->
assertThat(charSequence).isInstanceOf<Spanned>()
val spanned = charSequence as Spanned
val spans = spanned.getSpans(0, spanned.length, Any::class.java)
.toList()
.sortedByDescending { spanned.getSpanStart(it) }
val tagString = buildString {
append(spanned)
for (span in spans) {
assertThat(span).isInstanceOf<ForegroundColorSpan>().given { colorSpan ->
assertThat(colorSpan.foregroundColor).isEqualTo(COLOR)
insert(spanned.getSpanEnd(colorSpan), "</color>")
insert(spanned.getSpanStart(colorSpan), "<color>")
}
}
}
assertThat(tagString).isEqualTo(expected)
}
}