Code review

This commit is contained in:
Valere 2019-11-16 12:32:50 +01:00 committed by Benoit Marty
parent 2a4cdec020
commit 62bae67080
7 changed files with 96 additions and 48 deletions

View File

@ -42,6 +42,7 @@ interface SendService {
* Method to send a text message with a formatted body. * Method to send a text message with a formatted body.
* @param text the text message to send * @param text the text message to send
* @param formattedText The formatted body using MessageType#FORMAT_MATRIX_HTML * @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] * @return a [Cancelable]
*/ */
fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String = MessageType.MSGTYPE_TEXT): Cancelable fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String = MessageType.MSGTYPE_TEXT): Cancelable

View File

@ -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 = "<a href=\"https://matrix.to/#/%1\$s\">%2\$s</a>"
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
}
}
}
}

View File

@ -17,7 +17,6 @@
package im.vector.matrix.android.internal.session.room.send package im.vector.matrix.android.internal.session.room.send
import android.media.MediaMetadataRetriever import android.media.MediaMetadataRetriever
import android.text.SpannableString
import androidx.exifinterface.media.ExifInterface import androidx.exifinterface.media.ExifInterface
import com.zhuinden.monarchy.Monarchy import com.zhuinden.monarchy.Monarchy
import im.vector.matrix.android.R 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.ReactionInfo
import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent 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.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.TimelineEvent
import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent
import im.vector.matrix.android.internal.database.helper.addSendingEvent 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 { private fun createTextContent(text: CharSequence, autoMarkdown: Boolean): TextContent {
if (autoMarkdown) { if (autoMarkdown) {
val source = transformPills(text, "[%2\$s](https://matrix.to/#/%1\$s)") val source = TextPillsUtils.processSpecialSpansToMarkdown(text)
?: text.toString() ?: text.toString()
val document = parser.parse(source) val document = parser.parse(source)
val htmlText = renderer.render(document) val htmlText = renderer.render(document)
@ -80,7 +79,7 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use
} }
} else { } else {
// Try to detect pills // Try to detect pills
transformPills(text, "<a href=\"https://matrix.to/#/%1\$s\">%2\$s</a>")?.let { TextPillsUtils.processSpecialSpansToHtml(text)?.let {
return TextContent(text.toString(), it) return TextContent(text.toString(), it)
} }
} }
@ -88,30 +87,6 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use
return TextContent(text.toString()) 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?) = private fun isFormattedTextPertinent(text: String, htmlText: String?) =
text != htmlText && htmlText != "<p>${text.trim()}</p>\n" text != htmlText && htmlText != "<p>${text.trim()}</p>\n"

View File

