Skip to content

Conversation

@simonspa
Copy link
Contributor

@simonspa simonspa commented Jul 25, 2020

This MR adds the new emoji picker from nextcloud-vue to the new message form of the Talk text chat. This fixes #1130 .

image

image

image

@simonspa
Copy link
Contributor Author

What works:

  • emoji picker popover from nextcloud-vue
  • select emoji, add to end of text input

What doesn't work:

  • add emoji to cursor position - we're losing the caret position when focusing the popover.
  • re-focus the input after closing the picker - we can call AdvencedInput's focusInput() but that sets caret to start again
  • (future) it would be nice to add a handleColonEvent to AdvancedInput to allow selecting emojis directly similar to the handleAtEvent for participant mentions.

I'd appreciate some help for fixing these issues. 😏

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.

Super @simonspa!
As a minor note, I would swap the attachment and emoji buttons positions :)

Co-authored-by: Joas Schilling <[email protected]>
Signed-off-by: Simon Spannagel <[email protected]>
@simonspa simonspa force-pushed the emoji_picker_vue branch 2 times, most recently from 48150ca to 78f0268 Compare July 27, 2020 14:04
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Copy link
Member

Looks good as a first version, CSS styling needs some adjustment (icon not centered on button and the buttons should be more to the left now, so the text input field is aligned with posted messages again).

  • add emoji to cursor position - we're losing the caret position when focusing the popover.
  • re-focus the input after closing the picker - we can call AdvencedInput's focusInput() but that sets caret to start again
    (future) it would be nice to add a handleColonEvent to AdvancedInput to allow selecting emojis directly similar to the handleAtEvent for participant mentions.

I have to admit I'm as clueless as you are. I tried to figure out cursor position handling once, but it is not working with one of the browsers all the time, no matter which attempt I tried. Maybe @danxuliu has an idea, otherwise this is what we use for now and then check later.

Due to some other merges a rebase seems needed.

danxuliu added 2 commits July 29, 2020 18:24
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Instead of always inserting the emojis at the end of the text now they
are inserted at the current caret position, also replacing any selected
text. If the input is not focused then the emoji will be inserted at the
end like before.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu
Copy link
Member

add emoji to cursor position - we're losing the caret position when focusing the popover.

I have been investigating this but unfortunately I had limited success. I have added a commit to insert the emoji at the caret position that works as expected with latest Firefox. But...

  • With older Firefox versions (at least, in Firefox 68; I do not know in which version it started to work) it does not work because clicking an emoji in the picker focuses that emoji, so the selection is cleared in the message input. This can be reproduced in the EmojiPicker documentation.
  • With Chromium it does not work because clicking on the button to show the emoji picker moves the selection to the button to show the menu to attach files (I have no idea why). However, around 1 out of 5 times it works as expected, and the selection/caret position is not modified in the message input when clicking on the button to show the emoji picker. Again, no idea why.
    It is worth noting that it can not be reproduced in the EmojiPicker documentation (it always works as expected there). If you select a text in the page and then click on the button to show the EmojiPicker the selected text is not cleared, so it seems to be specific to Talk 🤷

Besides that the message input loses the caret position for example when writing in the search box of the picker. I have implemented a hack to keep a copy of the selection in the message input when it loses the focus and use that range to do the insertion. However I find it a bit counterintuitive, as it breaks the expectation regarding focus and selections (if you select some text, move the focus to other place and then add an emoji the emoji will replace what was in the selection even if it is no longer selected), so I would rather not apply it.

Independently of the above I have also fixed adding the emojis in Firefox when new lines are included in the input (as in that case Firefox adds an extra <br> at the end, so instead of just adding the emoji it has to be added before the last <br>).

@simonspa I have pushed my commits as well as one from @nickvergessen to your branch. I hope you do not mind :-) Also feel free to squash/fixup your commits with them ;-)

@jancborchardt

This comment has been minimized.

@nickvergessen
Copy link
Member

It now uses vue-material-design-icons/EmoticonOutline (screenshot is outdated) which is correct.

@nickvergessen
Copy link
Member

Needs a rebase

I would say let's get this in because of the feature freeze soon, and then we try to fix the position and focus things in a follow up

@nickvergessen
Copy link
Member

Rebased in #3994

@nickvergessen
Copy link
Member

Thanks a lot for the PR @simonspa we merged it via #3994 and it will be part of the Nextcloud 20 release

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.

Emoji picker in the chat

5 participants