From 319f426b064868ba338bbc572dc63707ed1a16b0 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 22 May 2023 21:24:43 +0200 Subject: [PATCH] Media: align attachement source picker design with Figma --- .../messages/impl/MessagesStateProvider.kt | 4 +- .../features/messages/impl/MessagesView.kt | 74 ++++++++----------- .../messagecomposer/MessageComposerEvents.kt | 7 +- .../MessageComposerPresenter.kt | 23 +++--- .../messagecomposer/MessageComposerState.kt | 7 +- .../MessageComposerStateProvider.kt | 2 +- .../MessageComposerPresenterTest.kt | 7 +- 7 files changed, 48 insertions(+), 76 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt index d235232560..1919de78f0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt @@ -18,7 +18,6 @@ package io.element.android.features.messages.impl import androidx.compose.ui.tooling.preview.PreviewParameterProvider import io.element.android.features.messages.impl.actionlist.anActionListState -import io.element.android.features.messages.impl.messagecomposer.AttachmentSourcePicker import io.element.android.features.messages.impl.messagecomposer.aMessageComposerState import io.element.android.features.messages.impl.timeline.aTimelineItemContent import io.element.android.features.messages.impl.timeline.aTimelineItemList @@ -33,8 +32,7 @@ open class MessagesStateProvider : PreviewParameterProvider { get() = sequenceOf( aMessagesState(), aMessagesState().copy(hasNetworkConnection = false), - aMessagesState().copy(composerState = aMessageComposerState().copy(attachmentSourcePicker = AttachmentSourcePicker.AllMedia)), - aMessagesState().copy(composerState = aMessageComposerState().copy(attachmentSourcePicker = AttachmentSourcePicker.Camera)), + aMessagesState().copy(composerState = aMessageComposerState().copy(showAttachmentSourcePicker = true)), ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index a3b64db002..be38b7a59c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -37,6 +37,11 @@ import androidx.compose.material.ListItem import androidx.compose.material.ModalBottomSheetValue import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowBack +import androidx.compose.material.icons.filled.AttachFile +import androidx.compose.material.icons.filled.Collections +import androidx.compose.material.icons.filled.LocalLibrary +import androidx.compose.material.icons.filled.PhotoCamera +import androidx.compose.material.icons.filled.Videocam import androidx.compose.material.rememberModalBottomSheetState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.SnackbarHost @@ -61,7 +66,6 @@ import io.element.android.features.messages.impl.actionlist.ActionListEvents import io.element.android.features.messages.impl.actionlist.ActionListView import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction import io.element.android.features.messages.impl.attachments.Attachment -import io.element.android.features.messages.impl.messagecomposer.AttachmentSourcePicker import io.element.android.features.messages.impl.messagecomposer.AttachmentsState import io.element.android.features.messages.impl.messagecomposer.MessageComposerEvents import io.element.android.features.messages.impl.messagecomposer.MessageComposerView @@ -102,7 +106,7 @@ fun MessagesView( initialValue = ModalBottomSheetValue.Hidden, ) val composerState = state.composerState - val initialBottomSheetState = if (LocalInspectionMode.current && composerState.attachmentSourcePicker != null) { + val initialBottomSheetState = if (LocalInspectionMode.current && composerState.showAttachmentSourcePicker != null) { ModalBottomSheetValue.Expanded } else { ModalBottomSheetValue.Hidden @@ -154,8 +158,8 @@ fun MessagesView( state.eventSink(MessagesEvents.HandleAction(action, event)) } - LaunchedEffect(composerState.attachmentSourcePicker) { - if (composerState.attachmentSourcePicker != null) { + LaunchedEffect(composerState.showAttachmentSourcePicker) { + if (composerState.showAttachmentSourcePicker) { // We need to use this instead of `LocalFocusManager.clearFocus()` to hide the keyboard when focus is on an Android View localView.hideKeyboard() bottomSheetState.show() @@ -173,8 +177,7 @@ fun MessagesView( sheetState = bottomSheetState, displayHandle = true, sheetContent = { - MediaPickerMenu( - addAttachmentSourcePicker = composerState.attachmentSourcePicker, + AttachmentSourcePickerMenu( eventSink = composerState.eventSink ) } @@ -305,50 +308,33 @@ fun MessagesViewTopBar( ) } -@Composable -internal fun MediaPickerMenu( - addAttachmentSourcePicker: AttachmentSourcePicker?, - eventSink: (MessageComposerEvents) -> Unit, -) { - when (addAttachmentSourcePicker) { - null -> return - AttachmentSourcePicker.AllMedia -> AllMediaSourcePickerMenu(eventSink = eventSink) - AttachmentSourcePicker.Camera -> CameraSourcePickerMenu(eventSink = eventSink) - } -} - @OptIn(ExperimentalMaterialApi::class) @Composable -internal fun AllMediaSourcePickerMenu( +internal fun AttachmentSourcePickerMenu( eventSink: (MessageComposerEvents) -> Unit, modifier: Modifier = Modifier, ) { Column(modifier) { - ListItem(Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) }) { - Text(stringResource(R.string.screen_room_attachment_source_gallery)) - } - ListItem(Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) }) { - Text(stringResource(R.string.screen_room_attachment_source_files)) - } - ListItem(Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromCamera) }) { - Text(stringResource(R.string.screen_room_attachment_source_camera)) - } - } -} - -@OptIn(ExperimentalMaterialApi::class) -@Composable -internal fun CameraSourcePickerMenu( - eventSink: (MessageComposerEvents) -> Unit, - modifier: Modifier = Modifier, -) { - Column(modifier) { - ListItem(Modifier.clickable { eventSink(MessageComposerEvents.PickCameraAttachmentSource.Photo) }) { - Text(stringResource(R.string.screen_room_attachment_source_camera_photo)) - } - ListItem(Modifier.clickable { eventSink(MessageComposerEvents.PickCameraAttachmentSource.Video) }) { - Text(stringResource(R.string.screen_room_attachment_source_camera_video)) - } + ListItem( + modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) }, + icon = { Icon(Icons.Default.Collections, null) }, + text = { Text(stringResource(R.string.screen_room_attachment_source_gallery)) }, + ) + ListItem( + modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) }, + icon = { Icon(Icons.Default.AttachFile, null) }, + text = { Text(stringResource(R.string.screen_room_attachment_source_files)) }, + ) + ListItem( + modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.PhotoFromCamera) }, + icon = { Icon(Icons.Default.PhotoCamera, null) }, + text = { Text(stringResource(R.string.screen_room_attachment_source_camera_photo)) }, + ) + ListItem( + modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.VideoFromCamera) }, + icon = { Icon(Icons.Default.Videocam, null) }, + text = { Text(stringResource(R.string.screen_room_attachment_source_camera_video)) }, + ) } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt index 92ce191ce1..155e981fcc 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt @@ -30,11 +30,8 @@ sealed interface MessageComposerEvents { object DismissAttachmentMenu : MessageComposerEvents sealed interface PickAttachmentSource : MessageComposerEvents { object FromGallery : PickAttachmentSource - object FromCamera : PickAttachmentSource object FromFiles : PickAttachmentSource - } - sealed interface PickCameraAttachmentSource : MessageComposerEvents { - object Photo : PickCameraAttachmentSource - object Video : PickCameraAttachmentSource + object PhotoFromCamera : PickAttachmentSource + object VideoFromCamera : PickAttachmentSource } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 1b1bebf209..0a93834231 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -107,7 +107,7 @@ class MessageComposerPresenter @Inject constructor( mutableStateOf(MessageComposerMode.Normal("")) } - var attachmentSourcePicker: AttachmentSourcePicker? by remember { mutableStateOf(null) } + var showAttachmentSourcePicker: Boolean by remember { mutableStateOf(false) } LaunchedEffect(composerMode.value) { when (val modeValue = composerMode.value) { @@ -135,26 +135,23 @@ class MessageComposerPresenter @Inject constructor( is MessageComposerEvents.SendMessage -> appCoroutineScope.sendMessage(event.message, composerMode, text) is MessageComposerEvents.SetMode -> composerMode.value = event.composerMode MessageComposerEvents.AddAttachment -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = AttachmentSourcePicker.AllMedia + showAttachmentSourcePicker = true } - MessageComposerEvents.DismissAttachmentMenu -> attachmentSourcePicker = null + MessageComposerEvents.DismissAttachmentMenu -> showAttachmentSourcePicker = false MessageComposerEvents.PickAttachmentSource.FromGallery -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = null + showAttachmentSourcePicker = false galleryMediaPicker.launch() } MessageComposerEvents.PickAttachmentSource.FromFiles -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = null + showAttachmentSourcePicker = false filesPicker.launch() } - MessageComposerEvents.PickAttachmentSource.FromCamera -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = AttachmentSourcePicker.Camera - } - MessageComposerEvents.PickCameraAttachmentSource.Photo -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = null + MessageComposerEvents.PickAttachmentSource.PhotoFromCamera -> localCoroutineScope.ifMediaPickersEnabled { + showAttachmentSourcePicker = false cameraPhotoPicker.launch() } - MessageComposerEvents.PickCameraAttachmentSource.Video -> localCoroutineScope.ifMediaPickersEnabled { - attachmentSourcePicker = null + MessageComposerEvents.PickAttachmentSource.VideoFromCamera -> localCoroutineScope.ifMediaPickersEnabled { + showAttachmentSourcePicker = false cameraVideoPicker.launch() } } @@ -164,7 +161,7 @@ class MessageComposerPresenter @Inject constructor( text = text.value, isFullScreen = isFullScreen.value, mode = composerMode.value, - attachmentSourcePicker = attachmentSourcePicker, + showAttachmentSourcePicker = showAttachmentSourcePicker, attachmentsState = attachmentsState.value, eventSink = ::handleEvents ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt index 12965b7e55..9c6a2c650a 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt @@ -27,7 +27,7 @@ data class MessageComposerState( val text: StableCharSequence?, val isFullScreen: Boolean, val mode: MessageComposerMode, - val attachmentSourcePicker: AttachmentSourcePicker?, + val showAttachmentSourcePicker: Boolean, val attachmentsState: AttachmentsState, val eventSink: (MessageComposerEvents) -> Unit ) { @@ -40,8 +40,3 @@ sealed interface AttachmentsState { data class Previewing(val attachments: ImmutableList) : AttachmentsState data class Sending(val attachments: ImmutableList) : AttachmentsState } - -sealed interface AttachmentSourcePicker { - object AllMedia : AttachmentSourcePicker - object Camera : AttachmentSourcePicker -} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt index b55a168814..56d050e7b1 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt @@ -31,7 +31,7 @@ fun aMessageComposerState() = MessageComposerState( text = StableCharSequence(""), isFullScreen = false, mode = MessageComposerMode.Normal(content = ""), - attachmentSourcePicker = null, + showAttachmentSourcePicker = false, attachmentsState = AttachmentsState.None, eventSink = {} ) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt index 1f6f172c25..65740b6485 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt @@ -44,7 +44,6 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import io.element.android.libraries.mediapickers.api.PickerProvider import io.element.android.libraries.mediapickers.test.FakePickerProvider import io.element.android.libraries.mediaupload.api.MediaPreProcessor -import io.element.android.libraries.mediaupload.api.MediaSender import io.element.android.libraries.mediaupload.api.MediaUploadInfo import io.element.android.libraries.mediaupload.api.ThumbnailProcessingInfo import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor @@ -259,7 +258,7 @@ class MessageComposerPresenterTest { val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.AddAttachment) - assertThat(awaitItem().attachmentSourcePicker).isEqualTo(AttachmentSourcePicker.AllMedia) + assertThat(awaitItem().showAttachmentSourcePicker).isEqualTo(AttachmentSourcePicker.AllMedia) } } @@ -272,7 +271,7 @@ class MessageComposerPresenterTest { val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromCamera) - assertThat(awaitItem().attachmentSourcePicker).isEqualTo(AttachmentSourcePicker.Camera) + assertThat(awaitItem().showAttachmentSourcePicker).isEqualTo(AttachmentSourcePicker.Camera) } } @@ -287,7 +286,7 @@ class MessageComposerPresenterTest { skipItems(1) initialState.eventSink(MessageComposerEvents.DismissAttachmentMenu) - assertThat(awaitItem().attachmentSourcePicker).isNull() + assertThat(awaitItem().showAttachmentSourcePicker).isNull() } }