Conversation
SDK Size Comparison 📏
|
andremion
force-pushed
the
redesign/AND-1030-message-composer-actions-menu-redesign
branch
2 times, most recently
from
b4a8142 to
2996e7c
Compare
- Remove the overlay and dialog-like behavior from the `AttachmentsPicker` composable, making it a regular surface. - Update `AttachmentsPicker` to use `FilledIconToggleButton` for tab selection. - Set the `containerColor` on the `MessagesScreen` Scaffold to use the theme's app background.
This commit introduces a new `AttachmentTypePicker` composable. This new component acts as a tab selector for different attachment types like media, files, camera, and polls. Specific changes include: * Introducing the `AttachmentTypePicker` composable. * Adding new icons for each attachment type (media, files, camera, polls). * Adding new string resources for the attachment type labels. * Introducing a new `backgroundCoreSelected` color to `StreamColors` for the selected tab state. * Replacing `AttachmentPickerOptions` with the new `AttachmentTypePicker`. * Updating tests to cover the new `AttachmentTypePicker`.
…tsPickerMode is now passed from the view model
… internal state is now handled within the AttachmentsPickerViewModel when the picker is hidden, removing the need for an explicit `dismissAttachments()` call at the UI layer.
andremion
force-pushed
the
redesign/AND-1030-message-composer-actions-menu-redesign
branch
from
48b05cf to
83115c7
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR redesigns the attachment picker system by introducing a new Changes
Sequence DiagramsequenceDiagram
participant User
participant MessageComposer as Message<br/>Composer
participant AttachmentPickerMenu
participant AttachmentTypePicker
participant SpecificPicker as Specific<br/>Picker
participant AttachmentsPickerVM as AttachmentsPicker<br/>ViewModel
participant MessageComposerVM as MessageComposer<br/>ViewModel
User->>MessageComposer: Click attachments button
MessageComposer->>AttachmentPickerMenu: Show picker (isAttachmentPickerVisible=true)
AttachmentPickerMenu->>AttachmentTypePicker: Display available modes
User->>AttachmentTypePicker: Select mode (Images/Files/Camera/etc)
AttachmentTypePicker->>SpecificPicker: Route to specific picker
SpecificPicker->>SpecificPicker: Handle permissions/access
SpecificPicker->>User: Present selection UI
User->>SpecificPicker: Select attachments
SpecificPicker->>AttachmentsPickerVM: changeSelectedAttachments()
AttachmentsPickerVM->>AttachmentsPickerVM: Update Selection state
User->>AttachmentPickerMenu: Submit selection
AttachmentPickerMenu->>AttachmentsPickerVM: Get selected attachments
AttachmentsPickerVM->>MessageComposerVM: updateSelectedAttachments()
MessageComposerVM->>MessageComposer: Update with attachments
MessageComposer->>User: Display attachments in composer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt (1)
45-46:⚠️ Potential issue | 🟡 MinorDuplicate
@paramtag formetadataEmphasis.Line 45 appears to be a copy-paste error. It should document
metadataDefaultinstead, asmetadataEmphasisis already documented on line 46.📝 Proposed fix
-* `@param` metadataEmphasis Used for metadata and supplementary information. -* `@param` metadataEmphasis Used for emphasized metadata and supplementary information in secondary content areas. +* `@param` metadataDefault Used for metadata and supplementary information. +* `@param` metadataEmphasis Used for emphasized metadata and supplementary information in secondary content areas.stream-chat-android-compose/api/stream-chat-android-compose.api (2)
3682-3886:⚠️ Potential issue | 🟠 MajorStreamColors constructor expansion is a breaking ABI change
The data class now requires 78 positional parameters with 23 optional. While
defaultColors()anddefaultDarkColors()factory methods exist, any code creating custom instances must pass 78 required parameters. This breaks binary compatibility for Java callers and makes direct instantiation brittle. Consider providing a builder or expanding the factory methods to accept partial configurations.
2984-2999:⚠️ Potential issue | 🟠 MajorProvide migration guidance or add deprecation shims for ChatComponentFactory changes
MessageComposer,MessageComposerCommandSuggestionItemCenterContent, andMessageComposerLeadingContentinChatComponentFactorylack deprecation annotations and migration guidance. Custom implementations extending this interface will break without clear upgrade instructions.
🤖 Fix all issues with AI agents
In `@stream-chat-android-compose/api/stream-chat-android-compose.api`:
- Around line 2291-2309: Rename the public API class
AttachmentPickerCommandClickClick to AttachmentPickerCommandClick: update the
class declaration, its constructor, methods (component1, copy, copy$default,
getCommand), the synthetic/static fields (like $stable) and any references or
usages in the io/getstream/chat/android/compose/ui/messages/attachments/factory
package to the new name so the public API matches
AttachmentPickerCreatePollClick; ensure the Kotlin-generated signatures and any
serialization/reflection usages are updated so callers see
AttachmentPickerCommandClick instead of AttachmentPickerCommandClickClick.
- Around line 2185-2277: Add a deprecation/migration note to DEPRECATIONS.md
describing the migration path from the legacy AttachmentsPickerDialogFragment
(ui-components) to the new Compose APIs AttachmentPicker and
AttachmentPickerMenu: state the that AttachmentsPickerDialogFragment is
deprecated, list equivalent Compose entrypoints (AttachmentPicker,
AttachmentPickerMenu) and required viewmodel changes (AttachmentsPickerViewModel
/ MessageComposerViewModel), provide a short step-by-step migration checklist,
and include a clear deprecation timeline (deprecation date and planned removal
version) so users know when to transition.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/suggestions/commands/CommandSuggestionItem.kt`:
- Around line 147-156: The fallback logic is inconsistent: Command.imageRes
returns the GIPHY drawable for unknown commands but Command.isColouredImage only
returns true for CommandDefaults.GIPHY, causing tinted rendering of the colorful
GIPHY icon; update the logic so the two are consistent—either add an explicit
generic fallback drawable case and keep isColouredImage false, or make
Command.isColouredImage return true when imageRes would be the GIPHY fallback
(i.e., when name == CommandDefaults.GIPHY OR when imageRes resolves to the giphy
drawable), by adjusting the implementations of Command.imageRes and
Command.isColouredImage to match each other (referencing Command.imageRes,
Command.isColouredImage, and CommandDefaults.GIPHY/MUTE/UNMUTE).
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCameraPicker.kt`:
- Around line 56-94: The UI flag showRequiredCameraPermission can remain true
after permission is granted; in AttachmentCameraPicker inside the LaunchedEffect
observing cameraPermissionState?.status, clear showRequiredCameraPermission (set
it to false) as soon as cameraPermissionState?.status.isGranted before calling
captureMediaLauncher?.launch(Unit) so the permission prompt UI is dismissed;
update the branch that currently checks status.isGranted to reset
showRequiredCameraPermission and then proceed to launch, leaving other branches
unchanged.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerContent.kt`:
- Around line 72-73: The when branch in AttachmentPickerContent currently uses
TODO() for the System and CustomPickerMode cases, which will throw at runtime;
replace those TODO() calls with safe fallbacks: for System and CustomPickerMode
render a benign UI (e.g., an empty Box, a placeholder Text, or reuse the
existing default picker UI component used for other modes) and/or log the
unexpected mode, or prevent these modes from being selectable upstream; update
the AttachmentPickerContent when expression to handle is System and is
CustomPickerMode by returning the safe fallback UI (reference the
AttachmentPickerContent composable and the System/CustomPickerMode sealed
variants) so the picker never crashes if those modes are reached.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerMenu.kt`:
- Around line 61-76: The picker auto-closes because after calling
keyboardController?.hide() the IME inset stays briefly true and your
isKeyboardVisible LaunchedEffect immediately calls
attachmentsPickerViewModel.changeAttachmentState(false); fix by suppressing the
keyboard-visibility-driven close for a short window after you programmatically
hide the keyboard: when you call keyboardController?.hide() inside the
LaunchedEffect(isShowingAttachments) (where shouldCloseKeyboard and
isShowingAttachments are used), set a local mutable flag (e.g.,
suppressImeClose) and reset it after a small delay (e.g., 150-300ms) in the same
coroutine; then in the LaunchedEffect observing isKeyboardVisible, ignore the
visibility change if suppressImeClose is true or if isShowingDialog is true
before calling attachmentsPickerViewModel.changeAttachmentState(showAttachments
= false).
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/factory/AttachmentPickerAction.kt`:
- Around line 39-47: The public data class name
AttachmentPickerCommandClickClick has a duplicated "Click" and should be renamed
to a clearer public API (e.g., AttachmentPickerCommandClick); update the
declaration of AttachmentPickerCommandClickClick to the new name and adjust all
references/usages and imports throughout the codebase (including any places
creating or pattern-matching on AttachmentPickerCommandClickClick and the
AttachmentPickerAction sealed/type hierarchy) so the type remains binary- and
source-compatible within this module.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt`:
- Around line 66-73: The class KDoc for StreamTypography is missing `@param`
entries for the new public properties; update the class-level KDoc to include
`@param` captionDefault (used for default caption text with regular weight),
`@param` headingSm (used for small headings with semi-bold emphasis), `@param`
numericMd (used for medium numeric displays like counters/badges), and `@param`
numericXl (used for extra-large numeric displays), placing these lines alongside
the existing `@param` tags in the class KDoc so all public properties
(captionDefault, headingSm, numericMd, numericXl) are documented consistently.
In
`@stream-chat-android-compose/src/main/res/drawable/stream_compose_ic_attachment_file_picker.xml`:
- Around line 25-30: The drawable uses a fixed theme color (`#1A1B25`) which
prevents runtime tinting; in stream_compose_ic_attachment_file_picker.xml update
the path's strokeColor attribute (and any other hardcoded stroke/fill colors in
this drawable) to the standard placeholder color (use solid black "#000000") so
composables can apply tint at runtime; specifically change
android:strokeColor="#1A1B25" to android:strokeColor="#000000" (leave
android:fillColor as-is) to match other stream_compose_ic_* icons.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCameraPickerTest.kt`:
- Around line 34-38: The test function named default in
AttachmentCameraPickerTest should be renamed to a descriptive backticked test
name (e.g., `camera picker default snapshot`) to match test style; update the
function declaration fun default() to fun `your descriptive name`() inside
AttachmentCameraPickerTest and keep the body invoking snapshotWithDarkMode {
AttachmentCameraPicker() }, ensuring any references to the test function (if
any) are updated accordingly.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePickerTest.kt`:
- Around line 34-39: Rename the test function in AttachmentTypePickerTest from
fun default() to a backticked name fun `default`() to follow the project's
test-naming convention; update the declaration of the test function (the one
currently named default) to use backticks and ensure any usages or references
(e.g., in test runners or IDE) still resolve correctly.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.kt`:
- Around line 146-156: The test creates a fresh mock in its verifications so the
assertions never hit the real mocked instance; update the verifications to
assert on the mock instance used by the view model (storageHelper) instead of
creating a new one. Specifically, in the `When changing picker mode Should not
load attachments` test that constructs
`AttachmentsPickerViewModel(storageHelper, channelState)` and calls
`changeAttachmentPickerMode(Files)`, replace
`verify(mock<StorageHelperWrapper>(), never()).getFiles()` and
`verify(mock<StorageHelperWrapper>(), never()).getMedia()` with
`verify(storageHelper, never()).getFiles()` and `verify(storageHelper,
never()).getMedia()` so the test checks the actual mock passed into the
`AttachmentsPickerViewModel`.
🧹 Nitpick comments (26)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/FullscreenDialog.kt (2)
41-42: Consider defensive casting forDialogWindowProvider.The unchecked cast assumes
view.parentis always aDialogWindowProvider. While this holds true for the current ComposeDialogimplementation, it relies on internal behavior that could change. A safe cast would be more defensive:♻️ Proposed defensive casting
val view = LocalView.current - val dialogWindow = (view.parent as DialogWindowProvider).window + val dialogWindow = (view.parent as? DialogWindowProvider)?.window ?: return@Dialog
27-59: Consider adding a@StreamPreviewfor development convenience.While this is an internal utility, a preview function would help visualize the fullscreen dialog behavior during development. Based on learnings, Compose previews in this module should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt (2)
170-200: Missing KDoc for new public properties.The new color properties (
backgroundCoreSurfaceSubtle,borderCoreOnAccent,badgeText,badgeBgInverse,stateBgDisabled,stateTextDisabled,backgroundCoreSelected) are public API but lack corresponding@paramentries in the class-level KDoc (lines 26-124). Other properties in this class are documented.As per coding guidelines, public APIs should be documented with KDoc.
358-392: Ordering inconsistency:backgroundCoreSurfaceSubtleis misplaced.In
defaultColors(), the semantic color assignments follow a logical/alphabetical grouping wherebackgroundCoreSurfaceSubtlecomes afterbackgroundCoreSurface(line 273). Here indefaultDarkColors(), it's placed betweenaccentErrorandaccentNeutral, breaking the consistent ordering.Consider moving it after
backgroundCoreSurface(line 364) for consistency withdefaultColors().♻️ Suggested reordering
accentError = StreamPrimitiveColors.red400, - backgroundCoreSurfaceSubtle = StreamPrimitiveColors.neutral800, accentNeutral = StreamPrimitiveColors.neutral500, accentPrimary = StreamPrimitiveColors.blue400, accentSuccess = StreamPrimitiveColors.green400, backgroundCoreDisabled = StreamPrimitiveColors.slate800, backgroundCoreSurface = StreamPrimitiveColors.neutral700, + backgroundCoreSurfaceSubtle = StreamPrimitiveColors.neutral800, backgroundElevationElevation0 = StreamPrimitiveColors.baseBlack,stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPollPicker.kt (1)
114-124: Consider using@StreamPreviewhelper for consistency.The preview uses
@Previewdirectly. Other compose previews in the project use@StreamPreviewhelpers for consistency.Also, consider moving the no-arg overload (lines 122-124) before the preview for better code organization—production code typically precedes preview code.
Based on learnings: "Compose previews should use
StreamPreviewhelpers".stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPollPickerTest.kt (1)
34-39: Use backtick test name for consistency.Per coding guidelines, test methods should use backtick naming for readability.
♻️ Suggested naming
`@Test` - fun default() { + fun `default state`() { snapshotWithDarkMode { AttachmentPollPicker() } }As per coding guidelines: "Use backtick test names (for example:
fun \message list filters muted channels`()`) for readability".stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/attachments/images/ImagesPicker.kt (1)
284-298: Consider using@StreamPreviewhelpers for preview composables.The preview functions use the standard
@Previewannotation. As per coding guidelines, Compose previews in this module should use@StreamPreviewhelpers for consistency across the codebase.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/RequiredPermission.kt (2)
133-152: Consider using@StreamPreviewhelpers for preview composables.The preview functions use the standard
@Previewannotation. As per coding guidelines, Compose previews in this module should use@StreamPreviewhelpers.
60-72: Hardcoded system settings navigation in composable.
RequiredCameraPermissiondirectly invokescontext::openSystemSettingsas the click handler. This couples the UI to a specific navigation action, making the component less flexible and harder to test. Consider accepting the click handler as a parameter (likeRequiredStoragePermissiondoes) for consistency.♻️ Proposed refactor for consistency
`@Composable` internal fun RequiredCameraPermission( modifier: Modifier = Modifier, + onGrantPermissionClick: () -> Unit, ) { - val context = LocalContext.current RequiredPermission( modifier = modifier, icon = R.drawable.stream_compose_ic_attachment_camera_picker, title = R.string.stream_ui_message_composer_permission_camera_title, message = R.string.stream_ui_message_composer_permission_camera_message, - onGrantPermissionClick = context::openSystemSettings, + onGrantPermissionClick = onGrantPermissionClick, ) }Then update the call site to provide the action:
val context = LocalContext.current RequiredCameraPermission( onGrantPermissionClick = context::openSystemSettings, )stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/poll/CreatePollScreen.kt (1)
158-164: UseChatPreviewThemefor preview consistency.Other picker previews in this PR (e.g.,
AttachmentFilePickerPreview,AttachmentMediaPickerPreview) useChatPreviewTheme, which initializesChatClientbefore wrapping content inChatTheme. Consider usingChatPreviewThemehere for consistency and to ensureChatClientis properly initialized.Proposed fix
`@Preview` `@Composable` private fun CreatePollScreenPreview() { - ChatTheme { + ChatPreviewTheme { CreatePollScreen() } }stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentFilePicker.kt (1)
137-145: Consider extracting duplicatedshowErrorIfNeededto a shared utility.This function is identical to the one in
AttachmentsPickerSystemTabFactory.kt(lines 285-293). Consider extracting it to a shared location (e.g., a utility file) to avoid duplication.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/factory/AttachmentsPickerPollTabFactory.kt (1)
91-102: Consider using@StreamPreviewhelper for Compose previews.The preview uses standard
@Previewannotations. As per coding guidelines, Compose previews in this module should use@StreamPreviewhelpers for consistency across the codebase.stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCommandPickerTest.kt (1)
34-39: Consider using backtick test naming for readability.As per coding guidelines, test methods should use backtick names for better readability (e.g.,
fun `default snapshot renders correctly`()).Suggested change
`@Test` - fun default() { + fun `default snapshot renders correctly`() { snapshotWithDarkMode { AttachmentCommandPicker() } }stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentSystemPickerTest.kt (1)
35-42: Consider using backtick test naming for readability.The test correctly wraps
AttachmentSystemPickerinViewModelStoreto provide the requiredLocalViewModelStoreOwner. As per coding guidelines, consider using backtick naming for better readability.Suggested change
`@Test` - fun default() { + fun `default snapshot renders correctly`() { snapshotWithDarkMode { ViewModelStore { AttachmentSystemPicker() } } }stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentSystemPicker.kt (2)
75-76: Document or remove theLongMethod/MagicNumbersuppressions.
If you need the suppressions, add a short rationale; otherwise, drop them after refactoring.♻️ Suggested update
-@Suppress("LongMethod") +// Suppressed while picker orchestration remains in a single composable; consider extracting helpers. +@Suppress("LongMethod") @@ -@Suppress("MagicNumber")As per coding guidelines, **/*.kt: Use
@OptInannotations explicitly; avoid suppressions unless documented.Also applies to: 266-266
258-263: Use@StreamPreviewfor Compose previews.♻️ Suggested update
-@Preview(showBackground = true) +@StreamPreviewAs per coding guidelines, Compose previews should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCommandPicker.kt (1)
51-56: Switch preview to@StreamPreview.♻️ Suggested update
-@Preview(showBackground = true) +@StreamPreviewAs per coding guidelines, Compose previews should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerMenu.kt (1)
43-55: Add thread/state notes to the public KDoc.
The new public composable’s KDoc doesn’t mention thread expectations or state behavior.As per coding guidelines: Document public APIs with KDoc, including thread expectations and state notes.📝 Suggested KDoc addition
/** * An attachments picker menu that expands and collapses. * * `@param` attachmentsPickerViewModel The [AttachmentsPickerViewModel] used to read state and * perform actions. * `@param` composerViewModel The [MessageComposerViewModel] used to read state and * perform actions. + * + * `@note` This composable reads and mutates picker/composer state and should be invoked + * from the main (UI) thread in a Compose context. */stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerContent.kt (1)
33-42: Avoid@Suppress("LongParameterList") by grouping callbacks.
Consider a small parameter holder (e.g., callbacks/state data class) or document why the suppression is required, so the complexity is explicit. As per coding guidelines, Use@OptInannotations explicitly; avoid suppressions unless documented.stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/attachments/factory/AttachmentsPickerTabFactoriesTest.kt (1)
27-29: Use backtick test names for readability.
Consider renaming these tests to backtick-style names for consistency with the project convention. As per coding guidelines, Use backtick test names (for example:funmessage list filters muted channels()) for readability.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPicker.kt (2)
55-67: Consider extracting sections instead of@Suppress("LongMethod").
Splitting the system-picker and custom-picker branches into private composables would reduce size and make the suppression unnecessary. As per coding guidelines, Use@OptInannotations explicitly; avoid suppressions unless documented.
43-54: KDoc should document the new callbacks.
Public API docs are missingonTabClickandonAttachmentItemSelected; also consider noting any callback threading expectations if applicable. As per coding guidelines, Document public APIs with KDoc, including thread expectations and state notes.📌 Suggested KDoc additions
/** * Represents the bottom bar UI that allows users to pick attachments. * * `@param` attachmentsPickerViewModel ViewModel that loads the images or files and persists which * `@param` modifier Modifier for styling. * `@param` shape The shape of the bottom bar. * `@param` messageMode The message mode, used to determine if the default "Polls" tab is enabled. * items have been selected. + * `@param` onTabClick Handler invoked when a picker tab is tapped. + * `@param` onAttachmentItemSelected Handler when an attachment item selection changes. * `@param` onAttachmentsSelected Handler when attachments are selected and confirmed by the user. * `@param` onAttachmentPickerAction A lambda that will be invoked when an action is happened. * `@param` onDismiss Handler when the user dismisses the UI. */Also applies to: 62-65
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/suggestions/commands/CommandSuggestionList.kt (1)
113-118: Switch previews to@StreamPreview.
Please use the StreamPreview helpers for Compose previews in this file. As per coding guidelines, Compose previews should use@StreamPreviewhelpers.Also applies to: 121-130
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/composer/MessageComposer.kt (1)
538-589: Use@StreamPreviewfor the new previews.
Replace@Previewwith StreamPreview for the new attachment-picker visibility previews to match the project convention. As per coding guidelines, Compose previews should use@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.kt (2)
132-152: Consider a more robust matching strategy for attachment removal.The matching logic on lines 134-136 compares
attachmentMetaData.titlewithattachment.name, with a comment indicating this relies on how attachments are created. This coupling is fragile—if the attachment creation changes, this sync will break silently.Consider adding a unique identifier to
AttachmentPickerItemStatefor more reliable matching, or at minimum, logging/asserting when a match isn't found for debugging purposes.
143-150: Duplicated selection reordering logic.The logic for decrementing selection counts when an item is removed/deselected is duplicated between
removeSelectedAttachment(lines 143-150) andchangeSelectedAttachments(lines 166-173). Consider extracting this into a private helper function to reduce duplication.♻️ Suggested extraction
private fun updateSelectionsOnRemoval( attachments: List<AttachmentPickerItemState>, itemIndex: Int, removedCount: Int, ): List<AttachmentPickerItemState> { return attachments.mapIndexed { index, item -> when { index == itemIndex -> item.copy(selection = Selection.Unselected) item.selection is Selection.Selected && item.selection.count > removedCount -> item.copy(selection = Selection.Selected(count = item.selection.count - 1)) else -> item } } }Also applies to: 166-173
stream-chat-android-compose/api/stream-chat-android-compose.api
Outdated
Show resolved
Hide resolved
...o/getstream/chat/android/compose/ui/components/suggestions/commands/CommandSuggestionItem.kt
Outdated
Show resolved
Hide resolved
...ain/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCameraPicker.kt
Show resolved
Hide resolved
...in/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerContent.kt
Show resolved
Hide resolved
...android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt
Outdated
Show resolved
Hide resolved
stream-chat-android-compose/src/main/res/drawable/stream_compose_ic_attachment_file_picker.xml
Show resolved
Hide resolved
...tlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCameraPickerTest.kt
Show resolved
Hide resolved
...kotlin/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePickerTest.kt
Show resolved
Hide resolved
...otlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.kt
Show resolved
Hide resolved
The `AttachmentPickerCommandClickClick` action has been renamed to `AttachmentPickerCommandSelect` to fix a typo and better reflect the action of selecting a command.
gpunto
left a comment
gpunto
left a comment
There was a problem hiding this comment.
Great work!
I left some remarks/questions and, also, while testing I noticed that the media attachment picker doesn't close after sending the message
...va/io/getstream/chat/android/compose/state/messages/attachments/AttachmentPickerItemState.kt
Outdated
Show resolved
Hide resolved
...ain/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentCameraPicker.kt
Show resolved
Hide resolved
.../main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentFilePicker.kt
Show resolved
Hide resolved
...main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentMediaPicker.kt
Show resolved
Hide resolved
.../src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPicker.kt
Outdated
Show resolved
Hide resolved
...in/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerContent.kt
Show resolved
Hide resolved
.../main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPollPicker.kt
Show resolved
Hide resolved
.../main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePicker.kt
Show resolved
Hide resolved
.../main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePicker.kt
Show resolved
Hide resolved
...android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt
Outdated
Show resolved
Hide resolved
The `AttachmentPollPicker` now immediately shows the poll creation dialog when it's first displayed, rather than waiting for the user to click the "Create Poll" button.
andremion
force-pushed
the
redesign/AND-1030-message-composer-actions-menu-redesign
branch
from
165795b to
41e1338
Compare
|
andremion
deleted the
redesign/AND-1030-message-composer-actions-menu-redesign
branch
2 participants
🎯 Goal
Redesign and refactor the Compose attachment picker and related composer UI for the new actions menu flow.
https://www.figma.com/design/Us73erK1xFNcB5EH3hyq6Y/Chat-SDK-Design-System?node-id=2797-80088&m=dev
🛠 Implementation details
Important
The API is not final and will be revised in a follow-up PR. That PR will introduce component factories to support the sample location picker and will also remove the legacy tab factories.
🎨 UI Changes
system.picker.webm
custom.picker.webm
🧪 Testing
useDefaultSystemMediaPicker = false, and add attachments/polls/commands, with and without permissions, checking the permissions flow.useDefaultSystemMediaPicker = true, and add attachments/polls/commands.Note
To check the camera permission flow, you have to declare
<uses-permission android:name="android.permission.CAMERA" />in the sample app manifestSummary by CodeRabbit
New Features
Bug Fixes
Style