Improve typing notification animations #2386

jmartinesp · 2024-02-13T12:03:10Z

Type of change

Feature
Bugfix
Technical
Other : UX improvement

Content

Make typing notifications appear using an animation, and keep the extra bottom space after the first one is displayed.
Also, simplify how MigrationScreenView animation works, since I forgot AnimatedVisibility() existed.

Motivation and context

Iterate on the initial implementation.

Screenshots / GIFs

typing_2.mp4

Tests

Open a room, scroll to the bottom, wait until you receive typing notifications. You should see:

An extra space is added at the bottom once the first typing notification appears.
The typing notifications will appear and disappear using a fade in/out animation.
The extra space is still there after the first notification disappears, so we only resize it once.

Tested devices

Physical
Emulator
OS version(s): 14

Checklist

Changes have been tested on an Android device or Android emulator with API 23
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-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/element-hq/element-x-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

github-actions · 2024-02-13T12:08:22Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/PHWA83

jmartinesp · 2024-02-13T12:30:11Z

...ping_TypingNotificationView_null_TypingNotificationView-Day-60_60_null_8,NEXUS_5,1.0,en].png

The screenshot is not very representative 😅 . There should be an empty space at the bottom, I'll try to make it visible against the theme background.

codecov · 2024-02-13T12:58:44Z

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (58a4cc2) 72.09% compared to head (730ed46) 72.11%.

Files	Patch %	Lines
...res/messages/impl/typing/TypingNotificationView.kt	83.33%	3 Missing and 6 partials ⚠️
...res/roomlist/impl/migration/MigrationScreenView.kt	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2386      +/-   ##
===========================================
+ Coverage    72.09%   72.11%   +0.01%     
===========================================
  Files         1358     1359       +1     
  Lines        32084    32124      +40     
  Branches      6272     6276       +4     
===========================================
+ Hits         23131    23166      +35     
- Misses        5699     5701       +2     
- Partials      3254     3257       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty

Thanks, please see my remarks, and let me know if you have any question

bmarty · 2024-02-13T13:49:29Z

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

+            ),
+            aMessagesState(
+                typingNotificationState = aTypingNotificationState(reserveSpace = true),
+            )


I would create a new provider, like MessagesStateForTypingProvider, and add those 2 new states here. Then use this provider in MessagesViewWithTypingPreview (I suspect that you did not see that Preview). The current state for MessagesViewWithTypingPreview will be the first state of this new provider. The idea is to split to previews into several files so that Android Studio can render them all, and faster. MessagesViewWithTypingPreview is also closer to the reality, since if does not render the Jump to bottom FAB.

Maybe the first added state here has no added value, I let you decide if you want to remove it.

bmarty · 2024-02-13T13:51:03Z

.../main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationPresenter.kt

+        var reserveSpace by remember { mutableStateOf(false) }
+        LaunchedEffect(renderTypingNotifications, typingMembersState.value) {
+            if (renderTypingNotifications && typingMembersState.value.isNotEmpty()) {
+                reserveSpace = true


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

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.

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.

bmarty · 2024-02-13T13:55:55Z

...l/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationView.kt

+        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))


Screen Reader may still read this invisible text (I am not 💯 sure though). So maybe disable the contentDescription adding .clearAndSetSemantics {} to the Modifier.

bmarty · 2024-02-13T13:59:33Z

...l/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationView.kt

@@ -99,6 +142,7 @@ internal fun TypingNotificationViewPreview(
    @PreviewParameter(TypingNotificationStateProvider::class) state: TypingNotificationState,
 ) = ElementPreview {
    TypingNotificationView(
+        modifier = Modifier.border(1.dp, Color.Blue),


Good idea, but maybe set the border only when state.reserveSpace == true && state.typingMembers.isNotEmpty to avoid impacting the existing screenshots

bmarty

Thanks for the update!

Also add screenshot for typing notifications in the timeline

sonarcloud · 2024-02-14T07:40:40Z

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

jmartinesp requested a review from a team as a code owner February 13, 2024 12:03

jmartinesp requested review from bmarty and removed request for a team February 13, 2024 12:03

jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Feb 13, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Feb 13, 2024

jmartinesp commented Feb 13, 2024

View reviewed changes

jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Feb 13, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Feb 13, 2024

bmarty reviewed Feb 13, 2024

View reviewed changes

jmartinesp force-pushed the misc/jme/improve-typing-notification-animations branch from c16e344 to eacc5fa Compare February 13, 2024 15:23

jmartinesp enabled auto-merge (squash) February 13, 2024 15:53

bmarty approved these changes Feb 13, 2024

View reviewed changes

jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Feb 13, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Feb 13, 2024

jmartinesp added 4 commits February 14, 2024 08:27

Improve typing notification animations

4ac5ba3

Add border to screenshots so the reserved space is visible

05fe29d

Also add screenshot for typing notifications in the timeline

Fix review comments

f7f5931

Fix Konsist failure

c09c570

jmartinesp force-pushed the misc/jme/improve-typing-notification-animations branch from 4a9fe0b to c09c570 Compare February 14, 2024 07:28

Update screenshots

730ed46

jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Feb 14, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Feb 14, 2024

jmartinesp merged commit 1b690c1 into develop Feb 14, 2024
18 checks passed

jmartinesp deleted the misc/jme/improve-typing-notification-animations branch February 14, 2024 08:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve typing notification animations #2386

Improve typing notification animations #2386

jmartinesp commented Feb 13, 2024 •

edited

Loading

github-actions bot commented Feb 13, 2024 •

edited

Loading

jmartinesp Feb 13, 2024

codecov bot commented Feb 13, 2024 •

edited

Loading

bmarty left a comment

bmarty Feb 13, 2024

bmarty Feb 13, 2024

bmarty Feb 13, 2024 •

edited

Loading

jmartinesp Feb 13, 2024

bmarty Feb 13, 2024

bmarty Feb 13, 2024

bmarty left a comment

sonarcloud bot commented Feb 14, 2024

Improve typing notification animations #2386

Improve typing notification animations #2386

Conversation

jmartinesp commented Feb 13, 2024 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Feb 13, 2024 • edited Loading

jmartinesp Feb 13, 2024

Choose a reason for hiding this comment

codecov bot commented Feb 13, 2024 • edited Loading

Codecov Report

bmarty left a comment

Choose a reason for hiding this comment

bmarty Feb 13, 2024

Choose a reason for hiding this comment

bmarty Feb 13, 2024

Choose a reason for hiding this comment

bmarty Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

jmartinesp Feb 13, 2024

Choose a reason for hiding this comment

bmarty Feb 13, 2024

Choose a reason for hiding this comment

bmarty Feb 13, 2024

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Feb 14, 2024

Quality Gate passed

jmartinesp commented Feb 13, 2024 •

edited

Loading

github-actions bot commented Feb 13, 2024 •

edited

Loading

codecov bot commented Feb 13, 2024 •

edited

Loading

bmarty Feb 13, 2024 •

edited

Loading