Use emoji font to show explicit emoji #1108
Conversation
This way we're making an explicit choice on all platforms. This change is NFC on iOS and Android, our supported targets.
This is NFC because on the affected platforms, the system will already fall back automatically to Apple Color Emoji when a character doesn't have a glyph found in any of the specified fonts but does in the Apple Color Emoji font.
The implementation is a bit gnarly, so it's important that the doc be clear.
This change isn't quite NFC: textStyleFromWidget will now match even if the Text widget has additional text in it beyond the targeted span. That seems fine, though: it's not likely to happen; and if for some reason it does, that's not really what this test is about.
Some unicode characters, like U+2764 (❤) or U+00AE (®) can have glyphs in non-Emoji fonts, resulting in incorrect rendering of such characters, where we specifically want an emoji to be displayed. So, explicitly mention "Apple Color Emoji" to be the font used on iOS/macOS for displaying the unicode emoji. This resolves part of zulip#1104, namely for reaction chips and autocomplete results. [greg: wrote test] Fixes-partly: zulip#1104
For emoji like ❤ U+2764 HEAVY BLACK HEART, this causes us to show a colorful, larger glyph like ❤️ from the emoji font, instead of a glyph like ❤︎ that comes from a plain-text font and has the color and size of plain text. Fixes: zulip#1104
There was a problem hiding this comment.
Thanks, LGTM!
I guess you stumbled on a case where current servers don't wrap unicode emoji in a special span 🙂 :
https://chat.zulip.org/#narrow/channel/7-test-here/topic/emoji/near/1993147
❤️️
So the Flutter app will still at least use an emoji's text variant when it appears in a code block. 🤷♂️ Seems OK; definitely not worth the effort to change especially before launch.
lib/widgets/text.dart
Outdated
| @@ -153,19 +153,24 @@ const kDefaultFontFamily = 'Source Sans 3'; | |||
|
|
|||
| /// The [TextStyle.fontFamilyFallback] for use with [kDefaultFontFamily]. | |||
| List<String> get defaultFontFamilyFallback => [ | |||
| if (_useNotoEmoji) 'Noto Color Emoji', | |||
| _useAppleEmoji ? 'Apple Color Emoji' : 'Noto Color Emoji', | |||
There was a problem hiding this comment.
I forget, how do we know 'Apple Color Emoji' is recognized at all?
…Oh I guess one way we know is: it managed to make things look wacky when we ranked it highest? In this experiment:
There was a problem hiding this comment.
In this experiment:
Well, I tried that experiment yesterday only on Android 🙂. @rajveermalviya showed it working on iOS in the screenshots in #1103, though:
Those are UnicodeEmojiWidget, so affected by the commit in #1103 that's cherry-picked here.
There was a problem hiding this comment.
OK, I've just gone and reproduced #1104 on an iPhone, in all three contexts:
- message content (as an emoji span)
- reaction chip
- emoji autocomplete results
and then confirmed that this PR fixes it in all three contexts.
test/widgets/content_test.dart
Outdated
| /// The "merged style" ([mergeSpanStylesOuterToInner]) of a text span | ||
| /// whose whole text matches the given pattern, under the given root span. | ||
| TextStyle? mergedStyleOfSubstring(InlineSpan rootSpan, Pattern spanPattern) { |
There was a problem hiding this comment.
content test [nfc]: Try to clarify mergedStyleOfSubstring a bit
Eep, yep, thanks for this improvement!
|
Thanks for the review!
Yeah. That's a case of #1107, the follow-up issue I split out (and milestoned for M7 Future). |
One of the two main commits here I cherry-picked from @rajveermalviya's #1103, and then wrote a test for.
Fixes: #1104
Selected commit messages
emoji: Use "Apple Color Emoji" font on iOS/macOS for UnicodeEmojiWidget
Some unicode characters, like U+2764 (❤) or U+00AE (®) can
have glyphs in non-Emoji fonts, resulting in incorrect
rendering of such characters, where we specifically want an
emoji to be displayed.
So, explicitly mention "Apple Color Emoji" to be the font used on
iOS/macOS for displaying the unicode emoji.
This resolves part of #1104, namely for reaction chips
and autocomplete results.
[greg: wrote test]
Fixes-partly: #1104
content: Use emoji font to show emoji nodes
For emoji like ❤ U+2764 HEAVY BLACK HEART, this causes us to show a
colorful, larger glyph like ❤️ from the emoji font, instead of a glyph
like ❤︎ that comes from a plain-text font and has the color and size
of plain text.
Fixes: #1104