Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat #198

stefanhaustein · 2017-08-30T23:47:27Z

Add an interface to enable the provider to do the spanning
Use support library spanning if available
Avoid empty array allocation

Includes fixes from PR #199, which probably should go in first.

…ode to use the drawable. Allows providers to provide optimized / not 1:1 resource based emoji

…le violation

…ential delete

… dependency is more explicit/obvious

…port library spanning functionality when available.

codecov · 2017-08-31T00:06:00Z

Codecov Report

Merging #198 into master will increase coverage by 0.11%.
The diff coverage is 45.83%.

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
+ Coverage   27.93%   28.04%   +0.11%     
==========================================
  Files          22       22              
  Lines         809      820      +11     
  Branches       89       90       +1     
==========================================
+ Hits          226      230       +4     
- Misses        565      571       +6     
- Partials       18       19       +1

Impacted Files	Coverage Δ
.../main/java/com/vanniktech/emoji/EmojiEditText.java	`0% <0%> (ø)`	⬆️
...rc/main/java/com/vanniktech/emoji/EmojiButton.java	`0% <0%> (ø)`	⬆️
.../main/java/com/vanniktech/emoji/EmojiTextView.java	`0% <0%> (ø)`	⬆️
...c/main/java/com/vanniktech/emoji/EmojiManager.java	`85.89% <73.33%> (-0.41%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b95744...4bf57e1. Read the comment docs.

vanniktech · 2017-09-01T08:08:38Z

I'll look at this PR shortly. I'm rather busy at the moment.

vanniktech

A few questions.

I really like the asList optimization

vanniktech · 2017-09-06T09:24:12Z

emoji/src/main/java/com/vanniktech/emoji/EmojiManager.java

+  }
+
+  /**
+   * Internal fallback method.


Does not really seem internal since it's public.

vanniktech · 2017-09-06T09:24:24Z

emoji/src/main/java/com/vanniktech/emoji/EmojiManager.java

+  /**
+   * Internal fallback method.
+   */
+  public static void replaceWithImagesImpl(final Context context, final Spannable text, final float emojiSize) {


Is it possible to keep this internal?

vanniktech · 2017-09-06T09:24:51Z

emoji/src/main/java/com/vanniktech/emoji/EmojiReplacer.java

+import android.content.Context;
+import android.text.Spannable;
+
+public interface EmojiReplacer {


some java doc what it can be used for and also that it'll be there since version 0.6.0

vanniktech · 2017-09-06T09:25:17Z

emoji/src/test/java/com/vanniktech/emoji/EmojiManagerTest.java

@@ -174,7 +174,7 @@ public void noProviderInstalled() {

    final Spannable text = new SpannableString(new String(new int[] { 0x1234, 0x4321 }, 0, 1));

-    EmojiManager.replaceWithImages(RuntimeEnvironment.application, text, 22);
+    EmojiManager.replaceWithImagesImpl(RuntimeEnvironment.application, text, 22);


why the function name change?

vanniktech · 2017-09-06T09:25:25Z

emoji/src/test/java/com/vanniktech/emoji/EmojiManagerTest.java

@@ -134,7 +134,7 @@ public void noProviderInstalled() {

    final Spannable text = new SpannableString(new String(new int[] { 0x1234 }, 0, 1));

-    EmojiManager.replaceWithImages(RuntimeEnvironment.application, text, 22);
+    EmojiManager.replaceWithImagesImpl(RuntimeEnvironment.application, text, 22);


why the function name change

vanniktech · 2017-09-09T22:39:46Z

I updated this PR for you. Is there anything that's missing in this branch now?

vanniktech · 2017-09-09T22:40:20Z

...google-compat/src/main/java/com/vanniktech/emoji/googlecompat/GoogleCompatEmojiProvider.java

@@ -32,4 +36,13 @@ public GoogleCompatEmojiProvider(@NonNull final EmojiCompat emojiCompat) {
      new FlagsCategory()
    };
  }
+
+  @Override


nit: same line as the method

stefanhaustein · 2017-09-10T22:31:36Z

Hi, sorry for the delay. The extra method is needed to fall back to the default after giving the manager an option to intercept. Maybe replaceWithImagesDefault would be a better name?

vanniktech · 2017-09-11T08:28:15Z

Yup replaceWithImagesDefault is better.

stefanhaustein · 2017-09-14T16:02:50Z

I have changed the code to avoid a public method using a fallback replacer.
The PR still contains an unrelated test app change, can pull this out into a separate PR, too, if you prefer.

PTAL

vanniktech

Thank you 👍

stefanhaustein and others added 22 commits August 23, 2017 00:01

Let Emoji provide a drawable in addition to the resource id. Switch c…

db8774b

…ode to use the drawable. Allows providers to provide optimized / not 1:1 resource based emoji

Add missing deprecation / nullability annotations

96bc46f

Make getDrawable() context parameter final to address a checkstyle ru…

845ee4d

…le violation

Keep deferred drawable materialization for EmojiSpan

77d6ac5

more explicit name for the on-demand drawable member, avoiding mNotation

0f1dfc5

Avoid mNotation

9d45eee

Merge remote-tracking branch 'upstream/master'

59413cc

emojicompat support library support starting point

20827ab

some size tuning

cbd524c

move from emoji-compat to emoji-google-compat

e715659

More renames

186d5ba

Avoid fullscreen text edit, which hides the emoji kbd control

53ca443

formatting

4c015bc

address review comments: formatting, visibility, init location, accid…

1826803

…ential delete

fix misplaced braces, hiding some emoji in the people category

67998b0

make the emoji compat config object a provider ctor parameter, so the…

a3620b0

… dependency is more explicit/obvious

remove blank line to make checkstyle happy

7b0d8a9

Provider ctor adjustement

6d3f61d

Merge remote-tracking branch 'upstream/master'

ab5a8e0

Don't crash when the emoji support libraray is not ready yet. Use Sup…

bc96370

…port library spanning functionality when available.

Don't crash when the emoji support libraray is not ready yet. Use Sup…

3d81ae4

…port library spanning functionality when available.

lint issues

432621d

stefanhaustein added 4 commits August 31, 2017 22:59

Add emoji modifier specifically where needed

5c8fc41

center text emoji horizontally in the corresponding drawable

b07424d

whitespace fix

8d8a251

revert buggy text centering

81fc5d7

stefanhaustein changed the title ~~Don't crash when the emoji support libraray is not ready yet~~ Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat Aug 31, 2017

vanniktech reviewed Sep 6, 2017

View reviewed changes

vanniktech mentioned this pull request Sep 6, 2017

Obey library loading state and add modifiers only where needed #199

Merged

Merge branch 'master' into master

713985a

vanniktech reviewed Sep 9, 2017

View reviewed changes

stefanhaustein added 3 commits September 13, 2017 22:57

Merge remote-tracking branch 'upstream/master'

34420fb

Merge branch 'master' of https://github.com/stefanhaustein/Emoji-1

d18c4dd

Made the fallback method not public by usina a fallback replacer

4bf57e1

vanniktech approved these changes Sep 14, 2017

View reviewed changes

vanniktech merged commit ebe1cb0 into vanniktech:master Sep 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat #198

Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat #198

stefanhaustein commented Aug 30, 2017 •

edited

Loading

codecov bot commented Aug 31, 2017 •

edited

Loading

vanniktech commented Sep 1, 2017

vanniktech left a comment

vanniktech Sep 6, 2017

vanniktech Sep 6, 2017

vanniktech Sep 6, 2017

vanniktech Sep 6, 2017

vanniktech Sep 6, 2017

vanniktech commented Sep 9, 2017

vanniktech Sep 9, 2017

stefanhaustein commented Sep 10, 2017

vanniktech commented Sep 11, 2017

stefanhaustein commented Sep 14, 2017 •

edited

Loading

vanniktech left a comment

Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat #198

Add infrastructure to let the provider perform emoji span replacements and utilize in emoji-google-compat #198

Conversation

stefanhaustein commented Aug 30, 2017 • edited Loading

codecov bot commented Aug 31, 2017 • edited Loading

Codecov Report

vanniktech commented Sep 1, 2017

vanniktech left a comment

Choose a reason for hiding this comment

vanniktech Sep 6, 2017

Choose a reason for hiding this comment

vanniktech Sep 6, 2017

Choose a reason for hiding this comment

vanniktech Sep 6, 2017

Choose a reason for hiding this comment

vanniktech Sep 6, 2017

Choose a reason for hiding this comment

vanniktech Sep 6, 2017

Choose a reason for hiding this comment

vanniktech commented Sep 9, 2017

vanniktech Sep 9, 2017

Choose a reason for hiding this comment

stefanhaustein commented Sep 10, 2017

vanniktech commented Sep 11, 2017

stefanhaustein commented Sep 14, 2017 • edited Loading

vanniktech left a comment

Choose a reason for hiding this comment

stefanhaustein commented Aug 30, 2017 •

edited

Loading

codecov bot commented Aug 31, 2017 •

edited

Loading

stefanhaustein commented Sep 14, 2017 •

edited

Loading