Integrate mentions in the composer #1799

jmartinesp · 2023-11-14T11:22:06Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Integrates a new version of the RTE library with better support for mentions.
Sets a callback in the mention suggestion selection view to actually insert the selection.
Creates MentionSpanProvider to provide the RTE composer with the custom ReplacementSpan for mentions.

Motivation and context

Screenshots / GIFs

Light	Dark

Tests

Open a room, type @ in the composer.
Select something from the list of suggestions.
A mention pill should be added to the editor and when sending, it'll be recognised as an actual mention.

Tested devices

Physical
Emulator
OS version(s): 11

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/vector-im/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/vector-im/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

codecov · 2023-11-14T11:54:07Z

Codecov Report

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

Comparison is base (4585597) 63.40% compared to head (8d7f24e) 63.45%.
Report is 39 commits behind head on develop.

❗ Current head 8d7f24e differs from pull request most recent head c8dcfd2. Consider uploading reports for the commit c8dcfd2 to get more accurate results

Files	Patch %	Lines
...aries/textcomposer/mentions/MentionSpanProvider.kt	71.15%	7 Missing and 8 partials ⚠️
...ent/android/libraries/textcomposer/TextComposer.kt	57.14%	2 Missing and 1 partial ⚠️
...ent/android/features/messages/impl/MessagesView.kt	75.00%	2 Missing ⚠️
.../element/android/libraries/core/data/FilterUpTo.kt	71.42%	1 Missing and 1 partial ⚠️
...ages/impl/mentions/MentionSuggestionsPickerView.kt	87.50%	0 Missing and 1 partial ⚠️
...s/impl/messagecomposer/MessageComposerPresenter.kt	83.33%	0 Missing and 1 partial ⚠️
...ssages/impl/messagecomposer/MessageComposerView.kt	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1799      +/-   ##
===========================================
+ Coverage    63.40%   63.45%   +0.04%     
===========================================
  Files         1293     1296       +3     
  Lines        33628    33727      +99     
  Branches      6982     7002      +20     
===========================================
+ Hits         21322    21401      +79     
- Misses        9117     9129      +12     
- Partials      3189     3197       +8

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

github-actions · 2023-11-17T12:05:06Z

📱 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/3ujdmH

- Add `MentionSpanProvider`. - Add custom colors needed for mentions. - Use the span provider to render mentions in the composer. - Allow selecting users from the mentions suggestions to insert a mention.

frebib · 2023-11-18T01:33:06Z

This works wonderfully! Nice work
There seems to be a small delay after hitting the @ symbol. Is that intended?

This was broken before.

jmartinesp · 2023-11-20T08:17:42Z

This works wonderfully! Nice work There seems to be a small delay after hitting the @ symbol. Is that intended?

Thanks for your comment, I hadn't noticed it myself, but I just realised the code is not working as expected: it's supposed to return the first 100 matching results for the mention, but it was actually returning all possible results, so in large rooms it could be quite slow. I just changed that.

ganfra · 2023-11-20T14:25:06Z

...pl/src/main/kotlin/io/element/android/libraries/textcomposer/mentions/MentionSpanProvider.kt

+                    append("mention", mentionSpan, 0)
+                    append(" to the current user and this is a ")
+                    append("mention", mentionSpan2, 0)
+                    append("to other user")


add space before => " to other user" ?

ganfra · 2023-11-20T14:28:33Z

...kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt

-sealed interface RoomMemberSuggestion {
-    data object Room : RoomMemberSuggestion
-    data class Member(val roomMember: RoomMember) : RoomMemberSuggestion
+sealed interface MentionSuggestion {


I'd extract this class outs of this file

ganfra · 2023-11-20T14:30:43Z

...pl/src/main/kotlin/io/element/android/libraries/textcomposer/mentions/MentionSpanProvider.kt

+
+    @Suppress("ComposableNaming")
+    @Composable
+    fun setup() {


make private?

Is this method called each time composer is recomposed?

make private?

I can make it internal at best, since it's used in the rememberMentionSpanProvider function.

Is this method called each time composer is recomposed?

I think so, yeah. Since those colors don't change I'm not sure if they will be re-calculated and re-assigned to each property, but I don't see any other way to be able to reach the @Composable color: we can't use remember or LaunchedEffect for that.

ganfra

Some small remarks otherwise LGTM

frebib · 2023-11-20T15:40:32Z

This works wonderfully! Nice work There seems to be a small delay after hitting the @ symbol. Is that intended?

Thanks for your comment, I hadn't noticed it myself, but I just realised the code is not working as expected: it's supposed to return the first 100 matching results for the mention, but it was actually returning all possible results, so in large rooms it could be quite slow. I just changed that.

Interestingly (prior to your changes 8d7f24e) for me it consistently takes around 1 second to open both in a group chat with a few participants (~6 iirc) as well as Matrix HQ

jmartinesp · 2023-11-20T15:52:47Z

Interestingly (prior to your changes 8d7f24e) for me it consistently takes around 1 second to open both in a group chat with a few participants (~6 iirc) as well as Matrix HQ

It's probably the .debounce(0.5.seconds) used to prevent too much computation when the user types really fast. Now that I'm aware of it the delay is quite noticeable, maybe it would be better to just remove this debounce so displaying the results is as immediate as possible.

jmartinesp · 2023-11-20T16:29:21Z

Interestingly (prior to your changes 8d7f24e) for me it consistently takes around 1 second to open both in a group chat with a few participants (~6 iirc) as well as Matrix HQ

It's probably the .debounce(0.5.seconds) used to prevent too much computation when the user types really fast. Now that I'm aware of it the delay is quite noticeable, maybe it would be better to just remove this debounce so displaying the results is as immediate as possible.

This should now work better: the initial time to display when using @ will be as short as possible with no artificial delays, then when we want to filter out some more by continue typing, there will be slightly shorter debounce than before (0.3s between search runs).

sonarcloud · 2023-11-20T16:59:32Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

jmartinesp force-pushed the feature/jme/integrate-mentions-with-composer branch from 8cabddd to 9270e29 Compare November 14, 2023 11:23

jmartinesp changed the title ~~WIP~~ Integrate mentions in the composer Nov 14, 2023

jmartinesp force-pushed the feature/jme/integrate-mentions-with-composer branch from 9270e29 to ab624c6 Compare November 15, 2023 14:26

jmartinesp mentioned this pull request Nov 16, 2023

Update wysiwyg to v2.17.0 #1827

Merged

1 task

jmartinesp and others added 8 commits November 17, 2023 13:23

Integrate mentions in the composer:

084c949

- Add `MentionSpanProvider`. - Add custom colors needed for mentions. - Use the span provider to render mentions in the composer. - Allow selecting users from the mentions suggestions to insert a mention.

Add tests for MentionSpanProvider

78dab4d

Have special logic for @room mentions, fix lint issues

385df27

Add preview for mentions, fix tests

4edf95d

Update screenshots

27809b8

Fix lint issue

1a7929b

Rename RoomMemberSuggestion to MentionSuggestion

a86abd6

Add changelog

cb33a13

jmartinesp force-pushed the feature/jme/integrate-mentions-with-composer branch from 33689de to cb33a13 Compare November 17, 2023 12:23

jmartinesp marked this pull request as ready for review November 17, 2023 12:36

jmartinesp requested a review from a team as a code owner November 17, 2023 12:36

jmartinesp requested review from ganfra and removed request for a team November 17, 2023 12:36

Only take up to 100 suggestions for the picker.

8d7f24e

This was broken before.

Fix lint issues

71d684c

ganfra reviewed Nov 20, 2023

View reviewed changes

ganfra approved these changes Nov 20, 2023

View reviewed changes

Improve the code following the PR suggestions

20b3b2f

Improve how debouncing the suggestions work

5cdb102

Update screenshots

c8dcfd2

jmartinesp merged commit a8fbb88 into develop Nov 20, 2023
13 checks passed

jmartinesp deleted the feature/jme/integrate-mentions-with-composer branch November 20, 2023 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate mentions in the composer #1799

Integrate mentions in the composer #1799

jmartinesp commented Nov 14, 2023 •

edited

Loading

codecov bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 17, 2023 •

edited

Loading

frebib commented Nov 18, 2023

jmartinesp commented Nov 20, 2023

ganfra Nov 20, 2023

ganfra Nov 20, 2023

ganfra Nov 20, 2023

ganfra Nov 20, 2023

jmartinesp Nov 20, 2023

ganfra left a comment

frebib commented Nov 20, 2023 •

edited

Loading

jmartinesp commented Nov 20, 2023

jmartinesp commented Nov 20, 2023

sonarcloud bot commented Nov 20, 2023

Integrate mentions in the composer #1799

Integrate mentions in the composer #1799

Conversation

jmartinesp commented Nov 14, 2023 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

codecov bot commented Nov 14, 2023 • edited Loading

Codecov Report

github-actions bot commented Nov 17, 2023 • edited Loading

frebib commented Nov 18, 2023

jmartinesp commented Nov 20, 2023

ganfra Nov 20, 2023

Choose a reason for hiding this comment

ganfra Nov 20, 2023

Choose a reason for hiding this comment

ganfra Nov 20, 2023

Choose a reason for hiding this comment

ganfra Nov 20, 2023

Choose a reason for hiding this comment

jmartinesp Nov 20, 2023

Choose a reason for hiding this comment

ganfra left a comment

Choose a reason for hiding this comment

frebib commented Nov 20, 2023 • edited Loading

jmartinesp commented Nov 20, 2023

jmartinesp commented Nov 20, 2023

sonarcloud bot commented Nov 20, 2023

jmartinesp commented Nov 14, 2023 •

edited

Loading

codecov bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 17, 2023 •

edited

Loading

frebib commented Nov 20, 2023 •

edited

Loading