[Media bottom sheet] Improve design, and hide Share and Download when rendered from the MediaViewer.

Closes #6907
This commit is contained in:
Benoit Marty
2026-06-02 15:22:04 +02:00
parent a095190902
commit 1aef3860c2
9 changed files with 70 additions and 60 deletions
@@ -16,6 +16,7 @@ sealed interface MediaBottomSheetState {
data object Hidden : MediaBottomSheetState data object Hidden : MediaBottomSheetState
data class Details( data class Details(
val fromGallery: Boolean,
val eventId: EventId?, val eventId: EventId?,
val canDelete: Boolean, val canDelete: Boolean,
val mediaInfo: MediaInfo, val mediaInfo: MediaInfo,
@@ -29,10 +29,14 @@ open class MediaBottomSheetStateDetailsProvider : PreviewParameterProvider<Media
aMediaBottomSheetStateDetails( aMediaBottomSheetStateDetails(
eventId = null, eventId = null,
), ),
aMediaBottomSheetStateDetails(
fromGallery = true,
),
) )
} }
fun aMediaBottomSheetStateDetails( fun aMediaBottomSheetStateDetails(
fromGallery: Boolean = false,
eventId: EventId? = EventId($$"$eventId"), eventId: EventId? = EventId($$"$eventId"),
canDelete: Boolean = true, canDelete: Boolean = true,
mediaInfo: MediaInfo = anImageMediaInfo( mediaInfo: MediaInfo = anImageMediaInfo(
@@ -40,6 +44,7 @@ fun aMediaBottomSheetStateDetails(
dateSentFull = "December 6, 2024 at 12:59", dateSentFull = "December 6, 2024 at 12:59",
), ),
) = MediaBottomSheetState.Details( ) = MediaBottomSheetState.Details(
fromGallery = fromGallery,
eventId = eventId, eventId = eventId,
canDelete = canDelete, canDelete = canDelete,
mediaInfo = mediaInfo, mediaInfo = mediaInfo,
@@ -10,7 +10,6 @@ package io.element.android.libraries.mediaviewer.impl.details
import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.fillMaxWidth
@@ -27,7 +26,6 @@ import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.res.stringResource import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.heading import androidx.compose.ui.semantics.heading
import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
@@ -102,34 +100,6 @@ fun MediaDetailsBottomSheet(
Spacer(modifier = Modifier.height(16.dp)) Spacer(modifier = Modifier.height(16.dp))
if (state.eventId != null) { if (state.eventId != null) {
HorizontalDivider() HorizontalDivider()
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.VisibilityOn())),
headlineContent = { Text(stringResource(CommonStrings.action_view_in_timeline)) },
onClick = {
onViewInTimeline(state.eventId)
}
)
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.ShareAndroid())),
headlineContent = { Text(stringResource(CommonStrings.action_share)) },
onClick = {
onShare(state.eventId)
}
)
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Forward())),
headlineContent = { Text(stringResource(CommonStrings.action_forward)) },
onClick = {
onForward(state.eventId)
}
)
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Download())),
headlineContent = { Text(stringResource(CommonStrings.action_download)) },
onClick = {
onDownload(state.eventId)
}
)
val mimeType = state.mediaInfo.mimeType val mimeType = state.mediaInfo.mimeType
val icon = when (mimeType) { val icon = when (mimeType) {
MimeTypes.Apk -> MimeTypes.Apk ->
@@ -148,11 +118,41 @@ fun MediaDetailsBottomSheet(
onOpenWith(state.eventId) onOpenWith(state.eventId)
} }
) )
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.VisibilityOn())),
headlineContent = { Text(stringResource(CommonStrings.action_view_in_timeline)) },
onClick = {
onViewInTimeline(state.eventId)
}
)
if (state.fromGallery) {
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.ShareAndroid())),
headlineContent = { Text(stringResource(CommonStrings.action_share)) },
onClick = {
onShare(state.eventId)
}
)
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Download())),
headlineContent = { Text(stringResource(CommonStrings.action_download)) },
onClick = {
onDownload(state.eventId)
}
)
}
ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Forward())),
headlineContent = { Text(stringResource(CommonStrings.action_forward)) },
onClick = {
onForward(state.eventId)
}
)
if (state.canDelete) { if (state.canDelete) {
HorizontalDivider() HorizontalDivider()
ListItem( ListItem(
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Delete())), leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Delete())),
headlineContent = { Text(stringResource(CommonStrings.action_delete)) }, headlineContent = { Text(stringResource(CommonStrings.action_delete_file)) },
style = ListItemStyle.Destructive, style = ListItemStyle.Destructive,
onClick = { onClick = {
onDelete(state.eventId) onDelete(state.eventId)
@@ -216,16 +216,14 @@ private fun SenderRow(
} }
@Composable @Composable
private fun ColumnScope.Title() { private fun Title() {
Text( Text(
modifier = Modifier modifier = Modifier
.align(Alignment.CenterHorizontally)
.padding(top = 16.dp, bottom = 8.dp, start = 16.dp, end = 16.dp) .padding(top = 16.dp, bottom = 8.dp, start = 16.dp, end = 16.dp)
.semantics { .semantics {
heading() heading()
}, },
text = stringResource(R.string.screen_media_details_title), text = stringResource(R.string.screen_media_details_title),
textAlign = TextAlign.Center,
style = ElementTheme.typography.fontBodyLgMedium, style = ElementTheme.typography.fontBodyLgMedium,
color = ElementTheme.colors.textPrimary, color = ElementTheme.colors.textPrimary,
) )
@@ -129,6 +129,7 @@ class MediaGalleryPresenter(
} }
is MediaGalleryEvent.OpenInfo -> coroutineScope.launch { is MediaGalleryEvent.OpenInfo -> coroutineScope.launch {
mediaBottomSheetState = MediaBottomSheetState.Details( mediaBottomSheetState = MediaBottomSheetState.Details(
fromGallery = true,
eventId = event.mediaItem.eventId(), eventId = event.mediaItem.eventId(),
canDelete = when (event.mediaItem.mediaInfo().senderId) { canDelete = when (event.mediaItem.mediaInfo().senderId) {
null -> false null -> false
@@ -145,6 +145,7 @@ class MediaViewerPresenter(
} }
is MediaViewerEvent.OpenInfo -> coroutineScope.launch { is MediaViewerEvent.OpenInfo -> coroutineScope.launch {
mediaBottomSheetState = MediaBottomSheetState.Details( mediaBottomSheetState = MediaBottomSheetState.Details(
fromGallery = false,
eventId = event.data.eventId, eventId = event.data.eventId,
canDelete = when (event.data.mediaInfo.senderId) { canDelete = when (event.data.mediaInfo.senderId) {
null -> false null -> false
@@ -277,6 +277,7 @@ fun MediaViewerView(
state.eventSink(MediaViewerEvent.ViewInTimeline(it)) state.eventSink(MediaViewerEvent.ViewInTimeline(it))
}, },
onShare = { onShare = {
// Note: share action is not rendered when the bottom sheet is opened from the media viewer
(currentData as? MediaViewerPageData.MediaViewerData)?.let { (currentData as? MediaViewerPageData.MediaViewerData)?.let {
state.eventSink(MediaViewerEvent.Share(currentData)) state.eventSink(MediaViewerEvent.Share(currentData))
} }
@@ -285,6 +286,7 @@ fun MediaViewerView(
state.eventSink(MediaViewerEvent.Forward(it)) state.eventSink(MediaViewerEvent.Forward(it))
}, },
onDownload = { onDownload = {
// Note: download action is not rendered when the bottom sheet is opened from the media viewer
(currentData as? MediaViewerPageData.MediaViewerData)?.let { (currentData as? MediaViewerPageData.MediaViewerData)?.let {
state.eventSink(MediaViewerEvent.SaveOnDisk(currentData)) state.eventSink(MediaViewerEvent.SaveOnDisk(currentData))
} }
@@ -45,7 +45,9 @@ class MediaDetailsBottomSheetTest {
@Test @Test
@Config(qualifiers = "h1024dp") @Config(qualifiers = "h1024dp")
fun `clicking on Share invokes expected callback`() = runAndroidComposeUiTest { fun `clicking on Share invokes expected callback`() = runAndroidComposeUiTest {
val state = aMediaBottomSheetStateDetails() val state = aMediaBottomSheetStateDetails(
fromGallery = true,
)
ensureCalledOnceWithParam(state.eventId) { callback -> ensureCalledOnceWithParam(state.eventId) { callback ->
setMediaDetailsBottomSheet( setMediaDetailsBottomSheet(
state = state, state = state,
@@ -55,6 +57,15 @@ class MediaDetailsBottomSheetTest {
} }
} }
@Test
@Config(qualifiers = "h1024dp")
fun `item Share is not displayed when opened from the media viewer`() = runAndroidComposeUiTest {
setMediaDetailsBottomSheet(
state = aMediaBottomSheetStateDetails(),
)
onNodeWithText(activity!!.getString(CommonStrings.action_share)).assertDoesNotExist()
}
@Test @Test
@Config(qualifiers = "h1024dp") @Config(qualifiers = "h1024dp")
fun `clicking on Forward invokes expected callback`() = runAndroidComposeUiTest { fun `clicking on Forward invokes expected callback`() = runAndroidComposeUiTest {
@@ -71,7 +82,9 @@ class MediaDetailsBottomSheetTest {
@Test @Test
@Config(qualifiers = "h1024dp") @Config(qualifiers = "h1024dp")
fun `clicking on Download invokes expected callback`() = runAndroidComposeUiTest { fun `clicking on Download invokes expected callback`() = runAndroidComposeUiTest {
val state = aMediaBottomSheetStateDetails() val state = aMediaBottomSheetStateDetails(
fromGallery = true,
)
ensureCalledOnceWithParam(state.eventId) { callback -> ensureCalledOnceWithParam(state.eventId) { callback ->
setMediaDetailsBottomSheet( setMediaDetailsBottomSheet(
state = state, state = state,
@@ -81,6 +94,15 @@ class MediaDetailsBottomSheetTest {
} }
} }
@Test
@Config(qualifiers = "h1024dp")
fun `item Download is not displayed when opened from the media viewer`() = runAndroidComposeUiTest {
setMediaDetailsBottomSheet(
state = aMediaBottomSheetStateDetails(),
)
onNodeWithText(activity!!.getString(CommonStrings.action_download)).assertDoesNotExist()
}
@Config(qualifiers = "h1024dp") @Config(qualifiers = "h1024dp")
@Test @Test
fun `clicking on Delete invokes expected callback`() = runAndroidComposeUiTest { fun `clicking on Delete invokes expected callback`() = runAndroidComposeUiTest {
@@ -90,8 +112,8 @@ class MediaDetailsBottomSheetTest {
state = state, state = state,
onDelete = callback, onDelete = callback,
) )
onNodeWithText(activity!!.getString(CommonStrings.action_delete)).assertExists() onNodeWithText(activity!!.getString(CommonStrings.action_delete_file)).assertExists()
clickOn(CommonStrings.action_delete) clickOn(CommonStrings.action_delete_file)
} }
} }
@@ -137,6 +137,7 @@ class MediaGalleryPresenterTest {
val state = awaitItem() val state = awaitItem()
assertThat(state.mediaBottomSheetState).isEqualTo( assertThat(state.mediaBottomSheetState).isEqualTo(
MediaBottomSheetState.Details( MediaBottomSheetState.Details(
fromGallery = true,
eventId = AN_EVENT_ID, eventId = AN_EVENT_ID,
canDelete = canDeleteOwn, canDelete = canDeleteOwn,
mediaInfo = item.mediaInfo, mediaInfo = item.mediaInfo,
@@ -184,6 +185,7 @@ class MediaGalleryPresenterTest {
val state = awaitItem() val state = awaitItem()
assertThat(state.mediaBottomSheetState).isEqualTo( assertThat(state.mediaBottomSheetState).isEqualTo(
MediaBottomSheetState.Details( MediaBottomSheetState.Details(
fromGallery = true,
eventId = AN_EVENT_ID, eventId = AN_EVENT_ID,
canDelete = canDeleteOther, canDelete = canDeleteOther,
mediaInfo = item.mediaInfo, mediaInfo = item.mediaInfo,
@@ -135,28 +135,6 @@ class MediaViewerViewTest {
) )
} }
@Test
@Config(qualifiers = "h1024dp")
fun `clicking on download emits expected Event`() {
val data = aMediaViewerPageData()
testBottomSheetAction(
data,
CommonStrings.action_download,
MediaViewerEvent.SaveOnDisk(data),
)
}
@Test
@Config(qualifiers = "h1024dp")
fun `clicking on share emits expected Event`() {
val data = aMediaViewerPageData()
testBottomSheetAction(
data,
CommonStrings.action_share,
MediaViewerEvent.Share(data),
)
}
@Config(qualifiers = "h1024dp") @Config(qualifiers = "h1024dp")
@Test @Test
fun `clicking on open in emits expected Event`() { fun `clicking on open in emits expected Event`() {