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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Emojis as a changing prop #167

Conversation

vcervellera-turbulent
Copy link

馃憢

Encountered issue:
We provide the picker with custom emojis based on different contexts, which means that in certain circumstances, the picker has custom emojis that will not be present at the next opening.
However, the CUSTOM_CATEGORY emojis array is built outside of the Picker class, and is kept in memory for all uses of the Picker, and therefore keeps any emojis its provided in memory, even when the Picker receives an empty custom prop in the future.

Fix:
Upon construction of the Picker, as we check if the custom prop has no length, I check if the CUSTOM_CATEGORY object contains emojis with a length, and reset it if so, to avoid contamination.

@savardc
Copy link

savardc commented Mar 1, 2018

Hi @EtienneLem, just making sure you haven't missed this fix. Understand if you're busy. Thanks!

@EtienneLem EtienneLem merged commit 90ce594 into missive:master Mar 2, 2018
@EtienneLem
Copy link
Member

@savardc Oh, thanks for the reminder.
@vcervellera-turbulent Thanks for the PR 馃

@EtienneLem
Copy link
Member

As a FYI: I noticed that we had other issues (#166) caused by the same thing. So although I did merge the PR, I sorta reverted it in the next commit (fed9a89) as it wasn鈥檛 required anymore.

These constants are now defined on each instance, instead of being shared. 馃槃

@vishalDhull0910
Copy link

vishalDhull0910 commented Sep 27, 2023

hey @vcervellera-turbulent can you please help me out here ,facing similar issue

how to reset react picker

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.

None yet

4 participants