Integrate WYSIWYG editor #7288

jmartinesp · 2022-10-04T16:22:11Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Adds the io.element.android:wysiwyg library, which features the new WYSIWYG editor. Modified MessageComposerFragment to be able to toggle between the current composer and the new one using a labs flag.

Motivation and context

We want to integrate the initial version of the WYSIWIG editor into the real app to be able to send rich text messages without having to use markdown.

Screenshots / GIFs

Before	After

Tests

Go to Settings > Labs, toggle 'Enable rich text editor'.
Open any room or DM.
Type any text using formatting and send it.
You can also test if formatted text is sent for message edits, quotes, replies, etc.

Tested devices

Physical
Emulator
OS version(s): 11, 12

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

jmartinesp · 2022-10-05T15:50:53Z

Please note that this is the just the initial version of the editor, so it lacks some features like autocomplete, lists, links, pills, etc. and might have some corner cases that we might have missed on our testing sessions, that's expected.

bmarty

Nice work!
Quick smoke test with the RTE enabled, seems to work fine.
The design will be updated? Could be nice to have something similar Element Web with popup:

bmarty · 2022-10-07T09:38:21Z

changelog.d/7288.feature

@@ -0,0 +1 @@
+Add WYSIWYG editor.


Can you add a 7288.sdk file to describe the changes in the SDK API? Thanks.

bmarty · 2022-10-07T09:43:13Z

...-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/timeline/TimelineEvent.kt

+        lastMessageContent.formattedBody
+    } else {
+        lastMessageContent?.body
+    } ?: return ""


Nit: maybe create an extension

fun MessageContent?.getFormattedBody(): String? { return if (this is MessageContentWithFormattedBody) { formattedBody } else { this?.body } }

