Introduce EmojiStore for some existing emoji logic #967
Conversation
lib/widgets/emoji_reaction.dart
Outdated
| final resolvedUrl = | ||
| doNotAnimate ? (emojiDisplay.resolvedStillUrl ?? emojiDisplay.resolvedUrl) | ||
| : emojiDisplay.resolvedUrl; |
There was a problem hiding this comment.
| final resolvedUrl = | |
| doNotAnimate ? (emojiDisplay.resolvedStillUrl ?? emojiDisplay.resolvedUrl) | |
| : emojiDisplay.resolvedUrl; | |
| final resolvedUrl = | |
| doNotAnimate | |
| ? (emojiDisplay.resolvedStillUrl ?? emojiDisplay.resolvedUrl) | |
| : emojiDisplay.resolvedUrl; |
or
| final resolvedUrl = | |
| doNotAnimate ? (emojiDisplay.resolvedStillUrl ?? emojiDisplay.resolvedUrl) | |
| : emojiDisplay.resolvedUrl; | |
| final resolvedUrl = doNotAnimate | |
| ? (emojiDisplay.resolvedStillUrl ?? emojiDisplay.resolvedUrl) | |
| : emojiDisplay.resolvedUrl; |
?
There was a problem hiding this comment.
Sure, I'll do the second of those.
test/model/emoji_test.dart
Outdated
| // Code matches against code, not against name. | ||
| checkDisplay(emojiCode: '123', emojiName: '100').isA<ImageEmojiDisplay>() | ||
| ..resolvedUrl.equals(eg.realmUrl.resolve('/emoji/123.png')) | ||
| ..resolvedStillUrl.isNull(); |
There was a problem hiding this comment.
I'm not sure I follow the comment. "Code" (the first word) means "emoji code", right, or does it mean the app code? What bug is this test written against?
There was a problem hiding this comment.
Yeah, it means "emoji code" as opposed to "emoji name". I'll make the comment less elliptical:
// Emoji code matches against emoji code, not against emoji name.The hypothetical bug here isn't a super likely one (because if we had it, then probably most uses would just fail and so we'd notice promptly in manual testing): it's that because both emoji codes and emoji names are just strings, we could take the given emoji name and look it up someplace where the keys are actually supposed to be emoji codes.
|
(CI should be resolved by rebasing past #968.) |
This gives us a good home to add the server's list of Unicode emoji.
This describes the semantics of part of Zulip's API.
We'll use this soon.
This will apply in other places we show an emoji, too, such as the emoji picker. What, you might ask, about the other place we already show emoji, namely in message content? We should probably apply this logic there too, but we'll leave that for later. Tracking it as issue zulip#966.
Some bits of data change in the one existing test that was using these; but not any data that the test looks at.
Now that this logic has been pulled out to model code, we can more easily write tests for it.
|
Thanks for the review! Merged after those changes. |
This is a refactoring that prepares us for #669, as well as for #388 and #670.