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

Improve typing notification animations #2386

Merged
merged 5 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,19 @@
package io.element.android.features.messages.impl.typing

import androidx.compose.runtime.Composable
import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.features.messages.impl.MessagesView
import io.element.android.features.messages.impl.aMessagesState
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight

@PreviewsDayNight
@Composable
internal fun MessagesViewWithTypingPreview() = ElementPreview {
internal fun MessagesViewWithTypingPreview(
@PreviewParameter(TypingNotificationStateForMessagesProvider::class) typingState: TypingNotificationState
) = ElementPreview {
MessagesView(
state = aMessagesState().copy(
typingNotificationState = aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
),
),
),
state = aMessagesState().copy(typingNotificationState = typingState),
onBackPressed = {},
onRoomDetailsClicked = {},
onEventClicked = { false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import io.element.android.features.preferences.api.store.SessionPreferencesStore
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.matrix.api.core.UserId
Expand All @@ -46,6 +47,7 @@ class TypingNotificationPresenter @Inject constructor(
override fun present(): TypingNotificationState {
val typingMembersState = remember { mutableStateOf(emptyList<RoomMember>()) }
val renderTypingNotifications by sessionPreferencesStore.isRenderTypingNotificationsEnabled().collectAsState(initial = true)

LaunchedEffect(renderTypingNotifications) {
if (renderTypingNotifications) {
observeRoomTypingMembers(typingMembersState)
Expand All @@ -54,9 +56,18 @@ class TypingNotificationPresenter @Inject constructor(
}
}

// This will keep the space reserved for the typing notifications after the first one is displayed
var reserveSpace by remember { mutableStateOf(false) }
LaunchedEffect(renderTypingNotifications, typingMembersState.value) {
if (renderTypingNotifications && typingMembersState.value.isNotEmpty()) {
reserveSpace = true
Copy link
Member

Choose a reason for hiding this comment

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

I think reserveSpace should be set back to false as soon as a new Event is received in the timeline.

Copy link
Member

@bmarty bmarty Feb 13, 2024

Choose a reason for hiding this comment

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

After discussing with @stefanceriu (who discussed with @amshakal) this is maybe not necesssary, but having an empty space at the bottom of the timeline is not great to me. Not a blocker then.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just spoke with @stefanceriu and he confirmed we've implemented the same behaviour in both platforms: the extra space should stay until you exit the room.

}
}

return TypingNotificationState(
renderTypingNotifications = renderTypingNotifications,
typingMembers = typingMembersState.value.toImmutableList(),
reserveSpace = reserveSpace,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ package io.element.android.features.messages.impl.typing
import io.element.android.libraries.matrix.api.room.RoomMember
import kotlinx.collections.immutable.ImmutableList

/**
* State for the typing notification view.
*/
data class TypingNotificationState(
/** Whether to render the typing notifications based on the user's preferences. */
val renderTypingNotifications: Boolean,
/** The room members currently typing. */
val typingMembers: ImmutableList<RoomMember>,
/** Whether to reserve space for the typing notifications at the bottom of the timeline. */
val reserveSpace: Boolean,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.element.android.features.messages.impl.typing

import androidx.compose.ui.tooling.preview.PreviewParameterProvider

class TypingNotificationStateForMessagesProvider : PreviewParameterProvider<TypingNotificationState> {
override val values: Sequence<TypingNotificationState>
get() = sequenceOf(
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
),
),
aTypingNotificationState(
typingMembers = listOf(aTypingRoomMember()),
reserveSpace = true
),
aTypingNotificationState(reserveSpace = true),
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,20 @@ class TypingNotificationStateProvider : PreviewParameterProvider<TypingNotificat
aTypingRoomMember(displayName = "Alice with a very long display name which means that it will be truncated"),
),
),
aTypingNotificationState(
typingMembers = emptyList(),
reserveSpace = true,
),
)
}

internal fun aTypingNotificationState(
typingMembers: List<RoomMember> = emptyList(),
reserveSpace: Boolean = false,
) = TypingNotificationState(
renderTypingNotifications = true,
typingMembers = typingMembers.toImmutableList(),
reserveSpace = reserveSpace,
)

internal fun aTypingRoomMember(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,27 @@

package io.element.android.features.messages.impl.typing

import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.expandVertically
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.animation.shrinkVertically
import androidx.compose.foundation.border
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
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 androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.alpha
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.res.pluralStringResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.clearAndSetSemantics
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.SpanStyle
import androidx.compose.ui.text.buildAnnotatedString
Expand All @@ -43,54 +58,89 @@ fun TypingNotificationView(
state: TypingNotificationState,
modifier: Modifier = Modifier,
) {
if (state.typingMembers.isEmpty() || !state.renderTypingNotifications) return
val typingNotificationText = computeTypingNotificationText(state.typingMembers)
Text(
modifier = modifier
.fillMaxWidth()
.padding(horizontal = 24.dp, vertical = 2.dp),
text = typingNotificationText,
overflow = TextOverflow.Ellipsis,
maxLines = 1,
style = ElementTheme.typography.fontBodySmRegular,
color = ElementTheme.colors.textSecondary,
)
val displayNotifications = state.typingMembers.isNotEmpty() && state.renderTypingNotifications

@Suppress("ModifierNaming")
@Composable fun TypingText(text: AnnotatedString, textModifier: Modifier = Modifier) {
Text(
modifier = textModifier,
text = text,
overflow = TextOverflow.Ellipsis,
maxLines = 1,
style = ElementTheme.typography.fontBodySmRegular,
color = ElementTheme.colors.textSecondary,
)
}

// Display the typing notification space when either a typing notification needs to be displayed or a previous one already was
AnimatedVisibility(
modifier = modifier.fillMaxWidth().padding(vertical = 2.dp),
visible = displayNotifications || state.reserveSpace,
enter = fadeIn() + expandVertically(),
exit = fadeOut() + shrinkVertically(),
) {
val typingNotificationText = computeTypingNotificationText(state.typingMembers)
Box(contentAlignment = Alignment.BottomStart) {
// Reserve the space for the typing notification by adding an invisible text
TypingText(
text = typingNotificationText,
textModifier = Modifier
.alpha(0f)
// Remove the semantics of the text to avoid screen readers to read it
.clearAndSetSemantics { }
)

// Display the actual notification
AnimatedVisibility(
visible = displayNotifications,
enter = fadeIn(),
exit = fadeOut(),
) {
TypingText(text = typingNotificationText, textModifier = Modifier.padding(horizontal = 24.dp))
}
}
}
}

@Composable
private fun computeTypingNotificationText(typingMembers: ImmutableList<RoomMember>): AnnotatedString {
val names = when (typingMembers.size) {
0 -> "" // Cannot happen
1 -> typingMembers[0].disambiguatedDisplayName
2 -> stringResource(
id = R.string.screen_room_typing_two_members,
typingMembers[0].disambiguatedDisplayName,
typingMembers[1].disambiguatedDisplayName,
)
else -> pluralStringResource(
id = R.plurals.screen_room_typing_many_members,
count = typingMembers.size - 2,
typingMembers[0].disambiguatedDisplayName,
typingMembers[1].disambiguatedDisplayName,
typingMembers.size - 2,
// Remember the last value to avoid empty typing messages while animating
var result by remember { mutableStateOf(AnnotatedString("")) }
if (typingMembers.isNotEmpty()) {
val names = when (typingMembers.size) {
0 -> "" // Cannot happen
1 -> typingMembers[0].disambiguatedDisplayName
2 -> stringResource(
id = R.string.screen_room_typing_two_members,
typingMembers[0].disambiguatedDisplayName,
typingMembers[1].disambiguatedDisplayName,
)
else -> pluralStringResource(
id = R.plurals.screen_room_typing_many_members,
count = typingMembers.size - 2,
typingMembers[0].disambiguatedDisplayName,
typingMembers[1].disambiguatedDisplayName,
typingMembers.size - 2,
)
}
// Get the translated string with a fake pattern
val tmpString = pluralStringResource(
id = R.plurals.screen_room_typing_notification,
count = typingMembers.size,
"<>",
)
}
// Get the translated string with a fake pattern
val tmpString = pluralStringResource(
id = R.plurals.screen_room_typing_notification,
count = typingMembers.size,
"<>",
)
// Split the string in 3 parts
val parts = tmpString.split("<>")
// And rebuild the string with the names
return buildAnnotatedString {
append(parts[0])
withStyle(SpanStyle(fontWeight = FontWeight.Bold)) {
append(names)
// Split the string in 3 parts
val parts = tmpString.split("<>")
// And rebuild the string with the names
result = buildAnnotatedString {
append(parts[0])
withStyle(SpanStyle(fontWeight = FontWeight.Bold)) {
append(names)
}
append(parts[1])
}
append(parts[1])
}
return result
}

@PreviewsDayNight
Expand All @@ -99,6 +149,7 @@ internal fun TypingNotificationViewPreview(
@PreviewParameter(TypingNotificationStateProvider::class) state: TypingNotificationState,
) = ElementPreview {
TypingNotificationView(
modifier = if (state.reserveSpace) Modifier.border(1.dp, Color.Blue) else Modifier,
state = state,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.element.android.features.messages.impl.typing

import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.Event
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.preferences.api.store.SessionPreferencesStore
Expand Down Expand Up @@ -53,6 +54,7 @@ class TypingNotificationPresenterTest {
val initialState = awaitItem()
assertThat(initialState.renderTypingNotifications).isTrue()
assertThat(initialState.typingMembers).isEmpty()
assertThat(initialState.reserveSpace).isFalse()
}
}

Expand Down Expand Up @@ -85,7 +87,7 @@ class TypingNotificationPresenterTest {
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aDefaultRoomMember)
// Preferences changes again
sessionPreferencesStore.setRenderTypingNotifications(false)
skipItems(1)
skipItems(2)
val finalState = awaitItem()
assertThat(finalState.renderTypingNotifications).isFalse()
assertThat(finalState.typingMembers).isEmpty()
Expand All @@ -108,6 +110,7 @@ class TypingNotificationPresenterTest {
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aDefaultRoomMember)
// User stops typing
room.givenRoomTypingMembers(emptyList())
skipItems(1)
val finalState = awaitItem()
assertThat(finalState.typingMembers).isEmpty()
}
Expand Down Expand Up @@ -140,6 +143,7 @@ class TypingNotificationPresenterTest {
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aKnownRoomMember)
// User stops typing
room.givenRoomTypingMembers(emptyList())
skipItems(1)
val finalState = awaitItem()
assertThat(finalState.typingMembers).isEmpty()
}
Expand All @@ -166,11 +170,38 @@ class TypingNotificationPresenterTest {
listOf(aKnownRoomMember).toImmutableList()
)
)
skipItems(1)
val finalState = awaitItem()
assertThat(finalState.typingMembers.first()).isEqualTo(aKnownRoomMember)
}
}

@Test
fun `present - reserveSpace becomes true once we get the first typing notification with room members`() = runTest {
val aDefaultRoomMember = createDefaultRoomMember(A_USER_ID_2)
val room = FakeMatrixRoom()
val presenter = createPresenter(matrixRoom = room)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.typingMembers).isEmpty()
room.givenRoomTypingMembers(listOf(A_USER_ID_2))
skipItems(1)
val updatedTypingState = awaitItem()
assertThat(updatedTypingState.reserveSpace).isTrue()
// User stops typing
room.givenRoomTypingMembers(emptyList())
// Is still true for all future events
val futureEvents = cancelAndConsumeRemainingEvents()
for (event in futureEvents) {
if (event is Event.Item) {
assertThat(event.value.reserveSpace).isTrue()
}
}
}
}

private fun createPresenter(
matrixRoom: MatrixRoom = FakeMatrixRoom().apply {
givenRoomInfo(aRoomInfo(id = roomId.value, name = ""))
Expand Down
Loading
Loading