bmarty · 2022-10-07T09:46:03Z

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt

            newBodyAutoMarkdown: Boolean,
            msgType: String,
            compatibilityText: String
    ): Event {
+        val content = if (newBodyFormattedText != null) {
+            TextContent(newBodyText.toString(), newBodyFormattedText.toString()).toMessageTextContent(msgType)


Maybe we should consider changing TextContent to have 2 CharSequence members.

I think it probably makes sense to keep using String for these, as both text and formattedText are expected to be the plain text representation of the contents (either a plain text string or the HTML format of that same text). I'm no expert on this part of the code though, so if you think we should change it I can do it.

Yes, you are right. Thanks

bmarty · 2022-10-07T09:49:24Z

vector/src/main/java/im/vector/app/core/extensions/EditText.kt

@@ -66,7 +67,7 @@ fun EditText.setTextIfDifferent(newText: CharSequence?): Boolean {
        // Previous text is the same. No op
        return false
    }
-    setText(newText)
+    setText(newText, TextView.BufferType.SPANNABLE)


Since the extension is used at other places, maybe add a parameter with default value to use TextView.BufferType.SPANNABLE on demand only?

Actually, this change wasn't needed, I forgot to revert a test. Great catch!

bmarty · 2022-10-07T09:51:09Z

vector/src/main/java/im/vector/app/features/command/CommandParser.kt

-                    if (message.isNotEmpty()) {
-                        ParsedCommand.ChangeDisplayNameForRoom(displayName = message)
+                    if (trimmedMessage.isNotEmpty()) {
+                        ParsedCommand.ChangeDisplayNameForRoom(displayName = trimmedMessage)


Could be better to reuse message name instead of trimmedMessage to avoid too many changes in this file.

Done, but I had to add a suppression for name shadowing.

bmarty · 2022-10-07T10:03:52Z

...r/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt

+                                        autoMarkdown = false
+                                )
+                            } else {
+                                room.sendService().sendFormattedTextMessage(parsedCommand.message.toString(), parsedCommand.formattedMessage)


For parity with the if block, maybe use named param, and 1 per line.

bmarty · 2022-10-07T10:05:01Z

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

+import im.vector.app.databinding.ViewRichTextMenuButtonBinding
+import io.element.android.wysiwyg.InlineFormat
+
+class RichTextComposerView @JvmOverloads constructor(


Maybe rename to RichTextComposerLayout (see my other remark about renaming)?

bmarty · 2022-10-07T10:07:53Z

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

+        defStyleAttr: Int = 0
+) : ConstraintLayout(context, attrs, defStyleAttr), MessageComposer {
+
+    val views: ComposerRichTextLayoutBinding


Could be private I think.

bmarty · 2022-10-07T10:08:41Z

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

+                override fun onTransitionCancel(transition: Transition) {}
+
+                override fun onTransitionStart(transition: Transition) {}
+            })


Could extend SimpleTransitionListener to avoid empty implementation.

bmarty · 2022-10-07T10:16:24Z

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

+    android:layout_marginHorizontal="2dp"
+    android:background="@android:color/transparent"
+    android:contentDescription="@string/app_name">
+    <!-- The contentDescription attr is populated programmatically. This is just to fix lint issues. -->


I think you want to add a tint, so that it works well on light and dark theme.

As replied above, the final designs will be implemented on another PR if that's fine.

…ges.

jmartinesp · 2022-10-10T09:54:07Z

Nice work! Quick smoke test with the RTE enabled, seems to work fine. The design will be updated? Could be nice to have something similar Element Web with popup:

The design that was chosen will keep this layout, we thought about using popups but they'd clash with Android native ones (copy, paste, select all, etc.) and there might not be room for all the options to include in the future.

The design will be updated in a future PR, adding background and tint colors to the buttons and other sizes and appearance adjustments.

sonarcloud · 2022-10-10T13:24:22Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

5.4% Coverage
15.3% Duplication

bmarty

Thanks for the update!

andybalaam · 2022-10-11T13:53:31Z

@bmarty since @jmartinesp is away, would you be able to merge this? Thanks!

jmartinesp self-assigned this Oct 4, 2022

jmartinesp mentioned this pull request Oct 5, 2022

Refactor: split TimelineFragment into MessageComposerFragment and VoiceRecorderFragment #7285

Merged

15 tasks

jmartinesp force-pushed the tech/integrate-wysiwyg branch 3 times, most recently from 754f7f3 to 0cbc8cb Compare October 5, 2022 15:46

jmartinesp marked this pull request as ready for review October 5, 2022 15:47

jmartinesp requested review from a team and yostyle and removed request for a team October 5, 2022 15:48

jmartinesp force-pushed the tech/integrate-wysiwyg branch from 0cbc8cb to 03ac243 Compare October 5, 2022 19:30

bmarty requested review from bmarty and removed request for yostyle October 7, 2022 09:36

bmarty reviewed Oct 7, 2022

View reviewed changes

jmartinesp added 11 commits October 10, 2022 10:40

Add WYSIWYG lib dependency

2521b83

Replace EditText with RichTextEditor

5badbda

Add bold button, fix sending formatting messages issues

00fc854

Add missing inline formatting buttons, make scrollview horizontal

cbdb197

Disable autocomplete for rich text editor

e14bea8

Add formatted text to messages sent, replies, quotes and edited messa…

ded508d

…ges.

Several fixes

994aee4

Add changelog

3f8fb10

Try to fix lint issues

945018e

Address review comments.

6cc187b

Exclude Epoxy KSP generated files from ktlint checks

820bf8b

jmartinesp force-pushed the tech/integrate-wysiwyg branch from 1609e96 to 820bf8b Compare October 10, 2022 09:50

jmartinesp requested a review from bmarty October 10, 2022 10:50

bmarty approved these changes Oct 10, 2022

View reviewed changes

jmartinesp merged commit def67b2 into develop Oct 11, 2022

jmartinesp deleted the tech/integrate-wysiwyg branch October 11, 2022 15:05

jmartinesp mentioned this pull request Oct 13, 2022

Apply design changes to WYSIWYG editor #7354

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate WYSIWYG editor #7288

Integrate WYSIWYG editor #7288

jmartinesp commented Oct 4, 2022 •

edited

Loading

jmartinesp commented Oct 5, 2022

bmarty left a comment

bmarty Oct 7, 2022

bmarty Oct 7, 2022

bmarty Oct 7, 2022

jmartinesp Oct 10, 2022

bmarty Oct 10, 2022

bmarty Oct 7, 2022

jmartinesp Oct 10, 2022

bmarty Oct 7, 2022

jmartinesp Oct 10, 2022

bmarty Oct 7, 2022

bmarty Oct 7, 2022

bmarty Oct 7, 2022

bmarty Oct 7, 2022

bmarty Oct 7, 2022

jmartinesp Oct 10, 2022

jmartinesp commented Oct 10, 2022 •

edited

Loading

sonarcloud bot commented Oct 10, 2022

bmarty left a comment

andybalaam commented Oct 11, 2022

Integrate WYSIWYG editor #7288

Integrate WYSIWYG editor #7288

Conversation

jmartinesp commented Oct 4, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

jmartinesp commented Oct 5, 2022

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

jmartinesp commented Oct 10, 2022 • edited Loading

sonarcloud bot commented Oct 10, 2022

bmarty left a comment

Choose a reason for hiding this comment

andybalaam commented Oct 11, 2022

jmartinesp commented Oct 4, 2022 •

edited

Loading

jmartinesp commented Oct 10, 2022 •

edited

Loading