Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image viewer glitch #3537

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Fix image viewer glitch #3537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1be7ea
media view : use overlay from room details
ganfra Sep 24, 2024
8102aa4
media viewer: fix glitch when transition from thumbnail to fullview
ganfra Sep 24, 2024
acae30b
Stop ignoring 2 tests and fix them.
bmarty Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .../impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -252,6 +253,6 @@ class RoomDetailsFlowNode @AssistedInject constructor(

@Composable
override fun View(modifier: Modifier) {
BackstackView()
BackstackWithOverlayBox(modifier)
}
}
21 changes: 6 additions & 15 deletions ...pi/src/main/kotlin/io/element/android/libraries/mediaviewer/api/viewer/MediaViewerView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -181,7 +179,6 @@ private fun MediaViewerPage(
mediaInfo = state.mediaInfo,
thumbnailSource = state.thumbnailSource,
isVisible = showThumbnail,
zoomableState = zoomableState
)
if (showError) {
ErrorView(
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 8 additions & 9 deletions ...rc/test/kotlin/io/element/android/libraries/mediaviewer/api/viewer/MediaViewerViewTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
}
Expand Down
Loading