missive / emoji-mart

🏪 One component to pick them all
https://missiveapp.com/open/emoji-mart
MIT License
8.67k stars 835 forks source link

TypeError when hovering custom emojis #118

Closed Gargron closed 7 years ago

Gargron commented 7 years ago

When the Picker is open and you're hovering of custom emojis, the following error appears in the console (albeit not blocking any important functions).

TypeError: emojiData.short_names is undefined

The stack trace leads to the "preview" component, which I am hiding with display: none because there is no other way to turn it off.

I believe this is because sanitize strips out short_names here:

https://github.com/missive/emoji-mart/blob/master/src/utils/index.js#L22-L31

Though don't quote me on that because I don't really understand that part of the code very well. All I can say is that this is how I insert custom emojis into the index:

// When emojis are loaded, I put them into the index by doing this (which
// is counter-intuitive but seems to do the job. You actually have to be careful
// to do it this way, because otherwise every search call would insert a new
// "Custom" category into the dataset
emojiIndex.search('', { custom: buildCustomEmojis(immutableListOfEmojisHere) });

// The original data of the emojis is "url" and "shortcode" (e.g. :octocat:), here is
// what buildCustomEmojis function does
export const buildCustomEmojis = customEmojis => {
  const emojis = [];

  customEmojis.forEach(emoji => {
    const shortcode = emoji.get('shortcode');
    const url       = emoji.get('url');
    const name      = shortcode.replace(':', '');

    emojis.push({
      id: name,
      name,
      short_names: [name],
      text: '',
      emoticons: [],
      keywords: [name],
      imageUrl: url,
      custom: true,
    });
  });

  return emojis;
};

As you can see, short_names is clearly set on input. Not sure what happens after that.

EtienneLem commented 7 years ago

Sorry for the delay. That part might’ve been overlooked or there may have been a regression. Thanks for the report, I’ll look into it.

EtienneLem commented 7 years ago

As a FYI: v2.0.0 introduced a showPreview props