Support Media and Entity References

It works well for our simple needs: reference media entities with a single image field.

@thomas.frobieter please review if that makes sense or if there is a better alternative already.

@clmsmidt: Which media version did you test this against? 1.x or media in core?

Thanks, the patch please needs a reroll and test against latest .dev. Can we get more feedback?

Could you please reroll and test the patch against 8.x-2.x branch?
Thank you!

Furthermore the missing points from #2 should be solved and it should be ensured that this doesn't break things with media_entity or when media is not used.

Hi guys, I improved patch for actual 2.x-dev version.

Patch with code style fix for code sniffer.

I don't get it why its failed, but test results shows anything alright.

By the way for someone who needs phowtoswipe support right now for project with media. #17 patch works perfectly as expected on PHP 7.1, Drupal 8.6 and latest 2.x version of photoswipe module.

No problem, the patch simply fails because there are no tests specified. The patch looks good so far but we should get some feedback. Could you please clarify if this is for media (core) or media_entity?

Thank you, good job!

I tested and created this patch on Media from core (Drupal 8.6).

But I agee that it need to more feedback.

Curious things I found:

1. I changed one line in photoswipe.theme.inc from

-  $entity = $item->getParent()->getParent()->getValue();
+  if ($item->entity instanceof MediaInterface && $item->entity->hasField($settings['photoswipe_reference_image_field'])) {
+    $item = $item->entity->get($settings['photoswipe_reference_image_field']);
+  }
+  $entity = $item->getParent()->getValue();

This is strange. Because with double getParent() I've got errors on every page with photoswipe. And I can't get the reason, why it's doubled. But with single call it works. I tested a little bit on legacy image fields, and it also works as expected, but I still can't understand why this done before, because media fails on it.

So this need to be tested by others as well. Maybe it can break formatter for legacy image field. 🤷‍

2. Updated to 2.x of module, my multiple images with media, and legacy image fields as well doesn't have "grouping" of images. Again, I don't know, this was broken by my patch, or this is normal for 2.x version and this is handled by other issue. Because I upgraded to 2.x only for this patch.

I am getting this error after applying the patch:

Error: Call to undefined method Drupal\media\Entity\Media::getFileUri() in template_preprocess_photoswipe_image_formatter() (line 29 of /Users/fotidim/Sites/devdesktop/drupal-8.6.1/modules/contrib/photoswipe/photoswipe.theme.inc) #0 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Theme/ThemeManager.php(287): template_preprocess_photoswipe_image_formatter(Array, 'photoswipe_imag...', Array) #1 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(437): Drupal\Core\Theme\ThemeManager->render('photoswipe_imag...', Array) #2 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, false) #3 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Template/TwigExtension.php(490): Drupal\Core\Render\Renderer->render(Array) #4 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/sites/default/files/php/twig/5ba16871bed57_field.html.twig_M2yHyNwhjg3kxV-d9xifXvB-S/tOssldP_9xk0wjupSVX6hE-r191hKlU4j2FQwCOvRzI.php(128): Drupal\Core\Template\TwigExtension->escapeFilter(Object(Drupal\Core\Template\TwigEnvironment), Array, 'html', NULL, true) #5 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(432): __TwigTemplate_e74a4433eec1a3e06c95293ba9f9e8f3d38c6ce8971a0bcab1f50e49712fde3a->doDisplay(Array, Array) #6 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(403): Twig_Template->displayWithErrorHandling(Array, Array) #7 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(411): Twig_Template->display(Array) #8 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/themes/engines/twig/twig.engine(64): Twig_Template->render(Array) #9 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Theme/ThemeManager.php(384): twig_render_template('themes/bootstra...', Array) #10 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(437): Drupal\Core\Theme\ThemeManager->render('field', Array) #11 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(450): Drupal\Core\Render\Renderer->doRender(Array) #12 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, false) #13 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Template/TwigExtension.php(490): Drupal\Core\Render\Renderer->render(Array) #14 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/sites/default/files/php/twig/5ba16871bed57_node.html.twig_96-rn3jRAJHEd4SuHdwrgrHqQ/0CQEqyJIMpnKZhcJf6jXTAeUJDerGu2TOY2zMl54x0U.php(111): Drupal\Core\Template\TwigExtension->escapeFilter(Object(Drupal\Core\Template\TwigEnvironment), Array, 'html', NULL, true) #15 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(432): __TwigTemplate_0cf7bf215c39faf5a00644ac5ce5353d0e73c36d98b0901542b2916c4bd4ff18->doDisplay(Array, Array) #16 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(403): Twig_Template->displayWithErrorHandling(Array, Array) #17 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/twig/twig/lib/Twig/Template.php(411): Twig_Template->display(Array) #18 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/themes/engines/twig/twig.engine(64): Twig_Template->render(Array) #19 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Theme/ThemeManager.php(384): twig_render_template('themes/bootstra...', Array) #20 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(437): Drupal\Core\Theme\ThemeManager->render('node', Array) #21 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, false) #22 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(226): Drupal\Core\Render\Renderer->render(Array, false) #23 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() #24 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(227): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) #25 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(117): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch)) #26 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch)) #27 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher)) #28 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher)) #29 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/symfony/http-kernel/HttpKernel.php(156): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent)) #30 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) #31 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #32 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #33 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/modules/page_cache/src/StackMiddleware/PageCache.php(184): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #34 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/modules/page_cache/src/StackMiddleware/PageCache.php(121): Drupal\page_cache\StackMiddleware\PageCache->fetch(Object(Symfony\Component\HttpFoundation\Request), 1, true) #35 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/modules/page_cache/src/StackMiddleware/PageCache.php(75): Drupal\page_cache\StackMiddleware\PageCache->lookup(Object(Symfony\Component\HttpFoundation\Request), 1, true) #36 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #37 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #38 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #39 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/core/lib/Drupal/Core/DrupalKernel.php(665): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #40 /Users/fotidim/Sites/devdesktop/drupal-8.6.1/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) #41 {main}.

