cx-media does not work with images with different ratio's

Describe the bug

cx-media does not work with images with different ratio's, but the Simple responsive banner is using it exactly like that.

Tell us the version of Spartacus

Feature Level 3.0, library version 3.0.0-next.3

To Reproduce

Steps to reproduce the behavior:

Go to the spartacus demo page: https://spartacus-demo.eastus.cloudapp.azure.com/electronics-spa/en/USD/
Open the page in the smallest viewports via dev tools
Look at the banners just below the navigation "Save big on..."
The tablet / desktop rendition get's used on mobile and the text on the image (which should not be on the image due to seo reasons anyway) is not readable

Expected behavior

The correct rendition for mobile get's displayed

Screenshots

Actual Mobile View:

Image renditions which should be used on mobile:

Desktop:

OS: Mac / Windows Actual Versions
Browser Chrome / Safari via Dev Tools Mobile View
Version Actual versions

Smartphone:

Device: Android / IOS actual versions
OS: Actual Versions]
Browser chrome / safari
Version Actual versions

Additional context

The cx-media uses the srcset property on the image tag. This property should receive different renditions of an image consisting all of the same ratio. Because the browser knows about the available space for that element and tries to find the best matching rendition also based on the display density (retina or not) and other factors.

If art direction is needed (different ratio's for different viewports), the picture tag needs to be used instead (I think, that's how it was before #2234 was merged.)

The current usage in the accelerator mixes both ways of using images, which leads to the wrong behaviour as described above

Considerations

The current implementation is a mix of both approaches and makes the srcset approach also not work if the author does not know exactly what he needs to do
The simple responsive banners of the accelerator should work correctly because as they are now, the component can not be used in reality (components with such a landscape ratio are just too small on mobile) without having art direction
As the authors can add 4 different images in the component dialog, we as developers cannot prevent that he adds different ratio's
Supporting both ways of doing images would be nice as most of the times, srcset ist fine, but sometimes one need art direction
For the srcset approach, one image field would be enough where the system should create the needed renditions defined by the mediaFormats

Ideal Case

The cms components would provide the option to choose between picture tag and srcset
The cx-media would distinguish between the chosen option and display either an image with srcset or a picture tag
The needed information to generate the media queries for the picture tag could also be defined via mediaFormats width parameter

Existing Tickets around the Topic

@janwidmer thanks for this report. I've evaluated the behaviour of the component, and drafted a documentation around it, see https://github.com/SAP/spartacus-docs/pull/845. Please note that this is a draft version (but appreciate your feedback).

Regarding the image not showing up, I do not think we have an actual bug, but we can do an improvement. He's what's going on:

the media srcset is provided with all media from the media container
the media format for each media in the container is aligned with the MediaConfig in Spartacus, to come up with the right width descriptor of the img srcset
the browser will select the right media from the srcset automatically. Please NOTE that the browser will not load a small image in case it has already loaded a larger one. in case you land at a desktop version, and open dev tools, you always need to reload the page to see the right image for the given size
we do not (yet) support the image density; modern devices (i.e. retina) use double pixel density to satisfy the high resolution. i've seen a "conflict" with the default configuration and the density of todays devices. For example, if you open the largest xs screensize (as you've demonstrated in this issue), it will never show the smallest image, because the smallest configured media format does not align with the density; the smallest size is configured to be 400px, but this will only show up if you set the responsive mode in devtools to width 200px.

In order to see the smallest media in the smallest viewport, I'd recommend to increase the media format sizes. We will consider to the same in our default config, as well as consider to add a density property to the media format configuration.

The example below shows a 200px viewport to illustrate that the smallest size image is actually shown. Please note that the image is not shown on larger viewport, as the pixel density needs a double size image.

Hey @tobi-or-not-tobi,

Thanks for the quick reaction. I agree that it is not a bug. :-) And as you explained, the wrong image is taken for displays with higher pixel density. That would all still be ok, because from a technical point of view, high density images are already possible, by defining more image formats with the same ratio, but with a bigger resolution.

