From 9451f8a8eb6978e6662760480bf44213f79458c1 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 27 Apr 2015 16:04:40 +0100 Subject: [PATCH] Issue #2123251 by Jelle_S, marcvangend, attiks, Sutharsan: Improve DX of responsive images; convert theme functions to new #type element --- .../responsive_image/responsive_image.module | 83 +++++++++---------- .../src/Element/ResponsiveImage.php | 30 +++++++ .../ResponsiveImageFormatter.php | 15 ++-- .../Tests/ResponsiveImageFieldDisplayTest.php | 19 ++--- .../responsive-image-formatter.html.twig | 19 +++++ .../templates/responsive-image.html.twig | 2 +- 6 files changed, 103 insertions(+), 65 deletions(-) create mode 100644 core/modules/responsive_image/src/Element/ResponsiveImage.php create mode 100644 core/modules/responsive_image/templates/responsive-image-formatter.html.twig diff --git a/core/modules/responsive_image/responsive_image.module b/core/modules/responsive_image/responsive_image.module index ad39ad802663..7637230001c4 100644 --- a/core/modules/responsive_image/responsive_image.module +++ b/core/modules/responsive_image/responsive_image.module @@ -74,11 +74,8 @@ function responsive_image_theme() { return array( 'responsive_image' => array( 'variables' => array( + 'link_path' => NULL, 'uri' => NULL, - 'width' => NULL, - 'height' => NULL, - 'alt' => '', - 'title' => NULL, 'attributes' => array(), 'responsive_image_style_id' => array(), ), @@ -86,58 +83,51 @@ function responsive_image_theme() { 'responsive_image_formatter' => array( 'variables' => array( 'item' => NULL, + 'item_attributes' => NULL, 'url' => NULL, - 'responsive_image_style_id' => array(), + 'responsive_image_style_id' => NULL, ), - 'function' => 'theme_responsive_image_formatter', ), ); } /** - * Returns HTML for a responsive image field formatter. + * Prepares variables for responsive image formatter templates. + * + * Default template: responsive-image-formatter.html.twig. * * @param array $variables * An associative array containing: * - item: An ImageItem object. - * - responsive_image_style_id: The ID of the responsive image style. + * - item_attributes: An optional associative array of HTML attributes to be + * placed in the img tag. + * - responsive_image_style_id: A responsive image style. * - url: An optional \Drupal\Core\Url object. - * - * @ingroup themeable */ -function theme_responsive_image_formatter($variables) { - $item = $variables['item']; - if (!isset($variables['responsive_image_style_id']) || empty($variables['responsive_image_style_id'])) { - $image_formatter = array( - '#theme' => 'image_formatter', - '#item' => $item, - '#url' => $variables['url'], - ); - return drupal_render($image_formatter); - } - - $responsive_image = array( - '#theme' => 'responsive_image', - '#width' => $item->width, - '#height' => $item->height, +function template_preprocess_responsive_image_formatter(&$variables) { + $variables['responsive_image'] = array( + '#type' => 'responsive_image', '#responsive_image_style_id' => $variables['responsive_image_style_id'], ); - if (isset($item->uri)) { - $responsive_image['#uri'] = $item->uri; - } - elseif ($entity = $item->entity) { - $responsive_image['#uri'] = $entity->getFileUri(); - $responsive_image['#entity'] = $entity; - } - $responsive_image['#alt'] = $item->alt; + $item = $variables['item']; + $attributes = array(); + // Do not output an empty 'title' attribute. if (Unicode::strlen($item->title) != 0) { - $responsive_image['#title'] = $item->title; + $attributes['title'] = $item->title; } - if (isset($variables['url'])) { - return \Drupal::l($responsive_image, $variables['url']); + $attributes['alt'] = $item->alt; + $attributes += $variables['item_attributes']; + if (($entity = $item->entity) && empty($item->uri)) { + $variables['responsive_image']['#uri'] = $entity->getFileUri(); + } + else { + $variables['responsive_image']['#uri'] = $item->uri; } - return drupal_render($responsive_image); + foreach (array('width', 'height') as $key) { + $variables['responsive_image']["#$key"] = $item->$key; + } + $variables['responsive_image']['#attributes'] = $attributes; } /** @@ -147,13 +137,9 @@ function theme_responsive_image_formatter($variables) { * * @param $variables * An associative array containing: - * - uri: Either the path of the image file (relative to base_path()) or a - * full URL. + * - uri: The URI of the image. * - width: The width of the image (if known). * - height: The height of the image (if known). - * - alt: The alternative text for text-based browsers. - * - title: The title text is displayed when the image is hovered in some - * popular browsers. * - attributes: Associative array of attributes to be placed in the img tag. * - responsive_image_style_id: The ID of the responsive image style. */ @@ -191,11 +177,16 @@ function template_preprocess_responsive_image(&$variables) { ), ), ); - foreach (array('alt', 'title', 'attributes') as $key) { - if (isset($variables[$key])) { - $variables['img_element']["#$key"] = $variables[$key]; - unset($variables[$key]); + if (isset($variables['attributes'])) { + if (isset($variables['attributes']['alt'])) { + $variables['img_element']['#alt'] = $variables['attributes']['alt']; + unset($variables['attributes']['alt']); + } + if (isset($variables['attributes']['title'])) { + $variables['img_element']['#title'] = $variables['attributes']['title']; + unset($variables['attributes']['title']); } + $variables['img_element']['#attributes'] = $variables['attributes']; } } diff --git a/core/modules/responsive_image/src/Element/ResponsiveImage.php b/core/modules/responsive_image/src/Element/ResponsiveImage.php new file mode 100644 index 000000000000..9da04f93ef17 --- /dev/null +++ b/core/modules/responsive_image/src/Element/ResponsiveImage.php @@ -0,0 +1,30 @@ +<?php +/** + * @file + * Contains \Drupal\responsive_image\Element\ResponsiveImage. + */ + +namespace Drupal\responsive_image\Element; + +use Drupal\Core\Render\Element\RenderElement; + +/** + * Provides a responsive image element. + * + * @RenderElement("responsive_image") + */ +class ResponsiveImage extends RenderElement { + + /** + * {@inheritdoc} + */ + public function getInfo() { + return [ + '#theme' => 'responsive_image', + '#attached' => [ + 'library' => ['core/picturefill'], + ], + ]; + } + +} diff --git a/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php index 3db169e60e73..64d0b7e682ba 100644 --- a/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php @@ -204,14 +204,16 @@ public function viewElements(FieldItemListInterface $items) { if (isset($link_file)) { $url = Url::fromUri(file_create_url($file->getFileUri())); } + // Extract field item attributes for the theme function, and unset them + // from the $item so that the field template does not re-render them. + $item = $file->_referringItem; + $item_attributes = $item->_attributes; + unset($item->_attributes); + $elements[$delta] = array( '#theme' => 'responsive_image_formatter', - '#attached' => array( - 'library' => array( - 'core/picturefill', - ), - ), - '#item' => $file->_referringItem, + '#item' => $item, + '#item_attributes' => $item_attributes, '#responsive_image_style_id' => $responsive_image_style ? $responsive_image_style->id() : '', '#url' => $url, '#cache' => array( @@ -219,7 +221,6 @@ public function viewElements(FieldItemListInterface $items) { ), ); } - return $elements; } } diff --git a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php index 2a84bc04a902..ccc09be1b10a 100644 --- a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php +++ b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php @@ -193,21 +193,15 @@ protected function doTestResponsiveImageFieldFormatters($scheme, $empty_styles = $display_options = array( 'type' => 'responsive_image', 'settings' => array( - 'image_link' => 'file' + 'image_link' => 'file', + 'responsive_image_style' => 'style_one', ), ); $display = entity_get_display('node', 'article', 'default'); $display->setComponent($field_name, $display_options) ->save(); - $image = array( - '#theme' => 'image', - '#uri' => $image_uri, - '#width' => 360, - '#height' => 240, - '#alt' => $alt, - ); - $default_output = '<a href="' . file_create_url($image_uri) . '">' . drupal_render($image) . '</a>'; + $default_output = '<a href="' . file_create_url($image_uri) . '"><picture'; $this->drupalGet('node/' . $nid); $cache_tags_header = $this->drupalGetHeader('X-Drupal-Cache-Tags'); $this->assertTrue(!preg_match('/ image_style\:/', $cache_tags_header), 'No image style cache tag found.'); @@ -285,7 +279,10 @@ protected function doTestResponsiveImageFieldFormatters($scheme, $empty_styles = ), ), ); - $default_output = drupal_render($fallback_image); + // The image.html.twig template has a newline after the <img> tag but + // responsive-image.html.twig doesn't have one after the fallback image, so + // we remove it here. + $default_output = trim(drupal_render($fallback_image)); $this->assertRaw($default_output, 'Image style large formatter displaying correctly on full node view.'); if ($scheme == 'private') { @@ -354,7 +351,7 @@ public function testResponsiveImageFieldFormattersEmptyMediaQuery() { $this->drupalGet('node/' . $nid); // Assert an empty media attribute is not output. - $this->assertNoPattern('@srcset="data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw== 1x".+?media="@'); + $this->assertNoPattern('@srcset="data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw== 1x".+?media=".+?/><source@'); // Assert the media attribute is present if it has a value. $thumbnail_style = ImageStyle::load('thumbnail'); diff --git a/core/modules/responsive_image/templates/responsive-image-formatter.html.twig b/core/modules/responsive_image/templates/responsive-image-formatter.html.twig new file mode 100644 index 000000000000..93caba88be4a --- /dev/null +++ b/core/modules/responsive_image/templates/responsive-image-formatter.html.twig @@ -0,0 +1,19 @@ +{# +/** + * @file + * Default theme implementation to display a formatted responsive image field. + * + * Available variables: + * - responsive_image: A collection of responsive image data. + * - url: An optional URL the image can be linked to. + * + * @see template_preprocess_responsive_image_formatter() + * + * @ingroup themeable + */ +#} +{% if url %} + <a href="{{ url }}">{{ responsive_image }}</a> +{% else %} + {{ responsive_image }} +{% endif %} diff --git a/core/modules/responsive_image/templates/responsive-image.html.twig b/core/modules/responsive_image/templates/responsive-image.html.twig index 9a8904351e85..a3a51a666b11 100644 --- a/core/modules/responsive_image/templates/responsive-image.html.twig +++ b/core/modules/responsive_image/templates/responsive-image.html.twig @@ -5,7 +5,7 @@ * * Available variables: * - sources: The attributes of the <source> tags for this <picture> tag. - * - fallback_image: The fallback <img> tag to use for this <picture> tag. + * - img_element: The controlling image, with the fallback image in srcset. * * @see template_preprocess() * @see template_preprocess_responsive_image() -- GitLab