diff --git a/core/modules/responsive_image/config/schema/responsive_image.schema.yml b/core/modules/responsive_image/config/schema/responsive_image.schema.yml index 430dc47..c6a33c6 100644 --- a/core/modules/responsive_image/config/schema/responsive_image.schema.yml +++ b/core/modules/responsive_image/config/schema/responsive_image.schema.yml @@ -10,7 +10,7 @@ responsive_image.mappings.*: label: type: label label: 'Label' - mappingDefinitions: + mapping_definitions: type: sequence label: 'Mapping definitions' sequence: @@ -31,7 +31,7 @@ responsive_image.mappings.*: multiplier: type: string label: 'Multiplier' - breakpointGroup: + breakpoint_group: type: string label: 'Breakpoint group' diff --git a/core/modules/responsive_image/responsive_image.module b/core/modules/responsive_image/responsive_image.module index ee10739..2a0faeb 100644 --- a/core/modules/responsive_image/responsive_image.module +++ b/core/modules/responsive_image/responsive_image.module @@ -13,6 +13,7 @@ use Drupal\Core\Url; use Drupal\responsive_image\Entity\ResponsiveImageMapping; use Drupal\Core\Image\ImageInterface; +use Drupal\Component\Utility\String; /** * The machine name for the empty image breakpoint image style option. @@ -207,16 +208,50 @@ function template_preprocess_responsive_image(&$variables) { * tag. In other words, this function provides the attributes for each * tag in a tag. * - * In a responsive image mapping, each breakpoint has a mapping defined for each - * of its multipliers. A mapping can be either of two types: 'sizes' (meaning - * it will output a tag with the 'sizes' attribute) or 'image_style' - * (meaning it will output a tag based on the selected image style for - * this breakpoint and multiplier). When all the images in the 'srcset' - * attribute of a tag have the same MIME type, the source tag will get - * a 'mime-type' attribute as well. This way we can gain some front-end - * performance because browsers can select which image ( tag) to load - * based on the MIME types they support (which, for instance, can be beneficial - * for browsers supporting WebP). + * In a responsive image mapping, each breakpoint has a mapping definition for + * each of its multipliers. A mapping definition can be either of two types: + * 'sizes' (meaning it will output a tag with the 'sizes' attribute) or + * 'image_style' (meaning it will output a tag based on the selected + * image style for this breakpoint and multiplier). A responsive image mapping + * can contain mapping definitions of mixed types (both 'image_style' and + * 'sizes'). For example: + * @code + * $responsiveImgMapping = entity_create('responsive_image_mapping', array( + * 'id' => 'mapping_one', + * 'label' => 'Mapping One', + * 'breakpoint_group' => 'responsive_image_test_module', + * )); + * ->addMappingDefinition('responsive_image_test_module.mobile', '1x', array( + * 'image_mapping_type' => 'image_style', + * 'image_mapping' => 'thumbnail', + * )) + * ->addMappingDefinition('responsive_image_test_module.narrow', '1x', array( + * 'image_mapping_type' => 'sizes', + * 'image_mapping' => array( + * 'sizes' => '(min-width: 700px) 700px, 100vw', + * 'sizes_image_styles' => array( + * 'large' => 'large', + * 'medium' => 'medium', + * ), + * ), + * )) + * ->save(); + * @endcode + * The above responsive image mapping will result in a tag similar to + * this: + * @code + * + * + * + * + * + * @endcode + * + * When all the images in the 'srcset' attribute of a tag have the same + * MIME type, the source tag will get a 'mime-type' attribute as well. This way + * we can gain some front-end performance because browsers can select which + * image ( tag) to load based on the MIME types they support (which, for + * instance, can be beneficial for browsers supporting WebP). * For example: * A tag can contain multiple images: * @code @@ -331,12 +366,10 @@ function responsive_image_build_source_attributes(ImageInterface $image, $variab // it. Because of this, the image styles for all multipliers of // this breakpoint should be merged into one srcset and the sizes // attribute should be merged as well. - if (!is_null($dimensions['width'])) { - $srcset[floatval($dimensions['width'])] = file_create_url(_responsive_image_image_style_url($image_style_name, $image->getSource())) . ' ' . $dimensions['width'] . 'w'; - } - else { - \Drupal::logger('responsive_image')->warning('Could not determine image width for @file using image style with ID: @image_style_name', array('@file' => $image->getSource(), '@image_style_name' => $image_style_name)); + if (is_null($dimensions['width'])) { + throw new \LogicException(String::format('Could not determine image width for @file using image style with ID: @image_style_name. This image style can not be used for a responsive image mapping definition using the \'sizes\' attribute.', array('@file' => $image->getSource(), '@image_style_name' => $image_style_name))); } + $srcset[floatval($dimensions['width'])] = file_create_url(_responsive_image_image_style_url($image_style_name, $image->getSource())) . ' ' . $dimensions['width'] . 'w'; $sizes = array_merge(explode(',', $mapping_definition['image_mapping']['sizes']), $sizes); } break; diff --git a/core/modules/responsive_image/src/Entity/ResponsiveImageMapping.php b/core/modules/responsive_image/src/Entity/ResponsiveImageMapping.php index 9a60f23..beca8d2 100644 --- a/core/modules/responsive_image/src/Entity/ResponsiveImageMapping.php +++ b/core/modules/responsive_image/src/Entity/ResponsiveImageMapping.php @@ -73,7 +73,7 @@ class ResponsiveImageMapping extends ConfigEntityBase implements ResponsiveImage * * @var array */ - protected $mappingDefinitions = array(); + protected $mapping_definitions = array(); /** * @var array @@ -85,7 +85,7 @@ class ResponsiveImageMapping extends ConfigEntityBase implements ResponsiveImage * * @var string */ - protected $breakpointGroup = ''; + protected $breakpoint_group = ''; /** * {@inheritdoc} @@ -98,12 +98,16 @@ public function __construct(array $values, $entity_type_id = 'responsive_image_m * {@inheritdoc} */ public function addMappingDefinition($breakpoint_id, $multiplier, array $mapping_definition) { + // Default 'image_mapping' to an empty string because the default for + // 'image_mapping_type' is 'image_style'. This means 'image_mapping' should + // be a string (the image style ID). If 'image_mapping_type' is 'sizes', the + // value for 'image_mapping' should be an array. $defaults = array( 'image_mapping_type' => 'image_style', 'image_mapping' => '', ); // If there is an existing mapping, overwrite it. - foreach ($this->mappingDefinitions as &$mapping) { + foreach ($this->mapping_definitions as &$mapping) { if ($mapping['breakpoint_id'] === $breakpoint_id && $mapping['multiplier'] === $multiplier) { $mapping = array( 'breakpoint_id' => $breakpoint_id, @@ -112,7 +116,7 @@ public function addMappingDefinition($breakpoint_id, $multiplier, array $mapping return $this; } } - $this->mappingDefinitions[] = array( + $this->mapping_definitions[] = array( 'breakpoint_id' => $breakpoint_id, 'multiplier' => $multiplier, ) + $mapping_definition + $defaults; @@ -134,12 +138,8 @@ public function hasMappingDefinitions() { public function getKeyedMappingDefinitions() { if (!$this->keyedMappingDefinitions) { $this->keyedMappingDefinitions = array(); - foreach($this->mappingDefinitions as $mapping) { + foreach($this->mapping_definitions as $mapping) { if (!$this->isEmptyMappingDefinition($mapping)) { - // Only return the selected image styles. - if ($mapping['image_mapping_type'] == 'sizes') { - $mapping['image_mapping']['sizes_image_styles'] = array_filter($mapping['image_mapping']['sizes_image_styles']); - } $this->keyedMappingDefinitions[$mapping['breakpoint_id']][$mapping['multiplier']] = $mapping; } } @@ -151,7 +151,7 @@ public function getKeyedMappingDefinitions() { * {@inheritdoc} */ public function getMappingDefinitions() { - return $this->get('mappingDefinitions'); + return $this->get('mapping_definitions'); } /** @@ -160,10 +160,10 @@ public function getMappingDefinitions() { public function setBreakpointGroup($breakpoint_group) { // If the breakpoint group is changed then the mapping definitions are // invalid. - if ($breakpoint_group !== $this->breakpointGroup) { + if ($breakpoint_group !== $this->breakpoint_group) { $this->removeMappingDefinitions(); } - $this->set('breakpointGroup', $breakpoint_group); + $this->set('breakpoint_group', $breakpoint_group); return $this; } @@ -171,14 +171,14 @@ public function setBreakpointGroup($breakpoint_group) { * {@inheritdoc} */ public function getBreakpointGroup() { - return $this->get('breakpointGroup'); + return $this->get('breakpoint_group'); } /** * {@inheritdoc} */ public function removeMappingDefinitions() { - $this->mappingDefinitions = array(); + $this->mapping_definitions = array(); $this->keyedMappingDefinitions = NULL; return $this; } @@ -188,7 +188,7 @@ public function removeMappingDefinitions() { */ public function calculateDependencies() { parent::calculateDependencies(); - $providers = \Drupal::service('breakpoint.manager')->getGroupProviders($this->breakpointGroup); + $providers = \Drupal::service('breakpoint.manager')->getGroupProviders($this->breakpoint_group); foreach ($providers as $provider => $type) { $this->addDependency($type, $provider); } @@ -204,7 +204,7 @@ public static function isEmptyMappingDefinition(array $mapping_definition) { case 'sizes': // The mapping definition must have a sizes attribute defined and one // or more image styles selected. - if ($mapping_definition['image_mapping']['sizes'] && array_filter($mapping_definition['image_mapping']['sizes_image_styles'])) { + if ($mapping_definition['image_mapping']['sizes'] && $mapping_definition['image_mapping']['sizes_image_styles']) { return FALSE; } break; diff --git a/core/modules/responsive_image/src/ResponsiveImageMappingForm.php b/core/modules/responsive_image/src/ResponsiveImageMappingForm.php index 2973413..cdb5474 100644 --- a/core/modules/responsive_image/src/ResponsiveImageMappingForm.php +++ b/core/modules/responsive_image/src/ResponsiveImageMappingForm.php @@ -89,7 +89,7 @@ public function form(array $form, FormStateInterface $form_state) { else { $description = $this->t('Select a breakpoint group from the installed themes.'); } - $form['breakpointGroup'] = array( + $form['breakpoint_group'] = array( '#type' => 'select', '#title' => $this->t('Breakpoint group'), '#default_value' => $responsive_image_mapping->getBreakpointGroup(), @@ -134,10 +134,13 @@ public function validate(array $form, FormStateInterface $form_state) { // Only validate on edit. if ($form_state->hasValue('keyed_mappings')) { // Check if another breakpoint group is selected. - if ($form_state->getValue('breakpointGroup') != $form_state->getCompleteForm()['breakpointGroup']['#default_value']) { + if ($form_state->getValue('breakpoint_group') != $form_state->getCompleteForm()['breakpoint_group']['#default_value']) { // Remove the mappings since the breakpoint ID has changed. $form_state->unsetValue('keyed_mappings'); } + // @todo Filter 'sizes_image_styles' to a normal array in + // https://www.drupal.org/node/2334387. For an example see + // \Drupal\Core\Block\BlockBase::validateConfigurationForm(). } } diff --git a/core/modules/responsive_image/src/Tests/ResponsiveImageAdminUITest.php b/core/modules/responsive_image/src/Tests/ResponsiveImageAdminUITest.php index 21b2ad9..38cee5d 100644 --- a/core/modules/responsive_image/src/Tests/ResponsiveImageAdminUITest.php +++ b/core/modules/responsive_image/src/Tests/ResponsiveImageAdminUITest.php @@ -44,13 +44,13 @@ public function testResponsiveImageAdmin() { // Add a new responsive image mapping, our breakpoint set should be selected. $this->drupalGet('admin/config/media/responsive-image-mapping/add'); - $this->assertFieldByName('breakpointGroup', 'responsive_image_test_module'); + $this->assertFieldByName('breakpoint_group', 'responsive_image_test_module'); // Create a new group. $edit = array( 'label' => 'Mapping One', 'id' => 'mapping_one', - 'breakpointGroup' => 'responsive_image_test_module', + 'breakpoint_group' => 'responsive_image_test_module', ); $this->drupalPostForm('admin/config/media/responsive-image-mapping/add', $edit, t('Save')); @@ -64,7 +64,7 @@ public function testResponsiveImageAdmin() { // Edit the group. $this->drupalGet('admin/config/media/responsive-image-mapping/mapping_one'); $this->assertFieldByName('label', 'Mapping One'); - $this->assertFieldByName('breakpointGroup', 'responsive_image_test_module'); + $this->assertFieldByName('breakpoint_group', 'responsive_image_test_module'); $cases = array( array('mobile', '1x'), @@ -83,7 +83,7 @@ public function testResponsiveImageAdmin() { // Save mappings for 1x variant only. $edit = array( 'label' => 'Mapping One', - 'breakpointGroup' => 'responsive_image_test_module', + 'breakpoint_group' => 'responsive_image_test_module', 'keyed_mappings[responsive_image_test_module.mobile][1x][image_mapping]' => 'thumbnail', 'keyed_mappings[responsive_image_test_module.narrow][1x][image_mapping]' => 'medium', 'keyed_mappings[responsive_image_test_module.wide][1x][image_mapping]' => 'large', diff --git a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php index 281d03e..cf7eba3 100644 --- a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php +++ b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php @@ -61,7 +61,7 @@ protected function setUp() { $this->responsiveImgMapping = entity_create('responsive_image_mapping', array( 'id' => 'mapping_one', 'label' => 'Mapping One', - 'breakpointGroup' => 'responsive_image_test_module', + 'breakpoint_group' => 'responsive_image_test_module', )); } @@ -119,10 +119,12 @@ protected function addTestMappings($empty_styles = FALSE) { } else { $this->responsiveImgMapping + // Test the output of an empty image. ->addMappingDefinition('responsive_image_test_module.mobile', '1x', array( 'image_mapping_type' => 'image_style', 'image_mapping' => RESPONSIVE_IMAGE_EMPTY_IMAGE, )) + // Test the output of the 'sizes' attribute. ->addMappingDefinition('responsive_image_test_module.narrow', '1x', array( 'image_mapping_type' => 'sizes', 'image_mapping' => array( @@ -133,6 +135,7 @@ protected function addTestMappings($empty_styles = FALSE) { ), ), )) + // Test the normal output of mapping to an image style. ->addMappingDefinition('responsive_image_test_module.wide', '1x', array( 'image_mapping_type' => 'image_style', 'image_mapping' => 'large', @@ -232,10 +235,13 @@ protected function doTestResponsiveImageFieldFormatters($scheme, $empty_styles = // Make sure the IE9 workaround is present. $this->assertRaw(''); $this->assertRaw(''); + // Assert the empty image is present. $this->assertRaw('data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='); $this->assertRaw('/styles/medium/'); + // Assert the output of the breakpoints. $this->assertRaw('media="(min-width: 0px)"'); $this->assertRaw('media="(min-width: 560px)"'); + // Assert the output of the 'sizes' attribute. $this->assertRaw('sizes="(min-width: 700px) 700px, 100vw"'); $this->assertPattern('/media="\(min-width: 560px\)".+?sizes="\(min-width: 700px\) 700px, 100vw"/'); $this->assertRaw('media="(min-width: 851px)"'); diff --git a/core/modules/responsive_image/tests/src/Unit/ResponsiveImageMappingConfigEntityUnitTest.php b/core/modules/responsive_image/tests/src/Unit/ResponsiveImageMappingConfigEntityUnitTest.php index 493ddb5..a8373c5 100644 --- a/core/modules/responsive_image/tests/src/Unit/ResponsiveImageMappingConfigEntityUnitTest.php +++ b/core/modules/responsive_image/tests/src/Unit/ResponsiveImageMappingConfigEntityUnitTest.php @@ -65,7 +65,7 @@ protected function setUp() { * @covers ::calculateDependencies */ public function testCalculateDependencies() { - $entity = new ResponsiveImageMapping(array('breakpointGroup' => 'test_group')); + $entity = new ResponsiveImageMapping(array('breakpoint_group' => 'test_group')); $entity->setBreakpointGroup('test_group'); $this->breakpointManager->expects($this->any()) @@ -306,7 +306,7 @@ public function testRemoveMappingDefinitions() { * @covers ::getBreakpointGroup */ public function testSetBreakpointGroup() { - $entity = new ResponsiveImageMapping(array('breakpointGroup' => 'test_group')); + $entity = new ResponsiveImageMapping(array('breakpoint_group' => 'test_group')); $entity->addMappingDefinition('test_breakpoint', '1x', array( 'image_mapping_type' => 'image_style', 'image_mapping' => 'large',