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.

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

[3369] video thumbnails compose #4096

Merged
merged 76 commits into from
Sep 26, 2022
Merged

Conversation

MarinTolic
Copy link
Contributor

@MarinTolic MarinTolic commented Aug 25, 2022

🎯 Goal

Closes the compose part of #3369

🛠 Implementation details

Implemented a new gallery including offline capabilitie sand preview content.

🎨 UI Changes

Add relevant screenshots

Mixed Video & Image Previews (Before) Mixed Video & Image Previews (After)
before after
Non cached images offline (Before) Non cached images offline (After)
before after
Offline Gallery (Before) Offline Gallery (After) Offline Gallery (After)
before after_no_tiles after_with_picker
Without the the ability to add more tiles (Before) With the ability to add more tiles (After)
before after

Going from offline mode to online mode

Messages list (Before) Messages list (After)
before_message_list.mp4
after_message_list.mp4
Gallery (Before) [Nothing happens] Gallery (After)
before_gallery.mp4
galery_after.mp4

🧪 Testing

Since this PR is quite large (apologies), the recommendation is to first do the practical testing part before reading any code.

Test Scenario A: List Offline Capabilities

  1. Open a channel with no cached images while offline
  2. Go online and do not scroll up or down

Expected: Images should load automatically when connection is regained

Test Scenario B: Gallery Offline Capabilities

  1. Open a channel with no cached images while offline
  2. Open non-cached attachments in a gallery
  3. Go online

Expected: Images should load automatically when connection is regained

Test Scenario C: Message List

  1. Send or find images with mixed video & image attachments
  2. If sending try to vary the number of attachments and the ratio of their type (Images : Videos ratio)

Expected: The play button should be loaded only above videos. Both videos and images should be displayed properly.

Test Scenario D: Gallery

There are a few things to test here.

  1. That switching videos and images works well
  2. That playing video works well and that previews do not get removed when they should not
  3. That attachments can be deleted in the right manner
  4. That other options such as reply and show in chat work too

Due to the way VideoPlayer is made and the fact that it's actually just a View, it's difficult to make it work as seamless as we would normally like.

Test Scenario E: Customizing MediaAttachmentFactory

Use the patch to customize MediaAttachmentFactory
Index: stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt b/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt
--- a/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt	(revision 684028d7c327fcfe74aed26e471a31d8e7241f3b)
+++ b/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt	(date 1662402566340)
@@ -16,6 +16,12 @@
 
 package io.getstream.chat.android.compose.ui.attachments
 
+import androidx.compose.foundation.Image
+import androidx.compose.foundation.layout.fillMaxSize
+import androidx.compose.material.Card
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.res.painterResource
+import io.getstream.chat.android.compose.R
 import io.getstream.chat.android.compose.ui.attachments.factory.FileAttachmentFactory
 import io.getstream.chat.android.compose.ui.attachments.factory.GiphyAttachmentFactory
 import io.getstream.chat.android.compose.ui.attachments.factory.LinkAttachmentFactory
@@ -46,7 +52,15 @@
         UploadAttachmentFactory(),
         LinkAttachmentFactory(linkDescriptionMaxLines),
         GiphyAttachmentFactory(),
-        MediaAttachmentFactory(),
+        MediaAttachmentFactory(maximumNumberOfPreviewedItems = 5, contentPlayButton = {
+            Card(modifier = Modifier.fillMaxSize(0.25f)) {
+                Image(painter = painterResource(id = R.drawable.stream_compose_ic_download), contentDescription = null)
+            }
+        }) {
+            Card(modifier = Modifier.fillMaxSize(0.25f)) {
+                Image(painter = painterResource(id = R.drawable.stream_compose_ic_download), contentDescription = null)
+            }
+        },
         FileAttachmentFactory(),
     )
 

Test Scenario F: Disabling video thumbnails

