Better emoji height management #117

rubengees · 2017-05-04T22:05:44Z

This would fix #116.
This change is not backwards-compatible.

Contents of this Pull Request

The EmojiSpan now automatically calculates it's size based on the line height of the TextView it resides in. This has the advantage that there is no "jumping" when typing and weird visuals as described in #116.
This removes the custom emojiSize attr; The user is expected to change text size by the normal methods (textAppearance or textSize)

Before	After

If you look closely, you can see the "jump" at the left GIF.
(Sorry about the bad quality of the GIFs...)

Code changes to achieve this

The EmojiSpan now gets the lineHeight of the EmojiTextView or EmojiEditText passed as the size. The getSize method was overriden to make this work.
The custom emojiSize attr and all associated methods have been removed.
The adapter items now have their textAppearance set to TextAppearance.AppCompat.Large for showcasing how to change the size.

The EmojiSpan is now drawn with the exact line height of the TextView it is in. This avoids jumping lines when typing in an EditText or weird visuals when having a line with and one without emojis in a TextView. Furthermore the custom attr "emojiSize" was removed, as it is now automatically calculated. To still be able to choose the size, the textAppearance of the TextView should be changed. Another change is a large textAppearance for the adapter items, to showcase different heights.

codecov · 2017-05-04T22:19:46Z

Codecov Report

Merging #117 into master will increase coverage by 0.56%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   23.93%   24.49%   +0.56%     
==========================================
  Files          19       19              
  Lines         656      641      -15     
  Branches       71       69       -2     
==========================================
  Hits          157      157              
+ Misses        486      471      -15     
  Partials       13       13

vanniktech

I like it. This bit of moving up and down has always bothered me. About backwards compatible, that's not a problem since we're still PRE v1.

vanniktech · 2017-05-08T11:37:54Z

app/src/main/res/layout/activity_main.xml

-                android:layout_height="match_parent"
-                android:orientation="vertical"
-                tools:context=".MainActivity">
+    xmlns:app="http://schemas.android.com/apk/res-auto"


can you revert those changes?

vanniktech · 2017-05-08T11:38:07Z

app/src/main/res/layout/adapter_chat.xml

-      android:textIsSelectable="true"
-      tools:text="my text"
-      />
+    <com.vanniktech.emoji.EmojiTextView


also here please revert the white space changes

vanniktech · 2017-05-08T11:38:19Z

app/src/main/res/layout/adapter_chat.xml

+        android:layout_height="wrap_content"
+        android:textAppearance="@style/TextAppearance.AppCompat.Large"
+        android:textIsSelectable="true"
+        tools:text="my text"/>


also here revert the /> change

vanniktech · 2017-05-08T11:38:26Z

app/src/main/res/layout/adapter_chat.xml

@@ -3,14 +3,13 @@
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
-    android:orientation="vertical"
-    >
+    android:orientation="vertical">


> should go on the same line

vanniktech · 2017-05-08T11:38:31Z

app/src/main/res/layout/dialog_main.xml

-              android:layout_height="wrap_content"
-              android:orientation="vertical"
-              tools:context=".MainDialog">
+    xmlns:app="http://schemas.android.com/apk/res-auto"


vanniktech · 2017-05-08T11:38:35Z

app/src/main/res/layout/dialog_main.xml

@@ -39,8 +39,7 @@
            android:layout_weight="1"
            android:imeOptions="actionSend"
            android:inputType="textCapSentences|textMultiLine"
-            android:maxLines="3"
-            app:emojiSize="26sp"/>
+            android:maxLines="3"/>


Don't know what to do about this one...

Yup this one is okay. Sorry for the confusion

vanniktech · 2017-05-08T11:39:13Z

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

@@ -30,4 +31,9 @@

    return drawable;
  }
+
+  @Override public int getSize(final Paint paint, final CharSequence text, final int start,


why all of the parameters if all they do is just nothing?

Well that's the signature of the base method, so we have to specify all of these parameters.

Oh okay, didn't see the @Override

rubengees · 2017-05-08T14:48:04Z

Everything done! I intuitively hit Strg + Alt + L after editing which always leads to such changes.

vanniktech · 2017-05-08T14:53:11Z

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

@@ -30,4 +31,9 @@

    return drawable;
  }
+
+  @Override public int getSize(final Paint paint, final CharSequence text, final int start,


I think this method can be just deleted right?

No, we need it to specify the size of the Span to the plattform.

vanniktech · 2017-05-08T14:54:10Z

@rubengees no worries about the formatting. There's still the getSize method which should be deleted, right?

rubengees · 2017-05-08T14:56:01Z

I tried it without the method just now and it did not work.

vanniktech

Thanks a lot for this one :)

vanniktech · 2017-05-08T14:58:41Z

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

@@ -30,4 +31,9 @@

    return drawable;
  }
+
+  @Override public int getSize(final Paint paint, final CharSequence text, final int start,


Oh okay, didn't see the @Override

Better emoji height management (#117) Adjust generator for emoji 5.0 (#118) Update all emojis to emoji 5.0 (#119) Merge branch 'master' into latest More

rubengees added 2 commits May 4, 2017 23:44

Formatting

faf1b0b

vanniktech requested changes May 8, 2017

View reviewed changes

rubengees added 3 commits May 8, 2017 16:42

Formatting

1748f87

Formatting

d9dbb65

Formatting

abecff1

vanniktech requested changes May 8, 2017

View reviewed changes

vanniktech approved these changes May 8, 2017

View reviewed changes

vanniktech merged commit 79a690d into vanniktech:master May 8, 2017

rubengees deleted the better-emoji-height branch May 8, 2017 15:00

vanniktech mentioned this pull request Jun 24, 2017

Add back Emoji Size and use line height as the default. #146

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better emoji height management #117

Better emoji height management #117

rubengees commented May 4, 2017 •

edited

Loading

codecov bot commented May 4, 2017 •

edited

Loading

vanniktech left a comment

vanniktech May 8, 2017

vanniktech May 8, 2017

vanniktech May 8, 2017

vanniktech May 8, 2017

vanniktech May 8, 2017

vanniktech May 8, 2017

rubengees May 8, 2017

vanniktech May 8, 2017

vanniktech May 8, 2017

rubengees May 8, 2017

vanniktech May 8, 2017

rubengees commented May 8, 2017

vanniktech May 8, 2017

rubengees May 8, 2017

vanniktech commented May 8, 2017

rubengees commented May 8, 2017

vanniktech left a comment

vanniktech May 8, 2017

Better emoji height management #117

Better emoji height management #117

Conversation

rubengees commented May 4, 2017 • edited Loading

Contents of this Pull Request

Code changes to achieve this

codecov bot commented May 4, 2017 • edited Loading

Codecov Report

vanniktech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubengees commented May 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanniktech commented May 8, 2017

rubengees commented May 8, 2017

vanniktech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubengees commented May 4, 2017 •

edited

Loading

codecov bot commented May 4, 2017 •

edited

Loading