$this->photoswipeAssetManager->attach() conflicts with twig_field_value "|field_value"

Created on 10 October 2023, about 1 year ago

Problem/Motivation

$this->photoswipeAssetManager->attach() conflicts with twig_field_value |field_value filter for field formatters.

Steps to reproduce

Instead of rendering the whole field regularly, use {{ content.field_image|field_value }}
See the library is not being attached

Proposed resolution

In https://git.drupalcode.org/project/photoswipe/-/blob/5.x/src/Plugin/Fiel... attach the library to each item instead of the field.

While that may lead to duplication, it also works if the field is handled like this.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Fixed

Version

5.0

Component

Code

Created by

🇩🇪Germany Anybody Porta Westfalica

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Anybody
  • @anybody opened merge request.
  • Assigned to thomas.frobieter
  • Status changed to Needs review about 1 year ago
  • 🇩🇪Germany Anybody Porta Westfalica

    @thomas.frobieter please try, if it fixes the issue (and still works for all other cases).

    Another alternative would have been to move the ->attach() into template_preprocess_photoswipe_(responsive_)image_formatter in photoswipe.theme.inc but then we'd also have to handle the overwritten (third party / formatter) settings there. Might also be a viable solution if this one shouldn't work.

    We should try this with care, as there's a risk of multi-initialization now. Attaching to the field was definitely the smarter and less risky approach.

  • 🇩🇪Germany Anybody Porta Westfalica

    I also reported the issue over at twig_field_value: 🐛 |field_value removes attached libraries from $build Active is I think it might be seen as a bug over there!
    If they agree and find a fix, we should consider reverting this.

  • Status changed to RTBC about 1 year ago
  • Looks good! Library is loaded correctly together with the field_value formatter

  • Issue was unassigned.
  • 🇩🇪Germany Anybody Porta Westfalica

    Ok tests are also all green, so let's merge this into dev and see if we get any errors reported. I made a fresh rc3 release before with the other fix.

  • Status changed to Fixed about 1 year ago
    • Anybody committed 6935e6e7 on 5.x
      Issue #3392968 by Anybody: $this->photoswipeAssetManager->attach()...
  • 🇩🇪Germany Anybody Porta Westfalica

    Merged into dev.

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

Production build 0.71.5 2024