Use the patch to disable video thumbnails
Index: stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt b/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt
--- a/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt	(revision 684028d7c327fcfe74aed26e471a31d8e7241f3b)
+++ b/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt	(date 1662402784654)
@@ -93,7 +93,7 @@
         val channelId = intent.getStringExtra(KEY_CHANNEL_ID) ?: return
 
         setContent {
-            ChatTheme(dateFormatter = ChatApp.dateFormatter) {
+            ChatTheme(dateFormatter = ChatApp.dateFormatter, videoThumbnailsEnabled = false) {
                 MessagesScreen(
                     channelId = channelId,
                     onBackPressed = { finish() },
  1. Open the list and check that video thumbnails are disabled
  2. Open the gallery including the picker within it and check that the thumbnails are disabled

Test Scenario G: Testing out the ability to independently change the size of Video, Image and group previews in ChatTheme.dimens

Use the patch to apply independent sizing
Index: stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt b/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt
--- a/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt	(revision 684028d7c327fcfe74aed26e471a31d8e7241f3b)
+++ b/stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt	(date 1662403075563)
@@ -470,7 +470,7 @@
             selectedChannelMenuUserItemWidth = 80.dp,
             selectedChannelMenuUserItemHorizontalPadding = 8.dp,
             selectedChannelMenuUserItemAvatarSize = 64.dp,
-            attachmentsContentImageWidth = 250.dp,
+            attachmentsContentImageWidth = 150.dp,
             attachmentsContentGiphyWidth = 250.dp,
             attachmentsContentGiphyHeight = 200.dp,
             attachmentsContentLinkWidth = 250.dp,
@@ -505,12 +505,12 @@
             quotedMessageAttachmentEndPadding = 0.dp,
             groupAvatarInitialsXOffset = 1.5.dp,
             groupAvatarInitialsYOffset = 2.5.dp,
-            attachmentsContentImageMaxHeight = 600.dp,
+            attachmentsContentImageMaxHeight = 200.dp,
             attachmentsContentVideoMaxHeight = 400.dp,
             attachmentsContentMediaGridSpacing = 2.dp,
-            attachmentsContentVideoWidth = 250.dp,
-            attachmentsContentGroupPreviewWidth = 250.dp,
-            attachmentsContentGroupPreviewHeight = 250.dp,
+            attachmentsContentVideoWidth = 100.dp,
+            attachmentsContentGroupPreviewWidth = 400.dp,
+            attachmentsContentGroupPreviewHeight = 200.dp,
         )
     }
 }

☑️Contributor Checklist

General

  • Assigned a person / code owner group (required)
  • Thread with the PR link started in a respective Slack channel (#android-chat-core or #android-chat-ui) (required)
  • PR targets the develop branch
  • PR is linked to the GitHub issue it resolves

Code & documentation

  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (KDocs, docusaurus, tutorial)

☑️Reviewer Checklist

  • UI Components sample runs & works
  • Compose sample runs & works
  • UI Changes correct (before & after images)
  • Bugs validated (bugfixes)
  • New feature tested and works
  • Release notes and docs clearly describe changes
  • All code we touched has new or updated KDocs

🎉 GIF

gif

… Images and Video and its accompanying composables.
…diaGalleryPreviewActivity` and receives results from it instead of `ImagePreviewActivity`
…droid into feature/3369-video-thumbnails-compose

� Conflicts:
�	CHANGELOG.md
�	stream-chat-android-client/api/stream-chat-android-client.api
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/ChatApi.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/ChannelClient.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/models/UploadedFile.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/uploader/FileUploader.kt
�	stream-chat-android-client/src/main/java/io/getstream/chat/android/client/uploader/StreamFileUploader.kt
�	stream-chat-android-client/src/test/java/io/getstream/chat/android/client/uploader/StreamFileUploaderTest.kt
�	stream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/client/helpers/MyFileUploader.java
�	stream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/client/helpers/MyFileUploader.kt
�	stream-chat-android-state/src/main/java/io/getstream/chat/android/offline/message/attachments/internal/AttachmentUploader.kt
�	stream-chat-android-state/src/test/java/io/getstream/chat/android/offline/channel/controller/attachment/AttachmentUploaderTests.kt
�	stream-chat-android-state/src/test/java/io/getstream/chat/android/offline/channel/controller/attachment/UploadAttachmentsIntegrationTests.kt
… number of previewd thumbnails, including odd numbers.
…me specific grid spacing for media preview
…reen and implement retry procedure when reconnecting after having a failed load due to being offline.
… track that in real time in the gallery so it is pointless
… track that in real time in the gallery so it is pointless
Copy link
Contributor

@filbabic filbabic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☑️Reviewer Checklist

  • Compose sample runs & works
  • UI Changes correct (before & after images)
  • Bugs validated (bugfixes)
  • New feature tested and works
  • Release notes and docs clearly describe changes
  • All code we touched has new or updated KDocs

This is certainly one of the best improvements to the UI part of Compose we've had in a long time and will bring lots of value to our users!

Overall, amazing job - I did leave some comments, so please clean them up and then we can merge!

MarinTolic and others added 10 commits September 16, 2022 14:15
…droid into feature/3369-video-thumbnails-compose

� Conflicts:
�	DEPRECATIONS.md
�	stream-chat-android-compose/api/stream-chat-android-compose.api
�	stream-chat-android-compose/detekt-baseline.xml
�	stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt
�	stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamDimens.kt
@filbabic filbabic merged commit 413a14b into develop Sep 26, 2022
@filbabic filbabic deleted the feature/3369-video-thumbnails-compose branch September 26, 2022 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants