-
-
Notifications
You must be signed in to change notification settings - Fork 123
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
Initial support for Unicode emojis #963
base: main
Are you sure you want to change the base?
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
I tried running Prettier and got tons of changes, none of which are related to the code I touched. I guess I ignore this test failure then? |
@palant your prettier is probably running without following the repo's config? You can run it on specific files too. Few notes:
To be honest, I'm not a fan of adding an emoji picker — I personally use the OS's native emoji picker (and 3rd-party native emoji picker apps) that works across all web sites and apps. Also, if I wanted to add one, probably could use this https://github.com/nolanlawson/emoji-picker-element but the concerns are the huge file sizes (images, shortcode mappings, locale files) and maintenance work required to update it every year as the folks from Unicode Consortium keep adding more. But I'm interested to see how this works. Preprocessed data is nicely done. If you try |
It clearly does respect the repo’s config, it corrects double quotes into single quotes for example. I’m running the same command as that test, yet it finds way more issues for some reason.
These are screenshots made on Linux (KDE): Note that I explicitly put the categories of the server’s custom emojis on top (it’s also what Mastodon does). This doesn’t quite work for the “other” category which is always placed at the bottom, but this could be addressed.
Quite frankly, I’d consider it more confusing if they were separate. As far as users are concerned, there is no difference between custom emojis and Unicode ones. Short code clashes don’t really seem to be an issue (other than for React which can be addressed). It’s a bit confusing if multiple icons show up with the same shortcut in the search, but the users can just select the icon they like. And Unicode emojis aren’t inserted as their shortcuts but rather as the actual Unicode symbol.
I am aware of that. Still, they don’t show up for me despite clicking. But that’s a side issue, I can look into it if the general direction is approved.
Well, I didn’t add one – there is one already, and it doesn’t do too badly. It could do with a category selector at the top, otherwise I don’t see big issues however. Linux doesn’t have a native emoji picker, and native emoji pickers aren’t always too convenient. Besides, they won’t let you pick the server’s custom emojis, and providing a unified experience for all emojis no matter the source would be beneficial.
I did look into this one first. I then decided that adding the necessary functionality to the existing emoji picker would be simpler. It works with the same data, but its preprocessing won’t reduce file size quite as much. The only benefit I see would be detection of broken emojis – something that would be fairly easy to address. An emoji font would be quite heavy. The decision whether to use one is independent of an emoji picker however. It would be a way to provide a unified experience across all platforms, including the ones with less than optimal emoji support. There is already #476 on that at the very least.
The bundle sizes aren’t affected, these JSON files aren’t compiled into the bundles. I considered compiling them into the bundles and using dynamic loading to make sure only the required locales are being loaded, but this turned out unnecessarily complex. So it’s static files and The file sizes vary between 260 KiB (Chinese) and 375 KiB (Ukrainian). Altogether it’s 6.5 MiB which could be reduced to 4.6 MiB by removing data duplicating the |
Ok, it’s definitely a versions mismatch. I’ve been running prettier 3.4.1, the test is running some version that is considerably older. It isn’t version 2.8.0 which is listed in If I may suggest, it would be a good idea to add |
This is not meant to be a complete fix for #131 and #400, I didn’t want this PR to get too complex. This is only a first step in the right direction.
This makes sure that data from the
emojibase-data
package is preprocessed and placed into theassets/emojis
directory. There is one file for each locale, with some files being identical to English becauseemojibase-data
doesn’t have translations for Czech for example.The
getCustomEmojis
function has been modified to load both custom emojis and the locale-specific Unicode ones. The callers can then handle Unicode emojis with minimal changes.Issues explicitly not addressed here:
emojibase-data
translation varies. For example, the gender neutral “scientist” is translated as “Wissenschaftler(in)” to German, the female “woman scientist” as “Wissenschaftlerin”. Both get the shortcode “wissenschaftlerin” – and Phanpy is using that shortcode as key, so React will complain about duplicate keys. Probably best to address this upstream, though some solution for duplicate shortcodes might also be in order.