Skip to content
This repository was archived by the owner on May 23, 2025. It is now read-only.

EmojiCompat support#600

Merged
connyduck merged 51 commits into
tuskyapp:masterfrom
C1710:fEmojiCompat
May 10, 2018
Merged

EmojiCompat support#600
connyduck merged 51 commits into
tuskyapp:masterfrom
C1710:fEmojiCompat

Conversation

@C1710
Copy link
Copy Markdown
Contributor

@C1710 C1710 commented Apr 24, 2018

So I finally added EmojiCompat :D
Please note that I don't know anything about license compatibility...
There are currently two fonts available: Twemoji and Blobmoji.
The font is loaded from the apps private external storage folder (/storage/emulated/0/Android/data/com.keylesspalace.tusky/files/EmojiCompat.ttf).
This method ensures that using EmojiCompat is optional (it will use a dummy configuration if no font is present) and customizable.
It should work on any component expect for the Account Header in the Navigation Drawer.

C1710 added 11 commits April 15, 2018 13:00
…in a file somewhere inside the device's storage
Emoji font should now be located at [Private external app directory]/files/EmojiCompat.ttf
# Conflicts:
#	app/build.gradle
#	app/src/main/java/com/keylesspalace/tusky/TuskyApplication.java
Since this EmojiCompat-implementation does not rely on BundledEmojiCompat, there's no reason to have it enabled.
Since connection isn't assigned to (I tried doing so), it can be declared final/val again.
@charlag
Copy link
Copy Markdown
Collaborator

charlag commented Apr 24, 2018

Wow, that's wonderful, thank you!
I'm not sure I understand it completely.
There are two configurations - assets and file and I am confused.
I'm also confused about how we should ship this - make two builds?
I'm probably missing something, I would be glad if you could explain it, thx!

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 24, 2018

These two configurations are used because of some issues if the EmojiCompat font is missing.
Usually it will try to use FileEmojiCompatConfig.
It will look into the file-directory and look for the font file called EmojiCompat.ttf - it can be downloaded manually into this directory.
Since the app will crash if EmojiCompat isn't initialized properly, I needed to add a fallback solution.
So I just made a minimal (and very small) EmojiCompat font which only consists of 6(?) emojis and won't replace them if they are present on the device (which is like always).
So in this fallback case it will use AssetEmojiCompatConfig to load this dummy font.

This solution ensures that the user is able to

  • use the app without a large APK to download
  • use the app with their favourite emoji font (as of now I only made a font for Twemoji and Blobmoji
  • use the app without any EmojiCompat font (this is the fallback solution I mentioned earlier) - although EmojiCompat is actually enabled and "working", but it doesn't affect anything.

A more user-friendly solution would be to make the fonts available online and to add an option (e.g. in Settings) to download one of them.
As far as I understood, you can even refresh EmojiCompat at runtime but I think restarting the app shouldn't be too difficult.
So all in all this solution won't do anything by default, but it offers the possibility to add EmojiCompat without having to create a new fork of this app.

@charlag
Copy link
Copy Markdown
Collaborator

charlag commented Apr 24, 2018

Thank you a lot! ❤️
I just wasn't sure that users can actually download the font because it's in the private folder.
Yeah, we can add some button which will do this automatically but this would be super great to have now too!
I'll let @connyduck have a look at this and thanks once again!

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 24, 2018

But AssetEmojiCompatConfig loads the font file from the app's internal asset folder which can't be accessed by other apps.
FileEmojiCompatConfig is able to load the font file from anywhere on the external storage.
I chose the app's private folder because it doesn't require storage permission to read files from here.
On the other hand it could get you a FileNotFoundException which would cause huge problems as EmojiCOmpat isn't designed to work without or with a missing font.

@connyduck
Copy link
Copy Markdown
Collaborator

Ok this is super awesome, but not yet userfriendly enough. Users should be able to select their preferred style in the settings + then we download the file to the device. I can implement that if you dont want to.

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 24, 2018

@connyduck I can try adding this option, but I'm not very sure about these things 😅

@connyduck
Copy link
Copy Markdown
Collaborator

@C1710 What are your concerns?

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 24, 2018

I don't know how Tusky handles all these things with settings and so on😂 - integrating EmojiCompat was relatively easy though...
The question is: Where should the fonts be hosted?
And I'm not sure about this attribution stuff.
For Twemoji there are instructions at: https://github.com/twitter/twemoji#attribution-requirements
For Blobmoji: I would just say the EmojiCompat font is licensed under the SIL OFL, but I'm still not sure about the original Noto emoji font. While the font itself is licensed under the SIL OFL, the sources are licensed as Apache 2.0 (the same would apply to Blobmoji but at least for the parts I made I wouldn't require any attribution or something like this).
All in all I don't think attribution for Blobmoji would be required but at least for Twemoji there could be some attribution - maybe.

