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 skin tone option #38

Merged
merged 4 commits into from
Apr 18, 2020
Merged

Conversation

Glennmen
Copy link
Contributor

@Glennmen Glennmen commented Apr 3, 2020

  • Default no skin tone (keep same behaviour)
  • Added environment variable to change skin tone (0, 1, 2 ,3, 4)
  • Added shift modifier to copy emoji without skin tone

fixes #31

This was my first time working on an Alfred workflow 😅 but it was a lot of fun. I am open for feedback.

First credits to https://github.com/muan/mojibar for inspiration how to add skin tone to an emoji.
Also there are a lot of emoji that should support skin tone but the emojilib is a little out of date it seems. Hopefully it will get updated soon.
I also didn't know if I should add skin tone to Copy code :+1:, I couldn't find any information if there is any spec for this or if every app implements this in their own format.

For now I used an environment variable skin_tone, but a good next improvement would be adding a new Alfred command to change skin tone.

- Default no skin tone
- Added environment variable to change skin tone (0, 1, 2 ,3, 4)
- Added `shift` modifier to copy emoji without skin tone
@jsumners
Copy link
Owner

jsumners commented Apr 4, 2020

What happens if a skin tone doesn't exist? Does it fall back to the neutral tone?

@Glennmen
Copy link
Contributor Author

Glennmen commented Apr 4, 2020

@jsumners Do you mean when you set a skin tone value higher than 4? It will fallback to the neutral tone.

@jsumners
Copy link
Owner

jsumners commented Apr 4, 2020

Also there are a lot of emoji that should support skin tone but the emojilib is a little out of date it seems.

So if a user has set that they prefer some alternative tone, and the emoji they are choosing doesn't support it, what happens? Can you paste a gif of such a search?

@Glennmen
Copy link
Contributor Author

Glennmen commented Apr 4, 2020

@jsumners That is what this line is for:

https://github.com/Glennmen/alfred-emoji/blob/2973921b00747ef706d16c838becd47811bb3db0/lib/genicons.js#L18

It will check if the emoji has skin tone support.
I am also writing some extra tests right now, will commit it in a few minutes and see if I can make a video.

@Glennmen
Copy link
Contributor Author

Glennmen commented Apr 4, 2020

@jsumners Here is the gif, I hope this answers your question 😄
emoji skin tone

Currently emojilib is very outdated it seems, that is why those smiles currently don't have a skin tone.

@jsumners
Copy link
Owner

jsumners commented Apr 6, 2020

Thank you for the gif; it answers my question clearly.

Currently emojilib is very outdated it seems, that is why those smiles currently don't have a skin tone.

I know. They are working on a restructuring on the library to follow Unicode releases. It's a difficult problem to tackle. I used up a whole day on it a while back and got basically nowhere (https://twitter.com/jsumners79/status/1211105577188970497). I may get back to it soon 🤷‍♂ .

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Co-Authored-By: James Sumners <james@sumners.email>
@Glennmen
Copy link
Contributor Author

Glennmen commented Apr 6, 2020

@jsumners If emojilib does the major update, it seems it will have a lot of breaking changes, feel free to tag me and if I have the time I will be happy to implement them. I went through the emojilib PR and unicode-emoji-json yesterday and it seems doable 🤔

I applied your suggested changed 🎉

@Glennmen Glennmen requested a review from jsumners April 6, 2020 13:42
@Glennmen
Copy link
Contributor Author

Glennmen commented Apr 6, 2020

There seems to be an error in one of the travis builds that are not related to the changes 🤔

@Glennmen
Copy link
Contributor Author

@jsumners Anything you want me to change? Just let me know 😄

@jsumners
Copy link
Owner

No. I just need to get around to dealing with it. I want to add in some GitHub Actions (maybe).

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.

skin tone
2 participants