Skip to content
This repository

Media query support in custom font loader #63

Closed
wants to merge 1 commit into from

5 participants

Mark Perkins Sean McBride David Demaree Alex Bell Bram Stein
Mark Perkins

I've had a few responsive projects recently where I've wanted to only load webfonts in for larger screened devices, to avoid the extra size and http requests for mobile devices.

This small change makes it possible to specify a media attribute to be added to the <link> element that gets injected into the head (for the custom loader option), by adding a 'media' item, as follows:

WebFontConfig = {
  custom: { families: ['OneFont', 'AnotherFont'],
    urls: [ 'http://myotherwebfontprovider.com/stylesheet1.css',
      'http://yetanotherwebfontprovider.com/stylesheet2.css' ],
   media: 'screen and (min-width: 480px)' }
};
Sean McBride seanami commented on the diff October 16, 2012
src/core/domhelper.js
((8 lines not shown))
104 104
     'rel': 'stylesheet',
105 105
     'href': src
106  
-  });
  106
+  };
  107
+  if ( typeof media !== 'undefined' ) {
  108
+    opts.media = media;
1
Sean McBride Collaborator
seanami added a note October 16, 2012

Let's use opts['media'] format instead of opts.media to make it explicit that the compiler shouldn't try to rename it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sean McBride seanami commented on the diff October 16, 2012
src/core/domhelper.js
@@ -99,13 +99,18 @@ webfont.DomHelper.prototype.removeElement = function(node) {
99 99
  * @param {string} src The URL of the stylesheet.
100 100
  * @return {Element} a link element.
1
Sean McBride Collaborator
seanami added a note October 16, 2012

The new media parameter needs to be documented as an optional param. In addition, Google Closure Compiler conventions call for naming optional arguments like opt_media to clarify their use. Check out this page for info on how to properly annotate params: https://developers.google.com/closure/compiler/docs/js-for-compiler

I think the right way to annotate this one would be:

@param {string=} opt_media The media attribute for the stylesheet (optional).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sean McBride seanami commented on the diff October 16, 2012
src/custom/customcss.js
@@ -22,7 +22,7 @@ webfont.CustomCss.prototype.load = function(onReady) {
22 22
   for (var i = 0, len = urls.length; i < len; i++) {
23 23
     var url = urls[i];
24 24
 
25  
-    this.domHelper_.insertInto('head', this.domHelper_.createCssLink(url));
  25
+    this.domHelper_.insertInto('head', this.domHelper_.createCssLink(url, this.configuration_['media']));
1
Sean McBride Collaborator
seanami added a note October 16, 2012

Can we add to the comment at the top of this file to give an example of this option in use?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sean McBride
Collaborator

Hey @allmarkedup, thanks for the contribution! This looks like a good feature to add, and I just had a few style comments that I've made on the pull request.

In addition, these changes need some accompanying tests (for both domhelper.js and customcss.js). Can you add some?

Finally, I wondered if you considered what will happen in the case where the fonts aren't loaded due to media restrictions. The library will still watch for the fonts to render on the page, but after the timeout (5 seconds) without them rendering, they'll be marked as inactive. Barring a way to interpret the media rule in JS and immediately tell whether fonts will be active or not, I think this is probably a reasonable behavior. Just wanted to make sure you'd thought it through.

David Demaree
Owner

@allmarkedup @seanami Most current browsers implement the window.matchMedia API that can be used to tell programmatically whether a media query will return true or not. IE 10 supports it but 9 and older versions do not, and older iOS/Android phones may also lack support, so there may need to be some fallback strategy for those devices. But generally I think this is a great addition, if we can make it matchMedia aware that could be great for people working on responsive design projects.

Alex Bell

What happens if the author writes:

media: 'screen and (min-width: 800px)' }

and the user loads the page with his 640x960px phone in portrait? The fonts aren't requested. Fine. But then suppose that, after waiting five seconds, the user rotates his phone into landscape. Now the media query condition is met, and so the fonts in the link element with the media query will be downloaded. But, because the user waited five seconds, the callbacks to the fontinactive events have already fired! Chaos ensues. It's conceivable that you could actually iron out all the edge cases around scenarios like this one, but is it really a good use of library development time?

Responsive is hard. There are serious inconsistencies in the mobile space surrounding resize and orientationchange events, and also around the new viewport units of vmin and vmax. Do you really want to get into this? matchMedia is great, but the polyfills have problems with resize, and they will bloat the library by another couple of k, precious space on mobile.

@allmarkedup I really like the idea of reducing http requests for mobile. This is a smart pattern! But this can be rudimentarily accomplished by querying the screen or viewport on document ready and loading the library, the custom code, and the fonts all inside a simple conditional. If the author wants to listen for resize he can do that too with one line of code. Then the mobile user doesn't even have to load the library, which is the lightest solution of all. In other words: why bother downloading the library and writing the link into the head if the condition it describes doesn't apply?

Bram Stein bramstein closed this June 03, 2013
Bram Stein
Collaborator

I'm closing this pull request because there hasn't been much activity, and @baloneysandwiches points out some very good cases against merging this into core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Sep 14, 2012
Mark Perkins Add media query option for the custom loader 9710769
This page is out of date. Refresh to see the latest.
11  src/core/domhelper.js
@@ -99,13 +99,18 @@ webfont.DomHelper.prototype.removeElement = function(node) {
99 99
  * @param {string} src The URL of the stylesheet.
100 100
  * @return {Element} a link element.
101 101
  */
102  
-webfont.DomHelper.prototype.createCssLink = function(src) {
103  
-  return this.createElement('link', {
  102
+webfont.DomHelper.prototype.createCssLink = function(src, media) {
  103
+  var opts = {
104 104
     'rel': 'stylesheet',
105 105
     'href': src
106  
-  });
  106
+  };
  107
+  if ( typeof media !== 'undefined' ) {
  108
+    opts.media = media;
  109
+  }
  110
+  return this.createElement('link', opts);
107 111
 };
108 112
 
  113
+
109 114
 /**
110 115
  * Creates a link to a javascript document.
111 116
  * @param {string} src The URL of the script.
2  src/custom/customcss.js
@@ -22,7 +22,7 @@ webfont.CustomCss.prototype.load = function(onReady) {
22 22
   for (var i = 0, len = urls.length; i < len; i++) {
23 23
     var url = urls[i];
24 24
 
25  
-    this.domHelper_.insertInto('head', this.domHelper_.createCssLink(url));
  25
+    this.domHelper_.insertInto('head', this.domHelper_.createCssLink(url, this.configuration_['media']));
26 26
   }
27 27
   onReady(families);
28 28
 };
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.