In field photoswipe formatter select correct media field. This error throws when entity_reference field selected in formatter not targets to actual image entity. I think it's need to be handled somehow.

Maybe we need to filter such fields.

This patch must be more reliable.

I've add addition condition for field options.

diff -u b/src/Plugin/Field/FieldFormatter/PhotoswipeFieldFormatter.php b/src/Plugin/Field/FieldFormatter/PhotoswipeFieldFormatter.php
--- b/src/Plugin/Field/FieldFormatter/PhotoswipeFieldFormatter.php
+++ b/src/Plugin/Field/FieldFormatter/PhotoswipeFieldFormatter.php
@@ -127,7 +127,7 @@
         ->getFieldDefinitions($target_type, $bundle);
     }
     $fields = array_filter($fields, function (FieldDefinitionInterface $field) {
-      return $field->getType() === 'image';
+      return $field->getType() === 'image' && $field->getName() !== 'thumbnail';
     });
 
     $field_options = [];

Actually thumbnail is a property for media entity. And not so reliable and I think no one will use it for photoswipe. But who knows, any suggestions?

@niklan now it does not appear for me at all. Just to double check I am using Drupal 8.6.1 with core Media module and PhotoSwipe 8.x-2.2. I am creating a Media reference with 6 potential values and setting the "Media type" to Image.

https://imgur.com/a/bhoGy27

@axaios this is strange. That's work for me.

If you using patch 17, and select in photoswipe formatter settings correct image field, error is gone?

@Niklan I can't get it to appear again. It seems to be flaky or I am missing a setting. I exported the demo website in a Dev Desktop archive. You can find it here: https://drive.google.com/open?id=1O8PTHDlKN29Q3lfp6Tq6kk1IDEEply5W username: admin password: admin. Let me know when you download it so I can delete it.

@axaios I've downloaded archive. I'll try it later.

#24 worked for me.

I have put this formatter on (core) media entity reference field. I think the expected behaviour should be that all references should be displayed in one gallery instead of gallery for each image.

Because of limited time I could fix this with just adding to lines.
The interdiff is based on 2.x-dev and patch #24 applied

@Niklan I got it to work. Patch 24 worked for me as well although still returns an error if "Node image style for first image" is "No special style.".

My wrongdoing was the way I was applying the patch. Sorry I am noob when it comes to patches. I am branch and PR guy :)

On a related issue: I am not getting next/previous arrows. Is that expected behaviour?

@axaios glad to hear you find the solution.

Here is the same patch as #24, but contains diffs from #30. So it's also adds support for Photoswiper gallerie which mentioned in #32.

