Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add support for custom emojis #85

Merged
merged 18 commits into from May 27, 2017
Merged

[WIP] Add support for custom emojis #85

merged 18 commits into from May 27, 2017

Conversation

sonicdoe
Copy link

Closes #2. Even though this already works, I’m not sure if I’m happy with some of the internal changes (hence the [WIP]). @EtienneLem Let me know what you think.

An overview of the internal changes I made to support custom emojis:

  • The custom category is handled similarly to the recent and search categories.
  • The logic for generating the search property is now split out into a separate file so we can use it from both scripts/build-data.js and src/utils.
  • <Emoji /> supports custom emojis by checking the emoji.custom property and using emoji.imageUrl to display it (the props themselves remain unchanged).
  • Because <Picker /> passes an already sanitized emoji object to <Preview /> it failed for custom emojis. To work around this, I merge the original emoji object (which was passed in via the custom prop) before passing it on to <Preview /> in handleEmojiOver().
  • EmojiIndex#search now accepts a custom property which will add those emojis to data.emojis, data.categories, and emojisList so that the rest of EmojiIndex#search works without any changes.
  • The anchor icon (an asterisk) is just a quick sketch on my part. If anyone wants to provide a better image, I’d be happy to add it.

Perhaps the custom props could be an object that looks like { name: 'Custom', emojis: [], anchorUrl: '' } so that the anchor can also be customized. I like the idea of customizing the name through I18N, so either works for me.

For the moment, I have opted for I18N. If we allow a custom image for the custom category, I think it would make sense to allow custom images for all categories. I’d therefore suggest a prop akin to backgroundImageFn, maybe anchorImageFn.

This is important if custom emojis don’t specify any emoticons, for example.
<Preview /> calls getData() itself which will fail for custom emojis because there is no universally available data source for custom emojis. To work around this, we pass the custom emoji data (which <Picker /> received via the `custom` prop) directly to <Preview />.
This is important when hovering over a custom emoji while searching.
@EtienneLem
Copy link
Member

Awesome. I’ll take a look either later today or perhaps this weekend.

@sonicdoe
Copy link
Author

No worries. I’ll also need to make sure that it works fine with other props such as include, exclude, and emojisToShowFilter. I have only tested the very basic stuff so far.

@EtienneLem EtienneLem merged commit 1e87212 into missive:master May 27, 2017
@EtienneLem
Copy link
Member

Welp, I think that’s it! That’s amazing, thank you so much! 🤘

@sonicdoe sonicdoe deleted the custom-emojis branch May 28, 2017 10:09
@sonicdoe
Copy link
Author

Thank you for implementing the rest of the necessary changes and for releasing v1.0.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants