Plain text editor implementation based on markdown input #2840

jmartinesp · 2024-05-14T11:50:43Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Added MarkdownTextInput, a TextInput counterpart that uses a simple EditText under the hood, instead of the RTE library. This will be the default editor unless the user explicitly enables rich text formatting.
Added MarkdownTextEditorState to act as a way to interact with the EditText from Compose code.
Wrapped both RichTextEditorState and MarkdownTextEditorState into a TextEditorState sealed interface with some helper functions.
Moved the parsing of the current editor state to send a message to MessageComposerPresenter from the View, as well as adding mention spans to the plain text editor. Sadly, this means having to add a MentionSpanProvider to presenter, which makes it more difficult to test. I also had to add isTesting and showFormatting properties to the presenter so it can be easily tested. I'll probably iterate on this later.
Removed the 'enable rich text editor' option from advanced settings, replaced it with a build config option in MessageComposerConfig in the appconfig module.

Known issues:

Mentions can't be converted from/to plain text and rich text yet. I'll handle this in another PR.
We probably want to keep track of whether the text formatting mode was last enabled so we can restore the text editor with that same value when navigating from one room to the other. This will also be handled in another PR.

Needs matrix-org/matrix-rich-text-editor#978.

Motivation and context

The RTE library has been problematic for some users, most of which didn't want to use rich text at all, so enabling

Screenshots / GIFs

I made the 2 editors look exactly the same, even replaced the editor used in some previews and the golden screenshots didn't change.

Tests

Thoroughly test the plain text editor and enabling / disabling text formatting, editing messages, replying to messages, etc.

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-05-14T12:02:27Z

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

codecov · 2024-05-14T14:16:19Z

Codecov Report

Attention: Patch coverage is 82.59861% with 75 lines in your changes are missing coverage. Please review.

Project coverage is 74.34%. Comparing base (cddb02f) to head (ca4691e).

Files	Patch %	Lines
...ent/android/libraries/textcomposer/TextComposer.kt	77.33%	3 Missing and 14 partials ⚠️
...tcomposer/components/markdown/MarkdownTextInput.kt	74.24%	1 Missing and 16 partials ⚠️
...ries/textcomposer/model/MarkdownTextEditorState.kt	82.69%	2 Missing and 7 partials ⚠️
...s/impl/messagecomposer/MessageComposerPresenter.kt	82.50%	4 Missing and 3 partials ⚠️
...android/libraries/textcomposer/ComposerModeView.kt	92.63%	2 Missing and 5 partials ⚠️
...id/libraries/textcomposer/model/TextEditorState.kt	75.00%	0 Missing and 6 partials ⚠️
...oid/libraries/textcomposer/mentions/MentionSpan.kt	66.66%	0 Missing and 3 partials ⚠️
...aries/textcomposer/mentions/MentionSpanProvider.kt	81.25%	2 Missing and 1 partial ⚠️
...composer/components/markdown/StableCharSequence.kt	77.77%	2 Missing ⚠️
...ages/impl/mentions/MentionSuggestionsPickerView.kt	87.50%	0 Missing and 1 partial ⚠️
... and 3 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2840      +/-   ##
===========================================
+ Coverage    74.30%   74.34%   +0.03%     
===========================================
  Files         1530     1536       +6     
  Lines        36526    36734     +208     
  Branches      7055     7123      +68     
===========================================
+ Hits         27142    27309     +167     
- Misses        5698     5700       +2     
- Partials      3686     3725      +39

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

- Fix autofocus of message composer. - Remove `Message` data class, fetch the details in `MessagesPresenter` instead. - Remove `enable rich text` option from advanced settings, set it as a build configuration instead. - Fix tests

ElementBot · 2024-05-16T07:05:39Z

	Warnings
⚠️	gradle/libs.versions.toml#L6 - A newer version of com.android.tools.build:gradle than 8.4.0 is available: 8.4.1
⚠️	gradle/libs.versions.toml#L12 - A newer version of androidx.core:core-ktx than 1.13.0 is available: 1.13.1
⚠️	gradle/libs.versions.toml#L18 - A newer version of androidx.datastore:datastore-preferences than 1.0.0 is available: 1.1.1
⚠️	gradle/libs.versions.toml#L21 - A newer version of androidx.lifecycle:lifecycle-runtime-ktx than 2.7.0 is available: 2.8.0
⚠️	gradle/libs.versions.toml#L22 - A newer version of androidx.activity:activity-compose than 1.8.2 is available: 1.9.0

	Messages
📖	gradle/libs.versions.toml#L136 - There are multiple dependencies junit:junit but with different version
📖	gradle/libs.versions.toml#L138 - There are multiple dependencies androidx.test.ext:junit but with different version
📖	gradle/libs.versions.toml#L207 - There are multiple dependencies junit:junit but with different version
📖	gradle/libs.versions.toml#L208 - There are multiple dependencies androidx.test.ext:junit but with different version

Generated by 🚫 dangerJS against ca4691e

bmarty

Thanks, 2 small remarks after a static review, I will test it this morning.

bmarty · 2024-05-15T19:28:05Z

.../textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/TextComposer.kt

