Add option to use native emoji #731

felixfbecker · 2019-01-18T20:11:14Z

Closes #672

Adds a new option "native" to the emoji menu.

Uses a MutationObserver to swap emoji img elements with the actual emoji text, and injects CSS to show the native emoji in the editor.

Also added a bunch more docblocks with type info.

I did not enable it by default because unfortunately some emoji are rendered as multiple characters, which is an Electron bug I wasn't able to work around: electron/electron#11874

Despite this flaw (which I hope will be fixed in the future) I would still use this option and think it is good to have.

CvX

Hey, thank you for this PR! 🙂 I like the idea of using native (or specifically, Apple's) emoji.

There are some things that need to be fixed first, though. Other than these, I'll also look into electron/electron#11874 issue.

browser.css

browser.js

emoji.js

To sort by "completeness"

felixfbecker · 2019-01-20T13:56:36Z

@CvX resolved all outstanding issues

felixfbecker · 2019-01-20T14:14:11Z

Build for testing: https://we.tl/t-9V4ZMO0boX

CvX · 2019-01-20T14:18:39Z

Some things are still broken:

Native emoji is displayed with various alpha color values (0.7 in the sidebar, 0.9 in the conversation when using the dark mode, etc.). .native-emoji should have its color set to something like rgba(0, 0, 0, 255).
Emojis in the reaction popover are misaligned:
Emojis in the "insert emoji" popover are misaligned too:
Emoticons (e.g. ":)") in the message composer are no longer being replaced with emoji while typing the message

felixfbecker · 2019-01-20T14:20:57Z

Native emoji is displayed with various alpha color values (0.7 in the sidebar, 0.9 in the conversation when using the dark mode, etc.). .native-emoji should have its color set to something like rgba(0, 0, 0, 255).

Could you elaborate on this? Shouldn't the emoji be displayed like the surrounding text?

CvX · 2019-01-20T14:25:28Z

I think emoji should always be displayed as they are – full opacity, no tinting. That's how they are displayed with other emoji sets anyway.

CvX · 2019-01-20T14:34:40Z

Ouch, here's a biggie:

Send ":)", restart Caprine, send another ":P". The last message in the conversation list doesn't update. Try sending "test", you end up with this:

felixfbecker · 2019-01-20T15:16:18Z

The error that causes the "Could not display composer" is

DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
    at Xg (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:81303)
    at fi (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:101019)
    at ei (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:98043)
    at ci (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:97387)
    at ji (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:104465)
    at hd (https://static.xx.fbcdn.net/rsrc.php/v3iLl54/yp/l/en_US/EYCXyA9EU03.js:47:32140)
    at HTMLDocument.n (https://static.xx.fbcdn.net/rsrc.php/v3/yF/r/KAUB_fijJwW.js:103:2698)
    at Object.applyWithGuard (https://static.xx.fbcdn.net/rsrc.php/v3/yF/r/KAUB_fijJwW.js:55:2237)
    at t.fire (https://static.xx.fbcdn.net/rsrc.php/v3/yF/r/KAUB_fijJwW.js:263:7487)
    at HTMLDocument.<anonymous> (https://static.xx.fbcdn.net/rsrc.php/v3/yF/r/KAUB_fijJwW.js:263:6644)

It seems like messenger doesn't like us messing with their DOM nodes?

CvX · 2019-01-20T15:55:39Z

Yeah… 😕

…I have an idea… that involves removing the MutationObserver and rendering emojis to canvas. 🙈

You know how currently the emoji replacement introduced in #720 uses redirects to change which emoji image is getting displayed? Well, it's possible to redirect to a dataURL! 😃

So we can render emoji (taken from the original URL and processes with String.fromCodePoint()) on-demand to a canvas, convert them to dataURL format and redirect to it. 😃

felixfbecker · 2019-01-20T15:56:52Z

I'm kind of out of ideas on this one. I think what we're seeing here is React holding a references to the old img that we replaced with a span. When it discovers that it needs to remove that img as part of a rerender, it calls removeChild(), which fails because the img is not anymore in the DOM and it really should be removing the new span instead.

The emojis in the message editor are possible to replace without this error because they use spans with background-image, so we can modify the element's textContent in place and just remove the background-image style. But emojis in the message list are imgs, which cannot have contents, not even with display: contents or pseudo elements.

felixfbecker · 2019-01-20T15:59:58Z

Yeah… 😕

…I have an idea… that involves removing the MutationObserver and rendering emojis to canvas. 🙈

You know how currently the emoji replacement introduced in #720 uses redirects to change which emoji image is getting displayed? Well, it's possible to redirect to a dataURL! 😃

So we can render emoji (taken from the original URL and processes with String.fromCodePoint()) on-demand to a canvas, convert them to dataURL format and redirect to it. 😃

Nice, that should work. But it'll have to be done through IPC I think since the interceptor is defined in the main process.

CvX · 2019-01-20T16:05:03Z

Yup, the strategy would be pretty much like in #731 (comment). 🙂

felixfbecker · 2019-01-20T16:19:12Z

It's a bit annoying that there is no real IPC layer used yet, like JSON RPC or comlink.

felixfbecker · 2019-01-20T17:17:35Z

Seems to work

felixfbecker · 2019-01-20T17:50:51Z

Need to tweak sizing on Windows

felixfbecker · 2019-01-20T18:14:56Z

Should be all fixed! 🎉

Messenger’s default should be first.

emoji.js

This reverts commit aa42ea4.

CvX · 2019-01-27T16:19:04Z

There are a couple of broken emojis:

An update on "broken emojis" issue:

As per electron/electron#16530 Electron 5.0.0-beta.1 contains a newer Chromium version that fixes most (though not all) emoji rendering problems:

It seems that there's a change in emoji's vertical offset when rendering, so on 5.0.0-beta.1 they're cut off a bit at the top. It's something to look out for when we upgrade after 5.0 stabilizes.

Anyway, I'm marking this one as "fixed".
The other 3 issues remain (128px emojis, "MaxListenersExceededWarning", and facebook's thumbs-up).

felixfbecker · 2019-01-28T16:29:02Z

The earliest I can get to this is next week, anyone is welcome to push/PR to the branch.

felixfbecker · 2019-01-30T14:52:01Z

Merged in master with change to TypeScript

CvX · 2019-01-30T14:58:41Z

Thank you! 🎉🚀

…felixfbecker/native-emoji # Conflicts: # package.json # source/emoji.ts

CvX · 2019-01-31T14:00:49Z

Merged in the master once again, and fixed the Facebook's thumbs-up:

felixfbecker · 2019-02-02T15:20:23Z

Btw I just discovered OffscreenCanvas which is also available in WebWorkers. I wonder if it's available in the Electron main process?

CvX · 2019-02-02T15:28:12Z

When I suggested the use of canvas I originally thought we could use the offscreen variant, but when I tried it… nope! 😄 Wasn't available in the context of the main process.

sindresorhus · 2019-02-02T18:43:19Z

https://electronjs.org/docs/tutorial/offscreen-rendering

CvX · 2019-02-03T10:06:01Z

Hm, I don't think Electron's offscreen rendering is a good fit here. Keeping a spare (hidden) frameless window just for rendering emoji seems excessive. 😃

CvX

🚀

felixfbecker · 2019-02-19T00:49:45Z

All done!

Fixes #672 Co-authored-by: Jarek Radosz <[email protected]>

sindresorhus · 2019-02-20T07:53:42Z

Nice work, everyone! 🎉

felixfbecker · 2019-02-22T15:58:27Z

🚢

CvX requested changes Jan 18, 2019

View reviewed changes

browser.css Outdated Show resolved Hide resolved

browser.js Outdated Show resolved Hide resolved

browser.js Outdated Show resolved Hide resolved

emoji.js Outdated Show resolved Hide resolved

felixfbecker added 2 commits January 20, 2019 14:38

Add option to use native emoji

9efed9d

Get emoji from src URL instead of alt text

b1df780

felixfbecker force-pushed the native-emoji branch from fadf2f5 to b1df780 Compare January 20, 2019 13:40

felixfbecker added 3 commits January 20, 2019 14:43

Remove native emoji "icon"

417749f

Handle large emojis

7398df4

Move native option to the top

0e367e1

To sort by "completeness"

Use canvas approach

430dea2

felixfbecker force-pushed the native-emoji branch from 172d656 to 430dea2 Compare January 20, 2019 17:18

felixfbecker added 2 commits January 20, 2019 19:00

Fix sizing on Windows

9de30c2

Account for different emoji font on mac

68761b8

felixfbecker and others added 4 commits January 20, 2019 19:16

Use is.macos

5b1c690

Disable some jsdoc linting rules.

01b197d

Fix a jsdoc linter warning.

edc1ff2

Move native emoji set to the second spot in the menu.

c62ca4d

Messenger’s default should be first.

felixfbecker commented Jan 21, 2019

View reviewed changes

emoji.js Outdated Show resolved Hide resolved

CvX and others added 4 commits January 21, 2019 16:04

Revert "Change the native emoji label depending on the platform."

1037f58

This reverts commit aa42ea4.

Rename “Native” emoji option to “System”.

b7f615b

Change emoji menu order, FB 3.0 label, add a separator.

107a7ba

Add docblocks

be4b86d

Merge branch 'master' into native-emoji

19c2cdf

CvX added 2 commits January 31, 2019 14:32

Merge branch 'master' of github.com:sindresorhus/caprine into pr-731/…

cf49e1f

…felixfbecker/native-emoji # Conflicts: # package.json # source/emoji.ts

Fix the Facebook’s thumbs-up icon in the “Change Emoji” popover

f449c03

Merge branch 'master' into pr-731/felixfbecker/native-emoji

32b0f47

felixfbecker added 3 commits February 19, 2019 00:56

Merge branch 'master' into native-emoji

d8475d3

Render at higher resolution

502c90d

Increase listener limit

ec2cb28

CvX approved these changes Feb 19, 2019

View reviewed changes

Optimize emoji rendering on Windows

39e0117

CvX requested a review from sindresorhus February 19, 2019 09:46

sindresorhus merged commit 1d1c2d5 into sindresorhus:master Feb 20, 2019

sindresorhus pushed a commit that referenced this pull request Feb 20, 2019

Add option to use native emoji (#731)

2717359

Fixes #672 Co-authored-by: Jarek Radosz <[email protected]>

felixfbecker deleted the native-emoji branch February 22, 2019 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to use native emoji #731

Add option to use native emoji #731

felixfbecker commented Jan 18, 2019 •

edited

Loading

CvX left a comment

felixfbecker commented Jan 20, 2019 •

edited

Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019 •

edited

Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

CvX commented Jan 20, 2019 •

edited

Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

CvX commented Jan 27, 2019

felixfbecker commented Jan 28, 2019

felixfbecker commented Jan 30, 2019

CvX commented Jan 30, 2019

CvX commented Jan 31, 2019

felixfbecker commented Feb 2, 2019

CvX commented Feb 2, 2019

sindresorhus commented Feb 2, 2019

CvX commented Feb 3, 2019

CvX left a comment

felixfbecker commented Feb 19, 2019

sindresorhus commented Feb 20, 2019

felixfbecker commented Feb 22, 2019

Add option to use native emoji #731

Add option to use native emoji #731

Conversation

felixfbecker commented Jan 18, 2019 • edited Loading

CvX left a comment

Choose a reason for hiding this comment

felixfbecker commented Jan 20, 2019 • edited Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019 • edited Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

CvX commented Jan 20, 2019 • edited Loading

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

CvX commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

felixfbecker commented Jan 20, 2019

CvX commented Jan 27, 2019

felixfbecker commented Jan 28, 2019

felixfbecker commented Jan 30, 2019

CvX commented Jan 30, 2019

CvX commented Jan 31, 2019

felixfbecker commented Feb 2, 2019

CvX commented Feb 2, 2019

sindresorhus commented Feb 2, 2019

CvX commented Feb 3, 2019

CvX left a comment

Choose a reason for hiding this comment

felixfbecker commented Feb 19, 2019

sindresorhus commented Feb 20, 2019

felixfbecker commented Feb 22, 2019

felixfbecker commented Jan 18, 2019 •

edited

Loading

felixfbecker commented Jan 20, 2019 •

edited

Loading

CvX commented Jan 20, 2019 •

edited

Loading

CvX commented Jan 20, 2019 •

edited

Loading