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

Please review the Ionicons font-face definition #2599

Closed
oliversalzburg opened this issue Nov 24, 2014 · 10 comments
Closed

Please review the Ionicons font-face definition #2599

oliversalzburg opened this issue Nov 24, 2014 · 10 comments
Milestone

Comments

@oliversalzburg
Copy link

I'm also experiencing the issue reported at ionic-team/ionicons#144, looked into the @font-face declaration and was slightly confused.

We're using Ionicons with our Ionic Framework app, and I feel like the font-face declaration is counter-productive in this use case (which, to my understanding, is the primary use case of the font).

First of all, I agree with @mowcixo, I don't see how the query on the URL has any benefit when being used in an app bundle. And even if the behavior on Windows Phone 8 is a bug in mobile IE 11, if removing the query from the src attribute allows the font to be used on the platform, why not consider changing it? After all, the current definition even includes #iefix.

Second, the src attribute seems unnecessarily verbose. It is my understanding that .eot is included on the web for IE8 compatibility, something that should have no relevance in the Ionic framework.
I guess, the .ttf enables support Android <4.4, so it is probably required, but should be ordered below the .woff IMHO.
I don't see the need for the .svg, as all browsers that support .svg fonts, also support WOFF. To my understanding, this also holds true on the web.

Please keep in mind that all font variants are usually packaged into the app bundle and contribute to the overall size. The .svg and .eot weigh 380 KB which might not be required in the package.

For testing purposes, I slimmed down the font-face to:

@font-face {
  font-family: "Ionicons";
  src: url("../fonts/ionicons.woff") format("woff"), url("../fonts/ionicons.ttf") format("truetype");
  font-weight: normal;
  font-style: normal; }

Which worked great on BlackBerry 10 and Windows Phone 8 (yes, I do realize that I tested only on platforms you don't support).

I'd be interested to hear your thoughts on this.

@mowcixo
Copy link

mowcixo commented Nov 24, 2014

Great point @oliversalzburg, because of that issue I have to write a hook in my Ionic/Cordova apps.

@oliversalzburg
Copy link
Author

I've now also successfully tested this on Android 4.4 (Nexus 4).

@drewrygh drewrygh added the css label Dec 5, 2014
@oliversalzburg
Copy link
Author

I don't understand why this hasn't gotten any response :( Because this is such a pain for us, we're now maintaining https://github.com/hartwig-cordova/ionic-bower.

The fix itself is in https://github.com/hartwig-cordova/ionic-bower/tree/fix/fonts. We publish -wp8 tags with the fix applied. We use https://github.com/hartwig-cordova/ionic-bower/tree/v1.0.0-beta.14-wp8 ourselves.

@mhartington
Copy link
Contributor

Hey there, so ionicons have a double life - as the icons for ionic and an all purpose font-icon for the web. That impacts how we've setup ionicons

  1. version query string: This forces the browser to load the most recent icons and not a cached version when they get updated.
  2. Again, since ionicons can be used for the web, and not just compiled apps, they have to support the full gamut of browsers.

https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face

@mowcixo idea of a hook to remove the unnecessary files to save on file size in interesting. @mowcixo would you be able to open an issue for this on the cli repo?

For now, we are going to keep the fonts the way the are as we're close to 1.0.

@mhartington
Copy link
Contributor

Will revisit post 1.0

@mhartington mhartington added this to the 1.1 milestone Feb 13, 2015
@oliversalzburg
Copy link
Author

@mhartington I understand that Ionicons can also be used standalone, but I don't see why the integration in the Ionic Framework can't be adjusted since there is a separate file for this and the SCSS from Ionicons isn't used in the Framework.

@mowcixo
Copy link

mowcixo commented Feb 16, 2015

Besides, I have not tested in IE for desktop, but I suppose it fails on it
too.... It's not a target for Ionicons to work on IE ?

2015-02-16 14:38 GMT+01:00 Oliver Salzburg [email protected]:

@mhartington https://github.com/mhartington I understand that Ionicons
can also be used standalone, but I don't see why the integration in the
Ionic Framework can't be adjusted since there is a separate file
https://github.com/driftyco/ionic/blob/master/scss/ionicons/_ionicons-font.scss
for this and the SCSS from Ionicons isn't used in the Framework.

Reply to this email directly or view it on GitHub
#2599 (comment).

Moisés Gramary Barbosa
[email protected]
0034 687800099

One bright morning when my work is over, Man will fly away home.

@oliversalzburg
Copy link
Author

@mowcixo Works fine on desktop IE11

@mowcixo
Copy link

mowcixo commented Feb 16, 2015

Yes, you are right, and in 8, 9 and 10 too. I'll see if I can make an
assembled version of my hook and propose a pull request on Ionic CLI.

2015-02-16 14:48 GMT+01:00 Oliver Salzburg [email protected]:

@mowcixo https://github.com/mowcixo Works fine on desktop IE11

Reply to this email directly or view it on GitHub
#2599 (comment).

Moisés Gramary Barbosa
[email protected]
0034 687800099

One bright morning when my work is over, Man will fly away home.

@astik
Copy link

astik commented Jul 25, 2016

Hi all,
i understand that ionicons is an independant project with its own life. But, like @oliversalzburg, I don't undestand why the ionicons integration supports all the not-needed features for mobile targets (EOT, SVG). As of today (ionic 1.3.1), ionicons is still bundled with all fonts flavors.

In ionic.scss, instead of :

@import
  // Ionicons
  "ionicons/ionicons.scss",

we could have :

@import
  // Ionicons
  "ionicons/ionicons-variables";
  "ionicons-font-for-ionic";
  "ionicons/ionicons-icons";

And in ionicons-font-for-ionic, inject the code from @oliversalzburg :

@font-face {
  font-family: $ionicons-font-family;
  src: url("#{$ionicons-font-path}/ionicons.woff") format("woff"),
  url("#{$ionicons-font-path}/ionicons.ttf") format("truetype");
  font-weight: normal;
  font-style: normal;
}

.ion {
  display: inline-block;
  font-family: $ionicons-font-family;
  speak: none;
  font-style: normal;
  font-weight: normal;
  font-variant: normal;
  text-transform: none;
  text-rendering: auto;
  line-height: 1;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
}

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants