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.

Sign up for GitHub

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

Jump to bottom

[Feature] Add filterFunctions prop to enable emojis filtering #34

Merged
merged 4 commits into from
Apr 30, 2019
Merged

[Feature] Add filterFunctions prop to enable emojis filtering #34

merged 4 commits into from
Apr 30, 2019

Conversation

darekg11
Copy link
Contributor

@darekg11 darekg11 commented Apr 6, 2019

Relate to any issue?

#5
It is a temporary workaround that would allow user of this library to avoid missing emojis being render as X

Breaking change?

Nope, by default all emojis will be rendered as before.

What this PR does?

Adds new filterFunctions prop allowing user to pass an array of functions used to filter emojis list before rendering it

Example of usage:

	filterFunctionByUnicode = emoji => {
		return emoji.lib.added_in === "6.0" || emoji.lib.added_in === "6.1"
	}

	<EmojiInput
		onEmojiSelected={this.handleEmojiSelected}
		ref={emojiInput => this._emojiInput = emojiInput}
		resetSearch={this.state.reset}
		loggingFunction={this.verboseLoggingFunction.bind(this)}
		verboseLoggingFunction={true}
		filterFunctions={[this.filterFunctionByUnicode]}
	/>

Change Preview:

Before on my Huawei P8Lite running Android 6.0:
before

After enabling only 6.0 Unicode:
after

@darekg11
Add filterFunctions prop allowing user to specify filter functions th…
60feb0c 
…at are invoked for every emoji before actual rendering making it possible to show only subset of emojis instead of all of them always
@darekg11 darekg11 mentioned this pull request Apr 6, 2019
Closed
@darekg11
Copy link
Contributor Author

darekg11 commented Apr 6, 2019

@rijn Let me know what You think : )

rijn
rijn requested changes Apr 7, 2019
View reviewed changes
Copy link
Collaborator

@rijn rijn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit. 😍 Thanks for working on this.

src/EmojiInput.js
@@ -298,7 +301,7 @@ class EmojiInput extends React.PureComponent {
.map((v, k) => [
{ char: category[k].key, categoryMarker: true, ...category[k] }
]);
_(emoji)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename it to emojis

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the param of function to emojis instead of emoji - let me know if this is what You meant.

src/EmojiInput.js Outdated
@@ -287,6 +287,9 @@ class EmojiInput extends React.PureComponent {
return e1.char !== e2.char;
});

this.filtered = this.props.filterFunctions.length === 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's directly connect the filter functions to lodash chain, e.g.

_(emojis)
  .filter(emoji => _.every ...)
  ...

Also _.every will return true if the first argument is empty array, so it's safe not to check whether the length is zero

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. 👍

darekg11 added 2 commits April 7, 2019 10:58
@darekg11
Chain lodash call to filter emojis. Drop checking for 0 length of fil…
b3176f6 
…terFunctions props as every will return true if collection is empty. Rename emoji param to emojis
@darekg11
Add call to value at the end of Lodash chain. Add missing semicolon
fcf92d5
rijn
rijn requested changes Apr 8, 2019
View reviewed changes
Copy link
Collaborator

@rijn rijn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes required. Also, I realized that filterFunctions will not be frequently changed. Therefore, moving the emojis that being filtered by filterFunctions to the state might be a better approach. I'll create another issue to follow this.

example/src/EmojiInput.js Outdated
let dataProvider = new DataProvider((e1, e2) => {
return e1.char !== e2.char;
});

this.filteredEmojis = _(emojis).pickBy(emoji => _.every(this.props.filterFunctions, fn => fn(emoji))).value();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to assign filteredEmojis to this component? The value will only be used inside the function, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filteredEmoji is not needed anymore so I have removed that.

example/src/EmojiInput.js Outdated
@@ -298,7 +299,7 @@ class EmojiInput extends React.PureComponent {
.map((v, k) => [
{ char: category[k].key, categoryMarker: true, ...category[k] }
]);
_(emoji)
_(this.filteredEmojis)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do the following

_(emojis)
  .values()
  .filter(emoji => _.every(this.props.filterFunctions, fn => fn(emoji)))
  .each(e => {
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed 👍

rijn
rijn approved these changes Apr 8, 2019
View reviewed changes
Copy link
Collaborator

@rijn rijn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely. Thanks a lot!

@darekg11
Copy link
Contributor Author

darekg11 commented Apr 9, 2019

No problem 👌

@sskhandek
Copy link
Owner

Thank you for all the work here @rijn and @darekg11

@sskhandek sskhandek merged commit 0c51ffe into sskhandek:master Apr 30, 2019
@darekg11
Copy link
Contributor Author

darekg11 commented May 4, 2019

No problem, glad to help.

@darekg11 darekg11 deleted the feature/allow-to-filter-rendered-emojis branch May 4, 2019 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rijn rijn rijn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@darekg11 @sskhandek @rijn