Fix pillification not working for non formatted message bodies (#3201)
* Fix pillification not working for non formatted message bodies Pure Markdown bodies weren't being 'pillified' so their mentions were turned into UI elements in the timeline. A new `pillifiedBody` property was added to `TimelineItemTextBasedContent` to fix this. * Use shorter version of `textWithMentions` computation
This commit is contained in:
committed by
GitHub
parent
27226e382a
commit
a6b6d2ab7a
@@ -571,7 +571,7 @@ internal fun TimelineItemEventRowPreview() = ElementPreview {
|
||||
event = aTimelineItemEvent(
|
||||
senderDisplayName = "Sender with a super long name that should ellipsize",
|
||||
isMine = isMine,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
content = aTimelineItemTextContent(
|
||||
body = "A long text which will be displayed on several lines and" +
|
||||
" hopefully can be manually adjusted to test different behaviors."
|
||||
),
|
||||
|
||||
@@ -34,7 +34,7 @@ internal fun TimelineItemEventRowForDirectRoomPreview() = ElementPreview {
|
||||
ATimelineItemEventRow(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = it,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
content = aTimelineItemTextContent(
|
||||
body = "A long text which will be displayed on several lines and" +
|
||||
" hopefully can be manually adjusted to test different behaviors."
|
||||
),
|
||||
|
||||
@@ -41,6 +41,7 @@ internal fun TimelineItemEventRowTimestampPreview(
|
||||
event = event.copy(
|
||||
content = oldContent.copy(
|
||||
body = str,
|
||||
pillifiedBody = str,
|
||||
),
|
||||
reactionsState = aTimelineItemReactions(count = 0),
|
||||
),
|
||||
|
||||
@@ -32,7 +32,7 @@ internal fun TimelineItemEventRowWithManyReactionsPreview() = ElementPreview {
|
||||
ATimelineItemEventRow(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = isMine,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
content = aTimelineItemTextContent(
|
||||
body = "A couple of multi-line messages with many reactions attached." +
|
||||
" One sent by me and another from someone else."
|
||||
),
|
||||
|
||||
@@ -41,9 +41,7 @@ internal fun TimelineItemEventRowWithRRPreview(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = false,
|
||||
sendState = null,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
body = "A message from someone else"
|
||||
),
|
||||
content = aTimelineItemTextContent(body = "A message from someone else"),
|
||||
timelineItemReactions = aTimelineItemReactions(count = 0),
|
||||
readReceiptState = TimelineItemReadReceipts(state.receipts),
|
||||
),
|
||||
@@ -55,9 +53,7 @@ internal fun TimelineItemEventRowWithRRPreview(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = true,
|
||||
sendState = state.sendState,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
body = "A message from me"
|
||||
),
|
||||
content = aTimelineItemTextContent(body = "A message from me"),
|
||||
timelineItemReactions = aTimelineItemReactions(count = 0),
|
||||
readReceiptState = TimelineItemReadReceipts(state.receipts),
|
||||
),
|
||||
@@ -69,9 +65,7 @@ internal fun TimelineItemEventRowWithRRPreview(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = true,
|
||||
sendState = state.sendState,
|
||||
content = aTimelineItemTextContent().copy(
|
||||
body = "A last message from me"
|
||||
),
|
||||
content = aTimelineItemTextContent(body = "A last message from me"),
|
||||
timelineItemReactions = aTimelineItemReactions(count = 0),
|
||||
readReceiptState = TimelineItemReadReceipts(state.receipts),
|
||||
),
|
||||
|
||||
@@ -48,9 +48,7 @@ internal fun TimelineItemEventRowWithReplyContentToPreview(
|
||||
event = aTimelineItemEvent(
|
||||
isMine = it,
|
||||
timelineItemReactions = aTimelineItemReactions(count = 0),
|
||||
content = aTimelineItemTextContent().copy(
|
||||
body = "A reply."
|
||||
),
|
||||
content = aTimelineItemTextContent(body = "A reply."),
|
||||
inReplyTo = inReplyToDetails,
|
||||
displayNameAmbiguous = displayNameAmbiguous,
|
||||
groupPosition = TimelineItemGroupPosition.First,
|
||||
|
||||
@@ -77,14 +77,13 @@ internal fun getTextWithResolvedMentions(content: TimelineItemTextBasedContent):
|
||||
val userProfileCache = LocalRoomMemberProfilesCache.current
|
||||
val lastCacheUpdate by userProfileCache.lastCacheUpdate.collectAsState()
|
||||
val mentionSpanTheme = LocalMentionSpanTheme.current
|
||||
val formattedBody = remember(content.formattedBody, mentionSpanTheme, lastCacheUpdate) {
|
||||
content.formattedBody?.let { formattedBody ->
|
||||
updateMentionSpans(formattedBody, userProfileCache)
|
||||
mentionSpanTheme.updateMentionStyles(formattedBody)
|
||||
formattedBody
|
||||
}
|
||||
val formattedBody = content.formattedBody ?: content.pillifiedBody
|
||||
val textWithMentions = remember(formattedBody, mentionSpanTheme, lastCacheUpdate) {
|
||||
updateMentionSpans(formattedBody, userProfileCache)
|
||||
mentionSpanTheme.updateMentionStyles(formattedBody)
|
||||
formattedBody
|
||||
}
|
||||
return SpannableString(formattedBody ?: content.body)
|
||||
return SpannableString(textWithMentions)
|
||||
}
|
||||
|
||||
private fun updateMentionSpans(text: CharSequence, cache: RoomMemberProfilesCache): Boolean {
|
||||
|
||||
@@ -36,6 +36,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextContent
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVoiceContent
|
||||
import io.element.android.features.messages.impl.utils.TextPillificationHelper
|
||||
import io.element.android.libraries.androidutils.filesize.FileSizeFormatter
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlagService
|
||||
@@ -69,6 +70,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
|
||||
private val featureFlagService: FeatureFlagService,
|
||||
private val htmlConverterProvider: HtmlConverterProvider,
|
||||
private val permalinkParser: PermalinkParser,
|
||||
private val textPillificationHelper: TextPillificationHelper,
|
||||
) {
|
||||
suspend fun create(
|
||||
content: MessageContent,
|
||||
@@ -126,6 +128,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
|
||||
val body = messageType.body.trimEnd()
|
||||
TimelineItemTextContent(
|
||||
body = body,
|
||||
pillifiedBody = textPillificationHelper.pillify(body),
|
||||
htmlDocument = null,
|
||||
plainText = body,
|
||||
formattedBody = null,
|
||||
@@ -215,6 +218,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
|
||||
val body = messageType.body.trimEnd()
|
||||
TimelineItemTextContent(
|
||||
body = body,
|
||||
pillifiedBody = textPillificationHelper.pillify(body),
|
||||
htmlDocument = messageType.formatted?.toHtmlDocument(permalinkParser = permalinkParser),
|
||||
formattedBody = parseHtml(messageType.formatted) ?: body.withLinks(),
|
||||
isEdited = content.isEdited,
|
||||
@@ -224,6 +228,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
|
||||
val body = messageType.body.trimEnd()
|
||||
TimelineItemTextContent(
|
||||
body = body,
|
||||
pillifiedBody = textPillificationHelper.pillify(body),
|
||||
htmlDocument = null,
|
||||
formattedBody = body.withLinks(),
|
||||
isEdited = content.isEdited,
|
||||
|
||||
@@ -21,6 +21,7 @@ import org.jsoup.nodes.Document
|
||||
|
||||
data class TimelineItemEmoteContent(
|
||||
override val body: String,
|
||||
override val pillifiedBody: CharSequence = body,
|
||||
override val htmlDocument: Document?,
|
||||
override val plainText: String = htmlDocument?.toPlainText() ?: body,
|
||||
override val formattedBody: CharSequence?,
|
||||
|
||||
@@ -84,8 +84,12 @@ fun aTimelineItemNoticeContent() = TimelineItemNoticeContent(
|
||||
|
||||
fun aTimelineItemRedactedContent() = TimelineItemRedactedContent
|
||||
|
||||
fun aTimelineItemTextContent() = TimelineItemTextContent(
|
||||
body = "Text",
|
||||
fun aTimelineItemTextContent(
|
||||
body: String = "Text",
|
||||
pillifiedBody: CharSequence = body,
|
||||
) = TimelineItemTextContent(
|
||||
body = body,
|
||||
pillifiedBody = pillifiedBody,
|
||||
htmlDocument = null,
|
||||
formattedBody = null,
|
||||
isEdited = false,
|
||||
|
||||
@@ -21,6 +21,7 @@ import org.jsoup.nodes.Document
|
||||
|
||||
data class TimelineItemNoticeContent(
|
||||
override val body: String,
|
||||
override val pillifiedBody: CharSequence = body,
|
||||
override val htmlDocument: Document?,
|
||||
override val plainText: String = htmlDocument?.toPlainText() ?: body,
|
||||
override val formattedBody: CharSequence?,
|
||||
|
||||
@@ -19,13 +19,30 @@ package io.element.android.features.messages.impl.timeline.model.event
|
||||
import androidx.compose.runtime.Immutable
|
||||
import org.jsoup.nodes.Document
|
||||
|
||||
/**
|
||||
* Represents a text based content of a timeline item event (a message, a notice, an emote event...).
|
||||
*/
|
||||
@Immutable
|
||||
sealed interface TimelineItemTextBasedContent : TimelineItemEventContent {
|
||||
/** The raw body of the event, in Markdown format. */
|
||||
val body: String
|
||||
|
||||
/** The body of the event, with mentions replaced by their pillified version. */
|
||||
val pillifiedBody: CharSequence
|
||||
|
||||
/** The parsed HTML DOM of the formatted event body. */
|
||||
val htmlDocument: Document?
|
||||
|
||||
/** The formatted body of the event, already parsed and with the DOM translated to Android spans. */
|
||||
val formattedBody: CharSequence?
|
||||
|
||||
/** The plain text version of the event body. This is the Markdown version without actual Markdown formatting. */
|
||||
val plainText: String
|
||||
|
||||
/** Whether the event has been edited. */
|
||||
val isEdited: Boolean
|
||||
|
||||
/** The raw HTML body of the event. */
|
||||
val htmlBody: String?
|
||||
get() = htmlDocument?.body()?.html()
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ import org.jsoup.nodes.Document
|
||||
|
||||
data class TimelineItemTextContent(
|
||||
override val body: String,
|
||||
override val pillifiedBody: CharSequence = body,
|
||||
override val htmlDocument: Document?,
|
||||
override val plainText: String = htmlDocument?.toPlainText() ?: body,
|
||||
override val formattedBody: CharSequence?,
|
||||
|
||||
@@ -19,6 +19,8 @@ package io.element.android.features.messages.impl.utils
|
||||
import android.text.Spannable
|
||||
import android.text.SpannableStringBuilder
|
||||
import androidx.core.text.getSpans
|
||||
import com.squareup.anvil.annotations.ContributesBinding
|
||||
import io.element.android.libraries.di.RoomScope
|
||||
import io.element.android.libraries.matrix.api.core.MatrixPatternType
|
||||
import io.element.android.libraries.matrix.api.core.MatrixPatterns
|
||||
import io.element.android.libraries.matrix.api.core.RoomAlias
|
||||
@@ -30,14 +32,19 @@ import io.element.android.libraries.textcomposer.mentions.MentionSpan
|
||||
import io.element.android.libraries.textcomposer.mentions.MentionSpanProvider
|
||||
import javax.inject.Inject
|
||||
|
||||
class TextPillificationHelper @Inject constructor(
|
||||
interface TextPillificationHelper {
|
||||
fun pillify(text: CharSequence): CharSequence
|
||||
}
|
||||
|
||||
@ContributesBinding(RoomScope::class)
|
||||
class DefaultTextPillificationHelper @Inject constructor(
|
||||
private val mentionSpanProvider: MentionSpanProvider,
|
||||
private val permalinkBuilder: PermalinkBuilder,
|
||||
private val permalinkParser: PermalinkParser,
|
||||
private val roomMemberProfilesCache: RoomMemberProfilesCache,
|
||||
) {
|
||||
) : TextPillificationHelper {
|
||||
@Suppress("LoopWithTooManyJumpStatements")
|
||||
fun pillify(text: CharSequence): CharSequence {
|
||||
override fun pillify(text: CharSequence): CharSequence {
|
||||
val matches = MatrixPatterns.findPatterns(text, permalinkParser).sortedByDescending { it.end }
|
||||
if (matches.isEmpty()) return text
|
||||
|
||||
|
||||
@@ -44,7 +44,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
|
||||
import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemPollContent
|
||||
import io.element.android.features.messages.impl.typing.TypingNotificationPresenter
|
||||
import io.element.android.features.messages.impl.utils.TextPillificationHelper
|
||||
import io.element.android.features.messages.impl.utils.FakeTextPillificationHelper
|
||||
import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerPlayer
|
||||
import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerPresenter
|
||||
import io.element.android.features.messages.impl.voicemessages.timeline.FakeRedactedVoiceMessageManager
|
||||
@@ -787,7 +787,7 @@ class MessagesPresenterTest {
|
||||
timelineController = TimelineController(matrixRoom),
|
||||
draftService = FakeComposerDraftService(),
|
||||
mentionSpanProvider = mentionSpanProvider,
|
||||
pillificationHelper = TextPillificationHelper(mentionSpanProvider, FakePermalinkBuilder(), FakePermalinkParser(), RoomMemberProfilesCache()),
|
||||
pillificationHelper = FakeTextPillificationHelper(),
|
||||
roomMemberProfilesCache = RoomMemberProfilesCache(),
|
||||
).apply {
|
||||
showTextFormatting = true
|
||||
|
||||
@@ -33,6 +33,7 @@ import io.element.android.features.messages.impl.timeline.factories.event.Timeli
|
||||
import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemDaySeparatorFactory
|
||||
import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory
|
||||
import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
|
||||
import io.element.android.features.messages.impl.utils.FakeTextPillificationHelper
|
||||
import io.element.android.features.messages.test.timeline.FakeHtmlConverterProvider
|
||||
import io.element.android.features.poll.test.pollcontent.FakePollContentStateFactory
|
||||
import io.element.android.libraries.androidutils.filesize.FakeFileSizeFormatter
|
||||
@@ -62,6 +63,7 @@ internal fun TestScope.aTimelineItemsFactory(
|
||||
featureFlagService = FakeFeatureFlagService(),
|
||||
htmlConverterProvider = FakeHtmlConverterProvider(),
|
||||
permalinkParser = FakePermalinkParser(),
|
||||
textPillificationHelper = FakeTextPillificationHelper(),
|
||||
),
|
||||
redactedMessageFactory = TimelineItemContentRedactedFactory(),
|
||||
stickerFactory = TimelineItemContentStickerFactory(
|
||||
|
||||
@@ -35,6 +35,7 @@ import io.element.android.features.messages.impl.messagecomposer.MessageComposer
|
||||
import io.element.android.features.messages.impl.messagecomposer.MessageComposerPresenter
|
||||
import io.element.android.features.messages.impl.messagecomposer.MessageComposerState
|
||||
import io.element.android.features.messages.impl.timeline.TimelineController
|
||||
import io.element.android.features.messages.impl.utils.FakeTextPillificationHelper
|
||||
import io.element.android.features.messages.impl.utils.TextPillificationHelper
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes
|
||||
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
|
||||
@@ -1362,12 +1363,7 @@ class MessageComposerPresenterTest {
|
||||
permalinkParser: PermalinkParser = FakePermalinkParser(),
|
||||
mentionSpanProvider: MentionSpanProvider = MentionSpanProvider(permalinkParser),
|
||||
roomMemberProfilesCache: RoomMemberProfilesCache = RoomMemberProfilesCache(),
|
||||
textPillificationHelper: TextPillificationHelper = TextPillificationHelper(
|
||||
mentionSpanProvider = mentionSpanProvider,
|
||||
permalinkBuilder = permalinkBuilder,
|
||||
permalinkParser = permalinkParser,
|
||||
roomMemberProfilesCache = roomMemberProfilesCache,
|
||||
),
|
||||
textPillificationHelper: TextPillificationHelper = FakeTextPillificationHelper(),
|
||||
isRichTextEditorEnabled: Boolean = true,
|
||||
draftService: ComposerDraftService = FakeComposerDraftService(),
|
||||
) = MessageComposerPresenter(
|
||||
|
||||
@@ -35,6 +35,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextContent
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
|
||||
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVoiceContent
|
||||
import io.element.android.features.messages.impl.utils.FakeTextPillificationHelper
|
||||
import io.element.android.features.messages.test.timeline.FakeHtmlConverterProvider
|
||||
import io.element.android.libraries.androidutils.filesize.FakeFileSizeFormatter
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes
|
||||
@@ -697,6 +698,7 @@ class TimelineItemContentMessageFactoryTest {
|
||||
featureFlagService = featureFlagService,
|
||||
htmlConverterProvider = FakeHtmlConverterProvider(htmlConverterTransform),
|
||||
permalinkParser = permalinkParser,
|
||||
textPillificationHelper = FakeTextPillificationHelper(),
|
||||
)
|
||||
|
||||
private fun createStickerContent(
|
||||
|
||||
@@ -35,7 +35,7 @@ import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class TextPillificationHelperTest {
|
||||
class DefaultTextPillificationHelperTest {
|
||||
@Test
|
||||
fun `pillify - adds pills for user ids`() {
|
||||
val text = "A @user:server.com"
|
||||
@@ -119,7 +119,7 @@ class TextPillificationHelperTest {
|
||||
permalinkBuilder: FakePermalinkBuilder = FakePermalinkBuilder(),
|
||||
mentionSpanProvider: MentionSpanProvider = MentionSpanProvider(permalinkparser),
|
||||
roomMemberProfilesCache: RoomMemberProfilesCache = RoomMemberProfilesCache(),
|
||||
) = TextPillificationHelper(
|
||||
) = DefaultTextPillificationHelper(
|
||||
mentionSpanProvider = mentionSpanProvider,
|
||||
permalinkBuilder = permalinkBuilder,
|
||||
permalinkParser = permalinkparser,
|
||||
@@ -0,0 +1,25 @@
|
||||
/*
|
||||
* Copyright (c) 2024 New Vector Ltd
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package io.element.android.features.messages.impl.utils
|
||||
|
||||
class FakeTextPillificationHelper(
|
||||
private val pillifyLambda: (CharSequence) -> CharSequence = { it }
|
||||
) : TextPillificationHelper {
|
||||
override fun pillify(text: CharSequence): CharSequence {
|
||||
return pillifyLambda(text)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user