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

Issue #1468582 by Schnitzel, lewisnyman, dcmouyard, alanburke, tstoeckler, edward_or, janusman: Add mobile friendly meta tags to the html.tpl.php. #386

Merged
merged 1 commit into from
Aug 22, 2014

Conversation

hellomrcat
Copy link
Contributor

@quicksketch
Copy link
Member

Nice! I'll need to have @jenlampton review this one. It's great to get these in here but we'll have to re-evaluate the approach. D8 eventually went with the equivalent of backdrop_add_html_head() to add these tags, meaning they all ended up as part of the single $head variable.

function system_module_test_preprocess_html(&$variables) {
$variables['default_mobile_metatags'] = FALSE;
}
?>
Copy link
Member

Choose a reason for hiding this comment

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

We should remove this closing PHP tag.

@jenlampton
Copy link
Member

Yeah, I don't like the extra variable added into the template. If we removed that the changes in core/includes/theme.inc and core/modules/system/html.tpl.php would go away.

We'll need to add a change in common.inc, to backdrop_get_html_head, as follows.

/**
 * Retrieves output to be displayed in the HEAD tag of the HTML page.
 */
function backdrop_get_html_head() {
  $elements = backdrop_add_html_head();

  // Always add the default mobile meta tags.
  $elements['common-viewport'] = array(
    '#type' => 'html_tag',
    '#tag' => 'meta',
    '#attributes' => array(
      'name' => 'viewport',
      'content' => 'width=device-width',
    ),
  );
  $elements['common-cleartype'] = array(
    '#type' => 'html_tag',
    '#tag' => 'meta',
    '#attributes' => array(
      'http-equiv' => 'cleartype',
      'content' => 'on',
    ),
  );

  backdrop_alter('html_head', $elements);
  return backdrop_render($elements);
}

@hellomrcat can you take a stab at this one, and see if the same test from this issue still passes?

@jenlampton jenlampton removed their assignment Aug 14, 2014
…kler, edward_or, janusman: Add mobile friendly meta tags to the html.tpl.php.
@quicksketch quicksketch merged commit 99b15c9 into backdrop:1.x Aug 22, 2014
@quicksketch
Copy link
Member

Super! It looks like D8 ultimately removed the cleartype meta tag in https://www.drupal.org/node/1717090, so I dropped that in the merge. Previously we had Seven adding the mobile meta tag, but that's not necessary if core is adding it. I removed that also as part of the merge. Thanks!

@hellomrcat hellomrcat deleted the 200-html branch August 22, 2014 22:06
quicksketch added a commit to quicksketch/backdrop that referenced this pull request Dec 21, 2014
quicksketch added a commit to quicksketch/backdrop that referenced this pull request Dec 22, 2014
quicksketch added a commit that referenced this pull request Dec 27, 2014
Issue #386: Report the update server as 'not implemented' until it starts reporting HTTP 200.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants