Rich Text Editor #292

aviraldg · 2016-05-27T04:51:05Z

Opening this now for tracking fix to element-hq/element-web#572. There is still a significant amount of work left:

fixing regressions
getting markdown to highlight and work correctly
figuring out whitespace issues
saving and restoring editor state (maybe blocked on draft-js itself)
editor toolbar

- use <p> for unstyled blocks - fix return key bug - editor placeholder

… feature-rte

dbkr · 2016-06-10T09:25:30Z

src/RichText.js

@@ -0,0 +1,82 @@
+import {Editor, ContentState, convertFromHTML, DefaultDraftBlockRenderMap, DefaultDraftInlineStyle, CompositeDecorator} from 'draft-js';
+const ReactDOM = require('react-dom');


You seem to have a mix of import styles here - unsure if we're planning on switching over, but we should probably at least be consistent within a file

Not quite sure how that happened -- will fix.

dbkr · 2016-06-10T11:25:35Z

Made quite a few comments: I think some things are probably just from it being WIP and the rest is just code style (javascript being terrible for this because there are so many ways of writing javascript). The two general things I would say are:

if in doubt, stick to the conventions of the existing code base (or check with us if you'd like to improve the style)
Some comments would be good just to explain what more complex bits of code are doing

As per the chat on #vector-dev, the style you have is more modern, so rather than change it to match the rest of the codebase, we're going to start moving the codebase over, so let's keep your ES6 class components and annotations etc.

But all the important stuff like how it's working is looking good, and it seems to be working nicely! Plus, having the bulk of the logic in a separate class is great.

As per the chat on #vector-dev, the plan going forward is:

Fix up any style things
Put in the old markdown stuff as 'markdown mode' and make it the default while we polish richtext mode
Get the old autocomplete working: possibly only in markdown mode for now, since if richtext mode is not the default, this can come later without regressing on features.

Hopefully made the code a little bit clearer.

Since it was not correctly extracting the selected part of the text string

Add basic Markdown highlighting

dbkr · 2016-06-13T17:59:52Z

src/RichText.js

+};
+
+const MARKDOWN_REGEX = {
+    LINK: /(?:\[([^\]]+)\]\(([^\)]+)\))|\<(\w+:\/\/[^\>]+)\>/g,


I guess we've done this to fit into the same model as richtext, but doing this manually with regexes rather than using the markd library feels like a major step back here. There must be lots of markdown syntax we don't support?

So this *only *handles the preview that's rendered in the composer (which
for obvious reasons can't be a proper rendering) -- the sent message still
uses the markdown library.

On Mon, Jun 13, 2016 at 11:29 PM David Baker notifications@github.com
wrote:

In src/RichText.js
#292 (comment)
:

+import * as sdk from './index';
+
+const BLOCK_RENDER_MAP = DefaultDraftBlockRenderMap.set('unstyled', {

element: 'p' // draft uses
by default which we don't really like, so we're using

+});

+const STYLES = {

BOLD: 'strong',

CODE: 'code',

ITALIC: 'em',

STRIKETHROUGH: 's',

UNDERLINE: 'u'
+};

