-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Use system emoji uniformly accross app when system emoji option is enabled #13327
Conversation
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 |
8c5ae8c
to
651ec41
Compare
03166a8
to
5cbbabb
Compare
4c85b31
to
8a36eeb
Compare
8a36eeb
to
5d40e79
Compare
8631e8c
to
94a330d
Compare
94a330d
to
56a9494
Compare
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. |
…to ensure missing glyphs are filled in.
56a9494
to
d7c57c0
Compare
Please keep this open, I'd still like to merge this if possible! |
I hope to see this merged too :) |
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! |
That's awesome, I'm super excited to see this land, thank you for reviewing! |
First time contributor checklist
Contributor checklist
Fixes #1234
syntaxDescription
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!