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 emoji font-family #9956

Closed
wants to merge 5 commits into from
Closed

Add emoji font-family #9956

wants to merge 5 commits into from

Conversation

810
Copy link
Contributor

@810 810 commented Apr 17, 2016

Now you can see colored icons instead of a black icon

Test instructions:

You need utf8mb4 collation

  1. go to http://getemoji.com/ copy some emoji's

  2. create new/edit article, paste the emoji's

before patch:
emo1

after:
emo2

Now you can see colored icons instead of a black icon
@andrepereiradasilva
Copy link
Contributor

how to test?

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

@andrepereiradasilva instructions added

@andrepereiradasilva
Copy link
Contributor

Using windows 10.

In firefox, ie 11 and edge i see coloful emojis after patch.
In chrome doesn't work.

Also shouldn't this be added to beez3 too?

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

chrome is using the blink engine, so it doesn't work

I can add it later to beez3

@andrepereiradasilva
Copy link
Contributor

ok i see, chrome as not added support for those fonts.

@richard67
Copy link
Member

@810 "I can add it later to beez3": Means we shall wait with testing until you added it to this PR here? Or means you will make another PR for that so we already shall test this one?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

@richard67 updated the beez3 css

@richard67
Copy link
Member

@810 For me there is no difference between before and after patch regarding colours of emojis. Some have colours, some have not.

Only when I remove the fonts "Helvetica Neue", Helvetica, Arial, sans-serif, "Apple Color Emoji" from the body tag's properties e.g. in Firebug, then I see a change for the emojis listed in the last section "Emojis that work in your Twitter Name, Twitter Bio, and Ask.fm questions" at http://getemoji.com/.

I tested on Windows 10 with latest versions of Firefox and Google Chrome.

Just clearing broswer cache does not help.

Am I missing something, e.g. related to cached fonts?

@andrepereiradasilva When you tested, did you verify with pre-patched, and was there a change? And which emojis did you use?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@Fedik
Copy link
Member

Fedik commented Apr 17, 2016

test result a lot depend from OS and browser,
for me it will always fail 😄

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

not all emoji's are standard implanted on all browsers. I added the same font-family as GitHub/facebook/ect

@richard67
Copy link
Member

Well then I better not test because I cannot replicate the issue and any change from this PR.

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

emo3

There is indeed not all icons are translated

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

actually, after testing, what @richard67 said it's true. with or without the PR the emojis stays the same in any browser on windows.

I never noticed the emoji already had color in other browsers besides chrome :D

@richard67
Copy link
Member

A snakeoil PR? (joke)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@andrepereiradasilva
Copy link
Contributor

i think the emoji rendering or not are OS/browser related. and @810 it trying to make a fallback in OS/browsers combinations that don't render them, right?

@810 what is your browser/OS combination?

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

Win10 - insiders build
edge/ie11/ff/chrome (not working)

@richard67
Copy link
Member

I just tested again with Firefor and now also with Microsoft's Edge on same OS (Win 10), with the emojis I mentioned before, the last section "Emojis that work in your Twitter Name, Twitter Bio, and Ask.fm questions" at http://getemoji.com/.

And now I could see a slight difference:

MS Edge before patch:
screen shot 2016-04-17 at 15 43 50

MS Edge after patch:
screen shot 2016-04-17 at 15 43 58

Firefox before patch:
screen shot 2016-04-17 at 15 44 03

Firefox after patch:
screen shot 2016-04-17 at 15 44 10

So this PR at least seems to make things a bit better in certain cases without doing harm in other cases.

And with the Firefox test I either was blind when testing 1st time, or there was stuff cached so it looked the same, don't know now what went wrong.

@810 Would you consider this being a good test?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

you can test it here also on the comment: just paste the emoji ;)

@richard67
Copy link
Member

@810

you can test it here also on the comment: just paste the emoji ;)

How you mean that?

And is my test sufficient and good now, or not? You did not answer that.

@810
Copy link
Contributor Author

810 commented Apr 17, 2016

I thought there will be more icons that have colors, Also I see other issue I will update the pr

@mikeveeckmans
Copy link

I have tested this item ✅ successfully on e5e8efe

Test OK

Mac OSX (10.11.4) , tested on firefox and chrome.

Emoji shown in article title, article content and H2, H3
Tested on Protostar, Beez3 & 3rdparty template


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@richard67
Copy link
Member

@810 In Protostar the font-families you have added are applied. But in Beez3 not. The reason is following code in "templates/beez3/css/layout.css" beginning at line 13:

body#shadow {
font-family: arial,sans-serif;
}

This made me check in which other css we have font-families set.

They are a lot.

Limiting my search to the templates folder (for frontend templates) I still get a lot:

