Skip to content

Commit

Permalink
Merge pull request #7647 from thunderbird/fix_RecipientLayoutCreator
Browse files Browse the repository at this point in the history
Fix `RecipientLayoutCreator` to retain contact name colors
  • Loading branch information
cketti authored Feb 16, 2024
2 parents 2f0f2a2 + c352ee9 commit 60cb3de
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 36 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.fsck.k9.ui.messageview

import android.text.SpannableStringBuilder
import androidx.core.text.toSpanned

private const val LIST_SEPARATOR = ", "

/**
Expand All @@ -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,
Expand All @@ -30,7 +42,7 @@ internal class RecipientLayoutCreator(

if (recipientNames.size == 1) {
return RecipientLayoutData(
recipientNames = recipientsFormat.format(recipientNames.first()),
recipientList = createRecipientList(recipientNames),
additionalRecipients = null,
)
}
Expand All @@ -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
Expand All @@ -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)

totalWidth <= availableWidth
}
}

return totalWidth <= availableWidth
private fun createRecipientList(recipientNames: List<CharSequence>): CharSequence {
return recipientNames.joinTo(
buffer = SpannableStringBuilder(),
separator = LIST_SEPARATOR,
prefix = recipientsPrefix,
postfix = recipientsSuffix,
).toSpanned()
}
}

Expand All @@ -90,7 +109,7 @@ private fun StringBuilder.toStringOrNull(): String? {
}

internal data class RecipientLayoutData(
val recipientNames: CharSequence,
val recipientList: CharSequence,
val additionalRecipients: String?,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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()
}

Expand All @@ -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")
}

Expand All @@ -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)
}
}

0 comments on commit 60cb3de

Please sign in to comment.