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

added property to Emoji class to be used for pattern matching purposes #486

Conversation

johnearlelevado
Copy link

@johnearlelevado johnearlelevado commented Jan 19, 2021

This is to fix issues #485 and #484. I added a new property for Emoji for pattern matching purposes. The issue was that whenever the emoji has more than one code point, EmojiUtils.isOnlyEmojis() doesn't work.

Copy link
Owner

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

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

@rubengees is this a valid approach to fix the problem?

@mario
Copy link
Collaborator

mario commented Jul 21, 2021

@vanniktech if it works and it's better than what we currently have, why not? :)

@mario
Copy link
Collaborator

mario commented Sep 26, 2021

@rubengees poke here again :P

@vanniktech
Copy link
Owner

@tricksilver04 I've added test for this: #578

I tried your changes locally and unignored the test, however it's failing for the red emoji heart. Can you please have another look?

@rubengees
Copy link
Collaborator

@vanniktech I debugged this a bit and have the following findings:

  • Some emojis can optionally have a variant selector which denotes the previous emoji to be rendered as an emoji instead of text. We have multiple emojis that can have this variant selector.
  • Our datasource includes information about this. If the emoji can have a variant selector, the non_qualified field is not null. I tried to extend the generator to take this into account and made the tests pass except for the red heart exclamation.
  • I saw that we already specifically handle the star emoji with a variant selector. Interestingly that emoji does not appear in our data source as such a case.
  • The red heart exclamation seems to be either invalid or a special case. The code points are [10083, 65039, 65039] or 2763-FE0F-FE0F in hex. 2763-FE0F would be the heart exclamation with variant selector, but as we can see it has an additional variant selector! I'm not sure if multiple variant selectors are valid but e.g. browsers seem to ignore the second one.

I can think of two solutions:

  • Strip all variant selectors from the text before processing it in either the utils or when rendering the images. We want to display everything as an emoji either way.
  • Extend the generator to include this information in our code and handle it accordingly. Weird cases like the double variant selector would not work properly though I think.

@vanniktech
Copy link
Owner

I saw that we already specifically handle the star emoji with a variant selector.

Yes if I remember correctly, we've had a report about this in the past and did a fix for it. (without thinking about all of the other emojis like described here)


Can we do a mix that we adjust the generator to include the information and just get rid of the double variant selector ourself?

@vanniktech
Copy link
Owner

@rubengees I just saw this

if (dataEntry.unified === "2B50") {

This is where we handle the star

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants