isotope / core

Core repository of Isotope eCommerce, an eCommerce extension for Contao Open Source CMS
https://isotopeecommerce.org
136 stars 107 forks source link

Fix responsive images definitions when loaded from database #2420

Open contaoacademy opened 1 year ago

contaoacademy commented 1 year ago

Currently, the pixel densities of the images are correctly used for the gallery.

However, there is no 100% support for all features from Contao. For example, images are not converted to webp. Also, the lazy attribute is not added in the source code

screen-20230216_IYiy5I4j

screen-20230216_xWEmmxjD

fritzmg commented 1 year ago

I can't confirm the issue. Format conversion works as expected.

fritzmg commented 1 year ago

I can confirm that this issue occurs if you use an image size from the database 👍

When using an image size from the bundle config it works.

rburch commented 1 year ago

I'm not sure what you mean by use image size from the database vs bundle config but conversion and lazy don't work for me either.

fritzmg commented 1 year ago

In Contao you can define responsive image sizes via the database - i.e. via the back end in your theme - or via the contao/core-bundle bundle configuration under contao.image.sizes.

rburch commented 1 year ago

I'm configuring it in the Contao Image Theme settings. So that must be pulling from the database once configured.

contaoacademy commented 1 year ago

You could do the following in config.yml. Example:

# config/config.yaml
contao:
  image:
    sizes:
      shop-main-image:
        width: 300
        height: 300
        resize_mode: crop
        zoom: 100
        css_class: configyml
        lazy_loading: true
        densities: 1.5x, 2x
        formats:
          jpg: [ webp, jpg ]
          png: [ webp, png ]
aschempp commented 1 year ago

@fritzmg any idea what's causing this?

fritzmg commented 1 year ago

Haven't looked too much into it yet. In theory it should work. While Isotope still uses the deprecated Image::create and Picture::create methods, they are supposed to support image sizes from the database (in whichever format).

fritzmg commented 1 year ago

It's actually a bug in Contao itself. Contao\Picture does not support formats from the database as they are not being set here: https://github.com/contao/contao/blob/d428d705a30fa0a6c81a39f90a21a19b2f7f0de1/core-bundle/src/Resources/contao/library/Contao/Picture.php#L223-L236 For formats defined by sizes from the config it works, because the contao.image.picture_factory takes care of that.

However, since Contao\Picture is deprecated, I am not sure if this should be fixed there at all. @ausi what do you think?

ausi commented 1 year ago

However, since Contao\Picture is deprecated, I am not sure if this should be fixed there at all.

As the contao.image.picture_factory exists since Contao 4.3, Isotope should use the service instead of Contao\Picture I think. Not sure if we should add new features to Contao\Picture, but if it can be done easily I think I’d be in favor of adding it to make it more consistent (with the image sizes from the config).

fritzmg commented 1 year ago

We would have to copy this code: https://github.com/contao/contao/blob/d428d705a30fa0a6c81a39f90a21a19b2f7f0de1/core-bundle/src/Image/PictureFactory.php#L153-L176

However, since Isotope requires at least Contao 4.9, it should use the contao.image.picture_factory instead imho.