fix: Show composite emojis enlarged, too #3427
Conversation
src/main/java/org/thoughtcrime/securesms/components/emoji/AutoScaledEmojiTextView.java
Show resolved
Hide resolved
| if (++graphemeCount > max) break; | ||
| String grapheme = text.substring(start, end); | ||
| if (!emojiRegex.matcher(grapheme).matches()) return 0; | ||
| if (++graphemeCount > max) return 0; |
There was a problem hiding this comment.
now the returned value is never > max so the if condition checking emojiCount > 8 can be removed, the function doc-string needs also to indicate this. I would prefer returning -1 in this case then we can check for emojiCount > 0
There was a problem hiding this comment.
did you test the performance impact of regex over the whole text vs doing it for each grapheme? in theory it should be cheap since we do it at most in 8 graphemes and break the loop otherwise
since now this is more robust and we don't loop over whole text, maybe makes sense to increase the length restriction from 21 to 56
there is another PR modifying this, maybe first merge that one and rebase
|
To test the changes in this pull request, install this apk: |
There are a lot of composite emojis like the flags and the family emojis. Here, the grapheme starts with an emoji, but then come some other characters that modify the first emoji (e.g. all flags start with the flag emoji). This PR shows those as big, too, by checking whether a grapheme _starts_ with an emoji to determine whether it's an emoji.
Hocuri
force-pushed
the
hoc/more-emojis
branch
from
4888873
to
a5ff3b2
Compare
|
To test the changes in this pull request, install this apk: |
| if (++graphemeCount > max) break; | ||
| String grapheme = text.substring(start, end); | ||
| if (!emojiRegex.matcher(grapheme).matches()) return 0; | ||
| if (++graphemeCount > max) return 0; |
There was a problem hiding this comment.
| if (++graphemeCount > max) return 0; | |
| if (++graphemeCount > max) return -1; |
| int emojiCount = countGraphemes(text, 8); | ||
| if (emojiCount > 8) { | ||
| int emojiCount = countEmojis(text, 8); | ||
| if (emojiCount > 8 || emojiCount == 0) { |
There was a problem hiding this comment.
| if (emojiCount > 8 || emojiCount == 0) { | |
| if (emojiCount <= 0) { |
| @@ -50,36 +50,34 @@ private float getTextScale(String text) { | |||
| if (text.length() > 21 || text.isEmpty() || Character.isLetter(text.charAt(0))) { | |||
There was a problem hiding this comment.
| if (text.length() > 21 || text.isEmpty() || Character.isLetter(text.charAt(0))) { | |
| if (text.length() > 56 || text.isEmpty() || Character.isLetter(text.charAt(0))) { |
adbenitez
changed the title
There are a lot of composite emojis like the flags and the family emojis. Here, the grapheme starts with an emoji, but then come some other characters that modify the first emoji (e.g. all flags start with the flag emoji).
This PR shows those as big, too, by checking whether a grapheme starts with an emoji to determine whether it's an emoji.