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

Return skin emoji on search #321

Merged
merged 7 commits into from
Mar 23, 2019

Conversation

pederjohnsen
Copy link

Returns emojis with skin data using skin tone from store in searches.

This is useful when doing emoji search for an autocomplete component.
Previously it would just return the default emoji data for an emoji, but now we get the skin related data too depending on what skin tone is set in the store.

Split out from #250

this.emojis[id] = {}
for (let skinTone = 1; skinTone <= 6; skinTone++) {
this.emojis[id][skinTone] = getSanitizedData({id, skin: skinTone}, skinTone, this.set, this.data)
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be smaller if it were an array, but an object of IDs 1-6 is fine

@nolanlawson
Copy link

Awesome PR. The only small issue is that we need to run yarn prettier to apply the right code style, but other than that this is great. Love the new tests as well. Tested it out in the storybook and it's definitely an improvement. Thanks!

@nolanlawson nolanlawson merged commit d9eda6a into missive:master Mar 23, 2019
@pederjohnsen
Copy link
Author

@nolanlawson hmm that's odd, I have prettier setup in my editor to format on save and looks like there is a prettier config in the repo, so would've thought it should've caught any issues 🤔

@pederjohnsen pederjohnsen mentioned this pull request Mar 23, 2019
@pederjohnsen pederjohnsen deleted the return_skin_emoji_on_search branch March 23, 2019 22:09
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