Problem/Motivation
Based on the twig_tweak drupal_image() complications found in #3356042: drupal_image() needs #width / #height values for fully working image cache scale, lazy-loading and img attributes we should discuss if it's possible and more stable to replace large parts of our custom code by utilizing field formatters on the image field, using drupal_field() with a display formatter array instead.
That way we could utilize all existing image formatters (responsive, photoswipe, ...) and their settings (as array).
Due to the genius implementation of flexible_image.html.twig and its parameters, this should already be possible in large parts.
This is where drupal_image() is used in this module:
https://git.drupalcode.org/search?search=drupal_image&nav_source=navbar&...
We should first have a look if it's possible and what's possible and play around a bit, my hope would be the following improvements:
- Less risk of custom logic using drupal_image() with it's discussable custom implementation
- Automatically solving things like "alt" attribute, further attributes, libraries etc. indirectly via the field formatter magically
- Better caching through Drupal fields itself (performance) - @Chi already mentioned that might be a harder issue with drupal_image() and imagecache calculations at runtime
- Being able to utilize what's implemented in the different field formatters already without having to initialize photoswipe ourselves etc.
=> More stable implementation in general!
If #3211892: Allow custom URL() to be used for image fields is finished, we could even use the "image_link" setting to link the image to arbitrary URLs. That should be pushed forward then.
In drowl_media we were already able to solve this: #3359595: Replace twig_tweak's drupal_image() where possible by drupal_field() (but that was a more simple case!)
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork drowl_paragraphs-3359603
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
Comment #2
anybodyComment #3
anybodyComment #4
anybodyComment #6
anybodyMhm maybe easier than we thought... just an example implementation to start with. Have a look and try @thomas.frobieter what you think! :)
Comment #7
anybody@thomas.frobieter: I guess this should be reviewed and merged. Sadly, I think this isn't present in D10VL anymore, but we can patch it in. First review the other issue, which was simpler.
I thought we had already fixed both, but not the case!
Comment #8
thomas.frobieterWe have to check if this will do any changes to the markup, and it very likely will. So there's a huge potential of destroying existing sites.
I'll test this, when I have more time available.
Comment #9
thomas.frobieterThe change causes:
TypeError: Drupal\twig_tweak\TwigTweakExtension::drupalField(): Argument #1 ($field_name) must be of type string, array given, called in /var/www/vhosts/web10.v1750.ncsrv.de/httpdocs/vorlagen/drupal10/vendor/twig/twig/src/Environment.php(361) : eval()'d code on line 133 in Drupal\twig_tweak\TwigTweakExtension::drupalField() (line 198 of modules/contrib/twig_tweak/src/TwigTweakExtension.php).
I'm on it.
Edit:
Okay, it should be:
{{ drupal_field('field_paragraphs_image', 'paragraph', paragraph.id(), {type: 'responsive_image', settings: {responsive_image_style: responsive_image_size}})|field_value }}instead (image_field = field array):
{{ drupal_field(image_field, 'paragraph', paragraph.id(), {type: 'responsive_image', settings: {responsive_image_style: responsive_image_size}})|field_value }}However, the formatter styles are not set. A simple link formatter is printed
<a href="/de/media/35/edit" hreflang="zxx">map_example.png</a>Trying to fix that now.
Edit 2:
This won't work this way, 'field_paragraphs_image' is a (media) reference field, which doesn't have a 'responsive_image' formatter available.
So two possible ways here I guess:
1) Grab the image field from the referenced media entity (favorite)
2) Use the "field formatter" modules formatter (Field formatter with inline settings), the 'settings' part will likely become horrible this way
Edit 3:
Got it, we dont need to grab the image field from the media reference, our include already relies on the original image field, not on the media reference field. So we only need to get the media.id() + the field name from that field and pass it to drupal_field().
Further good news: No markup change, so this fix is not a breaking change.
MR incoming.
Comment #10
thomas.frobieterDone, please review:
https://git.drupalcode.org/project/drowl_paragraphs/-/merge_requests/57/diffs
I have tested all the Paragraph bundles that use this include and everything seems to work fine.
Comment #11
anybodyFine, as we both tested and review this, let's merge it! :)
Comment #13
anybody