+            }
+        }
+        is TextEditorState.Markdown -> {
+            val placeholder = if (composerMode.inThread) {


Maybe extract the placeholder computation to limit code duplication?

bmarty · 2024-05-15T19:32:02Z

...otlin/io/element/android/libraries/textcomposer/components/markdown/ImmutableCharSequence.kt

+import io.element.android.libraries.core.extensions.orEmpty
+
+@Stable
+class ImmutableCharSequence(initialText: CharSequence = "") {


The name is a bit confusing me, since the CharSequence is actually mutable. Maybe rename to StableCharSequence?

bmarty

Doing some test. The Markdown composer is working as expected, but when in Rich, the composer is not emptied when the message is sent, so user can sent the same message several times.

Probably related, but if instead of sending again I close the rich text mode, the application crashes

java.lang.IndexOutOfBoundsException: setSpan (1 ... 1) ends beyond length 0

jmartinesp · 2024-05-16T10:17:20Z

@bmarty your issue above should be fixed in b73a63b.

I also added changes for your other comments.

bmarty

Thanks, working as expected now!

jmartinesp · 2024-05-21T10:21:22Z

Nice! I'll release a RTE version compatible with these changes and merge the PR.

…t-editor

sonarcloud · 2024-05-21T11:24:13Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

jmartinesp · 2024-05-21T11:59:21Z

The Maestro job got stuck after finishing so it was in a limbo. I just force merged.

jmartinesp force-pushed the misc/jme/plain-text-editor branch from 5e7592d to 285ecf0 Compare May 14, 2024 11:53

jmartinesp added Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. labels May 14, 2024

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

jmartinesp force-pushed the misc/jme/plain-text-editor branch 2 times, most recently from 5445910 to dc32b1b Compare May 15, 2024 07:53

jmartinesp changed the title ~~WIP: plain text editor implementation~~ Plain text editor implementation based on markdown input May 15, 2024

jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label May 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 May 15, 2024

jmartinesp marked this pull request as ready for review May 15, 2024 09:24

jmartinesp requested a review from a team as a code owner May 15, 2024 09:24

jmartinesp requested review from bmarty and removed request for a team May 15, 2024 09:24

jmartinesp and others added 13 commits May 15, 2024 11:51

Add plain text editor based on markdown input

489fde7

Improve layout, fix lots of minor issues

be4e0fb

Fix switching between plain text and formatted editors

d22c267

Changes:

698fd95

- Fix autofocus of message composer. - Remove `Message` data class, fetch the details in `MessagesPresenter` instead. - Remove `enable rich text` option from advanced settings, set it as a build configuration instead. - Fix tests

Use snapshot version so the CI can at least run

070f1e4

Fix lint issues

d13927a

Fix MentionSpanProvider

67726d7

Some cleanup and new tests

70eccfc

Fix simple composer screenshot

5b877cf

Add more tests and previews

6871ae3

Some more cleanup

7009d7a

Update screenshots

241e3e8

Add changelog

7c45bb7

jmartinesp force-pushed the misc/jme/plain-text-editor branch from 57f0c8a to 7c45bb7 Compare May 15, 2024 09:51

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

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

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

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

Try fixing Maestro tests

c8e87c1

jmartinesp force-pushed the misc/jme/plain-text-editor branch from 71cad2a to c8e87c1 Compare May 16, 2024 07:04

bmarty reviewed May 16, 2024

View reviewed changes

bmarty requested changes May 16, 2024

View reviewed changes

jmartinesp added 3 commits May 16, 2024 11:06

Fix sending messages not clearing the right text editor state

b73a63b

Rename ImmutableCharSequence, move placeholder generation

93e1590

Fix maestro tests

1e079d3

jmartinesp requested a review from bmarty May 16, 2024 10:17

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

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

bmarty approved these changes May 21, 2024

View reviewed changes

jmartinesp mentioned this pull request May 21, 2024

Version 2.37.3 matrix-org/matrix-rich-text-editor#982

Merged

jmartinesp added 2 commits May 21, 2024 13:17

Bump RTE library to released v2.37.3

4864da3

Merge remote-tracking branch 'origin/develop' into misc/jme/plain-tex…

ca4691e

…t-editor

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

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

jmartinesp enabled auto-merge (squash) May 21, 2024 11:36

jmartinesp disabled auto-merge May 21, 2024 11:58

jmartinesp merged commit 880ebb4 into develop May 21, 2024
25 of 26 checks passed

jmartinesp deleted the misc/jme/plain-text-editor branch May 21, 2024 11:58

frebib mentioned this pull request May 22, 2024

Text composer no longer auto-capitalises the first lettter #2896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plain text editor implementation based on markdown input #2840

Plain text editor implementation based on markdown input #2840

jmartinesp commented May 14, 2024 •

edited

github-actions bot commented May 14, 2024 •

edited

codecov bot commented May 14, 2024 •

edited

ElementBot commented May 16, 2024 •

edited

bmarty left a comment

bmarty May 15, 2024

bmarty May 15, 2024

bmarty left a comment •

edited

jmartinesp commented May 16, 2024 •

edited

bmarty left a comment

jmartinesp commented May 21, 2024

sonarcloud bot commented May 21, 2024

jmartinesp commented May 21, 2024

Plain text editor implementation based on markdown input #2840

Plain text editor implementation based on markdown input #2840

Conversation

jmartinesp commented May 14, 2024 • edited

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented May 14, 2024 • edited

codecov bot commented May 14, 2024 • edited

Codecov Report

ElementBot commented May 16, 2024 • edited

bmarty left a comment

Choose a reason for hiding this comment

bmarty May 15, 2024

Choose a reason for hiding this comment

bmarty May 15, 2024

Choose a reason for hiding this comment

bmarty left a comment • edited

Choose a reason for hiding this comment

jmartinesp commented May 16, 2024 • edited

bmarty left a comment

Choose a reason for hiding this comment

jmartinesp commented May 21, 2024

sonarcloud bot commented May 21, 2024

Quality Gate passed

jmartinesp commented May 21, 2024

jmartinesp commented May 14, 2024 •

edited

github-actions bot commented May 14, 2024 •

edited

codecov bot commented May 14, 2024 •

edited

ElementBot commented May 16, 2024 •

edited

bmarty left a comment •

edited

jmartinesp commented May 16, 2024 •

edited