Responsive image format for media

It's a big downside.
If I have many different node types and displays with references to the media image, I need to create as many displays for media Image entity as many responsive image types I have.
Example:

node--article--slide reference to media--image--article-slide (shows image field like a specific responsive type 'article-slide')
node--article--teaser reference to media--image--article-teaser (shows image field like a specific responsive type 'article-teaser')
node--article--small reference to media--image--article-small (shows image field like a specific responsive type 'article-small')

Would be great to have the Responsive image field formatter for the entity-reference filed type.

Here is the additional field formatter which implements the feature.

I created a core patch from nortmas formatter file against core 8.5.x-dev to test out. I'm guessing this would go directly into the responsive_image module. I'm using composer to apply the patch. This is my first core module patch and had to make sure the path was /modules instead of /core/modules. Let me know if this isn't the proper way to setup a core patch.

FYI, there is already a contrib module which adds this: Responsive Media Thumbnail

The file in that module has the difference of extending from *ResponsiveImageFormatter* rather than ImageFormatter like the example here does.

Updated #7 to add the new field formatter to the FieldFormatter plugins that ship with the responsive_image module. Also cleaned-up a couple of coding standards violations.

FWIW, I think this should be a core patch rather than a contrib module since both responsive image and media entities are stable core features.

Updating the patch from #10 with one that removes /core from the patch path, since that is implied when applying the patch. In #10 the file is created in docroot/core/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php

This is looking good. Just a few comments and code nits below. It should also have some tests to ensure it's behaving as expected.

1. +++ b/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php
   @@ -0,0 +1,303 @@
   +    $plugin_id,
   +    $plugin_definition,
   +    FieldDefinitionInterface $field_definition,
   +    array $settings,
   +    $label,
   +    $view_mode,
   +    array $third_party_settings,
   +    AccountInterface $current_user,
   +    ImageStyleStorageInterface $image_style_storage,
   +    RendererInterface $renderer,
   +    $responsive_image_style_storage,
   +    LinkGeneratorInterface $link_generator
   

   I wonder if rather than overriding the parent constructor, since it already takes so many parameters, if instead we could use the setter pattern in the create() method:

   Something like:

   $instance = parent::create(...);
   $instance->setRenderer(...);
   $instance->setResponsiveImageStorage(...);
   $instance->setLinkGenerator(...);
   

   where those 3 setter methods can be protected.

   (See this blog post for details on this pattern.)

   This is just a suggestion/point of discussion, not necessarily required (I'm not sure if core is using that pattern elsewhere yet or not.)

2. +++ b/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php
   @@ -0,0 +1,303 @@
   +      '#title' => t('Responsive image style'),
   ...
   +      '#title' => t('Link image to'),
   ...
   +      '#empty_option' => t('Nothing'),
   ...
   +      $summary[] = t('Responsive image style: @responsive_image_style', ['@responsive_image_style' => $responsive_image_style->label()]);
   ...
   +        'content' => t('Linked to content'),
   +        'file' => t('Linked to file'),
   ...
   +      $summary[] = t('Select a responsive image style.');
   

   Nit: should use $this->t() as elsewhere in the patch.

3. +++ b/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php
   @@ -0,0 +1,303 @@
   +    if ($style_id && $style = ResponsiveImageStyle::load($style_id)) {
   

   Since the class already has responsive image style storage injected, this can be $this->responsiveImageStyleStorage->load($style_id) I think.

4. +++ b/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php
   @@ -0,0 +1,303 @@
   +    if ($image_link_setting == 'content') {
   

   Nit, this should be ===

@jhedstrom thanks for the setter pattern suggestion; really happy I learnt about that; will come in handy in a lot of cases. Based on your comment on whether core is using it or not, I did a quick find and it is being used in a few places, one of them being Drupal\update\UpdateSettingsForm. They're not even using a setter method there, but instead setting the property directly, which I reproduced in my patch.

@SamLerner when I tried to apply your patch at #12, it was applied to the wrong directory; I had to add /core again to the patch file in order for it to apply properly, so I added it to my new patch as well. Other patches for core also seem to have that in.

I have addressed all of the points mentioned in #13 except for tests; will add some as soon as I can, if somebody else doesn't get to it first.

#14 forked for me. Drupal 8.9.7

Update patch for 9.x
Replace entity.manager to entity_type.manager
See interdiff file

Formatter looks great, but issue still needs test coverage

This is great work, really useful. It took me a while to get the patch in #19 working, until I found out that the new file was placed in /core/core/modules.. can't figure out why. But after I moved the file to the correct location, life was good!

BarisW add to your composer.file this line https://gitlab.com/batkor/ease/-/blob/master/composer.json#L41

Hm, maybe I do misunderstand the issue and its goal and the regarding patch but I cannot review or confirm the patch functionality since I do not see any change in the formatter options of a media reference field like I would expect here. Where should the responsive image format appear. I expect it to appear under /admin/structure/types/manage/nodetype/display when I choose a formatter for the media reference. Drupal 9.4.x dev with responsive image enabled and patch applied successfully via git apply -v.

Issue summary needs a description of the problem, etc. It currently has none.

Issue summary added, please review.

The plugin should declare a dependency on the Responsive Image module. Without it the use of this plugin results in the below error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "responsive_image_style" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (regel 143 van /var/www/html/htdocs/core/lib/Drupal/Core/Entity/EntityTypeManager.php).

Fixed bugs from #28

Add interdiff #19 - #31

This is really a responsive image formatter for media thumbnails so that it bypasses media display modes altogether. Retitling a bit and adding related issues - this will also need to add image loading attribute support if that lands first.

The title change is not correct and is not in line with the functionality of this formatter. The formatter configuration allows selecting all possible responsive image styles, much more than the Thumbnail style. And more, the introducing a new formatter only suitable for the thumbnail image, make the added value of this formatter almost zero.

Reworked the patch to use the file from the media source field instead of the thumbnail.

The ticket still needs tests for a new formatter.

Patch from #37 does not seem to apply to 9.5.3.

git apply -v --directory=web responsive-style-for-media-9.5-37.patch
Checking patch web/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php...
error: while searching for:
      ],
    ];

    $link_types = [
      'content' => $this->t('Content'),
      'file' => $this->t('File'),
    ];
    $elements['image_link'] = [
      '#title' => $this->t('Link image to'),
      '#type' => 'select',
      '#default_value' => $this->getSetting('image_link'),
      '#empty_option' => $this->t('Nothing'),
      '#options' => $link_types,
    ];

    return $elements;

error: patch failed: web/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php:148
error: web/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php: patch does not apply
Checking patch web/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php...

Fixed it with a new patch, only a single character is different.

Update: I think my problem is because the patch of this issue seems to collide with patch from https://www.drupal.org/project/drupal/issues/3192234
(https://www.drupal.org/files/issues/2022-09-30/3192234-228-9.5.x.patch).

Nice work, still needs tests as of #21, then it's ready for review.

Loading attribute is not set with that formatter!
It is missing 2 lines before $elements[$delta] is set:
When I find time I'll provide patch.

$image_loading_settings = $this->getSetting('image_loading');
$item_attributes['loading'] = $image_loading_settings['attribute'];

$elements[$delta] = [
.....

Here's the #39 patch with the loading attribute that was missing

The #45 patch causes this error
ParseError: Unclosed '{' on line 164 in Composer\Autoload\{closure}() (line 173 of core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveMediaImageFormatter.php).
so I've re-created it.