+const MARKDOWN_REGEX = {

LINK: /(?:[([^]]+)](([^)]+)))|<(\w+://[^>]+)>/g,

I guess we've done this to fit into the same model as richtext, but doing
this manually with regexes rather than using the markd library feels like a
major step back here. There must be lots of markdown syntax we don't
support?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/matrix-org/matrix-react-sdk/pull/292/files/df69d1de44810d2c6d8a338be33b73ee8829d177#r66838004,
or mute the thread
https://github.com/notifications/unsubscribe/AAOlRL8py71C3WdN0zy-XEyk2avtK7rtks5qLZqZgaJpZM4IoK58
.

Ah right, I see. Still doesn't feel great to have two different types of parsing for the markdown, but if it's not on the critical message path then probably ok.

I know, I was complaining about it on #vector-dev too, but there's very
little that can be done here.

On Tue, Jun 14, 2016 at 12:13 AM David Baker notifications@github.com
wrote:

In src/RichText.js
#292 (comment)
:

+import * as sdk from './index';
+
+const BLOCK_RENDER_MAP = DefaultDraftBlockRenderMap.set('unstyled', {

element: 'p' // draft uses
by default which we don't really like, so we're using

+});

+const STYLES = {

BOLD: 'strong',

CODE: 'code',

ITALIC: 'em',

STRIKETHROUGH: 's',

UNDERLINE: 'u'
+};

+const MARKDOWN_REGEX = {

LINK: /(?:[([^]]+)](([^)]+)))|<(\w+://[^>]+)>/g,

Ah right, I see. Still doesn't feel great to have two different types of
parsing for the markdown, but if it's not on the critical message path then
probably ok.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/matrix-org/matrix-react-sdk/pull/292/files/df69d1de44810d2c6d8a338be33b73ee8829d177#r66845074,
or mute the thread
https://github.com/notifications/unsubscribe/AAOlRHcbqZZTJzWOzcFdS4A5Xabm-zmlks5qLaTZgaJpZM4IoK58
.

Why can't you parse it to markdown then render that markdown as the preview?

Because at the end of the day, it still needs to be Markdown: using rendered Markdown will strip formatting characters and mangle tables, code blocks, etc. such that it becomes more like a rich text editor than a markdown one.

mangle tables, code blocks, etc

I don't see how you can sensibly display these things "as-you-type" Markdown. If we can't do all of markdown, we'll end up with a mix of some stuff that is easy to render as markdown and some stuff which is too difficult to render sensibly so we don't bother. I'm not sure I see the value in this.

If we do have a way of representing all of Markdown as you type which doesn't make me want to stab the input box, then surely we should be using the actual hooks given to us in the marked library to implement custom Renderers for things like strong, em, codespan, etc. This would make things a lot cleaner and doesn't introduce subtle formatting differences between our hand-crafted regex and the marked parser. See https://github.com/chjj/marked/blob/88ce4df47c4d994dc1b1df1477a21fb893e11ddc/lib/marked.js#L857

kegsay · 2016-06-14T10:47:15Z

src/RichText.js

+            () => true, // always return true => don't filter any ranges out
+            (start, end) => {
+                // map style names to elements
+                let tags = block.getInlineStyleAt(start).map(style => STYLES[style]);


If block.getInlineStyleAt returns a style that is not in STYLES, then STYLE[style] will be undefined which will populate tags with undefined entries. This will lead to tags like <undefined> on :43. You need to sanity check more.

While this shouldn't happen, since STYLES lists all the builtin styles currently supported, I agree that it'll be safer to check.

subscribernamegoeshere · 2024-02-02T15:48:52Z

will the element desktop/web clients ever support simple stuff such as strikethrough, bold, italic, whatever or some simple markings colors etc for example to edit/redact/mark/highlight etc ones owns chats and text etc later in a point in time to make visually slightly visible modifications etc? i am a noob about the element chat clients (web/desktop). thanks.

Initial version of rich text editor

001011d

aviraldg mentioned this pull request May 27, 2016

Rich Text Editor element-hq/element-web#1553

Merged

minor improvements

fe76eb9

- use <p> for unstyled blocks - fix return key bug - editor placeholder

aviraldg force-pushed the feature-rte branch from 07c539e to fe76eb9 Compare May 28, 2016 06:32

Merge branch 'develop' into feature-rte

96526c2

aviraldg mentioned this pull request Jun 1, 2016

😄 Emoji autocomplete and unicode emoji to image conversion using emojione. #255

Closed

aviraldg added 3 commits June 9, 2016 23:53

user and room decorators, history & typing notifs

29cdd1f

Fix MessageComposerInput.setLastTextEntry

4e0720d

Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…

bf8e56e

… feature-rte

dbkr reviewed Jun 10, 2016
View reviewed changes

aviraldg and others added 5 commits June 11, 2016 15:52

rte improvements, markdown mode

e4217c3

cleanup, better comments, markdown hotkeys

b960d22

get /commands working again

c0d7629

Basic Markdown highlighting

a5a3e4e

Fix highlighting behaviour on switch

8f45f16

Hopefully made the code a little bit clearer.

pferreir and others added 3 commits June 12, 2016 01:08

Fix behaviour of modifyText

294a8ef

Since it was not correctly extracting the selected part of the text string

Minimal house cleaning

e75a28b

Add basic Markdown highlighting

df69d1d

Add basic Markdown highlighting

dbkr reviewed Jun 13, 2016
View reviewed changes

aviraldg added 2 commits June 13, 2016 23:57

Merge branch 'develop' into feature-rte

74527a4

use rte labs setting

34be17c

kegsay reviewed Jun 14, 2016
View reviewed changes

aviraldg added 3 commits June 14, 2016 19:10

fixes and improvements in RichText

2606ea9

use constants for keycodes in RTE

8cb086e

more RTE fixes

ba69e43

dbkr merged commit 5199cd0 into matrix-org:develop Jun 14, 2016

aviraldg mentioned this pull request Jun 19, 2016

Rich text input element-hq/element-web#572

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rich Text Editor #292

Rich Text Editor #292

aviraldg commented May 27, 2016 •

edited

Loading

dbkr Jun 10, 2016

aviraldg Jun 10, 2016

dbkr commented Jun 10, 2016

dbkr Jun 13, 2016

aviraldg Jun 13, 2016

dbkr Jun 13, 2016

aviraldg Jun 13, 2016

kegsay Jun 14, 2016

aviraldg Jun 14, 2016

kegsay Jun 14, 2016

kegsay Jun 14, 2016

aviraldg Jun 14, 2016

subscribernamegoeshere commented Feb 2, 2024

		@@ -0,0 +1,82 @@
		import {Editor, ContentState, convertFromHTML, DefaultDraftBlockRenderMap, DefaultDraftInlineStyle, CompositeDecorator} from 'draft-js';
		const ReactDOM = require('react-dom');

Rich Text Editor #292

Rich Text Editor #292

Conversation

aviraldg commented May 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbkr commented Jun 10, 2016

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

subscribernamegoeshere commented Feb 2, 2024

aviraldg commented May 27, 2016 •

edited

Loading