But this issue a bit for another task :)

Set this issue to needs review from others.

Change again to needs review :) There are not tests.

Patch 33 works fine for me!

#33 is working fine

Ok lets get this into dev version... We'll keep it there for some weeks and further feedback. Thank you all.

Added support for oembed media
First thing I'm not a coder so I just tried to solve a problem I had mostly copying the code from the oembed formatter. I think the code can be improved quite a lot...
In my case I use a block to render media entities, images or eombed (youtube) videos, setting a display view with a photoswipe formatter.
Works properly for my needs.

I am using 8.x-2.x from #40.
And I can confirm that error same as in #22 still exists and workaround from #31 works (choosing something else than "No special style" for fist image). Looks like a bug.
Thanks, everyone for a good job!

@niklan re #21: Aha, now I see who changed double getParent() to single getParent() and broke something that worked in 8.x-2.2 :)
With double getParent() and NO Media involved (a simple image field in a simple node) $entity->getEntityTypeId() returned what it was supposed to, with a single getParent() I am getting a "call to getEntityTypeId() on null" error.
I guess we need to make sure that we are getting the entity object correctly in both Media and non-Media context (including Views!). Frankly, all this getParent()->getParent() stuff doesn't look like clean professional code to me, there must be a more straightforward / transparent way of getting the entity? Unfortunately I'm clueless in this particular realm.

OK, so we need $entity to point to the current entity when PhotoSwipe field formatter is applied to:
- an image field in entity display
- an image field in a view
- a Media field in entity display
- a Media field in a view

In the Colorbox module, the entity is simply passed to the template_preprocess_HOOK as variables['entity'], so it's very transparent in the .theme.inc and all real processing is done in the field formatter. Should we go the same way? I tried the following changes with "an image field in entity display" and "an image field in a view" situation and it seems to work for me. As I don't know anything about Media and cannot be trusted to test it reliably, could anyone test if it works in the Media context? And if it doesn't, fix it within a conditional block pertaining to Media fields only, so that it doesn't break the non-Media functionality?

Whao @ndlarge, your patchfrom #45 works great and fixes a lot of problems for media / entity reference.
One problem we still see is that if all but the first image are set to hidden in the formatter, they are not shown at all. The expected behaviour would be to only add their link markup but without image to add them to the gallery. But perhaps the reason for that bug isn't part of this issue. If you have an idea, you're welcome...

So far your patch improves the situation a lot, thank you!

Thank you @ndlarge for new patch and fixing my mistake :)

@Anybody are you sure this bug is exist? On clean Drupal 8.6 with Photoswipe 2.x-dev + patch from #45, I can select image style "hide" for first image, and other images will be printed as expected, but first will be printed with class "hidden" in the DOM, and available in photoswipe gallery.

I also check out the code, and this behave as expected and I understand it. There is nothing wrong.

@Niklan, well the bug existed, but wasn't the fault of this module but is a conflict with inline_field_formatter which we were using in combination with photoswipe. With this patch applied (plus I added media_entity support for not yet upgraded projects) inline_field_formatter is no more required and it works as designed.

After a lot of testing I'll now create a new dev branch containing this patch. Please test and report if it's stable enough for a new stable 2.x release. Thank you all very much!

Thank you @Anybody, works for me with image fields in entity display and views contexts, didn't test with Media!

In my view, the media entity reference field is not able to find the image field when using the photoswipe formatter. No options appear under "Image field of the referenced entity", it's just empty. If I save without selecting an option, the same error appears as in #22. Is there a step I'm missing or is media still not usable with photoswipe in a view? I'm using Drupal 8.8 and Photoswipe 2.9 with core media

Tested this on a fresh Drupal 8.8 install with Photoswipe 2.9. referencing a node entity (with an image field) in a view.

- Created a view field for that referenced entity field
- Format selected Photoswipe
- No select options available under "Image field of the referenced entity"
- If I save anyway, get this error "Error: Call to undefined method Drupal\node\Entity\Node::getFileUri() in template_preprocess_photoswipe_image_formatter() (line 30 of /drupal_sandbox/web/modules/contrib/photoswipe/photoswipe.theme.inc)"

I don't see any previous 8.8 tests, anyone else have this working? Does this issue need to be reopened?

A hacky temporary workaround to my issue on #55 is here.