Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Add builtin emoji support #2187

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

Foo-Manroot
Copy link
Contributor

Before this series of commits, the emojis shown on the device where vendor-specific and relied on the keyboard to be inserted. Now, there's an integrated popup where one can insert emojis. Also, the displayed emojis are changed, as shown on the image:
screenshot

The library that I used was vannkitec's, but some changes were needed to integrate it on this project, so I made a couple of modifications for that purpose. Until the new build is released, those changes are only available on the snapshot.

The emojis used are the ones from emoji-one for personal reasons (those were the ones I liked the most); but it can be changed easily modifying the dependency.

For more discussion about the need for built-in emoji support, refer to #819

I tried to follow the contribution guides; but I may have missed something. In that case, I'd appreciate some comments and I'll try to fix whatever problem there may be.

Signed-off-by: Foo-Manroot [email protected]

@bmarty
Copy link
Member

bmarty commented May 23, 2018

Hi @Foo-Manroot ,

Thanks for the PR. Contribution guide has been followed :) . We will discuss internally to decide if we can merge it. Do you know what can be the impacts on the other client (iOS/Web) when emojis are sent?

Benoît

@Foo-Manroot
Copy link
Contributor Author

This is just an aesthetic change, as the message is the same. It just differs on its presentation to the user. On riot-desktop there's already a builtin emoji set (incidentally, the same set as the one I used, from EmojiOne), so they're viewed exactly the same.

As per other clients, they'll still see the emojis exactly the same as they've until now. For example, this is a comparative of this app before this pull request (the latest version on F-Droid), this app after the PR and Riot-desktop:

Before PR After PR Desktop
normal with_mod desktop

(the background colour of the second image is black because I had the two apps installed, and I wanted to distinguish between them... :D)

As I say, on the rest of clients the emojis will appear as always has been.

@progserega
Copy link
Contributor

progserega commented May 27, 2018

When users use android-riot app - they see emoji from android.
They are often better then on a PC.
If we are remove this "feature" and all emoji will be such as PC - this is will bad for users.

Compare emoji:

This is ugly (riot-pc):
1_riot_web2_riot_web

This is not bad (riot-android) (for samsung smartphones it is more nicely, then clear android):
1_riot_android
Very nicely (telegram):
1_tele 2_tele

Default emoji in riot is ugly.
This is not a problem for geeks and hackers - they are use textual smiles such :-) and :), :(
But it is major for any other girls and womans.
We do not want the matrix to use only geeks? I think We want, that matrix will use many many people. :-)

I sink, that it is good patch, but before apply this patch, we need replace default emoji. And then, all emoji will be some on all devices and they will be nice.

@Foo-Manroot
Copy link
Contributor Author

I... don't really know what's your point, @progserega; but I think there has been a consensus over at #android:matrix.org about giving the option to use these new font or the default one through configuration.

Right now I'm quite busy and I don't think I may be able to take a look at it in a while, but I encourage anyone to try to implement this as configurable feature.

@progserega
Copy link
Contributor

ok

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants