From 62bae6708071b9fcb84d23838d1b74657478f296 Mon Sep 17 00:00:00 2001 From: Valere Date: Sat, 16 Nov 2019 12:32:50 +0100 Subject: [PATCH] Code review --- .../api/session/room/send/SendService.kt | 1 + .../api/session/room/send/TextPillsUtils.kt | 67 +++++++++++++++++++ .../room/send/LocalEchoEventFactory.kt | 31 +-------- .../home/room/detail/RoomDetailFragment.kt | 27 +++++--- .../room/detail/composer/TextComposerView.kt | 5 +- .../riotx/features/html/MxLinkTagHandler.kt | 3 +- .../riotx/features/html/PillImageSpan.kt | 10 +-- 7 files changed, 96 insertions(+), 48 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/TextPillsUtils.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/SendService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/SendService.kt index e45069bcff..bdae5eaaa6 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/SendService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/SendService.kt @@ -42,6 +42,7 @@ interface SendService { * Method to send a text message with a formatted body. * @param text the text message to send * @param formattedText The formatted body using MessageType#FORMAT_MATRIX_HTML + * @param msgType the message type: MessageType.MSGTYPE_TEXT (default) or MessageType.MSGTYPE_EMOTE * @return a [Cancelable] */ fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String = MessageType.MSGTYPE_TEXT): Cancelable diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/TextPillsUtils.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/TextPillsUtils.kt new file mode 100644 index 0000000000..b50d5dd4a6 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/send/TextPillsUtils.kt @@ -0,0 +1,67 @@ +/* + * Copyright 2019 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 + * + * http://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 im.vector.matrix.android.api.session.room.send + +import android.text.SpannableString + +/** + * Utility class to detect special span in CharSequence and turn them into + * formatted text to send them as a Matrix messages. + * + * For now only support UserMentionSpans (TODO rooms, room aliases, etc...) + */ +object TextPillsUtils { + + private const val MENTION_SPAN_TO_HTML_TEMPLATE = "%2\$s" + + private const val MENTION_SPAN_TO_MD_TEMPLATE = "[%2\$s](https://matrix.to/#/%1\$s)" + + /** + * Detects if transformable spans are present in the text. + * @return the transformed String or null if no Span found + */ + fun processSpecialSpansToHtml(text: CharSequence): String? { + return transformPills(text, MENTION_SPAN_TO_HTML_TEMPLATE) + } + + /** + * Detects if transformable spans are present in the text. + * @return the transformed String or null if no Span found + */ + fun processSpecialSpansToMarkdown(text: CharSequence): String? { + return transformPills(text, MENTION_SPAN_TO_MD_TEMPLATE) + } + + private fun transformPills(text: CharSequence, template: String): String? { + val spannableString = SpannableString.valueOf(text) + val pills = spannableString + ?.getSpans(0, text.length, UserMentionSpan::class.java) + ?.takeIf { it.isNotEmpty() } + ?: return null + + return buildString { + var currIndex = 0 + pills.forEachIndexed { _, urlSpan -> + val start = spannableString.getSpanStart(urlSpan) + val end = spannableString.getSpanEnd(urlSpan) + // We want to replace with the pill with a html link + append(text, currIndex, start) + append(String.format(template, urlSpan.userId, urlSpan.displayName)) + currIndex = end + } + } + } +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt index 3f47b8fff3..becba6bffe 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt @@ -17,7 +17,6 @@ package im.vector.matrix.android.internal.session.room.send import android.media.MediaMetadataRetriever -import android.text.SpannableString import androidx.exifinterface.media.ExifInterface import com.zhuinden.monarchy.Monarchy import im.vector.matrix.android.R @@ -29,7 +28,7 @@ import im.vector.matrix.android.api.session.room.model.relation.ReactionContent import im.vector.matrix.android.api.session.room.model.relation.ReactionInfo import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent import im.vector.matrix.android.api.session.room.model.relation.ReplyToContent -import im.vector.matrix.android.api.session.room.send.UserMentionSpan +import im.vector.matrix.android.api.session.room.send.TextPillsUtils import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent import im.vector.matrix.android.internal.database.helper.addSendingEvent @@ -70,7 +69,7 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use private fun createTextContent(text: CharSequence, autoMarkdown: Boolean): TextContent { if (autoMarkdown) { - val source = transformPills(text, "[%2\$s](https://matrix.to/#/%1\$s)") + val source = TextPillsUtils.processSpecialSpansToMarkdown(text) ?: text.toString() val document = parser.parse(source) val htmlText = renderer.render(document) @@ -80,7 +79,7 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use } } else { // Try to detect pills - transformPills(text, "%2\$s")?.let { + TextPillsUtils.processSpecialSpansToHtml(text)?.let { return TextContent(text.toString(), it) } } @@ -88,30 +87,6 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use return TextContent(text.toString()) } - private fun transformPills(text: CharSequence, - template: String) - : String? { - val bufSB = StringBuffer() - var currIndex = 0 - SpannableString.valueOf(text).let { - val pills = it.getSpans(0, text.length, UserMentionSpan::class.java) - if (pills.isNotEmpty()) { - pills.forEachIndexed { _, urlSpan -> - val start = it.getSpanStart(urlSpan) - val end = it.getSpanEnd(urlSpan) - // We want to replace with the pill with a html link - bufSB.append(text, currIndex, start) - bufSB.append(String.format(template, urlSpan.userId, urlSpan.displayName)) - currIndex = end - } - bufSB.append(text, currIndex, text.length) - return bufSB.toString() - } else { - return null - } - } - } - private fun isFormattedTextPertinent(text: String, htmlText: String?) = text != htmlText && htmlText != "

${text.trim()}

\n" diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt index 6186bd1ac1..b6b6270602 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt @@ -16,7 +16,6 @@ package im.vector.riotx.features.home.room.detail -import android.annotation.SuppressLint import android.app.Activity.RESULT_OK import android.content.Context import android.content.DialogInterface @@ -61,6 +60,7 @@ import im.vector.matrix.android.api.session.content.ContentAttachmentData import im.vector.matrix.android.api.session.events.model.Event import im.vector.matrix.android.api.session.events.model.LocalEcho import im.vector.matrix.android.api.session.room.model.Membership +import im.vector.matrix.android.api.session.room.model.RoomMember import im.vector.matrix.android.api.session.room.model.message.* import im.vector.matrix.android.api.session.room.send.SendState import im.vector.matrix.android.api.session.room.timeline.Timeline @@ -406,7 +406,8 @@ class RoomDetailFragment @Inject constructor( composerLayout.composerRelatedMessageActionIcon.setImageDrawable(ContextCompat.getDrawable(requireContext(), iconRes)) composerLayout.sendButton.setContentDescription(getString(descriptionRes)) - avatarRenderer.render(event.senderAvatar, event.root.senderId ?: "", event.getDisambiguatedDisplayName(), composerLayout.composerRelatedMessageAvatar) + avatarRenderer.render(event.senderAvatar, event.root.senderId + ?: "", event.getDisambiguatedDisplayName(), composerLayout.composerRelatedMessageAvatar) composerLayout.expand { // need to do it here also when not using quick reply focusComposerAndShowKeyboard() @@ -419,7 +420,8 @@ class RoomDetailFragment @Inject constructor( if (text != composerLayout.composerEditText.text.toString()) { // Ignore update to avoid saving a draft composerLayout.composerEditText.setText(text) - composerLayout.composerEditText.setSelection(composerLayout.composerEditText.text?.length ?: 0) + composerLayout.composerEditText.setSelection(composerLayout.composerEditText.text?.length + ?: 0) } } @@ -589,7 +591,8 @@ class RoomDetailFragment @Inject constructor( // Add the span val user = session.getUser(item.userId) - val span = PillImageSpan(glideRequests, avatarRenderer, requireContext(), item.userId, user) + val span = PillImageSpan(glideRequests, avatarRenderer, requireContext(), item.userId, user?.displayName + ?: item.userId, user?.avatarUrl) span.bind(composerLayout.composerEditText) editable.setSpan(span, startIndex, startIndex + displayName.length, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) @@ -976,10 +979,10 @@ class RoomDetailFragment @Inject constructor( vectorBaseActivity.notImplemented("Click on user avatar") } - @SuppressLint("SetTextI18n") override fun onMemberNameClicked(informationData: MessageInformationData) { - session.getUser(informationData.senderId)?.let { - insertUserDisplayNameInTextEditor(it) + val userId = informationData.senderId + roomDetailViewModel.getMember(userId)?.let { + insertUserDisplayNameInTextEditor(userId, it) } } @@ -1169,9 +1172,9 @@ class RoomDetailFragment @Inject constructor( * @param text the text to insert. */ // TODO legacy, refactor - private fun insertUserDisplayNameInTextEditor(member: User) { + private fun insertUserDisplayNameInTextEditor(userId: String, memberInfo: RoomMember) { // TODO move logic outside of fragment - val text = member.displayName + val text = memberInfo.displayName if (null != text) { // var vibrate = false @@ -1195,7 +1198,8 @@ class RoomDetailFragment @Inject constructor( SpannableStringBuilder().apply { append(sanitizeDisplayName) setSpan( - PillImageSpan(glideRequests, avatarRenderer, requireContext(), member.userId, member), + PillImageSpan(glideRequests, avatarRenderer, requireContext(), userId, memberInfo.displayName + ?: userId, memberInfo.avatarUrl), 0, sanitizeDisplayName.length, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE @@ -1208,7 +1212,8 @@ class RoomDetailFragment @Inject constructor( SpannableStringBuilder().apply { append(sanitizeDisplayName) setSpan( - PillImageSpan(glideRequests, avatarRenderer, requireContext(), member.userId, member), + PillImageSpan(glideRequests, avatarRenderer, requireContext(), userId, memberInfo.displayName + ?: userId, memberInfo.avatarUrl), 0, sanitizeDisplayName.length, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/composer/TextComposerView.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/composer/TextComposerView.kt index 63e74d6f32..f3a29b8dc0 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/composer/TextComposerView.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/composer/TextComposerView.kt @@ -88,7 +88,10 @@ class TextComposerView @JvmOverloads constructor(context: Context, attrs: Attrib sendButton.setOnClickListener { val textMessage = text?.toSpannable() - callback?.onSendMessage(textMessage ?: "") + callback?.onSendMessage( + textMessage + ?: "" + ) } attachmentButton.setOnClickListener { diff --git a/vector/src/main/java/im/vector/riotx/features/html/MxLinkTagHandler.kt b/vector/src/main/java/im/vector/riotx/features/html/MxLinkTagHandler.kt index fdcbb12cd7..ecbf0da415 100644 --- a/vector/src/main/java/im/vector/riotx/features/html/MxLinkTagHandler.kt +++ b/vector/src/main/java/im/vector/riotx/features/html/MxLinkTagHandler.kt @@ -41,7 +41,8 @@ class MxLinkTagHandler(private val glideRequests: GlideRequests, when (permalinkData) { is PermalinkData.UserLink -> { val user = sessionHolder.getSafeActiveSession()?.getUser(permalinkData.userId) - val span = PillImageSpan(glideRequests, avatarRenderer, context, permalinkData.userId, user) + val span = PillImageSpan(glideRequests, avatarRenderer, context, permalinkData.userId, user?.displayName + ?: permalinkData.userId, user?.avatarUrl) SpannableBuilder.setSpans( visitor.builder(), span, diff --git a/vector/src/main/java/im/vector/riotx/features/html/PillImageSpan.kt b/vector/src/main/java/im/vector/riotx/features/html/PillImageSpan.kt index 414cd71de7..a192c71961 100644 --- a/vector/src/main/java/im/vector/riotx/features/html/PillImageSpan.kt +++ b/vector/src/main/java/im/vector/riotx/features/html/PillImageSpan.kt @@ -29,7 +29,6 @@ import com.bumptech.glide.request.target.SimpleTarget import com.bumptech.glide.request.transition.Transition import com.google.android.material.chip.ChipDrawable import im.vector.matrix.android.api.session.room.send.UserMentionSpan -import im.vector.matrix.android.api.session.user.model.User import im.vector.riotx.R import im.vector.riotx.core.glide.GlideRequests import im.vector.riotx.features.home.AvatarRenderer @@ -44,11 +43,8 @@ class PillImageSpan(private val glideRequests: GlideRequests, private val avatarRenderer: AvatarRenderer, private val context: Context, override val userId: String, - private val user: User?) : ReplacementSpan(), UserMentionSpan { - - override val displayName by lazy { - if (user?.displayName.isNullOrEmpty()) userId else user?.displayName!! - } + override val displayName: String, + private val avatarUrl: String?) : ReplacementSpan(), UserMentionSpan { private val pillDrawable = createChipDrawable() private val target = PillImageSpanTarget(this) @@ -57,7 +53,7 @@ class PillImageSpan(private val glideRequests: GlideRequests, @UiThread fun bind(textView: TextView) { tv = WeakReference(textView) - avatarRenderer.render(context, glideRequests, user?.avatarUrl, userId, displayName, target) + avatarRenderer.render(context, glideRequests, avatarUrl, userId, displayName, target) } // ReplacementSpan *****************************************************************************