A little bit problematically in my opinion is, that you mix the two responsive image approaches (picture with source tags = different ratio's / image with srcset and "width" descriptor = same ratio's) by having images with different ratio's wich end up in a srcset in your accelerator demo instance. As most mobile devices today have displays around 360 - 400px (rendered) width and high density screens, all those devices will pick the wrong image (in your accelerator demo).

Because as I wrote, the srcset approach with the "w" descriptor is really meant to get images with all the same ratio and only different resolutions. Because as far as I know, its not possible to mix pixel density descriptor and width descriptor, or am I wrong?

Because, even if you add high density handling to it by adding more renditions (e.g. a 800px mobile image in our banner case), in a hypothetical example with our banner, how would the browser know, if he needs to load the 800px wide mobile high density rendition or the 800px normal version on a tablet without high density support.. because at the end, the browser calculates the needed width of an image out of different factors, and if multiple images with that width exist, he might take the wrong one (regarding ratio and intended usage from the developer).

So in my opinion, in referral to your documentation, if you want to keep the freedom of different ratio's, you would need to adjust the cx-media component to render either img tags with srcset (for e.g. product image with same ratio's) or picture tag with source tags (for e.g. banner with different ratio's per viewport).

The media queries needed for the picture with source tags could still be derived from the mediaFormats by using the given width of a mediaFormat as media query input: min-width: 400px.

We did a proof of concept by adding a custom component to be used as cx-media template for the picture case because we have cases in our project, where we absolutely need different ratio's and it seems to work so far. Of course it would be nice, if the cx-media component would handle that out of the box. :-)

Of course, beside of all that, the other question is, if it is cool for content authors, if they always need to manually maintain 4 renditions of an image (or even 8 if we want to support high density screens). In that case, an automated service which just takes a source image in the resolution of the maximally needed width / height and then does the cropping automatically..

Update with Spartacus 2211.19.0: With this Release, the Media Component has been adjusted to support Source Sets (See https://help.sap.com/docs/SAP_COMMERCE_COMPOSABLE_STOREFRONT/10a8bc7f635b4e3db6f6bb7880e58a7d/93ffb557d3c14922bda14dfc8b4250b4.html?locale=en-US)

The updated Implementation of the cx-media component does NOT follow the specification of responsive images though. The sourceSet in Composable Storefront is build based on the media config, which holds several different widths for a certain image type:

// default-media.config.ts
export const mediaConfig: MediaConfig = {
  mediaFormats: {
    mobile: { width: 400 },
    tablet: { width: 770 },
    desktop: { width: 1140 },
    widescreen: { width: 1400 },
  },
};

When using this media config e.g. within the banner component, this leads to the following images resolutions being rendered within the source set:

480x320
770x350
960x330
1400x440

When using srcSet, the browser tries to pick the best matching image based on several factors based on the Width Descriptor and the sizes attribute. This picking logic only works though, if all images have THE SAME ratio, otherwise, it leads to the potentially wrong image being picked

The Banner on the Homepage displays the 770x350 Rendition on mobile devices, which is actually meant to be used for Tablet since it has a more wide ratio..:

The correct Image to display would be this one: https://spartacus-demo.eastus.cloudapp.azure.com:8443/medias/Elec-480x320-HomeSpeed-EN-01-480W.jpg?context=bWFzdGVyfGltYWdlc3wzMzkzMnxpbWFnZS9qcGVnfGFXMWhaMlZ6TDJoaU5DOW9NV1l2T0RjNU56TTFOakl5T0RZek9DNXFjR2N8ZTFjYjU4MzY5YmEzYmQ3MjUxZTc2M2Y4OTZhNzdhNzkyYTk4OTlhMGFkMTEwY2NiZjg5ZWMyZjhlNjBmMDRhNQ

You can read more about this topic here: https://web.dev/patterns/web-vitals-patterns/images/responsive-images.

The requirement of using different ratios, as it is done for the banner on the homepage of the electronics demo store, is called "Art direction". If different ratio's are required, one needs to use the picture tag with multiple source tags with media queries to define the different image ratio's.

To make both worlds (picture with sources tags with media queries & picture tag with just one image tag and srcset) work, the following requirements would be needed from my perspective:

Enhance the media config to be able to define the media type source tags vs sourceset
```
export enum MediaType {
SOURCES: 'sources'
SRCSET: 'srcset'
}
```

export interface MediaRendition: { x1Width: number; x2Width?: number; // optional, only needed for MediaType.SOURCES ratio: string; // e.g. '16-9' // needed for the aspect ratio calculation for the MediaType.SOURCES }

export interface MediaRenditions: { key: string: MediaRendition; // the key of these properties contains the width which can be used for Media Query Generation (MediaType.SOURCES) and sizes attribute string generation (MediaType.SRCSET) }

export interface MediaBundle: { mediaType: MediaType, ratio: string; // e.g. '16-9' // needed for the aspect ratio calculation for the MediaType.SRCSET mediaRenditions: MediaRenditions; }

export interface MediaConfig: {

}

export const mediaConfig: MediaConfig = { // 1. Media Bundle Type Sources to be used for Art Direction Case with different Ratios bundleBanner: { mediaType: MediaType.SOURCES, mediaFormats: { // the number of the property key, e.g. w960, should be used as the breakpoint for the respective source tag // same widths with different ratio's can be used to have an aspect ratio based media query to better support ultra wide screens w_1400_16_6: { x1Width: 1920, x2Width: 3840, ratio: '16-6' }, w_1400_16_9: { x1Width: 1920, x2Width: 3840, ratio: '16-9' }, w_960: { x1Width: 1400, x2Width: 2800, ratio: '16-9' }, w_768: { x1Width: 960, x2Width: 1920, ratio: '16-8' }, w_480: { x1Width: 768, x2Width: 1536, ratio: '16-8' }, w1: { x1Width: 480, x2Width: 960, ratio: '4-3' }, fallback: { x1Width: 1400, ratio: '16-9' } }, },

// 2. Media Bundle Type Srcset to be used for e.g. Product Images with the same ratio for all renditions bundleBanner: { mediaType: MediaType.SRCSET, ratio: '16-9' mediaRenditions: { w_120: { width: 120 }, w_240: { width: 240 }, w_360: { width: 360 }, w_480: { width: 480 }, w_960: { width: 960 }, w_1200: { width: 1200 }, w_1600: { width: 1600 }, }, } };


As the second example has no connection to viewports and feature specific places, it also makes sense to name the media formats something agnostic (like e.g. w_xxx, which offers the benefit, that the width of the image can be used for sizes calculation, see second html example below). The browser will later pick the appropriate rendition.

The above two examples should lead to the following html output:

// Media Bundle Type Sources, needs to be used for art direction // width & height need to be defined according to the image dimensions (defined in the backend for image magick rendition generation)

// Media Bundle Type Srcset, can be used at most other places, where no artdirection is needed // width and height can be a static value based on the ratio

Hello @janwidmer . Thank you very much for your detailed explanation. I have one question regarding your config.

  bundleBanner: {
    mediaType: MediaType.SRCSET,
    ratio: '16-9'
    mediaRenditions: {
      w_120: { width: 120 },
      w_240: { width: 240 },
      w_360: { width: 360 },
      w_480: { width: 480 },
      w_960: { width: 960 },
      w_1200: { width: 1200 },
      w_1600: { width: 1600 },
    },
  }
};

What the propose of adding ratio: '16-9' here? As far as I understand this config is for img with srcset, and img with srcset should have images with the same ratio. So I have a feeling that I'm missing something here.

Regarding our changes, I created some time ago a PR with changes that covers pretty much the same cases that you addressed. I wanted to contact with you to collect the expectations and check if my PR covers all of them. couple of things I want to discuss with you.

This is how config looks after changes:

export const mediaConfig: MediaConfig = {

// Old config without changes for <img>
  mediaFormats: {
    mobile: { width: 400 },
    tablet: { width: 770 },
    desktop: { width: 1140 },
    widescreen: { width: 1400 },
    // product media
    cartIcon: { width: 65 },
    thumbnail: { width: 96 },
    product: { width: 284 },
    zoom: { width: 515 },
  },

// New config dedicated for <picture> element
  pictureElementFormats: {
    mobile: {
      maxWidth: '767px',
    },
    tablet: {
      minWidth: '768px',
      maxWidth: '1024px',
    },
    desktop: {
      minWidth: '1025px',
      maxWidth: '1439px',
    },
    widescreen: {
      minWidth: '1440px',
    },
    retina_mobile: {

// Based on this config user could provide any query, it is only should be mapped to proper query that could be used in media attribute. The map is below in separate object.
      maxWidth: '786px',
      minDevicePixelRatio: 3,
    },
    retina_desktop: {
      minWidth: '1440px',
      minDevicePixelRatio: 2,
    },
  },

// In the case of sources in picture element, order - matters. Sources will be sorted based on this order array.
  pictureFormatsOrder: [
    'retina_desktop',
    'retina_mobile',
    'widescreen',
    'desktop',
    'tablet',
    'mobile',
  ],

// Map of media queries. Here should be placed all the keys used in formats and mapped to a proper media query
  mediaQueryMap: {
    minWidth: 'min-width',
    maxWidth: 'max-width',
    minHeight: 'min-height',
    maxHeight: 'max-height',
    minDevicePixelRatio: '-webkit-min-device-pixel-ratio',
    maxDevicePixelRatio: '-webkit-max-device-pixel-ratio',
    orientation: 'orientation',
    minAspectRatio: 'min-aspect-ratio',
    maxAspectRatio: 'max-aspect-ratio',
    minResolution: 'min-resolution',
    maxResolution: 'max-resolution',
  },
};

I see the difference in your config, you handled pixel density with 1x, 2x descriptors w_768: { x1Width: 960, x2Width: 1920,. I decided to not mix such descriptors with the media queries in the config. I think the same could be achieved by adding additional format. For example:

 mobile: {
    maxWidth: '767px',
 },
 retina_mobile: {
    maxWidth: '786px',
    minDevicePixelRatio: 3,
 },

Please feel free to comment if you see any case where it can't be done this way.

Choose of desired html element is handled by the @Input() in MediaComponent; @Input() usePictureElement: boolean = false;. The name is draft yet, I planned to use img by default and give a possibility to explicitly defined - usePicture, where it is needed. But there is the possibility that it will be reversed, to avoid the regression bug.

Additionally, I'm going to add 3 inputs:

Sizes
Width
Height

Added as input because values are dependent on specific place where it is used. We could add additionally those values to config also, so it could be provided globally or passed through the Input, but I cannot imagine the proper usage of this.

I think that all that we could do right now without any backend changes. If there will be a decision to extend the backoffice with additional fields for each image, and add some toggle img/picture, width, height then we can easily without breaking changes adjust MediaComponent and extend the source of data from input to BackendData || input.

Will be happy to read you comments. Thanks again, @janwidmer.

Hi @rmch91,

Regarding your first question about the property `ratio` in the `MediaBundle` Object:

Also for the image with the srcset attribute, you need to know the ratio (or more specific the width and height) of the image, to be able to render it on the image html tag. The browser needs that information to be able to reserve the appropriate space for the image until it is loaded and to prevent layout shifts. Since the ratio of every size is the same in the MediaType.SRCSET, it can be on the level of MediaBundle in my media config suggestion. I guess, the additional inputs width and height of the media component from your suggestion would serve that purpose as well.

Regarding your suggestion based on your PR:

I assume, that the existing media config would cover the MediaType.SRCSET and render an image tag with the srcset. For those configs, I would really recommend to adjust the default config names to not use the words mobile, tablet, desktop and widescreen anymore. It makes devs think, that those image sizes refer to the viewport / device sizes, but with MediaType.SRCSET, it's really the browser, who picks the appropriate size.. and if the teaser is being displayed as 25% width Component in a grid in Viewport Desktop, he might use the mobile / tablet size event on the largest viewport..

Out of that reason, we usually use property names like this, to prevent adding layout / device based descriptive names to the sizes:

mediaFormats: {
    teaser_400: { width: 400 },
    teaser_770: { width: 770 },
    teaser_1140: { width: 1140 },
    teaser_1400: { width: 1400 },

    // product media
    product_65: { width: 65 },
    product_96: { width: 96 },
    product_284: { width: 284 },
    product_515: { width: 515 },
    product_1200: { width: 1200 },
    product_1600: { width: 1600 },
  },

And normally, depending on the usage of the component, the number of display variations (e.g. usage in grid / visual options), we have 5-10 sizes per bundle (bundle meaning all sizes starting with "teaser_xxx" above or "product_xxx"). Especially for the Product when having the product zoom feature on the detail page, 515px width is by far not enough on a 27" screen..

Regarding your suggestion of pictureElementFormats, this could also work in my optionion. If I understand correctly, all of the keys listed in mediaQueryMap, can be used in the following example object?

retina_desktop: {
      minWidth: '1440px',
      minDevicePixelRatio: 2,
    },

The only missing information in my opinion for the picture case is the ratio (or width / height) of the image sizes for this bundle. Passing it as input to the media component will not work, since it is potentially different for every size of the bundle (since picture element would potentially only be used when art direction is needed.

Could you enhance the objects of pictureElementFormats to contain a ratio & width or width and height next to maxWidth / minWidth?

mobile: {
      maxWidth: '767px',
      width: 767,
      ratio: '4/3' // would be needed to reserve space until image is loaded (image width would also be needed to know), could also be provided by the backend..
    },
    tablet: {
      minWidth: '768px',
      maxWidth: '1024px',
      width: 1024,
      ratio: '16/9'
    },
    desktop: {
      minWidth: '1025px',
      maxWidth: '1439px',
      width: 1439,
      ratio: '16/6'
    },

By the way, when using media queries, it would be sufficient to just use min-width, as long as the order in pictureFormatsOrder is defined correctly..

Would it make sense to remove the word retina from the begin of the property name and add it at the end? Instead of retina_mobile => mobile_hdpi (retina is a marketing term invented by apple ;-)). Or make it even more reduced and define mobile_x2 as key, then you don't need the property minDevicePixelRatio anymore..

Thank you @janwidmer for the response. So regarding the aspect ration in config, yes, the desired media query to control the aspect ration could be added to queryMap and used in media formats object. Currently in my example in mediaQueryMap you can see minAspectRatio: 'min-aspect-ratio', and maxAspectRatio: 'max-aspect-ratio',. It is really up to users implementation and needs, the aspect-ratio without min/max prefix could be added and used. The service just combine together all the queries provided in config, and returns as a string:

 /**
   * Generates a CSS media query string from the given PictureElementQueries object.
   *
   * @param {PictureElementQueries} queries - An object containing media query properties.
   * @returns {string} A string representing the CSS media query.
   *
   * This method constructs a media query string by mapping the provided query properties
   * to their corresponding CSS media query features and joining them with "and".
   */
  protected generateMediaQuery(queries: PictureElementQueries): string {
    const queryMap = this.config?.mediaQueryMap;

    if (!queryMap) {
      return '';
    }

    return Object.keys(queries)
      .filter((key) => key in queryMap && queries[key] !== undefined)
      .map((key) => {
        const mediaFeature = queryMap[key];
        const value = queries[key];

        return `(${mediaFeature}: ${value})`;
      })
      .join(' and ');
  }

retina_desktop: {
      minWidth: '1440px',
      minDevicePixelRatio: 2,
      aspect-ratio: '9/16'
    },

If to talk about examples I provided, it was add only as an example of usage. Most of this properties will not exist in the default config, as well as formats name will not be changed. At least for now. With this PR we only want to provide additional flexibility for users, thanks to additional config for picture tag and configurable media queries. With this it should be enough flexible I think so will not need to override media component or service. Thats why, most probably that for now, such formats as retina_mobile, retina_desktop and queries for handling this will be removed before some backend changes.

I should ask about width and height. I think I didn't get it.

The only missing information in my opinion for the picture case is the ratio (or width / height) of the image sizes for this bundle. Passing it as input to the media component will not work, since it is potentially different for every size of the bundle (since picture element would potentially only be used when art direction is needed.

Why are you saying that passing it as an input will not work? for example if media component renders the img tag:

<img src="example.jpg" alt="A description of the image" width="600" height="400"
     srcset="example-small.jpg 300w, example-medium.jpg 600w, example-large.jpg 1200w"
     sizes="(max-width: 600px) 300px, (max-width: 1200px) 600px, 1200px">

Can't understand why it is not possible to pass it as an input to such component?

Thanks again :)

@rmch91 ok perfect, the logic to generate the media queries sounds good.

Regarding Aspect Ratio, that works well for the case where the media component renders an img tag:

<cx-media [container]="myMedia" [width]="600" [height]="400" [sizes]="(max-width: 600px) 300px, (max-width: 1200px) 600px, 1200px" />

// output
<img src="example.jpg" alt="A description of the image" width="600" height="400"
     srcset="example-small.jpg 300w, example-medium.jpg 600w, example-large.jpg 1200w"
     sizes="(max-width: 600px) 300px, (max-width: 1200px) 600px, 1200px">

But when rendering a picture tag with the media component, it is not working to pass in width and height, because the width and height will be different for every image format (Because the image ratio is different because of the art direction).

Lets say, that we want to render a banner component having the following images depending on the viewport (Basically what is used on the spartacus demo shop banner on the homepage):

Viewport XS: 4:3 Ratio
Viewport MD: 16:9 Ratio
Viewport LG: 16:6 Ratio

<cx-media [container]="myMedia" />

// output
<picture>
    <source media="(min-width: 1440px)" srcset="image-1920-16-9.jpg 1x, image-3890-16-9.jpg  2x" width="1920" height="720">
    <source media="(min-width: 1024px)" srcset="image-1440.jpg 1x, image-2800.jpg  2x" width="1440" height="540">
    <source media="(min-width: 768px)" srcset="image-1024.jpg 1x, image-1920.jpg 2x" width="1024" height="576">
    // fallback image
   <img src"image-1440.jpg" loading="lazy" fetchpriority="auto" decoding="auto" width="1440" height="540">
</picture>

As you can see in the above output, every source tag needs it's own width / height parameters to define the correct image ratio in the specific viewport. Because when the browser loads the page on a mobile device, you want him to reserve the correct space as an aspect ratio box of 4:3 until the image has been finished loading.

Therefore, having just width & height input params on the media component is not sufficient for this case.

Do you understand?

Ok, now I see, so in this case we could add it to config

  pictureElementFormats: {
    mobile: {
      mediaQueries: {
        maxWidth: '767px',
      },
      width: 100,
      height: 100,
    },
    tablet: {
      mediaQueries: {
        minWidth: '768px',
        maxWidth: '1024px',
      },
      width: 100,
      height: 100,
    },
    desktop: {
      mediaQueries: {
        minWidth: '1025px',
        maxWidth: '1439px',
      },
      width: 100,
      height: 100,
    },
    widescreen: {
      mediaQueries: {
        minWidth: '1440px',
      },
      width: 100,
      height: 100,
    },
    retina_mobile: {
      mediaQueries: {
        maxWidth: '786px',
        minDevicePixelRatio: 3,
      },
      width: 100,
      height: 100,
    },
    retina_desktop: {
      mediaQueries: {
        minWidth: '1440px',
        minDevicePixelRatio: 2,
      },
      width: 100,
      height: 100,
    },
  },

100 it just a dummy value of course, just to show the structure.

OK, perfect, like this it can work..

SAP / spartacus