Skip to content

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Oct 8, 2020

Requires nextcloud-libraries/nextcloud-vue#1433

The RichContenteditable component provides its own template for candidate mentions, so the Mention component is no longer used in the input field. However, it is still used when rendering messages, so it can not be removed. It would be good to avoid having duplicate logic (for example, to select the icon for a guest mention).

Pending:

  • Proper handling of new lines
  • Fill user data for MentionBubble (so once you select a mention the user name is shown in the text instead of the id)
  • Use the same icon for guests as what will be shown in the chat (instead of always using the user icon)
  • Send messages when pressing Enter (take into account Added setting to change preferred key for sending messages #4259 too)
  • Pasting files
  • Shortcut to focus chat
  • Dark mode
  • Check if the exported mixin from RichContenteditable can be used to solve the duplicated logic mentioned above
  • handle isCompositing event attribute to prevent sending message when using input methods: Prevent submitting the message when the user is composing #5939 (ideal is to streamline this in the vue lib directly)

@PVince81
Copy link
Member

PVince81 commented Oct 8, 2020

for new lines, see 1adf641
might save some time

@nickvergessen
Copy link
Member

That commit should go into the lib directly, so behaviour is consistent across all points of nextcloud

@nickvergessen
Copy link
Member

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.

@nickvergessen nickvergessen deleted the replace-custom-component-with-new-richcontenteditable branch October 7, 2021 16:48
@marcoambrosini marcoambrosini restored the replace-custom-component-with-new-richcontenteditable branch September 12, 2022 06:59
@marcoambrosini marcoambrosini force-pushed the replace-custom-component-with-new-richcontenteditable branch 2 times, most recently from af0c0fa to 20fd12b Compare October 5, 2022 12:16
@ShGKme ShGKme force-pushed the replace-custom-component-with-new-richcontenteditable branch from db8624b to 8144b9f Compare February 20, 2023 15:28
@ShGKme
Copy link
Contributor

ShGKme commented Feb 20, 2023

Replacement completion

  • Fix styling:
    • Fix paddings
    • Fix max-height
    • Add hover/active/focus styles
    • Remove unnecessary deep overriding
    • Use variable instead of constant when applicable
  • Migration:
    • Fix placeholder: placeholder-textplaceholder
    • Fix disabling: active-inputdisabled
    • Remove parsedText (now there is no difference with text)
  • Fix focus after joining a chat (with nextTick)
  • Add missed shortcuts:
    • ESC - blur input
    • c - focus input (once)
  • Remove the old AdvancedInput
  • Uninstall vue-at
  • l10n (no changes)
  • Linting

Checklist

  • No visual differences with the old AdvancedInput
    • +Dark theme
  • States:
    • Active/focus/hover
    • Disabled
  • Sending messages
  • Multiline
    • +Max lines
  • Pasting files
  • Focus when a chat is loaded
  • Emoji picker
  • Mentions
  • l10n
  • Shortcuts:
    • ESC - blur
    • c - focus only once
    • Enter on mentions/emojies doesn't send a message (worked in NcRichContenteditable)

1
1-2
2
2-2
3
3-2
4
4-2
5
5-2
6
7
8

@ShGKme
Copy link
Contributor

ShGKme commented Feb 20, 2023

Emoji Picker has a problem: it is supposed to replace the selection in the input, but the selection is lost after the emoji picker opens. So an emoji is always added to the end. Also, it checks <br> in the text while \n is used now. It adds a new black line if the emoji is picked on an empty line.

But all those problems are not caused by this PR.

addEmoji(emoji) {
const selection = document.getSelection()
const contentEditable = this.$refs.richContenteditable.$refs.contenteditable
// There is no select, or current selection does not start in the
// content editable element, so just append the emoji at the end.
if (!contentEditable.isSameNode(selection.anchorNode) && !contentEditable.contains(selection.anchorNode)) {
// Browsers add a "<br>" element as soon as some rich text is
// written in a content editable div (for example, if a new line
// is added the div content will be "<br><br>"), so the emoji
// has to be added before the last "<br>" (if any).
if (this.text.endsWith('<br>')) {
this.text = this.text.slice(0, this.text.lastIndexOf('<br>')) + emoji + '<br>'
} else {
this.text += emoji
}
return
}
// Although due to legacy reasons the API allows several ranges the
// specification requires the selection to always have a single
// range.
// https://developer.mozilla.org/en-US/docs/Web/API/Selection#Multiple_ranges_in_a_selection
const range = selection.getRangeAt(0)
// Deleting the contents also collapses the range to the start.
range.deleteContents()
const emojiTextNode = document.createTextNode(emoji)
range.insertNode(emojiTextNode)
this.text = contentEditable.innerHTML
range.setStartAfter(emojiTextNode)
},

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The focus should return on the input after adding an emoji via emoji picker

Other than that I'd say that even with the caveats @ShGKme mentions, this is an awesome improvement and we should finally get it in.

@ShGKme
Copy link
Contributor

ShGKme commented Feb 20, 2023

  • The focus should return on the input after adding an emoji via emoji picker

It doesn't work on both master and stable25, requiring total addEmoji rewriting. I'd make it in a separate PR.

@ShGKme
Copy link
Contributor

ShGKme commented Feb 20, 2023

Ref: #4265

@nickvergessen
Copy link
Member

It doesn't work on both master and stable25,

In that case it does not block getting this in.

@nickvergessen
Copy link
Member

So, who hits the green button?

@ShGKme ShGKme merged commit 71dcfea into master Feb 21, 2023
@ShGKme ShGKme deleted the replace-custom-component-with-new-richcontenteditable branch February 21, 2023 09:15
@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@nickvergessen
Copy link
Member

We are not backporting this one

@ShGKme
Copy link
Contributor

ShGKme commented Feb 21, 2023

We are not backporting this one

I guess the backport bot was triggered by an old message after the merge...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants