TheWidlarzGroup / rn-emoji-keyboard

Super performant, lightweight, fully customizable emoji picker 🚀
https://thewidlarzgroup.github.io/rn-emoji-keyboard/
MIT License
326 stars 59 forks source link

Source info property addition #126

Closed liellevibob closed 1 year ago

liellevibob commented 1 year ago

Problem: When using multiple <EmojiComponent/> it is necessary to identify which emoji component have changed (where was made the last emoji selection) in order to associate the appropriate context.

Solution: An optional property, sourceInfo, that be will be returned with onEmojiSelected callback, if passed by the <EmojiComponent/> caller

jakex7 commented 1 year ago

Hi @liellevibob, thank you for your suggestion, however, I am not entirely convinced that this is the right way to solve your problem. How will this way be better than using the example below?

      <EmojiPicker 
         onEmojiSelected={(emojiObject: EmojiType) => handlePick(emojiObject, "source")} 
         // ...
      />
liellevibob commented 1 year ago

Hi @liellevibob, thank you for your suggestion, however, I am not entirely convinced that this is the right way to solve your problem. How will this way be better than using the example below?

      <EmojiPicker 
         onEmojiSelected={(emojiObject: EmojiType) => handlePick(emojiObject, "source")} 
         // ...
      />

Hi, thanks for the reply! If I add an arrow function it will affect the performance of my application. Support through the library will also solve this problem!

oferRounds commented 1 year ago

@jakex7 just to add to what @liellevibob said – it is recommend not to use an arrow function on the render method, as in this case, React will see this function as new every time and will re-render on every change, regardless if it needs or not

jakex7 commented 1 year ago

I don't think that's necessary to include this functionality in the library, especially with source typed as any. Everyone can implement such a feature themselves with custom component like this:

type CustomEmojiPickerProps = {
  source?: any;
  onEmojiSelected: (emoji: EmojiType, source?: any) => void;
};
export const CustomEmojiPicker = ({ source, ...props }: CustomEmojiPickerProps & KeyboardProps) => {
  const onEmojiSelected = React.useCallback(
    (emoji: EmojiType) => {
      props.onEmojiSelected(emoji, source);
    },
    [props, source]
  );
  return <EmojiPicker {...props} onEmojiSelected={onEmojiSelected} />;
};

Additionally, I don't fully know your case, but I'm guessing you have multiple EmojiPickers on one screen. It would probably be more efficient to use the EmojiPicker once and survive with the arrow functions in render

liellevibob commented 1 year ago

I don't think that's necessary to include this functionality in the library, especially with source typed as any. Everyone can implement such a feature themselves with custom component like this:

type CustomEmojiPickerProps = {
  source?: any;
  onEmojiSelected: (emoji: EmojiType, source?: any) => void;
};
export const CustomEmojiPicker = ({ source, ...props }: CustomEmojiPickerProps & KeyboardProps) => {
  const onEmojiSelected = React.useCallback(
    (emoji: EmojiType) => {
      props.onEmojiSelected(emoji, source);
    },
    [props, source]
  );
  return <EmojiPicker {...props} onEmojiSelected={onEmojiSelected} />;
};

Additionally, I don't fully know your case, but I'm guessing you have multiple EmojiPickers on one screen. It would probably be more efficient to use the EmojiPicker once and survive with the arrow functions in render

Hi, thanks for the solution!! I wanted to update that I used a patch for the library. And in addition, I updated the pr which should now work. just so you'll know :)

oferRounds commented 1 year ago

@jakex7 thank you! Like your suggestion also!

jakex7 commented 1 year ago

I am closing this PR, as another way that does not require modification of the library has been proposed.