onSkinChange prop

[akxcv]

Hi! Thank you for the great library!

I've encountered a use case where I need to do something whenever a skin tone is selected in the picker. I propose we add an onSkinChange prop to Picker's API. I can submit a pull request if you think this is a good idea.

EDIT:

Also, because of this line, it's impossible to change the picker's skin tone from the outside of if the skin tone is already set in the store. That's also a use case I'm personally interested in. Is there any reason why we have to check if skin tone is present in the store?

[EtienneLem]

I propose we add an onSkinChange prop to Picker's API. I can submit a pull request if you think this is a good idea.

Sure! 👍

Is there any reason why we have to check if skin tone is present in the store?

Mmm, I guess my initial thought was to not allow skin change from the props when the user decides they want a certain skin tone. Props would be a way to set the default, but how can we tell if we should use the prop vs. the user’s choice? Perhaps a boolean forceSkin props could be introduced? What do you think?

[akxcv]

Hmm. Is there a use case where you'd change the skinTone prop thinking that would change the default? 🤔
In my opinion, the skin tone from the store should merely be a default for the skinTone prop (in case it wasn't explicitly provided).

// '6' in store
<Picker {...props} /> // => skinTone is 6
<Picker {...props} skinTone={2} /> // => skinTone is 2
// nothing in store
<Picker {...props} /> // => skinTone is 1
<Picker {...props} skinTone={2} /> // => skinTone is 2

Does this make sense?

[EtienneLem]

But if props is more important than store, then as soon as you pass a skin tone the user’s choice would be ignored. If that was to be the case, should passing skin remove the skin chooser completely? Otherwise the developer would have to pass the skin as the skin, but then I’m not sure that’s better than the current behavior and result would be the same.

[EtienneLem]

I guess the best thing would be to handle all the use cases:

• skin: Overwrites user’s choice. It’s up to the developer to handle skin in store if they want to.
• defaultSkin: Only used when no skin in store.
• showSkinTones: Boolean to toggle on/off the skin picker.

Would give max control to devs. Sounds like something that would work for you?

[akxcv]

Sure, sounds good. Although I suppose, that would introduce a breaking change into the API, is that okay?

[akxcv]

Cool, thank you!

[tarzak]

I see at some point in time, we had onSkinChange callback. But we don't anymore. We need to store skin tone for different users in our users' settings. Is it possible to return this prop? @EtienneLem

[duhaime]

@EtienneLem Is there any chance you could bump the @emoji-mart/data and @emoji-mart/react packages on npm to include this feature? We'd be grateful if you could!

[duhaime]

oh strange, I looked at the source and see the react package is a thin wrapper around the base package. But searching the source I don't see onSkinChange -- am I missing something or did this get removed?