beez3\css\layout.css: font-family: arial,sans-serif
beez3\css\layout.css: font-family: inherit;
beez3\css\nature.css: font-family: arial, helvetica, sans-serif;
beez3\css\print.css: font-family:Arial, Verdana, Helvetica, sans-serif;
beez3\css\template_rtl.css: font-family: 'Titillium Maps', Arial;
protostar\css\template.css: font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
protostar\css\template.css: font-family: inherit;
protostar\css\template.css: font-family: Monaco, Menlo, Consolas, "Courier New", monospace;
protostar\css\template.css: font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
protostar\css\template.css: font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
protostar\css\template.css: font-family: 'IcoMoon';
protostar\css\template.css: font-family: 'IcoMoon';
protostar\css\template.css: font-family: 'IcoMoon';
system\css\editor.css: font-family: Tahoma,Helvetica,Arial,sans-serif;
system\css\editor.css: font-family:Helvetica ,Arial,sans-serif;
system\css\editor.css: font-family: Arial, Helvetica,sans-serif;
system\css\editor.css: font-family: Helvetica,Arial,sans-serif;
system\css\editor.css: font-family: Arial, Helvetica, sans-serif;
system\css\error.css: font-family: helvetica, arial, sans-serif;
system\css\error.css: font-family: helvetica, arial, sans-serif;
system\css\error.css: font-family: helvetica, arial, sans-serif;
system\css\offline.css: font-family: Arial, Helvetica, Sans Serif; font-size: 14px;
system\css\offline.css: font-family: inherit;

Not to mention all those in administrator/templates and in the media folder (and subfolders).

Of course not all of these need to be updated by your change, but I think at least things like "body#shadow" or other "body#...", like the one I mentioned at the top.

Am not sure if @mikeveeckmans tested the fonts used, or if he just checked if the emojis show up (like his test result reads), which would be not a valid test in my opinion then because objective of this PR is another than showing emojis which do not show up before.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@810
Copy link
Contributor Author

810 commented Apr 19, 2016

@richard67 On css files you need just 1 time to load the new font. Then its included on every page.

That's why I just added It once.

@MDXBilal12
Copy link

I have tested this item ✅ successfully on e5e8efe


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@brianteeman
Copy link
Contributor

I'm confused - is this good to merge or not?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@Fedik
Copy link
Member

Fedik commented May 2, 2016

The Patch is good only for OS X users, and maybe some Win10 users

@universewrld
Copy link

recently find this official website with all Emoji symbols- http://www.unicode.org/emoji/charts/full-emoji-list.html

@gunjanpatel
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 24, 2016
@brianteeman
Copy link
Contributor

Based on the comment from @Fedik above I am removing the RTC status


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@zero-24
Copy link
Contributor

zero-24 commented Jun 24, 2016

@joomla-cms-bot please do the job that @brianteeman prepared for you 😄

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 24, 2016
@brianteeman
Copy link
Contributor

It also has merge conflicts that need to be resolved

@gunjanpatel
Copy link
Contributor

It needs to be in Need Review status then? not for resolving conflicts but based on previous comments. 😄


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@brianteeman
Copy link
Contributor

The Patch is good only for OS X users, and maybe some Win10 users

I am closing this PR. We would need to ensure it worked on all supported platforms and browsers before it could be accepted.

Thanks


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9956.

@brianteeman brianteeman closed this Sep 6, 2016
@810
Copy link
Contributor Author

810 commented Sep 6, 2016

All browser are ok for me now, with the latest builds
Edge, IE11, chrome,ff

@brianteeman
Copy link
Contributor

IE 8 & 9 ?

On 6 Sep 2016 3:09 p.m., "Jelle Kok" notifications@github.com wrote:

All browser are ok for me now, with the latest builds
Edge, IE11, chrome,ff


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#9956 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8YBAZ9STZ0IoLHmmVi7QtIJT72hQks5qnXQPgaJpZM4IJMPs
.

@810
Copy link
Contributor Author

810 commented Sep 6, 2016

@brianteeman
Copy link
Contributor

So it's still a problem for chrome?

@810
Copy link
Contributor Author

810 commented Sep 6, 2016

now its fixed:

wdw

@brianteeman
Copy link
Contributor

Reop

@brianteeman brianteeman reopened this Sep 6, 2016
@brianteeman
Copy link
Contributor

Reopened for further testing

@810
Copy link
Contributor Author

810 commented Oct 21, 2016

any update please

@ghost
Copy link

ghost commented Jan 6, 2017

should this PR be tested?

@810
Copy link
Contributor Author

810 commented Jan 6, 2017

yes please,

but we are waiting for approval of this pr

@ghost
Copy link

ghost commented Jan 6, 2017

I have tested this item 🔴 unsuccessfully on 1a4146b

Switch from utf8_general to utf8mb4-general, copied "Pale Emojis" from http://getemoji.com/ in Article:

@810
Copy link
Contributor Author

810 commented Jan 28, 2017

Latest news:

Microsoft Edge is the first browser that have now fully color support as default, no need to add fonts.
(Feature since build 15019.1000)

All other browsers still need the fonts.

PS windows server isn't supported at all.

@johnnydement
Copy link

I would suggest to go with emojione.com integration, there're free, CC, Open source standard and continuosly in development, they have several ways to integrate, would be great to have a homogeneous set, ascii ones don't always look too good... nor have skin tones (last time I checked them)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/9956.

@brianteeman
Copy link
Contributor

It has been a long time since this was proposed. It's not really going anywhere fast due to browser support issues and it's hardly a required feature.

@brianteeman brianteeman closed this Sep 9, 2017
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.

None yet