diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/LocalTimelineItemPresenterFactories.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/LocalTimelineItemPresenterFactories.kt new file mode 100644 index 0000000000..d93bee7817 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/LocalTimelineItemPresenterFactories.kt @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2023 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 io.element.android.features.messages.impl.timeline.di + +import androidx.compose.runtime.staticCompositionLocalOf + +/** + * Provides a [TimelineItemPresenterFactories] to the composition. + */ +val LocalTimelineItemPresenterFactories = staticCompositionLocalOf { + TimelineItemPresenterFactories(emptyMap()) +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/TimelineItemPresenterFactories.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/TimelineItemPresenterFactories.kt index 0574f7e903..e8997f5a1c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/TimelineItemPresenterFactories.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/TimelineItemPresenterFactories.kt @@ -18,13 +18,13 @@ package io.element.android.features.messages.impl.timeline.di import androidx.compose.runtime.Composable import androidx.compose.runtime.remember -import androidx.compose.runtime.staticCompositionLocalOf import com.squareup.anvil.annotations.ContributesTo import dagger.Module import dagger.multibindings.Multibinds import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.di.SingleIn import javax.inject.Inject /** @@ -40,38 +40,60 @@ interface TimelineItemPresenterFactoriesModule { } /** - * Wrapper around the [TimelineItemPresenterFactory] map multi binding. + * Room level caching layer for the [TimelineItemPresenterFactory] instances. * - * Its only purpose is to provide a nicer type name than: - * `@JvmSuppressWildcards Map, TimelineItemPresenterFactory<*, *>>`. - * - * A typealias would have been better but typealiases on Dagger types which use @JvmSuppressWildcards - * currently make Dagger crash. - * - * Request this type from Dagger to access the [TimelineItemPresenterFactory] map multibinding. + * It will cache the presenter instances in the room scope, so that they can be + * reused across recompositions of the timeline items that happen whenever an item + * goes out of the [LazyColumn] viewport. */ -data class TimelineItemPresenterFactories @Inject constructor( - val factories: @JvmSuppressWildcards Map, TimelineItemPresenterFactory<*, *>>, -) +@SingleIn(RoomScope::class) +class TimelineItemPresenterFactories @Inject constructor( + private val factories: @JvmSuppressWildcards Map, TimelineItemPresenterFactory<*, *>>, +) { + private val presenters: MutableMap> = mutableMapOf() -/** - * Provides a [TimelineItemPresenterFactories] to the composition. - */ -val LocalTimelineItemPresenterFactories = staticCompositionLocalOf { - TimelineItemPresenterFactories(emptyMap()) + /** + * Creates and caches a presenter for the given content. + * + * Will throw if the presenter is not found in the [TimelineItemPresenterFactory] map multi binding. + * + * @param C The [TimelineItemEventContent] subtype handled by this TimelineItem presenter. + * @param S The state type produced by this timeline item presenter. + * @param content The [TimelineItemEventContent] instance to create a presenter for. + * @param contentClass The class of [content]. + * @return An instance of a TimelineItem presenter that will be cached in the room scope. + */ + @Composable + fun rememberPresenter( + content: C, + contentClass: Class, + ): Presenter = remember(content) { + presenters[content]?.let { + @Suppress("UNCHECKED_CAST") + it as Presenter + } ?: factories.getValue(contentClass).let { + @Suppress("UNCHECKED_CAST") + (it as TimelineItemPresenterFactory).create(content).apply { + presenters[content] = this + } + } + } } /** - * Creates and remembers a presenter for the given content. + * Creates and caches a presenter for the given content. * * Will throw if the presenter is not found in the [TimelineItemPresenterFactory] map multi binding. + * + * @param C The [TimelineItemEventContent] subtype handled by this TimelineItem presenter. + * @param S The state type produced by this timeline item presenter. + * @param content The [TimelineItemEventContent] instance to create a presenter for. + * @return An instance of a TimelineItem presenter that will be cached in the room scope. */ @Composable -inline fun TimelineItemPresenterFactories.rememberPresenter( +inline fun TimelineItemPresenterFactories.rememberPresenter( content: C -): Presenter = remember(content) { - factories.getValue(C::class.java).let { - @Suppress("UNCHECKED_CAST") - (it as TimelineItemPresenterFactory).create(content) - } -} +): Presenter = rememberPresenter( + content = content, + contentClass = C::class.java +) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt index 1bbdedc22d..5df804c8ac 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt @@ -22,7 +22,6 @@ import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import com.squareup.anvil.annotations.ContributesTo import dagger.Binds import dagger.Module @@ -40,6 +39,7 @@ import io.element.android.libraries.architecture.runUpdatingState import io.element.android.libraries.di.RoomScope import io.element.android.libraries.ui.utils.time.formatShort import io.element.android.services.analytics.api.AnalyticsService +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import kotlin.time.Duration.Companion.milliseconds @@ -55,6 +55,7 @@ interface VoiceMessagePresenterModule { class VoiceMessagePresenter @AssistedInject constructor( voiceMessagePlayerFactory: VoiceMessagePlayer.Factory, private val analyticsService: AnalyticsService, + private val scope: CoroutineScope, @Assisted private val content: TimelineItemVoiceContent, ) : Presenter { @@ -70,13 +71,13 @@ class VoiceMessagePresenter @AssistedInject constructor( body = content.body, ) + private val play = mutableStateOf>(Async.Uninitialized) + private var progressCache: Float = 0f + @Composable override fun present(): VoiceMessageState { - val scope = rememberCoroutineScope() - val playerState by player.state.collectAsState(VoiceMessagePlayer.State(isPlaying = false, isMyMedia = false, currentPosition = 0L)) - val play = remember { mutableStateOf>(Async.Uninitialized) } val button by remember { derivedStateOf { @@ -90,7 +91,12 @@ class VoiceMessagePresenter @AssistedInject constructor( } } val progress by remember { - derivedStateOf { if (playerState.isMyMedia) playerState.currentPosition / content.duration.toMillis().toFloat() else 0f } + derivedStateOf { + if (playerState.isMyMedia) { + progressCache = playerState.currentPosition / content.duration.toMillis().toFloat() + } + progressCache + } } val time by remember { derivedStateOf { diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt index 0097245d27..26f4232531 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt @@ -31,6 +31,7 @@ import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMes import io.element.android.libraries.mediaplayer.test.FakeMediaPlayer import io.element.android.services.analytics.api.AnalyticsService import io.element.android.services.analytics.test.FakeAnalyticsService +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Test @@ -201,7 +202,7 @@ class VoiceMessagePresenterTest { } } -fun createVoiceMessagePresenter( +fun TestScope.createVoiceMessagePresenter( voiceMessageMediaRepo: VoiceMessageMediaRepo = FakeVoiceMessageMediaRepo(), analyticsService: AnalyticsService = FakeAnalyticsService(), content: TimelineItemVoiceContent = aTimelineItemVoiceContent(), @@ -217,5 +218,6 @@ fun createVoiceMessagePresenter( ) }, analyticsService = analyticsService, + scope = this, content = content, )