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

update Emojione assets to latest Emojione version #132

Merged
merged 3 commits into from
Apr 16, 2018

Conversation

oriolbcn
Copy link
Contributor

@oriolbcn oriolbcn commented Feb 9, 2018

Fixes #85

I have updated the Emojione related assets to the latest Emojione version, also adding all previously missing images (now all has_img_emojione are true 😄).

First off, I wanted to thank and congratulate you on the magnific work done, all the build process is elegantly created and easy to use. Using all the scripts and everything it's been easy to do this (except one part that I struggled with and I detail below). Cheers!

In the process of doing the update I have done some more necessary changes that I detail here.

Emojione build changes

Starting from Emojione 3.0, they have separated all assets into a dedicated repo, so I added a reference to the repo (submodule) from build/emojione

Also, I have adapted the script that gets the images to reference the new directory and done another change (which took me a while to figure out, that's the thing I struggled with) to transform the filenames in emojine-assets to fully qualified names (ZWJ sequences).

The thing is that emojione has named the files with their short form, for example 🏋🏿‍♀️ is named 1f3cb-1f3ff-2640.png (missing the optional 200d and fe03 characters). BUT in the catalog we build we search only by the fully qualified name. Fortunately for us, Emojione provides a mapping indicating the fully qualified name for each file. What I have done is to use this mapping to rename the files copied to this repo in their fully qualified form.

build_image changes

When building the images I had a problem with the pipes system, I think it was causing some sort of overflow issue in my machine (Mac OS), so I changed to the "normal" form of calling montage by passing the full ordered list of files to the command. I think this is easier to understand and more portable, but I may be perfectly missing something here.

bash script changes

Since I am using Mac, stat -c was not working since the syntax is different, I just extracted that into a method that executes the correct command depending on whether it's run on Mac or Linux.

other things

I have already done the whole build process:

  • Rebuilt emoji.json and emoji_pretty.json (the only changes are the has_img_emojione flags).
  • Built and optimized all images using the provided scripts.
  • Rebuilt table.htm(isn't it beautiful that now the Emojione column is full?)

@oriolbcn
Copy link
Contributor Author

I added a new commit to the PR to add also the non_qualified attribute to skin_variations, because iOS and Android sometimes output the Emojis without the \uFE0F character.

@vanniktech
Copy link

What's stopping this from getting merged?

@HarshitOnGitHub
Copy link

Hey! Firstly, I would like to thank everyone involved in this project for their amazing work. We @zulip use this project for building our emoji infrastructure. We were in the process of upgrading to the latest release of this project when we encountered the issue related to this PR. This is currently a release blocker for us. Any update on when this will get merged? Thanks!

@vanniktech
Copy link

@iamcal friendly ping - could we get a review here?

@iamcal
Copy link
Owner

iamcal commented Apr 16, 2018

this is great - thanks for all the work that went into this.

the pipe stuff might be due to php changes, assuming you're using a newer version than i was

@iamcal iamcal merged commit 5c87b66 into iamcal:master Apr 16, 2018
@iamcal
Copy link
Owner

iamcal commented Apr 16, 2018

v4.0.4 is pushing to npm now

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.

4 participants