Copying issue from issue #2128931: Work with media file field for 7, because in 8 is the same .

This would be lovely to work with the Media file field. I'm currently have to use views if I just want to output one image from my multi-value media fields.

Media requires a Rendered file formatter, so it would need to piggy back on that, or be a Media File formatter.

Just hoping...

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

aleix created an issue. See original summary.

aleix’s picture

Status: Active » Needs review
StatusFileSize
new3.29 KB

just with few adaptations it seems it's working with media fields.

neograph734’s picture

Confirming that this works for me as well. Thanks for your effort.

I am just wondering if the plugin name media_delta_formatter could cause name clashing issues in the future? Perhaps image_delta_formatter_media would be safer?

fgm’s picture

Issue summary: View changes
neograph734’s picture

Status: Needs review » Reviewed & tested by the community

I've been using for quite a while without any issues. Switching to RTBC.

mitsuko’s picture

Yes, it works very well.
A new version with this patch commited will be wonderful !

mitsuko’s picture

One more thing...

If you apply the patch and install the module image delta formatter before media module is also enabled, you get the following error : Class 'Drupal\media\Plugin\Field\FieldFormatter\MediaThumbnailFormatter' not found in include() (line 21 of modules/contrib/image_delta_formatter/src/Plugin/Field/FieldFormatter/MediaDeltaFormatter.php

That sounds like Media become a dependency for image delta formatter.
Maybe move the this patch/new formatter in a module/submodule with dependency marked could be the solution...

neograph734’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new7.44 KB

@Mitsuko, good find. Changing the status because of that.

I am wondering if we could also do this with isApplicable. Could you perhaps test by adding the following to the class?

use Drupal\Core\Field\FieldDefinitionInterface;

class ... {

  ...

  /**
   * {@inheritdoc}
   */
  public static function isApplicable(FieldDefinitionInterface $field_definition) {
    if (!\Drupal::moduleHandler()->moduleExists('media')) {
      return FALSE;
    }
    return parent::isApplicable($field_definition);
  }

}

If this does not work, a sub-module might indeed be an option.

mitsuko’s picture

@Neograph734, sorry, I can't apply the new patch, I get an error both with 8.x-1.1 and 8.x-1.x-dev.

neograph734’s picture

StatusFileSize
new3.6 KB

Thanks for the fast feedback. The file had a file encoding issue. I believe this one should work better.

UPDATE
To be sure, you got the error when you visited the mange display page? Then I think the issue still exists with this patch...

mitsuko’s picture

Yes the error occurs on display page and is still present with the last patch applied.
Thanks you for looking around this issue.

neograph734’s picture

I was about to update the patch and make this into a sub-module, but then started looking around. It appears that there are already other modules solving the same issue. Note that I have not tested any of the modules yet, but I will in the near future.

https://www.drupal.org/project/single_image_formatter
https://www.drupal.org/project/single_media
https://www.drupal.org/project/entity_delta_formatter

Since this issue has not received any maintainer feedback I am wondering if we should not switch to one of the other solutions, rather then duplicating the functionality here once more?

mitsuko’s picture

I tried the first 2 on the list before discover and switch to Image Delta Formatter.
IMHO, Image Delta Formatter is more efficient and granular.
You are not limited to 1 image, and I like especially squeeze the media display mode to pick the image style in the parent's entity display mode.

Indeed, the issue not seems priority for maintainers ;-) but module and patch #2 make the job so far ;-)

I created an issue fork with the submodule approach but it's a little bit messy because the generated diff patch is against the 7.x branch instead 8.x...

mitsuko’s picture

StatusFileSize
new4.55 KB

The good diff is here : https://git.drupalcode.org/issue/image_delta_formatter-3173945/-/merge_r....
I add the patch to the issue.

neograph734’s picture

@Mitsuko, I see. I was not aware of that. Thanks for your efforts then :)

I still have the concern I've raised in comment #3 though. Especially now this is becoming a sub-module proper naming might be important?

mitsuko’s picture

StatusFileSize
new4.68 KB

