knock requests : refine and clean banner

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/DefaultKnockRequestsBannerRenderer.kt

@@ -17,16 +17,13 @@ import javax.inject.Inject
 @ContributesBinding(RoomScope::class)
 class DefaultKnockRequestsBannerRenderer @Inject constructor(
     private val presenter: KnockRequestsBannerPresenter,
-): KnockRequestsBannerRenderer {
+) : KnockRequestsBannerRenderer {
-
     @Composable
     override fun View(modifier: Modifier, onViewRequestsClick: () -> Unit) {
         val state = presenter.present()
         KnockRequestsBannerView(
             state = state,
-            onDismissClick = {},
             onViewRequestsClick = onViewRequestsClick,
         )
     }
-
 }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerEvents.kt

@@ -0,0 +1,15 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.knockrequests.impl.banner
+
+import io.element.android.features.knockrequests.impl.KnockRequest
+
+sealed interface KnockRequestsBannerEvents {
+    data class Accept(val knockRequest: KnockRequest) : KnockRequestsBannerEvents
+    data object Dismiss : KnockRequestsBannerEvents
+}

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerPresenter.kt

@@ -8,12 +8,35 @@
 package io.element.android.features.knockrequests.impl.banner
 
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
+import kotlinx.collections.immutable.persistentListOf
 import javax.inject.Inject
 
-class KnockRequestsBannerPresenter @Inject constructor(): Presenter<KnockRequestsBannerState> {
+class KnockRequestsBannerPresenter @Inject constructor() : Presenter<KnockRequestsBannerState> {
     @Composable
     override fun present(): KnockRequestsBannerState {
-        return KnockRequestsBannerState.Hidden
+        var shouldShowBanner by remember { mutableStateOf(false) }
+
+        fun handleEvents(event: KnockRequestsBannerEvents) {
+            when (event) {
+                is KnockRequestsBannerEvents.Accept -> Unit
+                is KnockRequestsBannerEvents.Dismiss -> {
+                    shouldShowBanner = false
+                }
+            }
+        }
+
+        return KnockRequestsBannerState(
+            knockRequests = persistentListOf(),
+            acceptAction = AsyncAction.Uninitialized,
+            canAccept = false,
+            isVisible = shouldShowBanner,
+            eventSink = ::handleEvents,
+        )
     }
 }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerState.kt

@@ -8,7 +8,6 @@
 package io.element.android.features.knockrequests.impl.banner
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.Immutable
 import androidx.compose.ui.res.pluralStringResource
 import androidx.compose.ui.res.stringResource
 import io.element.android.features.knockrequests.impl.KnockRequest
@@ -18,42 +17,39 @@ import io.element.android.libraries.ui.strings.CommonPlurals
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.collections.immutable.ImmutableList
 
-@Immutable
+data class KnockRequestsBannerState(
-sealed interface KnockRequestsBannerState {
+    val isVisible: Boolean,
-    data object Hidden : KnockRequestsBannerState
+    val knockRequests: ImmutableList<KnockRequest>,
-    data class Visible(
+    val acceptAction: AsyncAction<Unit>,
-        val knockRequests: ImmutableList<KnockRequest>,
+    val canAccept: Boolean,
-        val acceptAction: AsyncAction<Unit>,
+    val eventSink: (KnockRequestsBannerEvents) -> Unit,
-        val canAccept: Boolean,
+) {
-    ) : KnockRequestsBannerState {
+    val subtitle = if (knockRequests.size == 1) {
+        knockRequests.first().userId.value
+    } else {
+        null
+    }
 
-        val subtitle = if (knockRequests.size == 1) {
+    val reason = if (knockRequests.size == 1) {
-            knockRequests.first().userId.value
+        knockRequests.first().reason
-        } else {
+    } else {
-            null
+        null
-        }
+    }
 
-        val reason = if (knockRequests.size == 1) {
+    @Composable
-            knockRequests.first().reason
+    fun formattedTitle(): String {
-        } else {
+        return when (knockRequests.size) {
-            null
+            0 -> ""
-        }
+            1 -> stringResource(CommonStrings.screen_room_single_knock_request_title, knockRequests.first().getBestName())
-
+            else -> {
-        @Composable
+                val firstRequest = knockRequests.first()
-        fun formattedTitle(): String {
+                val otherRequestsCount = knockRequests.size - 1
-            return when (knockRequests.size) {
+                pluralStringResource(
-                0 -> ""
+                    id = CommonPlurals.screen_room_multiple_knock_requests_title,
-                1 -> stringResource(CommonStrings.screen_room_single_knock_request_title, knockRequests.first().getBestName())
+                    count = otherRequestsCount,
-                else -> {
+                    firstRequest.getBestName(),
-                    val firstRequest = knockRequests.first()
+                    otherRequestsCount
-                    val otherRequestsCount = knockRequests.size - 1
+                )
-                    pluralStringResource(
-                        id = CommonPlurals.screen_room_multiple_knock_requests_title,
-                        count = otherRequestsCount,
-                        firstRequest.getBestName(),
-                        otherRequestsCount
-                    )
-                }
             }
         }
     }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerStateProvider.kt

@@ -16,15 +16,23 @@ import kotlinx.collections.immutable.toImmutableList
 class KnockRequestsBannerStateProvider : PreviewParameterProvider<KnockRequestsBannerState> {
     override val values: Sequence<KnockRequestsBannerState>
         get() = sequenceOf(
-            KnockRequestsBannerState.Hidden,
+            aKnockRequestsBannerState(),
-            aVisibleKnockRequestsBannerState(),
+            aKnockRequestsBannerState(
-            aVisibleKnockRequestsBannerState(
+                knockRequests = listOf(
+                    aKnockRequest(
+                        reason = "A very long reason that should probably be truncated, " +
+                            "but could be also expanded so you can see it over the lines, wow," +
+                            "very amazing reason, I know, right, I'm so good at writing reasons."
+                    )
+                )
+            ),
+            aKnockRequestsBannerState(
                 knockRequests = listOf(
                     aKnockRequest(),
                     aKnockRequest(displayName = "Alice")
                 )
             ),
-            aVisibleKnockRequestsBannerState(
+            aKnockRequestsBannerState(
                 knockRequests = listOf(
                     aKnockRequest(),
                     aKnockRequest(displayName = "Alice"),
@@ -32,24 +40,28 @@ class KnockRequestsBannerStateProvider : PreviewParameterProvider<KnockRequestsB
                     aKnockRequest(displayName = "Charlie")
                 )
             ),
-            aVisibleKnockRequestsBannerState(
+            aKnockRequestsBannerState(
                 canAccept = false
             ),
-            aVisibleKnockRequestsBannerState(
+            aKnockRequestsBannerState(
                 acceptAction = AsyncAction.Loading
             ),
-            aVisibleKnockRequestsBannerState(
+            aKnockRequestsBannerState(
-                acceptAction = AsyncAction.Failure(Throwable())
+                acceptAction = AsyncAction.Failure(Throwable("Failed to accept knock"))
             ),
         )
 }
 
-fun aVisibleKnockRequestsBannerState(
+fun aKnockRequestsBannerState(
     knockRequests: List<KnockRequest> = listOf(aKnockRequest()),
     acceptAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
     canAccept: Boolean = true,
-) = KnockRequestsBannerState.Visible(
+    isVisible: Boolean = true,
+    eventSink: (KnockRequestsBannerEvents) -> Unit = {}
+) = KnockRequestsBannerState(
     knockRequests = knockRequests.toImmutableList(),
     acceptAction = acceptAction,
-    canAccept = canAccept
+    canAccept = canAccept,
+    isVisible = isVisible,
+    eventSink = eventSink,
 )

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerView.kt

@@ -7,6 +7,9 @@
 
 package io.element.android.features.knockrequests.impl.banner
 
+import androidx.compose.animation.AnimatedVisibility
+import androidx.compose.animation.expandVertically
+import androidx.compose.animation.shrinkVertically
 import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Arrangement
@@ -27,6 +30,7 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.style.TextAlign
+import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.zIndex
@@ -52,96 +56,102 @@ private const val MAX_AVATAR_COUNT = 3
 @Composable
 fun KnockRequestsBannerView(
     state: KnockRequestsBannerState,
-    onDismissClick: () -> Unit,
     onViewRequestsClick: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    when (state) {
+    AnimatedVisibility(
-        is KnockRequestsBannerState.Hidden -> Unit
+        visible = state.isVisible,
-        is KnockRequestsBannerState.Visible -> VisibleKnockRequestsBannerView(
+        enter = expandVertically(),
-            state = state,
+        exit = shrinkVertically(),
-            onDismissClick = onDismissClick,
+        modifier = modifier,
-            onViewRequestsClick = onViewRequestsClick,
+    ) {
-            modifier = modifier
+        Surface(
-        )
+            shape = MaterialTheme.shapes.small,
+            color = ElementTheme.colors.bgCanvasDefaultLevel1,
+            shadowElevation = 24.dp,
+            modifier = Modifier.padding(16.dp),
+        ) {
+            KnockRequestsBannerContent(
+                state = state,
+                onViewRequestsClick = onViewRequestsClick,
+            )
+        }
     }
 }
 
 @Composable
-private fun VisibleKnockRequestsBannerView(
+private fun KnockRequestsBannerContent(
-    state: KnockRequestsBannerState.Visible,
+    state: KnockRequestsBannerState,
-    onDismissClick: () -> Unit,
     onViewRequestsClick: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    Surface(
+    fun onDismissClick() {
-        modifier.fillMaxWidth(),
+        state.eventSink(KnockRequestsBannerEvents.Dismiss)
-        shape = MaterialTheme.shapes.small,
+    }
-        color = ElementTheme.colors.bgCanvasDefaultLevel1,
+
-        shadowElevation = 24.dp
+    Column(
+        modifier
+            .fillMaxWidth()
+            .padding(all = 16.dp)
     ) {
-        Column(
+        Row {
-                Modifier
+            KnockRequestAvatarView(state.knockRequests)
-                        .fillMaxWidth()
+            Spacer(modifier = Modifier.width(10.dp))
-                        .padding(all = 16.dp)
+            Column(modifier = Modifier.weight(1f)) {
-        ) {
+                Text(
-            Row {
+                    text = state.formattedTitle(),
-                KnockRequestAvatarView(state.knockRequests)
+                    style = ElementTheme.typography.fontBodyMdMedium,
-                Spacer(modifier = Modifier.width(10.dp))
+                    color = MaterialTheme.colorScheme.primary,
-                Column(modifier = Modifier.weight(1f)) {
+                    textAlign = TextAlign.Start,
+                )
+                if (state.subtitle != null) {
                     Text(
-                        text = state.formattedTitle(),
+                        text = state.subtitle,
-                        style = ElementTheme.typography.fontBodyMdMedium,
+                        style = ElementTheme.typography.fontBodySmRegular,
-                        color = MaterialTheme.colorScheme.primary,
+                        color = MaterialTheme.colorScheme.secondary,
                         textAlign = TextAlign.Start,
                     )
-                    if (state.subtitle != null) {
-                        Text(
-                            text = state.subtitle,
-                            style = ElementTheme.typography.fontBodySmRegular,
-                            color = MaterialTheme.colorScheme.secondary,
-                            textAlign = TextAlign.Start,
-                        )
-                    }
                 }
-                Icon(
-                    modifier = Modifier.clickable(onClick = onDismissClick),
-                    imageVector = CompoundIcons.Close(),
-                    contentDescription = stringResource(CommonStrings.action_close)
-                )
-            }
-            if (state.reason != null) {
-                Spacer(modifier = Modifier.height(16.dp))
-                Text(
-                    text = state.reason,
-                    color = ElementTheme.colors.textPrimary,
-                    style = ElementTheme.typography.fontBodyMdRegular,
-                )
             }
+            Icon(
+                modifier = Modifier.clickable(onClick = ::onDismissClick),
+                imageVector = CompoundIcons.Close(),
+                contentDescription = stringResource(CommonStrings.action_close)
+            )
+        }
+        if (state.reason != null) {
             Spacer(modifier = Modifier.height(16.dp))
-            Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(12.dp)) {
+            Text(
-                if (state.knockRequests.size > 1) {
+                text = state.reason,
+                color = ElementTheme.colors.textPrimary,
+                style = ElementTheme.typography.fontBodyMdRegular,
+                maxLines = 2,
+                overflow = TextOverflow.Ellipsis,
+            )
+        }
+        Spacer(modifier = Modifier.height(16.dp))
+        Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(12.dp)) {
+            if (state.knockRequests.size > 1) {
+                Button(
+                    text = "View all",
+                    onClick = onViewRequestsClick,
+                    size = ButtonSize.MediumLowPadding,
+                    modifier = Modifier.weight(1f),
+                )
+            } else {
+                OutlinedButton(
+                    text = "View",
+                    onClick = onViewRequestsClick,
+                    size = ButtonSize.MediumLowPadding,
+                    modifier = Modifier.weight(1f),
+                )
+                if (state.canAccept) {
                     Button(
-                        text = "View all",
+                        text = "Accept",
-                        onClick = onViewRequestsClick,
+                        onClick = {},
                         size = ButtonSize.MediumLowPadding,
                         modifier = Modifier.weight(1f),
                     )
-                } else {
-                    OutlinedButton(
-                        text = "View",
-                        onClick = onViewRequestsClick,
-                        size = ButtonSize.MediumLowPadding,
-                        modifier = Modifier.weight(1f),
-                    )
-                    if (state.canAccept) {
-                        Button(
-                            text = "Accept",
-                            onClick = {},
-                            size = ButtonSize.MediumLowPadding,
-                            modifier = Modifier.weight(1f),
-                        )
-                    }
                 }
             }
         }
@@ -178,11 +188,11 @@ private fun KnockRequestAvatarListView(
                 Box(
                     contentAlignment = Alignment.Center,
                     modifier = Modifier
-                            .size(size = avatarSize)
+                        .size(size = avatarSize)
-                            .clip(CircleShape)
+                        .clip(CircleShape)
-                            .background(color = ElementTheme.colors.bgCanvasDefaultLevel1)
+                        .background(color = ElementTheme.colors.bgCanvasDefaultLevel1)
-                            .zIndex(-index.toFloat()),
+                        .zIndex(-index.toFloat()),
-                    ) {
+                ) {
                     Avatar(
                         modifier = Modifier.padding(2.dp),
                         avatarData = knockRequest.getAvatarData(AvatarSize.KnockRequestBanner),
@@ -197,8 +207,6 @@ private fun KnockRequestAvatarListView(
 internal fun KnockRequestsBannerViewPreview(@PreviewParameter(KnockRequestsBannerStateProvider::class) state: KnockRequestsBannerState) = ElementPreview {
     KnockRequestsBannerView(
         state = state,
-        onDismissClick = {},
         onViewRequestsClick = {},
-        modifier = Modifier.padding(16.dp)
     )
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt

@@ -238,9 +238,9 @@ class MessagesNode @AssistedInject constructor(
                 onCreatePollClick = this::onCreatePollClick,
                 onJoinCallClick = this::onJoinCallClick,
                 onViewAllPinnedMessagesClick = this::onViewAllPinnedMessagesClick,
-                knockRequestsBanner = { modifier ->
+                knockRequestsBannerView = {
                     knockRequestsBannerRenderer.View(
-                        modifier = modifier,
+                        modifier = Modifier,
                         onViewRequestsClick = this::onViewKnockRequestsClick
                     )
                 },

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt

@@ -118,7 +118,7 @@ fun MessagesView(
     onViewAllPinnedMessagesClick: () -> Unit,
     modifier: Modifier = Modifier,
     forceJumpToBottomVisibility: Boolean = false,
-    knockRequestsBanner: @Composable (Modifier) -> Unit,
+    knockRequestsBannerView: @Composable () -> Unit,
 ) {
     OnLifecycleEvent { _, event ->
         state.voiceMessageComposerState.eventSink(VoiceMessageComposerEvents.LifecycleEvent(event))
@@ -196,8 +196,8 @@ fun MessagesView(
             MessagesViewContent(
                 state = state,
                 modifier = Modifier
-                    .padding(padding)
+                        .padding(padding)
-                    .consumeWindowInsets(padding),
+                        .consumeWindowInsets(padding),
                 onContentClick = ::onContentClick,
                 onMessageLongClick = ::onMessageLongClick,
                 onUserDataClick = { hidingKeyboard { onUserDataClick(it) } },
@@ -216,7 +216,7 @@ fun MessagesView(
                 forceJumpToBottomVisibility = forceJumpToBottomVisibility,
                 onJoinCallClick = onJoinCallClick,
                 onViewAllPinnedMessagesClick = onViewAllPinnedMessagesClick,
-                knockRequestsBanner = knockRequestsBanner,
+                knockRequestsBannerView = knockRequestsBannerView,
             )
         },
         snackbarHost = {
@@ -286,13 +286,13 @@ private fun MessagesViewContent(
     forceJumpToBottomVisibility: Boolean,
     onSwipeToReply: (TimelineItem.Event) -> Unit,
     modifier: Modifier = Modifier,
-    knockRequestsBanner: @Composable (Modifier) -> Unit,
+    knockRequestsBannerView: @Composable () -> Unit,
 ) {
     Box(
         modifier = modifier
-            .fillMaxSize()
+                .fillMaxSize()
-            .navigationBarsPadding()
+                .navigationBarsPadding()
-            .imePadding(),
+                .imePadding(),
     ) {
         AttachmentsBottomSheet(
             state = state.composerState,
@@ -375,9 +375,7 @@ private fun MessagesViewContent(
                             onViewAllClick = onViewAllPinnedMessagesClick,
                         )
                     }
-                    Box(modifier = Modifier.padding(all = 16.dp)) {
+                    knockRequestsBannerView()
-                        knockRequestsBanner(Modifier)
-                    }
                 }
             },
             sheetContent = { subcomposing: Boolean ->
@@ -404,13 +402,13 @@ private fun MessagesViewComposerBottomSheetContents(
         Column(modifier = Modifier.fillMaxWidth()) {
             SuggestionsPickerView(
                 modifier = Modifier
-                    .heightIn(max = 230.dp)
+                        .heightIn(max = 230.dp)
-                    // Consume all scrolling, preventing the bottom sheet from being dragged when interacting with the list of suggestions
+                        // Consume all scrolling, preventing the bottom sheet from being dragged when interacting with the list of suggestions
-                    .nestedScroll(object : NestedScrollConnection {
+                        .nestedScroll(object : NestedScrollConnection {
-                        override fun onPostScroll(consumed: Offset, available: Offset, source: NestedScrollSource): Offset {
+                            override fun onPostScroll(consumed: Offset, available: Offset, source: NestedScrollSource): Offset {
-                            return available
+                                return available
-                        }
+                            }
-                    }),
+                        }),
                 roomId = state.roomId,
                 roomName = state.roomName.dataOrNull(),
                 roomAvatarData = state.roomAvatar.dataOrNull(),
@@ -458,8 +456,8 @@ private fun MessagesViewTopBar(
         title = {
             val roundedCornerShape = RoundedCornerShape(8.dp)
             val titleModifier = Modifier
-                .clip(roundedCornerShape)
+                    .clip(roundedCornerShape)
-                .clickable { onRoomDetailsClick() }
+                    .clickable { onRoomDetailsClick() }
             if (roomName != null && roomAvatar != null) {
                 RoomAvatarAndNameRow(
                     roomName = roomName,
@@ -514,9 +512,9 @@ private fun RoomAvatarAndNameRow(
 private fun CantSendMessageBanner() {
     Row(
         modifier = Modifier
-            .fillMaxWidth()
+                .fillMaxWidth()
-            .background(MaterialTheme.colorScheme.secondary)
+                .background(MaterialTheme.colorScheme.secondary)
-            .padding(16.dp),
+                .padding(16.dp),
         verticalAlignment = Alignment.CenterVertically,
         horizontalArrangement = Arrangement.Center
     ) {
@@ -545,6 +543,6 @@ internal fun MessagesViewPreview(@PreviewParameter(MessagesStateProvider::class)
         onJoinCallClick = {},
         onViewAllPinnedMessagesClick = { },
         forceJumpToBottomVisibility = true,
-        knockRequestsBanner = {},
+        knockRequestsBannerView = {},
     )
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/MessagesViewWithIdentityChangePreview.kt

@@ -40,7 +40,6 @@ internal fun MessagesViewWithIdentityChangePreview(
         onCreatePollClick = {},
         onJoinCallClick = {},
         onViewAllPinnedMessagesClick = {},
-        knockRequestsBanner = {}
+        knockRequestsBannerView = {}
-
     )
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt

@@ -533,7 +533,7 @@ private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setMessa
                 onCreatePollClick = onCreatePollClick,
                 onJoinCallClick = onJoinCallClick,
                 onViewAllPinnedMessagesClick = onViewAllPinnedMessagesClick,
-                knockRequestsBanner = {}
+                knockRequestsBannerView = {}
             )
         }
     }