Emoji Support Library integration #196
Conversation
…ode to use the drawable. Allows providers to provide optimized / not 1:1 resource based emoji
Codecov Report
@@ Coverage Diff @@
## master #196 +/- ##
=======================================
Coverage 27.93% 27.93%
=======================================
Files 22 22
Lines 809 809
Branches 88 88
=======================================
Hits 226 226
Misses 565 565
Partials 18 18
Continue to review full report at Codecov.
|
| @@ -17,6 +19,10 @@ | |||
| return; | |||
| } | |||
|
|
|||
| final EmojiCompat.Config config = new BundledEmojiCompatConfig(this); | |||
There was a problem hiding this comment.
can we move this into the Activity where we'll also install the provider?
| @@ -39,9 +39,8 @@ | |||
| android:layout_width="0dp" | |||
| android:layout_height="wrap_content" | |||
| android:layout_weight="1" | |||
| android:imeOptions="actionSend" | |||
| android:inputType="textCapSentences|textMultiLine" | |||
| android:maxLines="3"/> | |||
There was a problem hiding this comment.
can you restore the max lines?
emoji-google-compat/build.gradle
Outdated
| } | ||
|
|
||
| android { | ||
| compileSdkVersion rootProject.ext.compileSdkVersion as int |
There was a problem hiding this comment.
can you do 2 tabs instead of 4?
| import android.support.annotation.NonNull; | ||
| import com.vanniktech.emoji.emoji.Emoji; | ||
|
|
||
| public class GoogleCompatEmoji extends Emoji { |
There was a problem hiding this comment.
let's make it final so others can't extend it
| } | ||
|
|
||
| @NonNull | ||
| @Override |
| textPaint.setColor(0x0ffffffff); | ||
| } | ||
|
|
||
| @Override |
| /** | ||
| * An emoji drawable backed by a span generated by the Google emoji support library. | ||
| */ | ||
| public class GoogleCompatEmojiDrawable extends Drawable { |
There was a problem hiding this comment.
final & package private so others can't use it and we won't have to support it in the public API
| } | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
nit: same line as method function
| textPaint.setAlpha(alpha); | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
nit: same line as method function
| textPaint.setColorFilter(colorFilter); | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
nit: same line as method function
|
Done. Have created the categories manually by changing the generated google categories. Wanted to look into how sprite sheet metadata is handled for EmonjiOne etc. before changing the generator (and don't have nodejs installed on the laptop). |
There was a problem hiding this comment.
Oh nice I like. I also tested it locally and everything seems to work so far. 👍
Regarding the automatic generation, we can take care of that later.
Should we do the EmojiCompat.init(config); in the provider itself or should the user do this? I'm not that familiar with the google emoji api. If we let the user do it we should somehow check in GoogleCompatEmojiProvider that EmojiCompat was initialized. Another option would be to pass the EmojiCompatConfig into the Provider but then the module would need to bundle the support library.
|
@stefanhaustein This is great 👍! How do the categories differ from the normal Google ones? |
|
The categories should not differ at all it's just that for this artifact the google support library will take care of the images and provide them lazily via fonts so they won't blow up the apk. Adjusting the generator sounds perfect! Yup watched that issue regarding the ordering and there's no progress yet. |
|
The categories should be identical to the regular Google categories. The support library can be used either with bundled emoji (as currently in the demo app), or the initialization can download the font as needed (= not on Oreo). Both setup methods have different dependencies, which makes it tricky to include the library setup, see https://developer.android.com/guide/topics/ui/look-and-feel/emoji-compat.html |
|
@stefanhaustein Thanks for the clarification. I think we could also implement the lazy approach of the
I think this would require separate modules for Thoughts? |
|
I think it would be fine to leave the support lib setup to the user. The download case requires some download key metadata in the manifest anyway IIRC, and users might also want to use the support lib elsewhere, which might make full wrapping awkward. One option to make the dependency more explicit might be to hand in the emoji support lib config object to the GoogleCompatEmojiProvider constructor (and just make sure it's not null). We could still add emoji-compat-bundled on demand later if the extra setup really turns out to be a problem for users. To me, the most important next step seems to be moving span generation to the provider, so we can avoid the span -> drawable -> span indirection in the support library case. |
|
@stefanhaustein Ah sorry, I misunderstood how the Concerning the dependency explicity: I think there is room for improvements (The setup could be plug-and-play with optional configuration if wanted/needed), but you are right, other things like lib size and the span code are more important for now. Involving this PR, I have found an issue while testing it locally. Consider these screenshots:
The new |
I think that's the best way to go for now. The emoji support lib config object is available via the emoji library so we don't need to depend on the bundled one right? If so we should change this to be more explicit. |
… dependency is more explicit/obvious
|
Missing Emoji & Ctor parameter should be fixed now O:) |
| public GoogleCompatEmojiProvider(@NonNull final EmojiCompat.Config emojiCompatConfig) { | ||
| if (emojiCompatConfig == null) { | ||
| throw new NullPointerException(); | ||
| } |
There was a problem hiding this comment.
Let's also call the install function?
|
Let's also call the install function? |
|
Distributing the initialization seems confusing to me. How about changing the ctor param from Config to EmojiCompat instead, i.e. the result of EmojiCompat.init(config)? |
|
Let's do that I didn't know the init method returns something. |
|
Done! Merge? O:) |
|
Thanks a lot for all the good work :) |
Quite minimal
Potential followups: