Media: some ui improvements

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt

@@ -45,8 +45,8 @@ class AttachmentsPreviewNode @AssistedInject constructor(
 
     @Composable
     override fun View(modifier: Modifier) {
-        val state = presenter.present()
         ElementTheme(darkTheme = true) {
+            val state = presenter.present()
             AttachmentsPreviewView(
                 state = state,
                 onDismiss = this::navigateUp,

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt

@@ -22,14 +22,14 @@ import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
-import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.defaultMinSize
+import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
@@ -70,7 +70,10 @@ fun AttachmentsPreviewView(
     }
 
     Scaffold(modifier) {
-        Box {
+        Box(
+            modifier = Modifier.padding(it),
+            contentAlignment = Alignment.Center
+        ) {
             AttachmentPreviewContent(
                 attachment = state.attachment,
                 onSendClicked = ::postSendAttachment,
@@ -115,11 +118,10 @@ private fun AttachmentPreviewContent(
     onDismiss: () -> Unit
 ) {
     Column(
-        modifier = Modifier.fillMaxWidth(),
+        modifier = Modifier
+            .fillMaxSize()
+            .padding(top = 24.dp)
     ) {
-        Spacer(
-            modifier = Modifier.height(80.dp)
-        )
         Box(
             modifier = Modifier
                 .fillMaxWidth()

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/media/viewer/MediaViewerNode.kt

@@ -49,8 +49,8 @@ class MediaViewerNode @AssistedInject constructor(
 
     @Composable
     override fun View(modifier: Modifier) {
-        val state = presenter.present()
         ElementTheme(darkTheme = true) {
+            val state = presenter.present()
             MediaViewerView(
                 state = state,
                 modifier = modifier

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/media/viewer/MediaViewerView.kt

@@ -41,7 +41,7 @@ import io.element.android.libraries.designsystem.theme.components.Button
 import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.Scaffold
 import io.element.android.libraries.designsystem.theme.components.Text
-import io.element.android.libraries.ui.strings.R.string as StringR
+import io.element.android.libraries.ui.strings.R as StringR
 
 @Composable
 fun MediaViewerView(
@@ -62,7 +62,7 @@ fun MediaViewerView(
         ) {
             when (state.downloadedMedia) {
                 is Async.Success -> LocalMediaView(state.downloadedMedia.state)
-                is Async.Failure -> ErrorView("Error while downloading", ::onRetry)
+                is Async.Failure -> ErrorView(stringResource(id = StringR.string.error_unknown), ::onRetry)
                 else -> CircularProgressIndicator(
                     strokeWidth = 2.dp,
                 )
@@ -82,11 +82,11 @@ private fun ErrorView(
         horizontalAlignment = Alignment.CenterHorizontally,
     ) {
         Text(text = errorMessage)
-        Spacer(modifier = Modifier.size(8.dp))
+        Spacer(modifier = Modifier.size(16.dp))
         Button(
             onClick = onRetry
         ) {
-            Text(text = stringResource(id = StringR.action_retry))
+            Text(text = stringResource(id = StringR.string.action_retry))
         }
 
     }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemAspectRatioBox.kt

@@ -0,0 +1,42 @@
+/*
+ * 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.components.event
+
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.BoxScope
+import androidx.compose.foundation.layout.aspectRatio
+import androidx.compose.foundation.layout.heightIn
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import kotlin.math.min
+
+@Composable
+fun TimelineItemAspectRatioBox(
+    height: Int?,
+    aspectRatio: Float,
+    modifier: Modifier = Modifier,
+    content: @Composable BoxScope.() -> Unit,
+) {
+    val maxHeight = min(300, height ?: 0)
+    Box(
+        modifier = modifier
+            .heightIn(max = maxHeight.dp)
+            .aspectRatio(aspectRatio),
+        content = content
+    )
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt

@@ -16,37 +16,31 @@
 
 package io.element.android.features.messages.impl.timeline.components.event
 
-import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.foundation.layout.aspectRatio
-import androidx.compose.foundation.layout.heightIn
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
-import androidx.compose.ui.unit.dp
+import io.element.android.features.messages.impl.timeline.components.blurhash.BlurHashAsyncImage
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContent
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContentProvider
-import io.element.android.libraries.designsystem.components.blurhash.BlurHashAsyncImage
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.matrix.ui.media.MediaRequestData
-import kotlin.math.min
 
 @Composable
 fun TimelineItemImageView(
     content: TimelineItemImageContent,
     modifier: Modifier = Modifier,
 ) {
-    val maxHeight = min(300, content.height ?: Int.MAX_VALUE)
+    TimelineItemAspectRatioBox(
-    Box(
+        height = content.height,
+        aspectRatio = content.aspectRatio,
         modifier = modifier
-            .heightIn(max = maxHeight.dp)
-            .aspectRatio(content.aspectRatio),
-        contentAlignment = Alignment.Center,
     ) {
         BlurHashAsyncImage(
+            modifier = Modifier.fillMaxSize(),
             blurHash = content.blurhash,
             model = MediaRequestData(content.mediaSource, MediaRequestData.Kind.Content),
             contentScale = ContentScale.Fit,

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt

@@ -17,9 +17,7 @@
 package io.element.android.features.messages.impl.timeline.components.event
 
 import androidx.compose.foundation.Image
-import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.foundation.layout.aspectRatio
-import androidx.compose.foundation.layout.heightIn
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
@@ -29,29 +27,26 @@ import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
-import androidx.compose.ui.unit.dp
+import io.element.android.features.messages.impl.timeline.components.blurhash.BlurHashAsyncImage
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContent
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContentProvider
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
-import io.element.android.libraries.designsystem.components.blurhash.BlurHashAsyncImage
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.matrix.ui.media.MediaRequestData
-import kotlin.math.min
 
 @Composable
 fun TimelineItemVideoView(
     content: TimelineItemVideoContent,
     modifier: Modifier = Modifier,
 ) {
-    val maxHeight = min(300, content.height ?: Int.MAX_VALUE)
+    TimelineItemAspectRatioBox(
-    Box(
+        height = content.height,
+        aspectRatio = content.aspectRatio,
         modifier = modifier
-            .heightIn(max = maxHeight.dp)
-            .aspectRatio(content.aspectRatio),
-        contentAlignment = Alignment.Center,
     ) {
         BlurHashAsyncImage(
+            modifier = Modifier.fillMaxSize(),
             blurHash = content.blurhash,
             model = MediaRequestData(content.thumbnailSource, MediaRequestData.Kind.Content),
             contentScale = ContentScale.Fit,
@@ -59,9 +54,9 @@ fun TimelineItemVideoView(
         Image(
             painterResource(id = androidx.media3.ui.R.drawable.exo_ic_play_circle_filled),
             contentDescription = null,
-            colorFilter = ColorFilter.tint(MaterialTheme.colorScheme.onBackground)
+            colorFilter = ColorFilter.tint(MaterialTheme.colorScheme.onBackground),
+            modifier = Modifier.align(Alignment.Center),
         )
-
     }
 }
 

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt

@@ -41,13 +41,7 @@ class TimelineItemContentMessageFactory @Inject constructor() {
                 htmlDocument = messageType.formatted?.toHtmlDocument()
             )
             is ImageMessageType -> {
-                val height = messageType.info?.height?.toFloat()
+                val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height)
-                val width = messageType.info?.width?.toFloat()
-                val aspectRatio = if (height != null && width != null) {
-                    width / height
-                } else {
-                    0.7f
-                }
                 TimelineItemImageContent(
                     body = messageType.body,
                     height = messageType.info?.height?.toInt(),
@@ -59,13 +53,7 @@ class TimelineItemContentMessageFactory @Inject constructor() {
                 )
             }
             is VideoMessageType -> {
-                val height = messageType.info?.height?.toFloat()
+                val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height)
-                val width = messageType.info?.width?.toFloat()
-                val aspectRatio = if (height != null && width != null) {
-                    width / height
-                } else {
-                    0.7f
-                }
                 TimelineItemVideoContent(
                     body = messageType.body,
                     thumbnailSource = messageType.info?.thumbnailSource,
@@ -89,4 +77,12 @@ class TimelineItemContentMessageFactory @Inject constructor() {
             else -> TimelineItemUnknownContent
         }
     }
+
+    private fun aspectRatioOf(width: Long?, height: Long?): Float {
+        return if (height != null && width != null) {
+            width.toFloat() / height.toFloat()
+        } else {
+            0.7f
+        }
+    }
 }