Skip to content

Conversation

@jmechn
Copy link

@jmechn jmechn commented Aug 4, 2022

Had a look at the other isEmoji functions. One of my initial solutions I found was to use \p{Emoji}, however, it also accepts numbers and other symbols. There were other ones such as \p{Extended_Pictographic} but it didn't seem to work. I found that \p{Emoji_Presentation } worked but some emojis would not return true as they were considered symbols rather than an emoji, for example: ☂, ❤ , ©️, etc. Unfortunately the emoji unicode system is all over the place so I haven't been able to simplify the Regex to only accept the symbols that Emoji_Presentation cannot. Currently, the regex accepts emojis and emojis that are both symbols and emojis.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov
Copy link

codecov bot commented Aug 4, 2022

Codecov Report

Merging #2015 (c5584a4) into master (1bb14e8) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2015   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          104       105    +1     
  Lines         2203      2210    +7     
  Branches       477       478    +1     
=========================================
+ Hits          2203      2210    +7     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)
src/lib/isEmoji.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jmechn jmechn marked this pull request as draft August 4, 2022 14:51
@jmechn jmechn marked this pull request as ready for review August 4, 2022 14:51
@WikiRik
Copy link
Member

WikiRik commented Aug 8, 2022

This is an improvement to #1968 right?

removed .idea folder
@jmechn
Copy link
Author

jmechn commented Aug 8, 2022

This is an improvement to #1968 right?

Yes, rather than listing all the possible emojis, the regex takes ranges of emojis. However, the regex definitely can be improved further.

Copy link

@ParthJadhav ParthJadhav left a comment

Choose a reason for hiding this comment

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

LGTM... Regex could be simplified. But it works great !

@pano9000
Copy link
Contributor

one question that comes to my mind here:
What is the purpose for this validator?
Does it make sense to check if a string contains an emoji somewhere, or should it rather check, if the supplied string is one (or even several?) emoji characters?

I can see the latter being useful, but I can't really see any practical application for the former.

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.

4 participants