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

Add twimoji #105

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add twimoji #105

wants to merge 2 commits into from

Conversation

run1t
Copy link
Member

@run1t run1t commented Apr 23, 2016

Closes #23.

@SaturnusDJ
Copy link
Member

https://www.npmjs.com/package/emoji or https://www.npmjs.com/package/node-emoji no good?
Agree with @mrmayfield (#23 (comment)) that Twemoji doesn't look so good.

@stuwil stuwil self-assigned this Apr 23, 2016
@stuwil
Copy link
Contributor

stuwil commented Apr 23, 2016

Gonna merge this one in for now, as it's probably better than no emojis. Feel free to open a PR to swap the library.

return localStorage[cookieName];
};
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be here?

@SaturnusDJ
Copy link
Member

Not yet. That's in fact the code @run1t wrote to help me to create the profile menu entry in the new layout 3 branch.

@stuwil
Copy link
Contributor

stuwil commented Apr 23, 2016

Ok, cool. @run1t, can you remove that block from tinder-desktop.common.js then? Also, can we add the emoji filter to the Profile / Swipe bios?

@run1t
Copy link
Member Author

run1t commented Apr 23, 2016

Yep will do that

<link rel="stylesheet" href="css/normalize.min.css"/>
<link href='https://fonts.googleapis.com/css?family=Raleway' rel='stylesheet' type='text/css'>
<link href='https://fonts.googleapis.com/css?family=Merriweather' rel='stylesheet' type='text/css'>
<link rel="stylesheet" href="css/font-awesome.min.css">
<link rel="stylesheet" href="css/app.css"/>
<link rel="stylesheet" href="css/sweet-alert.css"/>


<script src="https://twemoji.maxcdn.com/2/twemoji.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this via bower?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

twemoji it's really heavy (almost 300mb) that's why I use a CDN here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holy guacamole. The source, or the dist?!?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bower install twemojiso I guess the dist

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just the source for dev. Alright, let's leave it as a CDN resource for now...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also privacy implications, maybe we want to ship it locally and avoid outside dependencies?

@stuwil stuwil added this to the 0.1.0 milestone Apr 24, 2016
@stuwil
Copy link
Contributor

stuwil commented Apr 24, 2016

capture d ecran 2016-04-24 a 11 05 59

@stuwil stuwil removed their assignment Apr 25, 2016
@mayeaux mayeaux self-assigned this Apr 25, 2016
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.

5 participants