Skip to content

Fixes #23 -- Add a selector for skin tone#43

Merged
treyhunner merged 2 commits intoTruthfulTechnology:mainfrom
bmispelon:skintone-part-deux
Jun 12, 2022
Merged

Fixes #23 -- Add a selector for skin tone#43
treyhunner merged 2 commits intoTruthfulTechnology:mainfrom
bmispelon:skintone-part-deux

Conversation

@bmispelon
Copy link
Copy Markdown
Collaborator

Features:

  • Live updates of tiles when changing the skin tone
  • Only emojis that supports the skin tone modifier characters are affected
  • Selected skin tone persists between searches and sessions (using the browser's localStorage)

Limitations:

  • Because of the way the fancy labels are implemented, you sometimes need to click twice on the skin tone selector to open it (once to focus, once to open the dropdown)
  • Skin tone support on individual emojis is a boolean (supplied by the library) and doesn't consider the user's actual support (it can vary based on system or browser version). This can result in slightly broken tiles where both the original emoji and the skin tone character are shown side by side instead of combined. I think it's acceptable considering that this "slightly broken" behavior already appears for some "advanced" combined emoji (for example the "service dog" emoji doesn't render correctly for me on chromium but works on firefox on the same machine).

@netlify
Copy link
Copy Markdown

netlify bot commented Jun 12, 2022

Deploy Preview for leafy-cocada-ad06dd ready!

Name Link
🔨 Latest commit ad52f97
🔍 Latest deploy log https://app.netlify.com/sites/leafy-cocada-ad06dd/deploys/62a62d0819201b00094e2073
😎 Deploy Preview https://deploy-preview-43--leafy-cocada-ad06dd.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bmispelon bmispelon changed the title Add a selector for skin tone Fixes #23 -- Add a selector for skin tone Jun 12, 2022
@treyhunner
Copy link
Copy Markdown
Member

Features:

  • Live updates of tiles when changing the skin tone
  • Only emojis that supports the skin tone modifier characters are affected
  • Selected skin tone persists between searches and sessions (using the browser's localStorage)

Nice work!

Limitations:

  • Because of the way the fancy labels are implemented, you sometimes need to click twice on the skin tone selector to open it (once to focus, once to open the dropdown)

It seems like disabling pointer-events in CSS on the label fixes this issue:

<label for="skintone" style="pointer-events: none;">Skin tone</label>

That might be worth doing (I experienced the double-clicking right away and I think it's worth fixing if we can).

  • Skin tone support on individual emojis is a boolean (supplied by the library) and doesn't consider the user's actual support (it can vary based on system or browser version). This can result in slightly broken tiles where both the original emoji and the skin tone character are shown side by side instead of combined. I think it's acceptable considering that this "slightly broken" behavior already appears for some "advanced" combined emoji (for example the "service dog" emoji doesn't render correctly for me on chromium but works on firefox on the same machine).

I don't have any individual skin tone set in my operating system but it looks like some emoji characters don't merge properly.

image

I thought maybe this was due to the OpenSansEmoji font, but it doesn't look like the browser is even serving up that font (I assume I messed something up in the CSS font-face config).

@bmispelon
Copy link
Copy Markdown
Collaborator Author

I've pushed a commit with the pointer-events: none workaround. I didn't know about that CSS property, thanks :)

As for the issue with the "unmerged" emoji, I see the same thing as your screenshot on my machine but only in chromium. Firefox renders those fine.

From what I understand the Unicode standard defines which emoji are "skintone-enabled" but each font is then responsible for actually implementing the merging. That explains why things can render differently on different OSes, different versions of the same OS, or even different browsers on the same OS.
As I mentioned, this issue of "unmerged" emoji is not limited to skintone and already appears with some more recent emoji.

One possible solution could be to do some <canvas> trickery to try and detect support in javascript. That seems a bit hacky and potentially expensive computation-wise.
Another solution I've been thinking about involves overflow-ing the emoji-char container in CSS so that in case the emoji is not merged correctly then at least it doesn't mess the layout too much. With some light js it might even be possible to style those emoji tiles a bit differently to indicate the error to the user.

@treyhunner
Copy link
Copy Markdown
Member

I tested this out on my phone and it seems to work much better than on my computer (the non-combining characters on my laptop all combine on the phone).

The non-combining issue seems to exacerbate the issue in #40. At least it only happens if/when users opt-in to using a modifier and if their OS has the similar issues to mine.

I think disabling pointer-events for labels is worth doing before merging. Thoughts @bmispelon?

@treyhunner
Copy link
Copy Markdown
Member

I've pushed a commit with the pointer-events: none workaround. I didn't know about that CSS property, thanks :)

Missed this before sending my last message.

One possible solution could be to do some <canvas> trickery to try and detect support in javascript. That seems a bit hacky and potentially expensive computation-wise. Another solution I've been thinking about involves overflow-ing the emoji-char container in CSS so that in case the emoji is not merged correctly then at least it doesn't mess the layout too much. With some light js it might even be possible to style those emoji tiles a bit differently to indicate the error to the user.

I was also thinking customizing overflow might fix this. 👍🏻

@treyhunner treyhunner merged commit 67bcb5e into TruthfulTechnology:main Jun 12, 2022
@bmispelon
Copy link
Copy Markdown
Collaborator Author

bmispelon commented Jun 12, 2022

Excellent, thanks 👍🏻

I'll make a note about overflow in #40 so we don't forget about it.

EDIT No I won't , I just saw that you've already closed that ticket 😅

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.

2 participants