#3296 Typing Notification update #5206

ahmed-radhouane · 2022-02-11T10:42:49Z

Issue #3296 Typing Notification update

Signed-off-by: Ahmed Radhouane Belkilani [email protected]

Type of change

Feature
Bugfix
Technical
Other :

Content

While user is in a room a typing notification appear in the toolbar with the sender name and a short message "user A is typing..."
This will be removed with the issues Update the top bar in a room #4642
In this implementation we added a new typing notification view at the bottom of the time line showing some loading dots animation and sending users avatars with new text to appear related to different states (one, two or more senders) . please refer to the Gifs.

Motivation and context

A new string resources has been added to the strings.xml file with a //TODO tag above.
please refer to the Figma file describing the new UI for the typing notification.

Screenshots / GIFs

On sender	Two sender

Multi-senders	RTL

Tests

Get into a room from user A connected with User B
Type a message in room as User A
Access to same room as User B ans see the typing notification at the bottom of the timeline.

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com./vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com./vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

github-actions · 2022-02-11T11:09:58Z

Unit Test Results

  84 files ±0   84 suites ±0 1m 2s ⏱️ +3s
157 tests ±0 157 ✔️ ±0 0 💤 ±0 0 ❌ ±0
504 runs ±0 504 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 91ab472. ± Comparison against base commit 924a4f8.

♻️ This comment has been updated with latest results.

github-actions · 2022-02-11T13:37:56Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
passed=
[org.matrix.android.sdk.account]
passed=
[org.matrix.android.sdk.internal]
passed=
[org.matrix.android.sdk.ordering]
passed=
[org.matrix.android.sdk.PermalinkParserTest]
passed=

ouchadam · 2022-02-11T16:52:00Z

.idea/shelf/Uncommitted_changes_before_rebase__Default_Changelist_.xml

@@ -0,0 +1,4 @@
+<changelist name="Uncommitted_changes_before_rebase_[Default_Changelist]" date="1644570474870" recycled="true" deleted="true">


looks like a bonus file was included!

it run away, catch it !

ouchadam · 2022-02-11T16:54:49Z

vector/src/main/java/im/vector/app/core/ui/views/TypingMessageAvatar.kt

