Restore intentional mentions in the markdown/plain text editor #3193

jmartinesp · 2024-07-15T10:50:56Z

Content

Add TextPillificationHelper to add pills to existing MD text by using regexes searching for raw ids and links pointing to permalinks with ids.
Use the helper when loading a draft, setting the text from the edit mode, or loading the MD version of the rich text content when toggling format.
Split MentionSpanProvider into MentionSpanProvider and MentionSpanTheme, which contains the info about colors, typography and spacing to use in MentionSpan.
Modify MentionSpan so those values can be update after the span creation.

Motivation and context

Fixes #2966

Screenshots / GIFs

No UI changes, I just renamed a preview.

Tests

In a room, without formatting enabled, write @room and @someotheruser.
Enable formatting, check the pills still work.
Disable formatting, the pills should still be there.
Go back to the room list, then back to the room. The draft should contain the pills too.
Send the message, check the pills are rendered fine.
Edit the message, check the pills are added correctly to the editor and once edited the message renders fine.

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 title will be used in the release note, it clearly define what will change for the user
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-07-15T10:57:12Z

📱 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/LcAWYx

codecov · 2024-07-15T11:23:32Z

Codecov Report

Attention: Patch coverage is 82.35294% with 27 lines in your changes missing coverage. Please review.

Project coverage is 76.15%. Comparing base (35fdc85) to head (b0002b2).
Report is 1 commits behind head on develop.

Files	Patch %	Lines
...oid/libraries/textcomposer/mentions/MentionSpan.kt	57.89%	6 Missing and 2 partials ⚠️
...ages/impl/timeline/DefaultHtmlConverterProvider.kt	22.22%	6 Missing and 1 partial ⚠️
...res/messages/impl/utils/TextPillificationHelper.kt	80.64%	1 Missing and 5 partials ⚠️
...ndroid/libraries/matrix/api/core/MatrixPatterns.kt	93.10%	1 Missing and 1 partial ⚠️
...ibraries/textcomposer/mentions/MentionSpanTheme.kt	93.10%	0 Missing and 2 partials ⚠️
...libraries/matrix/api/permalink/PermalinkBuilder.kt	0.00%	1 Missing ⚠️
...ent/android/libraries/textcomposer/TextComposer.kt	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3193      +/-   ##
===========================================
+ Coverage    76.12%   76.15%   +0.02%     
===========================================
  Files         1643     1645       +2     
  Lines        38668    38738      +70     
  Branches      7475     7496      +21     
===========================================
+ Hits         29437    29501      +64     
- Misses        5341     5343       +2     
- Partials      3890     3894       +4

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

ganfra · 2024-07-15T13:41:51Z

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

@@ -397,7 +402,22 @@ class MessageComposerPresenter @Inject constructor(
            canCreatePoll = canCreatePoll.value,
            attachmentsState = attachmentsState.value,
            memberSuggestions = memberSuggestions.toPersistentList(),
-            eventSink = { handleEvents(it) }
+            resolveMentionDisplay = remember(mentionSpanTheme) {


Can you just put that in a val before the actual creation of the MessageComposerState?

As a remembered lambda, you mean?

I mean I just prefer not creating instances in the state constuctor, should just map values.

ganfra

2 small remarks, otherwise LGTM

ganfra · 2024-07-15T13:46:12Z

...c/test/kotlin/io/element/android/features/messages/impl/utils/TextPillificationHelperTest.kt

+        val pillified = helper.pillify(text)
+        val mentionSpans = pillified.getMentionSpans()
+        assertThat(mentionSpans).hasSize(1)
+        assertThat(mentionSpans.firstOrNull()?.type).isEqualTo(MentionSpan.Type.USER)


Maybe change to this? mentionsSpans.firstOrNull()?.let { assertThat(...)....}

this is not equivalent test.

The assertion error wouldn't be the same, but we have the hasSize() check before. I could also either extract the value to a variable and keep it nullable or use first() instead and have the tests crash in that case.

I'd vote for using first(), it will not crash since line 52 is checking for the collection size.

Yeah sorry that was what I had in mind, not firstOrNull

sonarqubecloud · 2024-07-15T14:16:36Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jmartinesp added 4 commits July 15, 2024 12:49

Restore intentional mentions in the markdown/plain text editor

f36c6eb

Improve APIs, fix tests

3d6942f

Fix theme color preview

2c879b0

Fix lint issues

db7fadf

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

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

Update screenshots

7d7b96e

jmartinesp added the PR-Bugfix For bug fix label Jul 15, 2024

Add missing tests

ca0c43e

jmartinesp marked this pull request as ready for review July 15, 2024 12:10

jmartinesp requested a review from a team as a code owner July 15, 2024 12:10

jmartinesp requested review from ganfra and removed request for a team July 15, 2024 12:10

ganfra reviewed Jul 15, 2024

View reviewed changes

ganfra approved these changes Jul 15, 2024

View reviewed changes

Fix review comments

b0002b2

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

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

jmartinesp merged commit 2ff5fa6 into develop Jul 15, 2024
30 checks passed

jmartinesp deleted the fix/jme/restore-mentions-in-drafts-and-formatting-toggle branch July 15, 2024 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore intentional mentions in the markdown/plain text editor #3193

Restore intentional mentions in the markdown/plain text editor #3193

jmartinesp commented Jul 15, 2024 •

edited

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading

codecov bot commented Jul 15, 2024 •

edited

Loading

ganfra Jul 15, 2024

jmartinesp Jul 15, 2024

ganfra Jul 15, 2024

ganfra left a comment

ganfra Jul 15, 2024

bmarty Jul 15, 2024

jmartinesp Jul 15, 2024

bmarty Jul 15, 2024

ganfra Jul 15, 2024

sonarqubecloud bot commented Jul 15, 2024

Restore intentional mentions in the markdown/plain text editor #3193

Restore intentional mentions in the markdown/plain text editor #3193

Conversation

jmartinesp commented Jul 15, 2024 • edited Loading

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Jul 15, 2024 • edited Loading

codecov bot commented Jul 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganfra 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

sonarqubecloud bot commented Jul 15, 2024

Quality Gate passed

jmartinesp commented Jul 15, 2024 •

edited

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading

codecov bot commented Jul 15, 2024 •

edited

Loading