@ -16,7 +16,6 @@
package im.vector.riotx.features.home.room.detail package im.vector.riotx.features.home.room.detail
import android.annotation.SuppressLint
import android.app.Activity.RESULT_OK import android.app.Activity.RESULT_OK
import android.content.Context import android.content.Context
import android.content.DialogInterface 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.Event
import im.vector.matrix.android.api.session.events.model.LocalEcho 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.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.model.message.*
import im.vector.matrix.android.api.session.room.send.SendState import im.vector.matrix.android.api.session.room.send.SendState
import im.vector.matrix.android.api.session.room.timeline.Timeline 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.composerRelatedMessageActionIcon.setImageDrawable(ContextCompat.getDrawable(requireContext(), iconRes))
composerLayout.sendButton.setContentDescription(getString(descriptionRes)) 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 { composerLayout.expand {
// need to do it here also when not using quick reply // need to do it here also when not using quick reply
focusComposerAndShowKeyboard() focusComposerAndShowKeyboard()
@ -419,7 +420,8 @@ class RoomDetailFragment @Inject constructor(
if (text != composerLayout.composerEditText.text.toString()) { if (text != composerLayout.composerEditText.text.toString()) {
// Ignore update to avoid saving a draft // Ignore update to avoid saving a draft
composerLayout.composerEditText.setText(text) 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 // Add the span
val user = session.getUser(item.userId) 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) span.bind(composerLayout.composerEditText)
editable.setSpan(span, startIndex, startIndex + displayName.length, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) 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") vectorBaseActivity.notImplemented("Click on user avatar")
} }
@SuppressLint("SetTextI18n")
override fun onMemberNameClicked(informationData: MessageInformationData) { override fun onMemberNameClicked(informationData: MessageInformationData) {
session.getUser(informationData.senderId)?.let { val userId = informationData.senderId
insertUserDisplayNameInTextEditor(it) roomDetailViewModel.getMember(userId)?.let {
insertUserDisplayNameInTextEditor(userId, it)
} }
} }
@ -1169,9 +1172,9 @@ class RoomDetailFragment @Inject constructor(
* @param text the text to insert. * @param text the text to insert.
*/ */
// TODO legacy, refactor // TODO legacy, refactor
private fun insertUserDisplayNameInTextEditor(member: User) { private fun insertUserDisplayNameInTextEditor(userId: String, memberInfo: RoomMember) {
// TODO move logic outside of fragment // TODO move logic outside of fragment
val text = member.displayName val text = memberInfo.displayName
if (null != text) { if (null != text) {
// var vibrate = false // var vibrate = false
@ -1195,7 +1198,8 @@ class RoomDetailFragment @Inject constructor(
SpannableStringBuilder().apply { SpannableStringBuilder().apply {
append(sanitizeDisplayName) append(sanitizeDisplayName)
setSpan( setSpan(
PillImageSpan(glideRequests, avatarRenderer, requireContext(), member.userId, member), PillImageSpan(glideRequests, avatarRenderer, requireContext(), userId, memberInfo.displayName
?: userId, memberInfo.avatarUrl),
0, 0,
sanitizeDisplayName.length, sanitizeDisplayName.length,
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
@ -1208,7 +1212,8 @@ class RoomDetailFragment @Inject constructor(
SpannableStringBuilder().apply { SpannableStringBuilder().apply {
append(sanitizeDisplayName) append(sanitizeDisplayName)
setSpan( setSpan(
PillImageSpan(glideRequests, avatarRenderer, requireContext(), member.userId, member), PillImageSpan(glideRequests, avatarRenderer, requireContext(), userId, memberInfo.displayName
?: userId, memberInfo.avatarUrl),
0, 0,
sanitizeDisplayName.length, sanitizeDisplayName.length,
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE Spannable.SPAN_EXCLUSIVE_EXCLUSIVE

View File

@ -88,7 +88,10 @@ class TextComposerView @JvmOverloads constructor(context: Context, attrs: Attrib
sendButton.setOnClickListener { sendButton.setOnClickListener {
val textMessage = text?.toSpannable() val textMessage = text?.toSpannable()
callback?.onSendMessage(textMessage ?: "") callback?.onSendMessage(
textMessage
?: ""
)
} }
attachmentButton.setOnClickListener { attachmentButton.setOnClickListener {

View File

@ -41,7 +41,8 @@ class MxLinkTagHandler(private val glideRequests: GlideRequests,
when (permalinkData) { when (permalinkData) {
is PermalinkData.UserLink -> { is PermalinkData.UserLink -> {
val user = sessionHolder.getSafeActiveSession()?.getUser(permalinkData.userId) 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( SpannableBuilder.setSpans(
visitor.builder(), visitor.builder(),
span, span,

View File

@ -29,7 +29,6 @@ import com.bumptech.glide.request.target.SimpleTarget
import com.bumptech.glide.request.transition.Transition import com.bumptech.glide.request.transition.Transition
import com.google.android.material.chip.ChipDrawable import com.google.android.material.chip.ChipDrawable
import im.vector.matrix.android.api.session.room.send.UserMentionSpan 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.R
import im.vector.riotx.core.glide.GlideRequests import im.vector.riotx.core.glide.GlideRequests
import im.vector.riotx.features.home.AvatarRenderer import im.vector.riotx.features.home.AvatarRenderer
@ -44,11 +43,8 @@ class PillImageSpan(private val glideRequests: GlideRequests,
private val avatarRenderer: AvatarRenderer, private val avatarRenderer: AvatarRenderer,
private val context: Context, private val context: Context,
override val userId: String, override val userId: String,
private val user: User?) : ReplacementSpan(), UserMentionSpan { override val displayName: String,
private val avatarUrl: String?) : ReplacementSpan(), UserMentionSpan {
override val displayName by lazy {
if (user?.displayName.isNullOrEmpty()) userId else user?.displayName!!
}
private val pillDrawable = createChipDrawable() private val pillDrawable = createChipDrawable()
private val target = PillImageSpanTarget(this) private val target = PillImageSpanTarget(this)
@ -57,7 +53,7 @@ class PillImageSpan(private val glideRequests: GlideRequests,
@UiThread @UiThread
fun bind(textView: TextView) { fun bind(textView: TextView) {
tv = WeakReference(textView) tv = WeakReference(textView)
avatarRenderer.render(context, glideRequests, user?.avatarUrl, userId, displayName, target) avatarRenderer.render(context, glideRequests, avatarUrl, userId, displayName, target)
} }
// ReplacementSpan ***************************************************************************** // ReplacementSpan *****************************************************************************