Conversation
kmonahan
left a comment
There was a problem hiding this comment.
Another one that really needs a "Needs discussion" status.
I wholeheartedly agree that responsive images > non-responsive images. But a lot of the work here seems to be around creating or modifying custom Twig templates for images, and I don't think we should be doing that. We should use what Drupal provides us, which is a responsive image using img or picture depending on your Responsive Image style settings.
In the CTA, for example, the media now comes from a series of nested Twig templates and render functions that, to me, falsely creates the impression that I can pass in other modifiers to my image or modify the responsive image template. Before, it was clear that you were going to be handed an image element to slot in where needed.
I might be overthinking things here and maybe these are just meant as ways to easily add responsive images as placeholders in Storybook without having to handwrite a bunch of image sizes every time. If that's the case, it changes my opinion and I'm on board with it. What I want to avoid is something like we have with links, where we constantly have to tear apart the Drupal render array to feed it to our own template.
But like I said, needs discussion! I'll tag other people here for more perspectives.
|
Dan/Tommy and I just talked about this and I think a it might address the concerns KJ raised if we:
|
This PR adds responsive images to Storybook and updates components to use them.