validator.js icon indicating copy to clipboard operation
validator.js copied to clipboard

isEmojy added.

Open erfiboy opened this issue 4 years ago • 3 comments

#1889

erfiboy avatar Dec 23 '21 07:12 erfiboy

Codecov Report

Merging #1890 (ea013c5) into master (f055c11) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1890   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          102       102           
  Lines         2072      2072           
  Branches       472       472           
=========================================
  Hits          2072      2072           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f055c11...ea013c5. Read the comment docs.

codecov[bot] avatar Dec 23 '21 07:12 codecov[bot]

Thanks for this, please make the changes suggested and we should land this in.

profnandaa avatar Apr 22 '22 03:04 profnandaa

one tiny additional issue I see here:

What is the intended goal here?

  • a) check if a string contains any emojis OR
  • b) check if the string IS an emoji

Both of which could be valid use cases, I guess, but in any case expectation and result should match:

Currently the validator is called "isEmoji" - and to me this suggests that it tests case b) Howecer it actually tests case a) instead:

E.g. isEmojy("abc😂def") will return true

EDIT+ Ok, somehow I missed, that https://github.com/validatorjs/validator.js/pull/1968 exists That PR is adressing the issue above, and went for checking for case b) :-)

pano9000 avatar Dec 01 '22 21:12 pano9000