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

enhancement: functionality to disable the frequent emoji sorting #417

Conversation

SaraRandolph
Copy link

This is in reference to issue #366.

I added:

  • a bool value disableFrequentEmojiSort that defaults to false
  • functionality in storybook so we can all see it in action

I'm very open to any feedback anyone has so please let me know if I missed anything...its been known to happen 😉😂

Thank you guys!

@EtienneLem
Copy link
Member

Oh, it was that easy huh? 😅

One might argue that it shouldn’t even be an option, but the default non-configurable behaviour. What do you think? 🤔It does feel weird and hardly expected that these can change position when you want to insert multiple of the same emoji.

@SaraRandolph
Copy link
Author

@EtienneLem hmmm I know what you are saying about it seeming like unexpected behavior when clicking on it. BUT maybe since thats how its been built people who use emoji-mart are excepting that behavior and any change would be upsetting? 🤷

If it were just up to me, I would say we make the default behavior be to NOT update the emojis and give people to option to configure it, if they so choose. I can adjust the PR to reflect that and change the bool value to enableFrequentEmojiSort, if we want.

However, these things are not up to just me 😂 so I think that with either option, we can easily justify the decision. 🎉

@EtienneLem EtienneLem force-pushed the sara.randolph/enhancement/disable_frequent_emoji_sort branch from fde31fa to 56ae7b4 Compare March 16, 2020 15:10
@EtienneLem
Copy link
Member

If it were just up to me, I would say we make the default behavior be to NOT update the emojis and give people to option to configure it, if they so choose. I can adjust the PR to reflect that and change the bool value to enableFrequentEmojiSort, if we want.

I did just that, I’m confident this is the expected behavior by most. Will add this as a breaking change in the changelog.

Thanks for contributing @SaraRandolph 🙌

@Simon-Laux
Copy link

wouldn't be disableImmediateFrequentEmojiSort a better name?

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

3 participants