@connyduck
Copy link
Copy Markdown
Collaborator

I don't know how Tusky handles all these things with settings and so on😂 - integrating EmojiCompat was relatively easy though...

I would do it the following way: Add a new Preference between theme and font size. On click, show a dialog with the options (maybe even example emojis) and the download size. Add a text like "Tusky needs to download the emojis to your device in order for this to work".
After users selected a style, show a progressbar (this may be tricky to do with okhttp, indefinite progressbar is ok for now) and then just download the file, a subfolder Tusky of Environment.getExternalStorageDirectory() is probably best. After the download finished, verify the file is there (bonus: check hashsum) and save the selected style to SharedPreferences. On error show an error message and revert to default style.

On appstart, check the shared preferences for the selected style and then check if the required font file exists. If so, use it, otherwise fall back to default.

I can help you if you have questions about implementation details

The question is: Where should the fonts be hosted?

We could put it on https://tuskyapp.github.io/ ?

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 27, 2018

I just finished the implementation of EmojiCompat!
Remaining issues:

  • I didn't comment the code yet (I'll do this in the next few hours)
  • There's no option to stop a download
  • Some weird positioning of the radio buttons
  • You can't update a font or the font list
  • The code might be a bit chaotic - I'll need to do some cleanup
  • There's no progress shown
  • No checksums
  • There's no link to the font's sources. It is available in the files though to be easily implemented
  • I'll need to sync the fork in order to prevent Merge conflicts.

