From 30115fa2b982451b63b4c574a869a9c128a24c7e Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 27 Jun 2022 15:43:21 +0300 Subject: [PATCH] Code review fixes. --- .../timeline/factory/MessageItemFactory.kt | 56 ++++++++++++- .../factory/MessageItemFactoryHelper.kt | 82 ------------------- .../factory/PollItemViewStateFactoryTest.kt | 20 ++--- 3 files changed, 57 insertions(+), 101 deletions(-) delete mode 100644 vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactoryHelper.kt diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt index 9b6186c878..853fef8bc8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt @@ -16,8 +16,13 @@ package im.vector.app.features.home.room.detail.timeline.factory +import android.text.Spannable import android.text.SpannableStringBuilder import android.text.Spanned +import android.text.TextPaint +import android.text.style.AbsoluteSizeSpan +import android.text.style.ClickableSpan +import android.text.style.ForegroundColorSpan import android.view.View import dagger.Lazy import im.vector.app.R @@ -30,7 +35,6 @@ import im.vector.app.core.time.Clock import im.vector.app.core.utils.DimensionConverter import im.vector.app.core.utils.containsOnlyEmojis import im.vector.app.features.home.room.detail.timeline.TimelineEventController -import im.vector.app.features.home.room.detail.timeline.factory.MessageItemFactoryHelper.annotateWithEdited import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.home.room.detail.timeline.helper.AvatarSizeProvider import im.vector.app.features.home.room.detail.timeline.helper.ContentDownloadStateTrackerBinder @@ -253,7 +257,7 @@ class MessageItemFactory @Inject constructor( question: String, callback: TimelineEventController.Callback?, ) = if (informationData.hasBeenEdited) { - annotateWithEdited(stringProvider, colorProvider, dimensionConverter, question, callback, informationData) + annotateWithEdited(question, callback, informationData) } else { question }.toEpoxyCharSequence() @@ -554,7 +558,7 @@ class MessageItemFactory @Inject constructor( return MessageTextItem_() .message( if (informationData.hasBeenEdited) { - annotateWithEdited(stringProvider, colorProvider, dimensionConverter, linkifiedBody, callback, informationData) + annotateWithEdited(linkifiedBody, callback, informationData) } else { linkifiedBody }.toEpoxyCharSequence() @@ -572,6 +576,50 @@ class MessageItemFactory @Inject constructor( .movementMethod(createLinkMovementMethod(callback)) } + private fun annotateWithEdited( + linkifiedBody: CharSequence, + callback: TimelineEventController.Callback?, + informationData: MessageInformationData, + ): Spannable { + val spannable = SpannableStringBuilder() + spannable.append(linkifiedBody) + val editedSuffix = stringProvider.getString(R.string.edited_suffix) + spannable.append(" ").append(editedSuffix) + val color = colorProvider.getColorFromAttribute(R.attr.vctr_content_secondary) + val editStart = spannable.lastIndexOf(editedSuffix) + val editEnd = editStart + editedSuffix.length + spannable.setSpan( + ForegroundColorSpan(color), + editStart, + editEnd, + Spanned.SPAN_INCLUSIVE_EXCLUSIVE + ) + + // Note: text size is set to 14sp + spannable.setSpan( + AbsoluteSizeSpan(dimensionConverter.spToPx(13)), + editStart, + editEnd, + Spanned.SPAN_INCLUSIVE_EXCLUSIVE + ) + + spannable.setSpan( + object : ClickableSpan() { + override fun onClick(widget: View) { + callback?.onEditedDecorationClicked(informationData) + } + + override fun updateDrawState(ds: TextPaint) { + // nop + } + }, + editStart, + editEnd, + Spanned.SPAN_INCLUSIVE_EXCLUSIVE + ) + return spannable + } + private fun buildNoticeMessageItem( messageContent: MessageNoticeContent, @Suppress("UNUSED_PARAMETER") @@ -618,7 +666,7 @@ class MessageItemFactory @Inject constructor( return MessageTextItem_() .message( if (informationData.hasBeenEdited) { - annotateWithEdited(stringProvider, colorProvider, dimensionConverter, message, callback, informationData) + annotateWithEdited(message, callback, informationData) } else { message }.toEpoxyCharSequence() diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactoryHelper.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactoryHelper.kt deleted file mode 100644 index 0c4c7238e7..0000000000 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactoryHelper.kt +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright (c) 2022 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.app.features.home.room.detail.timeline.factory - -import android.text.Spannable -import android.text.SpannableStringBuilder -import android.text.Spanned -import android.text.TextPaint -import android.text.style.AbsoluteSizeSpan -import android.text.style.ClickableSpan -import android.text.style.ForegroundColorSpan -import android.view.View -import im.vector.app.R -import im.vector.app.core.resources.ColorProvider -import im.vector.app.core.resources.StringProvider -import im.vector.app.core.utils.DimensionConverter -import im.vector.app.features.home.room.detail.timeline.TimelineEventController -import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData - -object MessageItemFactoryHelper { - - fun annotateWithEdited( - stringProvider: StringProvider, - colorProvider: ColorProvider, - dimensionConverter: DimensionConverter, - linkifiedBody: CharSequence, - callback: TimelineEventController.Callback?, - informationData: MessageInformationData, - ): Spannable { - val spannable = SpannableStringBuilder() - spannable.append(linkifiedBody) - val editedSuffix = stringProvider.getString(R.string.edited_suffix) - spannable.append(" ").append(editedSuffix) - val color = colorProvider.getColorFromAttribute(R.attr.vctr_content_secondary) - val editStart = spannable.lastIndexOf(editedSuffix) - val editEnd = editStart + editedSuffix.length - spannable.setSpan( - ForegroundColorSpan(color), - editStart, - editEnd, - Spanned.SPAN_INCLUSIVE_EXCLUSIVE - ) - - // Note: text size is set to 14sp - spannable.setSpan( - AbsoluteSizeSpan(dimensionConverter.spToPx(13)), - editStart, - editEnd, - Spanned.SPAN_INCLUSIVE_EXCLUSIVE - ) - - spannable.setSpan( - object : ClickableSpan() { - override fun onClick(widget: View) { - callback?.onEditedDecorationClicked(informationData) - } - - override fun updateDrawState(ds: TextPaint) { - // nop - } - }, - editStart, - editEnd, - Spanned.SPAN_INCLUSIVE_EXCLUSIVE - ) - return spannable - } -} diff --git a/vector/src/test/java/im/vector/app/features/home/room/detail/timeline/factory/PollItemViewStateFactoryTest.kt b/vector/src/test/java/im/vector/app/features/home/room/detail/timeline/factory/PollItemViewStateFactoryTest.kt index 1f6020e639..e519652503 100644 --- a/vector/src/test/java/im/vector/app/features/home/room/detail/timeline/factory/PollItemViewStateFactoryTest.kt +++ b/vector/src/test/java/im/vector/app/features/home/room/detail/timeline/factory/PollItemViewStateFactoryTest.kt @@ -16,7 +16,6 @@ package im.vector.app.features.home.room.detail.timeline.factory -import com.airbnb.mvrx.test.MvRxTestRule import im.vector.app.R import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData import im.vector.app.features.home.room.detail.timeline.item.PollOptionViewState @@ -26,11 +25,8 @@ import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryDat import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLayout import im.vector.app.features.poll.PollViewState import im.vector.app.test.fakes.FakeStringProvider -import kotlinx.coroutines.test.UnconfinedTestDispatcher -import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo import org.junit.Before -import org.junit.Rule import org.junit.Test import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent import org.matrix.android.sdk.api.session.room.model.message.PollAnswer @@ -83,12 +79,6 @@ private val A_POLL_CONTENT = MessagePollContent( class PollItemViewStateFactoryTest { - private val testDispatcher = UnconfinedTestDispatcher() - - @get:Rule - val mvRxTestRule = MvRxTestRule( - testDispatcher = testDispatcher // See https://github.com/airbnb/mavericks/issues/599 - ) private lateinit var pollItemViewStateFactory: PollItemViewStateFactory private val stringProvider = FakeStringProvider() @@ -102,7 +92,7 @@ class PollItemViewStateFactoryTest { } @Test - fun `given a sending poll state then poll is not votable and option states are PollSending`() = runTest { + fun `given a sending poll state then poll is not votable and option states are PollSending`() { val sendingPollInformationData = A_MESSAGE_INFORMATION_DATA.copy(sendState = SendState.SENDING) val pollViewState = pollItemViewStateFactory.create( pollContent = A_POLL_CONTENT, @@ -123,7 +113,7 @@ class PollItemViewStateFactoryTest { } @Test - fun `given a sent poll state when poll is closed then poll is not votable and option states are Ended`() = runTest { + fun `given a sent poll state when poll is closed then poll is not votable and option states are Ended`() { val closedPollSummary = A_POLL_RESPONSE_DATA.copy(isClosed = true) val closedPollInformationData = A_MESSAGE_INFORMATION_DATA.copy(pollResponseAggregatedSummary = closedPollSummary) @@ -149,7 +139,7 @@ class PollItemViewStateFactoryTest { } @Test - fun `given a sent poll when undisclosed poll type is selected then poll is votable and option states are PollUndisclosed`() = runTest { + fun `given a sent poll when undisclosed poll type is selected then poll is votable and option states are PollUndisclosed`() { val pollViewState = pollItemViewStateFactory.create( pollContent = A_POLL_CONTENT, informationData = A_MESSAGE_INFORMATION_DATA, @@ -170,7 +160,7 @@ class PollItemViewStateFactoryTest { } @Test - fun `given a sent poll when my vote exists then poll is still votable and options states are PollVoted`() = runTest { + fun `given a sent poll when my vote exists then poll is still votable and options states are PollVoted`() { val votedPollData = A_POLL_RESPONSE_DATA.copy( totalVotes = 1, myVote = A_POLL_OPTION_IDS[0], @@ -205,7 +195,7 @@ class PollItemViewStateFactoryTest { } @Test - fun `given a sent poll when poll type is disclosed then poll is votable and option view states are PollReady`() = runTest { + fun `given a sent poll when poll type is disclosed then poll is votable and option view states are PollReady`() { val disclosedPollContent = A_POLL_CONTENT.copy( unstablePollCreationInfo = A_POLL_CONTENT.getBestPollCreationInfo()?.copy( kind = PollType.DISCLOSED_UNSTABLE