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

Clearing custom emojis from index and pool when necessary #172

Conversation

vcervellera-turbulent
Copy link

@vcervellera-turbulent vcervellera-turbulent commented Mar 12, 2018

👋

Problem & how to reproduce:
File emoji-index.js has four global variables, that will mutate whenever you search an emoji with the search input:

var originalPool = {}
var index = {}
var emojisList = {}
var emoticonsList = {}

Considering you have Emoji Pickers in different places (different communities), where the custom emojis that are provided to those pickers vary, if the index nor the pool get "contaminated" with the custom emojis of one Picker, every other will contain them in their search results.

Solution provided
_The best approach would probably have been to re-factor those four global variables to bind them to the Emoji Picker instead of having them be globally instantiated. However, the emoji-index.js util exports emojis and emoticons, therefore in order to avoid any further regression, I approached the issue differently to work on a patch rather than a refactor ~_

Instead of calling addCustomToPool at every search call triggered, I store the previously provided custom emojis and store them in a list, that I compare the custom emojis provided along the search with, to define whether we need to feed the pool (and the emojisList) with those. In addition to that, whenever there's a change identified between the previous, and the new custom emojis, I remove the entries of old custom emojis, and clear the indexes, which in the case of a new custom prop provided, is a reliable (and possibly the least expensive) way to clear the indexed results and pools.

I know this is a pretty specific fix for projects that are using emoji pickers in different contexts, sorry about that! 🙏

Edit: I reproduced the faulty behaviour in a sandbox so you can witness it: https://codesandbox.io/s/20wwk5nj0r

@EtienneLem
Copy link
Member

👋 Hey!

Sorry for the delay, this looks fantastic BTW. I’ll be merging and releasing very shortly ✌️

@vcervellera-turbulent
Copy link
Author

Hey @EtienneLem 👋 Any chance of a release this week including this fix? 🙏

@EtienneLem EtienneLem merged commit 933ea29 into missive:master Mar 27, 2018
@EtienneLem
Copy link
Member

Just released v2.5.1
Thanks again, it works —> https://codesandbox.io/s/q9oz8wzr7q 🙌

@vcervellera-turbulent
Copy link
Author

Woo! Thank you! 🙏

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.

2 participants