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

[5422] AttachmentsPickerSystemTabFactory configuration and bug fixes #5430

Merged
merged 17 commits into from
Oct 9, 2024

Conversation

VelikovPetar
Copy link

@VelikovPetar VelikovPetar commented Oct 4, 2024

🎯 Goal

Fixes couple of UI bugs in the new AttachmentsPickerSystemTabFactory and makes its content configurable.
Resolves: #5422

🛠 Implementation details

  1. Make AttachmentsPickerSystemTabFactory configurable: It now accepts a list of the following flags, instructing which types of attachment pickers it supports:
  • filesAllowed - if the file attachments picker is allowed
  • mediaAllowed - if the photo attachment picker is allowed
  • captureImageAllowed - if the photo capture option is allowed
  • captureVideoAllowed - if the video capture option is allowed
  • pollAllowed - if the poll option is allowed
    The AttachmentsPickerTabFactories#defaultFactoriesWithoutStoragePermissions is also extended with the same options, similar to the already existing AttachmentsPickerTabFactories#defaultFactories
  1. Fix for the case where on smaller screens, the picker options were 'squashed' - now the list of options is scrollable, so if there is not enough room for all options, the user will be able to scroll to see all options.
  2. Fix for the edge case where we must request the CAMERA permission, even when using the AttachmentsPickerSystemTabFactory: If the CAMERA permission is declared in the app manifest and is not granted, we still must request the permission, before staring the camera capture. (if the CAMERA permission is not declared in the Manifest, or is declared and granted, it works properly)
  3. Additionally, align the picker buttons size and spacing in the XML SDK to the Compose SDK.

🎨 UI Changes

Add relevant videos

SDK Before After
XML
xml_permission_before.mp4
xml_permission_after.mp4
Compose
compose_permission_before.mp4
compose_permission_after.updated.mp4

🧪 Testing

Permissions bug:

  1. Ensure the system media picker factory is used (useDefaultSystemMediaPicker)
  2. Ensure the CAMERA permission is declared in the Manifest, but is not granted
  3. Open a chat
  4. Tap on the add attachments button
  5. Tap on "Capture"
Patch for permissions bug testing
Subject: [PATCH] System media picker testing.
---
Index: stream-chat-android-ui-components-sample/src/main/AndroidManifest.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-ui-components-sample/src/main/AndroidManifest.xml b/stream-chat-android-ui-components-sample/src/main/AndroidManifest.xml
--- a/stream-chat-android-ui-components-sample/src/main/AndroidManifest.xml	
+++ b/stream-chat-android-ui-components-sample/src/main/AndroidManifest.xml	
@@ -19,6 +19,7 @@
     >
 
     <uses-permission android:name="android.permission.INTERNET" />
+    <uses-permission android:name="android.permission.CAMERA" />
 
     <application
         android:name="io.getstream.chat.ui.sample.application.App"
Index: stream-chat-android-compose-sample/src/main/AndroidManifest.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-compose-sample/src/main/AndroidManifest.xml b/stream-chat-android-compose-sample/src/main/AndroidManifest.xml
--- a/stream-chat-android-compose-sample/src/main/AndroidManifest.xml	
+++ b/stream-chat-android-compose-sample/src/main/AndroidManifest.xml	
@@ -19,6 +19,8 @@
     package="io.getstream.chat.android.compose.sample"
     >
 
+    <uses-permission android:name="android.permission.CAMERA" />
+
     <application
         android:name="io.getstream.chat.android.compose.sample.ChatApp"
         android:allowBackup="false"
Index: stream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xml b/stream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xml
--- a/stream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xml	
+++ b/stream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xml	
@@ -52,6 +52,7 @@
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintStart_toStartOf="parent"
         app:layout_constraintTop_toBottomOf="@+id/messageListView"
+        app:streamUiMessageComposerAttachmentsPickerSystemPickerEnabled="true"
         />
 
 </androidx.constraintlayout.widget.ConstraintLayout>
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	
+++ b/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.kt	
@@ -122,6 +122,7 @@
             val shapes = StreamShapes.defaultShapes()
             val messageComposerTheme = MessageComposerTheme.defaultTheme(isInDarkMode, typography, shapes, colors)
             ChatTheme(
+                useDefaultSystemMediaPicker = true,
                 isInDarkMode = isInDarkMode,
                 colors = colors,
                 shapes = shapes,

Button scrollability:

  1. Open the compose sample app on a smaller screen device
  2. Ensure the system media picker factory is used (useDefaultSystemMediaPicker)
  3. Ensure all 4 options from the attachments picker are shown
  4. The last option should not be 'squashed', but rather the whole list should be scrollable.

☑️Contributor Checklist

General

  • I have signed the Stream CLA (required)
  • 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

Please provide a suitable gif that describes your work on this pull request

@VelikovPetar VelikovPetar requested a review from a team October 4, 2024 16:05
@VelikovPetar VelikovPetar marked this pull request as ready for review October 4, 2024 16:05
@VelikovPetar VelikovPetar changed the title AttachmentsPickerSystemTabFactory configuration and big fixes AttachmentsPickerSystemTabFactory configuration and bug fixes Oct 4, 2024
@aleksandar-apostolov aleksandar-apostolov linked an issue Oct 7, 2024 that may be closed by this pull request
@kanat
Copy link
Collaborator

kanat commented Oct 7, 2024

@VelikovPetar could you please add the ticket number to the PR title like [PBE-XXX] <decription> and the ticket link to the PR description? 🙂

@VelikovPetar VelikovPetar changed the title AttachmentsPickerSystemTabFactory configuration and bug fixes [5422] AttachmentsPickerSystemTabFactory configuration and bug fixes Oct 8, 2024
@VelikovPetar
Copy link
Author

@VelikovPetar could you please add the ticket number to the PR title like [PBE-XXX] <decription> and the ticket link to the PR description? 🙂

I added the missing info, I think its more clear now 🙂

Copy link
Collaborator

@kanat kanat left a comment

Choose a reason for hiding this comment

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

LGTM

@kanat kanat merged commit e920821 into develop Oct 9, 2024
7 checks passed
@kanat kanat deleted the fix/system-attachments-picker-bugfixes branch October 9, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New attachment picker UI issues
3 participants