But there are a few things I can't do:

  1. The fonts still need to be hosted somewhere
  2. The font configuration is based on the JSON format. This includes a list of the available fonts which is downloaded as well. I added a sample version of it. (It's actually just missing the real links. I used the Blobmoji links for debugging)

Please be aware that I currently use a different branch than the one I started the pull request from

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 27, 2018

Oh and another issue is that you'll need to restart the app to select a new emoji font since @google blocked classes from outside the EMojiCompat library itself resetting the configuration 🤷‍♀️

Copy link
Copy Markdown
Collaborator

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a super cool and hard work

Of course I'm going to grunt anyway :D

I'd rather see new code in Kotlin but I'm not sure that you're comfortable with it.

I have thoughts that maybe we should put fonts in another repo? Because first, I'm not sure how they're licensed and second, people won't have to download fonts when they're downloading source code.

* @return the corresponding font. Will default to SYSTEM_DEFAULT if not in range.
*/
public static EmojiCompatFont byId(int id) {
if(id >= 0 && id < FONTS.length) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This formatting of if/else makes me sad

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 29, 2018

About Kotlin: I think I'm able to read code written in Kotlin, but I'm not sure that I'm able to actually write code in Kotlin.
About the fonts: I made a pull request for tuskyapp.github.io as @connyduck suggested in order to host the files there.
At least for Blobmoji I can say that it's either licensed under the SIL OFL or Apache 2.0 license, but I'd go for SIL OFL (I'm not sure about all this stuff as I didn't create most of the assets of it which makes it some kind of derivate work. The original NotoColorEmoji font is licensed under the OFL though. I think I should ask them on how licensing works in this case).
Twemoji is licensed under CC-BY-4.0, but they got some more permissive Attribution requirements.
And I simply forgot to remove the font files from this repo 😅

Copy link
Copy Markdown
Collaborator

@connyduck connyduck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting really good ^___^

Unfortunately it is not working anymore for me, it seems the folder where the emojis get stored is not created correctly?

Also I think EmojiTextView should be used conservatively, as it has a (small) perf setback over standard TextView. Usernames can not contain emojis, so no need to use EmojiTextView where only usernames are displayed.

android:paddingTop="24dp">

<TextView
<android.support.text.emoji.widget.EmojiTextView
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better leave it a TextView, its guaranteed to not contain emojis

@@ -0,0 +1,11 @@
<vector android:height="24dp" android:viewportHeight="128"
android:viewportWidth="128" android:width="24dp" xmlns:android="http://schemas.android.com/apk/res/android">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

android:height and android:width need to be 40dp or it will look blurry on some devices

@@ -0,0 +1,9 @@
<vector android:height="24dp" android:viewportHeight="36"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

android:height and android:width need to be 40dp or it will look blurry on some devices

PendingIntent mPendingIntent = PendingIntent.getActivity(
context,
// This is the codepoint of the party face emoji :D
0x1f973,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😋🎉

Comment thread fonts.json Outdated
@@ -0,0 +1,19 @@
[
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File not needed anymore?

Comment thread app/build.gradle Outdated
implementation('com.mikepenz:materialdrawer:6.0.7@aar') {
transitive = true
}
// GSON is used to parse the EmojiCompat font-list
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove this? (gson is included anyway via the retrofit converter)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this is a good idea? If we're depending on Gson not only for network than it's not bad to mention it explicitly?
Theoretically: we're getting rid of convertor. The person removes it. Other part of the app breaks. No good.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree, but GSON usage has been removed from this PR I cant find it anymore?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry, my bad, I fail to keep up with this fast-changing PR 😅

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 30, 2018

I just added all these changes including the removal of the GSON dependency.
I already included two files which can be used for some credits in the About screen, but since my train is just arriving at its destination, I don't have the time to implement it now

@C1710
Copy link
Copy Markdown
Contributor Author

C1710 commented Apr 30, 2018

Information about the emoji font's licensing is now available in the About screen.
It should now comply with both licenses.

@connyduck connyduck added this to the Tusky 1.9 milestone May 3, 2018
C1710 added 4 commits May 4, 2018 09:13
Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>
Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>
Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>
@connyduck connyduck merged commit 1108652 into tuskyapp:master May 10, 2018
nailyk-fr pushed a commit to nailyk-fr/Tusky that referenced this pull request Aug 4, 2018
* Add EmojiCompat

* EmojiCompat doesn' replace all emojis anymore

* This app should be now capable of loading a EmojiCompat-font located in a file somewhere inside the device's storage

* Should now replace all emojis

* Add EmojiCompat support to EditTextTyped

* Provide EmojiCompat fonts

* The app won't crash anymore when no emoji font is available.
Emoji font should now be located at [Private external app directory]/files/EmojiCompat.ttf

* Removed BundledEmojiCompat dependency

Since this EmojiCompat-implementation does not rely on BundledEmojiCompat, there's no reason to have it enabled.

* Update EditTextTyped.kt

Since connection isn't assigned to (I tried doing so), it can be declared final/val again.

* Update README.md

* Add some non-working emoji preferences

* Add a short font list for testing

* Finished implementation

* Add Twemoji to font list

* Update documentation, more comments

* Delete AssetEmojiCompat which is obsolete now

* Update the font list

* Update the font list

* Fix font list & add Exception handling for malformed JSON files (hopefully)

* More fixes. It should work now...

* Removed AssetEmojiCompat (again)

* Add most of the changes

* Improved the EmojiCompat dialog's style

* The font list is now based on a static layout without external files

* Re-add the real font URL for Twemoji

* Emoji-font captions are now translatable

* Removed one unused String (loading)

* Removed emoji fonts from this repo

* Applied changes from the PR change requests

* The correct emoji font will be selected after cancelling a change

* Add details on the EmojiCompat fonts available (not shown yet)

* Add licensing information on Twemoji and Blobmoji

* Reworked some strings

* Moved FileEmojiCompat to its own library

* Update FileEmojiCompat to the latest version (1.0.3)

* EmojiCompat bug should be fixed

* Better handling of failed downloads

* Removed one TODO

Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>

* Update emoji attribution strings

Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>

* Fixed some misspelled strings

Signed-off-by: Constantin A <10349490+C1710@users.noreply.github.com>
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