Signal-Android icon indicating copy to clipboard operation
Signal-Android copied to clipboard

Use system emoji uniformly accross app when system emoji option is enabled

Open danbrun opened this issue 1 year ago • 2 comments

First time contributor checklist

Contributor checklist

  • Pixel 8 Pro running Android 14
  • Emulator running Android 10
  • Emulator running Android 5.0
  • [x] My contribution is fully baked and ready to be merged as is
  • [x] I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Hi! I'm a long time Signal user and fan! However, there is one thing that has bugged me about the app, and that's the inconsistency of emoji when using the system emoji option. I've found some others have the same sentiment on the forum and in Github issues.

The comments from the Signal team I've seen didn't specify what the blocking technical issues were so I took a look at the code base and found this solution. I created an alternate drawable that leverages StaticLayout and the EmojiCompat library to draw the system emoji directly, and have the emoji provider return these drawables when the system emoji option is enabled. This should behave consistently with emoji in TextViews because AppCompat uses EmojiCompat under the hood to render glyphs with the system font where possible, while falling back to a font provided by Google Play Services for missing glyphs.

I tested this on my Pixel 8 Pro as well as on an Android 10 emulator and an Android 5.0 emulator (to demonstrate that EmojiCompat works and cover the SDK_INT check around the StaticLayout constructor). One thing to note when testing on an emulator is that there seems to be a known issue with EmojiCompat not pulling the fallback font on emulators, so to test those I used the EmojiCompat bundled configuration in my use-system-emoji-bundled branch. Below are some screenshots of the reactions picker to show the emojis working across Android versions and demonstrate that the EmojiCompat fallback works so outdated Android devices will not be missing any glyphs.

Android 14 Android 10 with EmojiCompat Android 10 without EmojiCompat Android 5 with EmojiCompat Android 5 without EmojiCompat

Thanks for your consideration!

danbrun avatar Dec 21 '23 05:12 danbrun

I like this PR a lot. Can you add a screenshot of a message with an emoji reaction on Android 5? I have no ability to merge it in but I am just curious how it looks. :)

farewelltospring avatar Jan 03 '24 10:01 farewelltospring

I like this PR a lot. Can you add a screenshot of a message with an emoji reaction on Android 5? I have no ability to merge it in but I am just curious how it looks. :)

Sure thing! Here is a screenshot of Android 5 reactions on messages

danbrun avatar Jan 09 '24 07:01 danbrun

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.

stale[bot] avatar Jun 14 '24 05:06 stale[bot]

Please keep this open, I'd still like to merge this if possible!

danbrun avatar Jun 14 '24 07:06 danbrun

I hope to see this merged too :)

farewelltospring avatar Jun 14 '24 09:06 farewelltospring

Hi there! So sorry for not seeing this PR earlier, that is one benefit of the stale bot :) Thank you so much for building this! This is great and super clever. One of my favorite public PR's I've seen, where it fixes a long-running issue with very little code :) The plan is for this to go out in 7.10 unless we run into any unforeseen issues, but everything seems to be working for me! Thanks again!

greyson-signal avatar Jun 14 '24 14:06 greyson-signal

That's awesome, I'm super excited to see this land, thank you for reviewing!

danbrun avatar Jun 18 '24 07:06 danbrun

Was just using Signal and noticed this change. Great improvement, and super cool to see that this came as a public contribution, thanks @danbrun and @greyson-signal ! 😄

PaulKlauser avatar Jul 10 '24 11:07 PaulKlauser

I just tried to switch to system emoji, but how should it work? Running on 7.11.3 the emoji icon disappear after toggle the setting to ON. :eyes:

UserX404 avatar Jul 19 '24 11:07 UserX404

@UserX404 Historically, we've assumed that if people want to use system emoji, then they would prefer their system keyboard... but the UX has changed a lot since that original decision (we've added stickers and gifs to that menu, meaning it doesn't really save any space to remove the emoji picker), so I think I'll change it in 7.13 so that the emoji picker is always present, regardless of the setting. Thanks for the feedback!

(If you're curious how to access it otherwise, reactions + typing in emoji via your system keyboard should show it working)

greyson-signal avatar Jul 19 '24 15:07 greyson-signal