Added setting to change preferred key for sending messages

[PVince81]

• Rebase after Replace custom component with new RichContenteditable #4333

Fixes #3268

Added checkbox in the settings dialog for users to choose
between Enter and Shift+Enter for sending messages.

The settingsStore was borrowed from #4231

Manual Tests

• TEST: enter key sends, shift-enter adds newlines
• TEST: when setting is enabled, shift-enter sends, enter adds newlines

[PVince81]

I meant to also post a screenshot, to show where the setting is, this is the best place I found so far where it doesn't look too much out of place:

[PVince81]

signoff bot was unhappy, I now simply squashed all, should be fine now

[PVince81]

@nickvergessen any objections to merging this ?

[nickvergessen]

With the setting changed, submitting:

fewfew
fwef

posts this in the chat:

<div>fewfew</div><div>fwef
</div>

(Firefox 81.0)

[PVince81]

this https://github.com/nextcloud/spreed/pull/4259/files#diff-3fab09c19bdf3c266b808d2335831664R412 had fixed it for me in Firefox 80...

hmmm, need to check more browsers then and/or fix it differently

[nickvergessen]

seems to not work in FF81?

[PVince81]

Apparently that fix only worked for Chrome, not Firefox...

I did more analysis and found out that if we leave the event untouched, then:

• enter key inserts div elements
• shift-enter inserts br elements

so to be consistent we'd need to find a way to also strip the div (did a few attempts but didn't get anything consistent)
or replace these with br

[PVince81]

Test result:

newlines with shift enter:
1
2

3


4

gives
newlines with shift enter:<br>1<br>2<br><br>3<br><br><br>4<br>

and with enter key:

newlines with enter:
1
2

3


4

gives <div>newlines with enter:</div><div>1</div><div>2</div><div><br></div><div>3</div><div><br></div><div><br></div><div>4<br></div>

😞

[PVince81]

I found a combination that works for both Firefox and Chromium for parsing:

			// first div doesn't count
			text = text.replace(/^<div>/g, '')

			// newlines from enter key
			//   extra newlines
			text = text.replace(/<div><br><\/div>/g, '\n')
			//   regular newlines
			text = text.replace(/<div>/g, '\n')

			// newlines from shift+enter key
			text = text.replace(/<br>/g, '\n')

			// clean up
			text = text.replace(/<\/div>/g, '')

			text = text.replace(/&nbsp;/g, ' ')

However, with emojis and "enter inserts newline" mode, type: "ab", insert emoji, "cd", then enter, then "ef" insert emoji.
At this stage, the emoji appears on a third line because of the extra "div" that the enter key inserted:

And I even managed to get this: 😱

Feels like a can of worms...

I'll try another alternative: preventing "div" insertion with preventDefault and then synthesizing a newline or br...

[PVince81]

made several attempts to insert a br instead of a div with

• document.execCommand('insertHTML', false, '<br>') => doesn't insert a BR when on the first line cursor at the end of text
• document.execCommand('insertHTML', false, '\n') => doesn't insert a BR when on the first line cursor at the end of text
• document.execCommand('insertHTML', false, '<br><br>') inserts a single newline in chrome, but a double in Firefox

inconsistencies...

[PVince81]

replacing <br> with \n works well in Chromium after detecting if first line or not.
however in Firefox it messes up...

[PVince81]

I went back to the original behavior of shift+enter before this PR and noticed that:

• Chromium inserts actual "\n" characters into the div
• Firefox inserts "
  " elements, but always has a non-deletable "
  " as a last node

I'll see if I can also have that extra "br" there.

(also need to test this with Edge and Safari...)

[PVince81]

When forcing having an initial "\n" inside the content editable:

• with display: inline-block in place:
  • Firefox
    • first time enter: works fine, rendered as <br>
    • adding newline in the middle of a string: works fine, rendered as <br>
  • Chromium
    • first time enter: works fine
    • adding newline in the middle of a string: double newlines! also: chromium renders <br> directly as newlines in the DOM instead of using the tag...
• without display: inline-block:
  • Firefox all fine as above
  • Chromium: first time enter doesn't work...

I think I'll abandon this approach...

Next I'll explore a hybrid approach:

• since we can strip divs in the final result, let's try stripping them earlier, every time enter is pressed
• when adding an emoji, to avoid element issues, also perfom the stripping before inserting the emoji

[PVince81]

I give up trying to hack this further. Too much time lost for this little feature.

I went with the approach where the "div" are stripped to reflect whatever the user typed in, this seems to work well most of the times.

There are situations in Chromium where inserting an emoji at the end might either end up in the last div element, or sometimes it appears after it. Despite exact same steps, the behavior seems to be randomly different, based on Chromium's mood. I suggest to investigate this later as part of the "emoji insertion position issues" ticket here #4265

[PVince81]

Tested with Chromium 85.0.4183.121, Firefox 80.0 and MS Edge / Win 10 from https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/

[nickvergessen]

So, do we do this? Or is it not needed anymore with #4333

[PVince81]

part for this PR will be needed for #4333 as well, at least the bits with the checkbox in settings.

but also, as I heard the rich editor will not handle enter keypresses for submitting, so these still need to be done from outside

@danxuliu are you fine if we move forward with this PR, knowing about future conflicts on #4333 ?

[danxuliu]

@danxuliu are you fine if we move forward with this PR, knowing about future conflicts on #4333 ?

Yes, I always intended to rebase #4333 on this once done, so no problem :-)

I would prefer not to use a server setting for this; I think that using the browser storage would be better in this case. Moreover that way the setting could be available for guest users too.

Also, in the new RichContenteditable component Ctrl+Enter is the combination used for submitting. Maybe it would be better to always use Shift+Enter for new lines, Ctrl+Enter for submitting, and make configurable whether Enter sends or adds a new line?

In any case, will adding a new line instead of sending when pressing Enter solve the original issue? Even if the message is not sent it would cause a new line to be added after each character, which will require that new line to be removed before typing the next character. Better than single character messages, but still problematic I think.

Unfortunately I have not been able to reproduce the original issue. I was able to press Enter to select Hiragana and Katakana characters without sending the message with Firefox and Chromium using ibus-anthy, ibus-mozc and fictx (in this case, Pinyin, not Japanese characters, but it should not make any difference) on GNU/Linux. So it would be good if @satorunsuto could try this pull request and tell us if it fixes the issue :-)

Finally, regarding the post-processing of divs when entering new lines, it seems to be working fine based on some little tests I made. Also the code looks good, and given the extensive testing by @PVince81, 👍

[PVince81]

@nickvergessen let's move this to backlog as it doesn't make sense to merge it before #4333 is done. When #4333 is done this PR here will need some adjustments.

[bushrang3r]

Thanks for everyone's work on this.
I'm just wondering, would a possible solution to this be to add a clickable send button next to the input field, and reserve the Enter key just for starting a new line?
This is the way Slack does it.

[nickvergessen]

A clickable button is already there

[bushrang3r]

Oops, my apologies. I somehow didn't see it.
I'll continue to watch this issue.

[bushrang3r]

Waiting eagerly for #4333 to get the 'Enter key for new line' functionality +1

Unfortunately I have not been able to reproduce the original issue. I was able to press Enter to select Hiragana and Katakana characters without sending the message with Firefox and Chromium using ibus-anthy, ibus-mozc and fictx (in this case, Pinyin, not Japanese characters, but it should not make any difference) on GNU/Linux.

To chime in on this comment, being able to reproduce this issue comes down to whether or not you can type Japanese at native speed. I'm currently administering a Nextcloud instance for a small Japanese team and the problem they have is obvious. If you think about how we type, we constantly hit superfluous keys, backspace, replace, backspace, replace. Typing at native speed is definitely not a linear, mistake-free activity. When it comes to selecting certain Japanese words with the Enter key at native speed, the Enter key is being hit very frequently and often superfluously. For English, it would be like using the space-bar to send the message. You would be constantly sending one word messages.

I would say this functionality - to have the Enter key insert a new line (and not send) - is essential.

[nickvergessen]

I will close the PR for now (as there was no active development since 6 months). Anyone can pick it up whenever they want and reopen the PR.