-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #209: Allow inserting emojis alongside text #202
base: master
Are you sure you want to change the base?
Conversation
@bLind17 Thank you for the PR. The emoji picker updates are great and seem like that should be the default behavior.
|
I have removed that commit, since I completely rewrote the chat window for my project, and I'm not using this code anymore 😉
I added that as a precaution, since I didn't know how you would deal with such major changes. I think it could be left out, but that would totally change the behaviour on existing projects, maybe there are some people out there who would be bothered by it?
Again, this is your choice, but I would never comprimise on usability to get cleaner code. If I select some text and then add an emoji, I expect the text to be overwritten, as that is the common behaviour. My new chat windows is based on your implementation and ideas, thank you for that! If you want to see it in action, you can check it out here (best two open two tabs and chat with yourself): My whole project is going to be open source at some point, so if you see something you like, I'll gladly let you check out my code. (So far I have added reactions and a reply-to feature and changed the emoji-picker positioning). See you around, cheers, Lukas! |
btw. if you add |
@bLind17 Thank you for your response.
Arguably. In the browser, clicking a focusable element (the emoji selector button, in this case) usually clears any selection.
I shouldn't have really used the word "compromise" in my previous comment as I see this as more of an implementation detail than a usability issue due to the above reason. An example of what complexity can obscure: the event listener added for By the way, I see the following error when trying to open the chat window at the link you shared and the window does not show up: |
While that may be true, I still feel like a chat window is like a tiny-text-editor and the behaviour makes sense. I checked with the WhatsApp web app and facebook chat, they both do it that way.
Well of course you are right with that, and I too consider the principle of clean code very imporant. The question is, what's the alternative? Once the focus goes to the emoji picker, the selection is cleared and we lose track of the caret, right?
Oh, thanks for the info. Works for me on the latest browser versions. I realize that the |
That's true; I concede. The event handler is needed for a proper implementation of this feature.
Starting from Chrome 85, yes. I'm currently on 81, and get left behind as I must manually update Ungoogled Chromium. This one's on me, too. Thanks for your responses. The additional prop, I will review the code soon and provide feedback, if any, or approve. |
Let me know what you decide about the default behaviour, I'll gladly remove the |
Sure. Also just noticed while testing that the undo buffer of the field gets messed up after an emoji is inserted (Cmd/Ctrl + Z). I wonder if that could be addressed if the emoji was inserted using a keyboard event. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
For my current project I needed to insert emojis into the input field rather then send them as a message directly.
This PR includes changes to realize that.
I have added a property
sendEmojisDirectly
that propagates towards theUserInput
.If set to
false
the new feature - respecting the last selection that has been made in the input fileld - will either insert the emoji at the caret or overwrite a selected range.