+        const val OVERLAP_FACT0R = -3 // =~ 30% to left
+    }
+
+    fun render(typingUsers: List<SenderInfo>, avatarRender: AvatarRenderer) {


have you noticed any performance issues with creating/dropping views as part of the typing flow?

I'm wondering if it's worth caching and reusing the imageviews instead of dynamically creating/removing them, probably not~

I do not think we need to handle cache at this level.
avatars count are limit to 4, and users could change rapidly in a short time.
in total the view took almost 1 second and disappear. Still I will try to renforce test on this.
Thank you

ouchadam · 2022-02-11T17:02:00Z

vector/src/main/java/im/vector/app/core/ui/views/TypingMessageDotsView.kt

+        animator.duration = DEFAULT_CIRCLE_DURATION
+        animator.startDelay = DEFAULT_START_ANIM_CIRCLE_DURATION * index
+        animator.repeatCount = ValueAnimator.INFINITE
+        animator.addUpdateListener {


could also use ObjectAnimator to avoid needing the separate callback, I don't think there's any difference other than syntax~

val animator = ObjectAnimator.ofFloat(circle, "alpha", DEFAULT_MAX_ALPHA, DEFAULT_MIN_ALPHA)

Changed !
Thanks

ouchadam · 2022-02-11T17:03:09Z

vector/src/main/res/layout/vector_settings_round_avatar.xml

@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="utf-8"?>
 <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
-    android:layout_width="40dp"
-    android:layout_height="40dp">
+    android:layout_width="24dp"


is this layout used anywhere else? this change might effect another screen~

It was an error, I've revert value to those of develop branch.
Thanks

ouchadam

great work! the change looks awesome 💯

added a few minor comments/queries

ouchadam

LGTM 💯

bmarty

Some remark before we can merge this PR

bmarty · 2022-02-17T09:16:58Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/sender/SenderInfo.kt

@@ -35,3 +36,5 @@ data class SenderInfo(
            else                                      -> "$displayName ($userId)"
        }
 }
+
+fun SenderInfo.toMatrixItem() = MatrixItem.UserItem(userId, displayName, avatarUrl)


All extensions like this are declared in the file MatrixItem.kt. Maybe move this one there for coherency.

Done.
Thanks !

bmarty · 2022-02-17T09:18:36Z

vector/src/main/java/im/vector/app/core/ui/views/TypingMessageView.kt

+        views = TypingMessageLayoutBinding.bind(this)
+    }
+
+    fun render(typingUsers: List<SenderInfo>, avatarRender: AvatarRenderer) {


Suggested change

fun render(typingUsers: List<SenderInfo>, avatarRender: AvatarRenderer) {

fun render(typingUsers: List<SenderInfo>, avatarRenderer: AvatarRenderer) {

Done.
Thanks !

bmarty · 2022-02-17T09:18:44Z

vector/src/main/java/im/vector/app/core/ui/views/TypingMessageView.kt

+
+    fun render(typingUsers: List<SenderInfo>, avatarRender: AvatarRenderer) {
+        views.usersName.text = typingHelper.getNotificationTypingMessage(typingUsers)
+        views.avatars.render(typingUsers, avatarRender)


Suggested change

views.avatars.render(typingUsers, avatarRender)

views.avatars.render(typingUsers, avatarRenderer)

Done.
Thanks !

bmarty · 2022-02-17T09:34:43Z

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -1597,6 +1601,17 @@ class TimelineFragment @Inject constructor(
        voiceMessageRecorderView.isVisible = false
    }

+    private fun renderTypingMessageNotification(roomSummary: RoomSummary?, state: RoomDetailViewState) {
+        if (!isThreadTimeLine() && roomSummary != null) {
+            views.typingMessageView.isInvisible = state.typingUsers.isNullOrEmpty()


So I understand than when there is no typing user, there is an empty area at the bottom of the timeline, right?

Yes, it's about 20dp, otherwise we will see the jump of the time line recycler view.

bmarty · 2022-02-17T09:36:10Z

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

+        if (!isThreadTimeLine() && roomSummary != null) {
+            views.typingMessageView.isInvisible = state.typingUsers.isNullOrEmpty()
+            state.typingUsers?.let { senders ->
+                views.typingMessageView.render(senders.take(MAX_TYPING_MESSAGE_USERS_COUNT), avatarRenderer)


Nit: I would rewrite to

state.typingUsers ?.take(MAX_TYPING_MESSAGE_USERS_COUNT) ?.let { typingUsers -> views.typingMessageView.render(typingUsers, avatarRenderer) }

Done.
Thanks !

bmarty · 2022-02-17T09:37:34Z

vector/src/main/res/layout/fragment_timeline.xml

+        android:layout_width="0dp"
+        android:paddingStart="20dp"
+        android:paddingEnd="20dp"
+        android:visibility="invisible"


Maybe make it visible in the layout editor

Done.
Thanks !

bmarty · 2022-02-17T09:38:06Z

vector/src/main/res/layout/fragment_timeline.xml

@@ -119,8 +129,7 @@
        android:visibility="gone"
        app:layout_constraintBottom_toBottomOf="parent"
        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintStart_toStartOf="parent"
-        tools:visibility="visible" />


Please revert this change. Layout editor should show all the views if possible

Done.
Thanks !

bmarty · 2022-02-17T09:38:42Z

vector/src/main/res/layout/typing_message_layout.xml

+        android:layout_height="0dp"
+        android:layout_marginStart="8dp"
+        android:gravity="center"
+        android:visibility="visible"


This line is not necessary

Done.
Thanks !

bmarty · 2022-02-17T09:39:42Z

vector/src/main/res/values/strings.xml

@@ -952,6 +952,9 @@
    <string name="room_one_user_is_typing">%s is typing…</string>
    <string name="room_two_users_are_typing">%1$s &#038; %2$s are typing…</string>
    <string name="room_many_users_are_typing">%1$s &#038; %2$s &#038; others are typing…</string>
+    <!--TODO #3296 add next two strings values -->


This TODO is not necessary. Adding new strings will be handled automatically by Weblate

Done.
Thanks !

bmarty · 2022-02-17T09:44:32Z

vector/src/main/java/im/vector/app/core/ui/views/TypingMessageAvatar.kt

+import org.matrix.android.sdk.api.session.room.sender.SenderInfo
+import org.matrix.android.sdk.api.util.toMatrixItem
+
+class TypingMessageAvatar @JvmOverloads constructor(


Another possible implementation could be like in the View ReadReceiptsView. I do not know if it is better or not though.

First Idea was to use this view, but too much differences with what is asked from UI team.
But, in other way TypingMessageAvatar could be used from showing the read receipts, with some modification.

ahmed-radhouane · 2022-02-18T07:46:46Z

@bmarty
Any manip to avoid the :

❌ Error: 1 file has more than 2500 lines:
./vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt (2513 lines)

which set a failing state for the Code Quality Checks ?

bmarty · 2022-02-18T13:25:11Z

@ahmed-radhouane you can increase the limit here (to 2800 maybe), we will have to split this class later.

- Adding a typing message notification view at the bottom of the timeline in rooms. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

- prevent timeline jump up while TypingMessageView populated. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

- revert vector_settings_round_avatar.xml file to develop state. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

- use ObjectAnimator instead of ValueAnimator in TypingMessageDots. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

- Fixes after review. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

bmarty · 2022-02-23T17:41:06Z

vector/src/main/java/im/vector/app/features/home/room/typing/TypingHelper.kt

@@ -42,4 +42,15 @@ class TypingHelper @Inject constructor(private val stringProvider: StringProvide
                        typingUsers[1].disambiguatedDisplayName)
        }
    }
+
+    fun getNotificationTypingMessage(typingUsers: List<SenderInfo>): String {
+        return when {


Do not change it, but I think we could have when(typingUsers.size())

Ah it's similar than the fun above :)

bmarty

Thanks for the update!

ouchadam reviewed Feb 11, 2022

View reviewed changes

ahmed-radhouane force-pushed the arb/issues/3296 branch from 7f51b05 to e3aae38 Compare February 11, 2022 17:53

ahmed-radhouane requested a review from ouchadam February 16, 2022 09:19

ouchadam approved these changes Feb 16, 2022

View reviewed changes

bmarty reviewed Feb 17, 2022

View reviewed changes

ahmed-radhouane force-pushed the arb/issues/3296 branch from bfd31fb to 0694cb2 Compare February 22, 2022 08:01

ahmed-radhouane and others added 6 commits February 23, 2022 11:34

#3296

da9fdf1

- Adding a typing message notification view at the bottom of the timeline in rooms. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

#3296

a9b8871

- prevent timeline jump up while TypingMessageView populated. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

Delete Uncommitted_changes_before_rebase__Default_Changelist_.xml

f6c6e92

#4642 [Review]

5deabfa

- revert vector_settings_round_avatar.xml file to develop state. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

#4642 [Review]

b39a3ab

- use ObjectAnimator instead of ValueAnimator in TypingMessageDots. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

#3296

91ab472

- Fixes after review. Signed-off-by: Ahmed Radhouane Belkilani <[email protected]>

ahmed-radhouane force-pushed the arb/issues/3296 branch from 0694cb2 to 91ab472 Compare February 23, 2022 10:35

bmarty reviewed Feb 23, 2022

View reviewed changes

bmarty approved these changes Feb 23, 2022

View reviewed changes

bmarty merged commit 93de61e into develop Feb 23, 2022

bmarty deleted the arb/issues/3296 branch February 23, 2022 17:57

		@@ -0,0 +1,4 @@
		<changelist name="Uncommitted_changes_before_rebase_[Default_Changelist]" date="1644570474870" recycled="true" deleted="true">

	fun render(typingUsers: List<SenderInfo>, avatarRender: AvatarRenderer) {
	fun render(typingUsers: List<SenderInfo>, avatarRenderer: AvatarRenderer) {

	views.avatars.render(typingUsers, avatarRender)
	views.avatars.render(typingUsers, avatarRenderer)

#3296 Typing Notification update #5206

#3296 Typing Notification update #5206

Conversation

ahmed-radhouane commented Feb 11, 2022

Issue #3296 Typing Notification update

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Feb 11, 2022 • edited Loading

Unit Test Results

github-actions bot commented Feb 11, 2022 • edited Loading

Matrix SDK

Integration Tests Results:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmed-radhouane commented Feb 18, 2022

bmarty commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 11, 2022 •

edited

Loading

github-actions bot commented Feb 11, 2022 •

edited

Loading