Following the lead of rupl and his work on the Modernizr module and Modernizr integration (see #1914478: [meta] Provide Modernizr integration for other contrib modules and #1735496: Geolocation Field: Integration with Modernizr 7.x-3.x), this is a request for @font-your-face integration with the Modernizr module.

As I understand it, the goal is to automate creation of custom Modernizr builds using hook_modernizr_info() which ensures that if @font-your-face is enabled, the Modernizr module knows it needs to include the @font-face feature test when fetching a custom build.

Comments

iwuv’s picture

Status: Active » Needs review
StatusFileSize
new590 bytes

Patch!

rupl’s picture

Status: Needs review » Needs work

Hey, this is awesome! This patch is perfecto as far as matching the Modernizr 7.x-3.0 API. However, I've made some changes and 3.1 will be much easier and more stable for the future.

Could we make the following changes?

  • Cut out the type and desc lines. I put all that stuff into a master file within the Modernizr module.
  • Change module to source (themes can supply tests so I made the term more generic)

Here is an example from #1935794: Move support for Modernizr into HTML5 submodule


/**
 * Implements hook_modernizr_info().
 */
function geolocation_html5_modernizr_info() {
  $tests = array();

  $tests['geolocation'] = array(
    'source' => 'Geolocation HTML5',
  );

  return $tests;
}


iwuv’s picture

StatusFileSize
new501 bytes

Thanks for the tip, @rupl! Here's a new patch to use instead of the first one. It works with the current Modernizr 7.x-3.x dev.

iwuv’s picture

Status: Needs work » Needs review

Changing issue status to needs review.

sreynen’s picture

This looks good to me, but I'll wait and see if rupl has any more feedback.

If it's not too late to give feedback on hook_modernizr_info(), it seems odd to me that it's asking for the module name from a function that has the module machine name (which could be used to lookup the human name) as a prefix.

rupl’s picture

Status: Needs review » Postponed

Hey Scott, you're totally right about the redundancy and it has been pointed out to me by two others. I'm using associative arrays for metadata because there's other stuff I plan to add in the future (caniuse slugs, Modernizr docs links) and those do not correspond to any Drupal machine data.

Maybe just hold tight and I'll work out that kink and we can commit this afterwards. FWIW, this patch follows the exact convention in the Modernizr API docs, and when we switch to the more intelligent system of gathering module names later it will not break existing implementations.

rupl’s picture

Status: Postponed » Active

Latest dev of Modernizr 7.x-3.x now auto-detects which module/theme is asking for tests. So the syntax can be as simple as:

/**
 * Implements hook_modernizr_info().
 */
function fontyourface_modernizr_info() {
  $tests = array();
  $tests[] = 'fontface';

  return $tests;
}

Thanks for the feedback, @sreynen

sreynen’s picture

Status: Active » Fixed

This is committed now.

rupl’s picture

Thanks dude!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.