Merge pull request #3537 from element-hq/feature/fga/fix_image_viewer_glitch

Fix image viewer glitch

Author: ganfra

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt

@@ -33,9 +33,10 @@ import io.element.android.features.roomdetails.impl.notificationsettings.RoomNot
 import io.element.android.features.roomdetails.impl.rolesandpermissions.RolesAndPermissionsFlowNode
 import io.element.android.features.userprofile.shared.UserProfileNodeHelper
 import io.element.android.features.userprofile.shared.avatar.AvatarPreviewNode
-import io.element.android.libraries.architecture.BackstackView
+import io.element.android.libraries.architecture.BackstackWithOverlayBox
 import io.element.android.libraries.architecture.BaseFlowNode
 import io.element.android.libraries.architecture.createNode
+import io.element.android.libraries.architecture.overlay.operation.show
 import io.element.android.libraries.core.mimetype.MimeTypes
 import io.element.android.libraries.di.RoomScope
 import io.element.android.libraries.matrix.api.core.RoomId
@@ -125,7 +126,7 @@ class RoomDetailsFlowNode @AssistedInject constructor(
                     }
 
                     override fun openAvatarPreview(name: String, url: String) {
-                        backstack.push(NavTarget.AvatarPreview(name, url))
+                        overlay.show(NavTarget.AvatarPreview(name, url))
                     }
 
                     override fun openPollHistory() {
@@ -186,7 +187,7 @@ class RoomDetailsFlowNode @AssistedInject constructor(
             is NavTarget.RoomMemberDetails -> {
                 val callback = object : UserProfileNodeHelper.Callback {
                     override fun openAvatarPreview(username: String, avatarUrl: String) {
-                        backstack.push(NavTarget.AvatarPreview(username, avatarUrl))
+                        overlay.show(NavTarget.AvatarPreview(username, avatarUrl))
                     }
 
                     override fun onStartDM(roomId: RoomId) {
@@ -252,6 +253,6 @@ class RoomDetailsFlowNode @AssistedInject constructor(
 
     @Composable
     override fun View(modifier: Modifier) {
-        BackstackView()
+        BackstackWithOverlayBox(modifier)
     }
 }

libraries/mediaviewer/api/src/main/kotlin/io/element/android/libraries/mediaviewer/api/viewer/MediaViewerView.kt

@@ -40,6 +40,7 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
+import coil.compose.AsyncImage
 import io.element.android.compound.tokens.generated.CompoundIcons
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.core.mimetype.MimeTypes
@@ -66,9 +67,6 @@ import me.saket.telephoto.flick.FlickToDismiss
 import me.saket.telephoto.flick.FlickToDismissState
 import me.saket.telephoto.flick.rememberFlickToDismissState
 import me.saket.telephoto.zoomable.ZoomSpec
-import me.saket.telephoto.zoomable.ZoomableState
-import me.saket.telephoto.zoomable.coil.ZoomableAsyncImage
-import me.saket.telephoto.zoomable.rememberZoomableImageState
 import me.saket.telephoto.zoomable.rememberZoomableState
 import kotlin.time.Duration
 
@@ -181,7 +179,6 @@ private fun MediaViewerPage(
                     mediaInfo = state.mediaInfo,
                     thumbnailSource = state.thumbnailSource,
                     isVisible = showThumbnail,
-                    zoomableState = zoomableState
                 )
                 if (showError) {
                     ErrorView(
@@ -316,24 +313,18 @@ private fun ThumbnailView(
     thumbnailSource: MediaSource?,
     isVisible: Boolean,
     mediaInfo: MediaInfo,
-    zoomableState: ZoomableState,
     modifier: Modifier = Modifier,
 ) {
-    AnimatedVisibility(
-        visible = isVisible,
-        enter = fadeIn(),
-        exit = fadeOut()
+    Box(
+        modifier = modifier.fillMaxSize(),
+        contentAlignment = Alignment.Center
     ) {
-        Box(
-            modifier = modifier.fillMaxSize(),
-            contentAlignment = Alignment.Center
-        ) {
+        if (isVisible) {
             val mediaRequestData = MediaRequestData(
                 source = thumbnailSource,
                 kind = MediaRequestData.Kind.File(mediaInfo.name, mediaInfo.mimeType)
             )
-            ZoomableAsyncImage(
-                state = rememberZoomableImageState(zoomableState),
+            AsyncImage(
                 modifier = Modifier.fillMaxSize(),
                 model = mediaRequestData,
                 contentScale = ContentScale.Fit,

libraries/mediaviewer/api/src/test/kotlin/io/element/android/libraries/mediaviewer/api/viewer/MediaViewerViewTest.kt

@@ -26,7 +26,6 @@ import io.element.android.tests.testutils.EventsRecorder
 import io.element.android.tests.testutils.clickOn
 import io.element.android.tests.testutils.ensureCalledOnce
 import io.element.android.tests.testutils.pressBack
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TestRule
@@ -81,7 +80,6 @@ class MediaViewerViewTest {
         eventsRecorder.assertSingle(expectedEvent)
     }
 
-    @Ignore("This test is not passing yet, maybe due to interaction with ZoomableAsyncImage?")
     @Test
     fun `clicking on image hides the overlay`() {
         val eventsRecorder = EventsRecorder<MediaViewerEvents>(expectEvents = false)
@@ -96,16 +94,17 @@ class MediaViewerViewTest {
         )
         // Ensure that the action are visible
         val contentDescription = rule.activity.getString(CommonStrings.action_open_with)
-        rule.onNodeWithContentDescription(contentDescription).assertHasClickAction()
+        rule.onNodeWithContentDescription(contentDescription)
+            .assertExists()
+            .assertHasClickAction()
         val imageContentDescription = rule.activity.getString(CommonStrings.common_image)
         rule.onNodeWithContentDescription(imageContentDescription).performClick()
-        // assertHasNoClickAction does not work as expected (?)
-        // rule.onNodeWithContentDescription(contentDescription).assertHasNoClickAction()
-        rule.onNodeWithContentDescription(contentDescription).performClick()
-        // No emitted event
+        // Give time for the animation (? since even by removing AnimatedVisibility it still fails)
+        rule.mainClock.advanceTimeBy(1_000)
+        rule.onNodeWithContentDescription(contentDescription)
+            .assertDoesNotExist()
     }
 
-    @Ignore("This test is not passing yet, maybe due to interaction with ZoomableAsyncImage?")
     @Test
     fun `clicking swipe on the image invokes the expected callback`() {
         val eventsRecorder = EventsRecorder<MediaViewerEvents>(expectEvents = false)
@@ -121,7 +120,7 @@ class MediaViewerViewTest {
                 onBackClick = callback,
             )
             val imageContentDescription = rule.activity.getString(CommonStrings.common_image)
-            rule.onNodeWithContentDescription(imageContentDescription).performTouchInput { swipeDown() }
+            rule.onNodeWithContentDescription(imageContentDescription).performTouchInput { swipeDown(startY = centerY) }
             rule.mainClock.advanceTimeBy(1_000)
         }
     }