From c3a281815b0800b44423b583772672633d051bb6 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:49:59 +0300 Subject: [PATCH] Drop the comments around the bidi isolation Review feedback: the helper name says what it does and the test names say why the preview needs it. --- .../impl/DefaultRoomLatestEventFormatter.kt | 2 -- .../android/libraries/eventformatter/impl/PrefixWith.kt | 9 +-------- .../impl/DefaultRoomLatestEventFormatterTest.kt | 3 --- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt index 6bdc8aa841..e7bbdb1e7f 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatter.kt @@ -189,8 +189,6 @@ class DefaultRoomLatestEventFormatter( if (isOutgoing) { sp.getString(CommonStrings.common_you) } else { - // The display name is user-provided and may be right-to-left, which would otherwise - // reorder the whole preview line. senderDisambiguatedDisplayName.bidiIsolate() } ) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt index 58dad3d7b2..f0965e95af 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/PrefixWith.kt @@ -17,14 +17,7 @@ import androidx.compose.ui.text.withStyle private const val FIRST_STRONG_ISOLATE = 0x2068 private const val POP_DIRECTIONAL_ISOLATE = 0x2069 -/** - * Wraps the string between Unicode isolate characters (FSI … PDI) so its own direction cannot - * reorder the text around it. - * - * A room list preview is rendered as a single paragraph, so a right-to-left display name otherwise - * flips the whole line — the message body ends up on the wrong side of the name and the trailing - * colon jumps to the front. See https://github.com/element-hq/element-x-android/issues/3338 - */ +/** Wraps the string between Unicode isolate characters so its own direction cannot reorder the text around it. */ internal fun String.bidiIsolate(): String = Char(FIRST_STRONG_ISOLATE) + this + Char(POP_DIRECTIONAL_ISOLATE) internal fun CharSequence.prefixWith(prefix: String): AnnotatedString { diff --git a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt index abc3cf280d..ecd0509269 100644 --- a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt +++ b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLatestEventFormatterTest.kt @@ -109,7 +109,6 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() { // And there is a bold span for the 'Sticker' part val boldSpanStyle = (result as AnnotatedString).spanStyles.lastOrNull { it.item.fontWeight == FontWeight.Bold } assertThat(boldSpanStyle).isNotNull() - // +2 for the isolate characters around the sender name, +2 for the ": " separator val spanStart = someoneElseId.value.length + 4 assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 7) assertThat(result.toString()).isEqualTo(expectedBody) @@ -171,7 +170,6 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() { ) val result = formatter.format(message, false) assertThat(result.toString()).isEqualTo("⁨$senderName⁩: $body") - // The bold span still covers exactly the name and its isolates, not the body. val boldSpanStyle = (result as AnnotatedString).spanStyles.first { it.item.fontWeight == FontWeight.Bold } assertThat(boldSpanStyle.start).isEqualTo(0) assertThat(boldSpanStyle.end).isEqualTo(senderName.length + 2) @@ -971,7 +969,6 @@ class DefaultRoomLatestEventFormatterTest : RobolectricTest() { // And there is a bold span for the 'Poll' part val boldSpanStyle = (result as AnnotatedString).spanStyles.lastOrNull { it.item.fontWeight == FontWeight.Bold } assertThat(boldSpanStyle).isNotNull() - // +2 for the isolate characters around the sender name, +2 for the ": " separator val spanStart = "Bob".length + 4 assertThat(boldSpanStyle!!.start..boldSpanStyle.end).isEqualTo(spanStart..spanStart + 4) }