sonata-project / SonataMediaBundle

Symfony SonataMediaBundle
https://docs.sonata-project.org/projects/SonataMediaBundle
MIT License
449 stars 496 forks source link

Changed default srcset behaviour #1386

Closed core23 closed 6 months ago

core23 commented 6 years ago

Subject

The media twig tag is broken.

Steps to reproduce

Define some image formats in the configuration.

Create a template with the following code:

{% media post.image, 'general' %}

Expected results

A simple image is displayed

<img 
src="/uploads/media/news/0001/01/thumb_235_news_general.jpeg" width="1080" height="200">

Actual results

An image with all available formats (include the raw reference format!) is create

<img src="/uploads/media/news/0001/01/thumb_235_news_general.jpeg" 
srcset="/uploads/media/news/0001/01/thumb_235_news_general.jpeg 1080w, /uploads/media/news/0001/01/thumb_235_news_recent.jpeg 260w, /uploads/media/news/0001/01/91f2c4a0f87a8ef98d14de5d7448a05813e823be.jpeg 1280w" 
sizes="(max-width: 1080px) 100vw, 1080px" 
width="1080" height="200">
hug963 commented 6 years ago

This is a feature of the media helper.

It is described in the docs: https://github.com/sonata-project/SonataMediaBundle/blob/3.x/docs/reference/helpers.rst

Why do you think it is an issue?

cussack commented 5 years ago

Try {% media post.image, 'general' with {'srcset': []} %}. But anyway, it should be a good thing to be responsive.

cussack commented 5 years ago

@core23 I found this to be an issue as well now. While it is probably a good thing to render responsive images by default, the reference image should not be included in the srcset. There is no programmatic control over its size and dimensions, so it should not be part of the documented „sensible defaults“.

What do you think? I need to remove it for a project anyway and can contribute the solution here.

core23 commented 5 years ago

There are some pros and cons for the current solution.

In my projects I use the different image dimensions for different pages. So I have a list page where I don't want to render a big size image in any way and a detail page which has a different purpose.

The best way would be a config key with srcset groups:

        default:
            group:
                list: [ 'small', 'icon' ]
                detail: [ 'big', 'reference' ]
            formats:
                small: { width: 100 , quality: 70}
                big:   { width: 500 , quality: 70, resizer: sonata.media.resizer.square}
                # You can pass through any custom option to resizer by using the resizer_options key
                icon:  { width: 32, quality: 70, resizer: your.custom.resizer, resizer_options: { custom_crop: true } }
{% media post.image, 'list' %}
cussack commented 5 years ago

True, that would be the best solution for my use cases as well. (Unnecessary BC break though, I would propose {% media post.image, 'fallback' with { group: 'list' } or better {% media post.image, 'fallback' with { srcset: 'list' }.

Anyway, do we agree that the reference image should only ever be included when it's mentioned explicitly? Then I would propose a two step solution to this issue:

  1. remove reference from default lists
  2. add format groups

Ok?

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

razvantomareea commented 3 years ago

This was also a breaking change for me, before upgrading the images were okay src="/uploads/media/news/0001/01/thumb_235_news_general.jpeg" width="1080" height="200"> after upgrading there is a the new srcset and this caused when user windows display settings is set to a 125% zoom or bigger the images are all strechted.

github-actions[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.