@Neograph734, yes, you're completely right, prefix the submodule name with main module name is good practice and more secure.
I changed the name according to your suggestion, fixed the target branch in the issue fork to 8.x-1.x, so the generated diff is ok.
Add a new patch nevertheless.

neograph734’s picture

Looks good to me. After updating the patch and enabling the module I've had to update the display style because (obviously) the previous formatter no longer exists. From there all works as expected.

The only think I noticed is that "Linked to content" is displayed twice, but that appears to be the case with all formatters and must be something that is inhereted from a parent class. So believe that is unrelated.

imclean’s picture

Just to add to the list in #12, field limiter provides something similar.

https://www.drupal.org/project/field_limiter

neograph734’s picture

Status: Needs review » Reviewed & tested by the community

Marking RTBC, been using this for half a year without any issues.

mitsuko’s picture

StatusFileSize
new4.48 KB

Update the patch for drupal 10 compatibility

fgm’s picture

Status: Reviewed & tested by the community » Needs work

@mitsuko: could you apply that patch to the existing MR at https://git.drupalcode.org/issue/image_delta_formatter-3173945/-/tree/31... ? As it stands, the current MR does not include D10 support.

Also, more importantly, I think there is no reason to have separate modules to support that use case: we could just as well have the two plugins in the same module, and spare the extra load for an additional module.

In addition, since both plugins ultimately derive from the same underlying ImageFormatter (via MediaThumbnailFormatter in the case of image_delta_formatter_media), it would make sense to just reuse the code instead of duplicating it. One possibility would be to defined a Trait holding that common code, and reduce both plugins to one-liners using that trait.

neograph734’s picture

Also, more importantly, I think there is no reason to have separate modules to support that use case: we could just as well have the two plugins in the same module, and spare the extra load for an additional module.

Please also check comment #7. It is split because of the dependency on the media module.

fgm’s picture

Status: Needs work » Needs review

@neogrpah734 indeed, since media is a core module and has all but replaced image module for most common use cases since Drupal 8.4, I do not consider the dependency on media to be an issue any longer: this is no longer 2021.

I just pushed a combined version on the MR, which also drops D8 support since it has been unsupported for almost 2 years already: comments welcome.

fgm’s picture

StatusFileSize
new12.43 KB

In patch form in case anyone prefers that.

fgm’s picture

StatusFileSize
new8.98 KB
fgm’s picture

Status: Needs review » Needs work

With the added dependency, the code is now missing a hook_update_N to enabled media if needed.

fgm’s picture

Status: Needs work » Needs review
StatusFileSize
new9.7 KB
new641 bytes

Added hook_update_N.

fgm’s picture

There was a typo in the media formatter name ("medial_delta_formatter").

Regarding the conditional loading with isApplicable, it couldn't work because to call the method, the class first needs to be loaded and that will fail if media.module is not loaded.

Another approach could be to compose the thumbnail formatter instead of inheriting from it, to support conditional loading, but it would make the code significantly more complex.

fgm credited Berdir.

fgm’s picture

@berdir suggested a smarter approach on Slack: the Media delta plugin is now conditional, which avoids both the extra submodule and depending on media.module and will still enable the Media formatter automatically if media is present.

I guess that's a better choice for everyone ?

  • fgm committed 93bbc8d9 on 8.x-1.x authored by Mitsuko
    Issue #3173945 by fgm, Mitsuko, aleix, Berdir: Work with media file...
fgm’s picture

Status: Needs review » Fixed

Merged that simplified version to current dev HEAD, thanks everyone.

Status: Fixed » Closed (fixed)

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

neograph734’s picture

@fgm thanks for all the work. Is there any chance you could create a tagged release with these changes included?

fgm’s picture

@Neograph734 the plan is to do it after D10.2 release, along with #3399449: Update Drupal / PHP support. Think it's worth doing one before ?

neograph734’s picture

Thanks for the fast reply! I believe that 10.2 is planned for next week or so? No need to create a new release now then.
I was only asking because I figured that I was still on the dev version.