Refactor of the new component and the helper method
Renamed both, now it's Image
Loaded low resolution placeholder loading afterwards the with other resolutions using the width descriptor making the browser to decide which image is the best match to load based on the actual size of the image on the page
It doesn't seem like the srcset properly responds to the images. I can see the /listing being loaded, but it's always loading the big image instead of one of the smaller sizes. Maybe the sizes attribute needs to be specified?
So srcset could describe all the available versions and their widths, but it may not be enough for browser to properly choose the size, because browser might load the image before it knows the eventual size of the rendered image area (and loading the largest one just in case). That's why sizes is needed to tell the browser the size of the image to help it to choose the right scale:
Also, because sizing is based on widths, Plone image scaling must provide those widths or images may be upscaled on browser. That's how I ended with image sizes like large 800:65536.
Isn't the @@images/image/listing size too small? 16px, as is the default, is no bigger then a small icon. Yes, it loads fast, but it's also ugly. I suggest maybe use tile (64px) or thumb (128px) as default?
For me the validator results look like I expected: Plone image scaling is too smart for srcset and that's why widths do not match. My indefinitive heights in image scale forces Plone always to scale by width, what should make it match.That should also prevent cropping mentioned by validator. Also sizes-attribute should be set (and that's the hardest one, because its values probably depend on themes).
@datakurre No worries, I haven't yet plugged the new image scales into Plone, this was with the default image scales. I am trying to get a feeling of what this whole problem space is about. The above comment, with the screenshot, wasn't aimed at you. Just documenting things as I encounter them
@nzambello My hope for this PR is that, in the end, it provides a comprehensive solution for responsive images. One issue I have though, and maybe I have read the code wrong, but it seems to only take Plone image content types into account (so basically, only the "image" field). I think this should be handled better and allow also handling content types with either non-standard image field or multiple image fields.
No problem. For me the key for understanding this was to acknowledge that there is no magic involved, but browser really must be able to decide the optimal scale from HTML only.
On 23. Jan 2021, 13.23 +0200, Tiberiu Ichim [email protected], wrote:
@nzambello My hope for this PR is that, in the end, it provides a comprehensive solution for responsive images. On issue I have though, and maybe I have read the code wrong, but it seems to only take Plone image content types into account (so basically, only the "image" field). I think this should be handled better and allow also handling content types with either non-standard image field or multiple image fields.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Another idea: when definining image scales, I think it could be interesting to also define resizing parameters. Looking at these parameters, for example (most important, fit type and positions. Right now this PR only defines width in the image scales, thought kitconcept.volto defines square images as well (which we may want to chose how to fit):
icon 32:32
tile 64:64
thumb 128:128
mini 200:65536
preview 400:65536
teaser 600:65536
large 800:65536
larger 1000:65536
great 1200:65536
huge 1600:65536
The square image formats would need to be cropped.
@sneridagh we now have the correct image scale being loaded only when the image is visible and the placeholder has loaded, so we have the actual size of the image in the page.
I don't fully like this behavior, but I think it's the best way we have without forcing developers to know and set the final image width. It would be an anti-responsive pattern I guess.
I'm not sure if understand the code right, but you did not seem to define sizes right?
I think it would be better, so that the browser knows the image size before it is loaded, even more so when using lazy loading.
I'm currently trying to solve this for classic UI.
On one hand enabling the browser to automatically select the right image scale is awesome.
But i would like to also allow art direction.
In the form of providing different cropped versions for smaller screens, like i could create via plone.app.imagecropping.
Art direction is only possible via source tags in combination with media queries.
This is more explicit and you can force the browser to choose e special scale which contains your cropped image.
With srcset and sizes the browser will choose what ever fit's in the moment, even a bigger scale, when the bigger scale is already in the cache. This way you might see the bigger scale on small screens which is fine from the quality point of view, but might not what you want. The bigger scale does not have the crop and looks bad on mobile.
Here is a good example of that problem and the solution: https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images
The challenge now is to find the right amount of flexibility without making the configuration to complex.
I would like to exchange ideas on that topic.
I'll join the team in Bucharest for the Beethoven Sprint, i hope we can make some progress then.
I'm not sure if understand the code right, but you did not seem to define sizes right? I think it would be better, so that the browser knows the image size before it is loaded, even more so when using lazy loading. I'm currently trying to solve this for classic UI. On one hand enabling the browser to automatically select the right image scale is awesome. But i would like to also allow art direction. In the form of providing different cropped versions for smaller screens, like i could create via plone.app.imagecropping. Art direction is only possible via source tags in combination with media queries. This is more explicit and you can force the browser to choose e special scale which contains your cropped image. With srcset and sizes the browser will choose what ever fit's in the moment, even a bigger scale, when the bigger scale is already in the cache. This way you might see the bigger scale on small screens which is fine from the quality point of view, but might not what you want. The bigger scale does not have the crop and looks bad on mobile. Here is a good example of that problem and the solution: https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images
The challenge now is to find the right amount of flexibility without making the configuration to complex. I would like to exchange ideas on that topic. I'll join the team in Bucharest for the Beethoven Sprint, i hope we can make some progress then.
@MrTango
image sizes are defined here https://github.com/plone/volto/blob/3660875628a9c721994a7de4a10309ad5735f1d9/src/config/ImageScales.jsx and inject in config here https://github.com/plone/volto/blob/3660875628a9c721994a7de4a10309ad5735f1d9/src/config/index.js#L117.
Browser try to determine the size of image here https://github.com/plone/volto/blob/3660875628a9c721994a7de4a10309ad5735f1d9/src/components/theme/Image/Image.jsx#L65 and loads the src-set maximum image size based on available width and pixel ratio.
In Volto we don't have yet the concept of 'cropped image' only the scaled image.
I think crop will be a next feature and is not a theme to take care now on Volto.
Do you set the sizes attribute somewhere?
This is important for the browser to know how big the image will be on the screen.
This will be used to calculate the needed image size by looking at the screen resolution (2x,3x aso) and the Intrinsic_Size of the image.
The image intrinsic size of the image is 800px. On a older 1x desktop screen that means the browser might prefer the image scale: https://picsum.photos/id/1011/800, but on a high dpi screen let's say 2x, the Browser would require the scale https://picsum.photos/id/1011/1600 1600w
If you don't provide the sizes attribute, the intrinsic size has to be calculated from CSS definitions, that takes more time.
PS. I'll be at the sprint in Bucharest the following days, let me know if we could sit together to discuss this and also my ideas for the srcset config in the backend. I made the default config much smaler now, but it is still valid to have it and gives us to the option to improve the frontends in time more.
Do you set the sizes attribute somewhere? This is important for the browser to know how big the image will be on the screen. This will be used to calculate the needed image size by looking at the screen resolution (2x,3x aso) and the Intrinsic_Size of the image.
The image intrinsic size of the image is 800px. On a older 1x desktop screen that means the browser might prefer the image scale: https://picsum.photos/id/1011/800, but on a high dpi screen let's say 2x, the Browser would require the scale https://picsum.photos/id/1011/1600 1600w
If you don't provide the sizes attribute, the intrinsic size has to be calculated from CSS definitions, that takes more time.
PS. I'll be at the sprint in Bucharest the following days, let me know if we could sit together to discuss this and also my ideas for the srcset config in the backend. I made the default config much smaler now, but it is still valid to have it and gives us to the option to improve the frontends in time more.
yes, for example, the html generated is this:
<picture class="volto-image"><source srcset="/huge.jpg/@@images/image/listing 16w, /huge.jpg/@@images/image/icon 32w, /huge.jpg/@@images/image/tile 64w, /huge.jpg/@@images/image/thumb 128w, /huge.jpg/@@images/image/mini 200w, /huge.jpg/@@images/image/midi 300w, /huge.jpg/@@images/image/preview 400w, /huge.jpg/@@images/image/teaser 600w, /huge.jpg/@@images/image/large 800w, /huge.jpg/@@images/image/larger 1000w, /huge.jpg/@@images/image/great 1200w, /huge.jpg/@@images/image/huge 1600w"><img src="/huge.jpg/@@images/image/listing" alt="" class="full-width" role="img" loading="lazy" style="width:100%;object-fit:cover"></picture>
we didn't set the real size of image in attributes because we don't know the real size of image here, we only know the miniature size.. this is the big problem of this implementation and need to think about it..
Another problem that we have here, is that srcset is injected in page only when image meets intersection observer.. it was a way to do the lazy loading of image... but i don't think it's the best way to do it, because this causes two image loading for one image:
the image set in src attribute of (/huge.jpg/@@images/image/listing)
@giuliaghisini I'm sorry for my previous comment (I now deleted) that made no sense.
Regarding sizes, could sizes="100vw" be better than nothing? At least then browser would never choose a size greater than the window itself. (Or does browser act like that by default?)
Or does it already work properly? Just loading listing size at first and the correct scale when CSS has been calculated?
I recall that my old PoC https://github.com/datakurre/my-volto-app/commit/89c86ba93db2ad404f45a7dfc5fc9dde686390e6#diff-3f5fdd396d4f51d380bccc043229925fc08f0b5c5bffa414d8e4e1cf9325e55bR19 only loaded a single scale, but I did not try that with loading="lazy" :thinking:
@giuliaghisini I'm sorry for my previous comment (I now deleted) that made no sense.
Regarding sizes, could sizes="100vw" be better than nothing? At least then browser would never choose a size greater than the window itself. (Or does browser act like that by default?)
Or does it already work properly? Just loading listing size at first and the correct scale when CSS has been calculated?
yes, it could be better than nothing.. it's the sizes of the 'placeholder' slots into which the browser can insert images from the srcset..
Maybe 100vw will be too much, it will cause some 'flickers' area in page when real image is loaded.. i think.. we have to try...
Maybe sizes could be setted in applySrcSet fn where we know the real image size.. but i don't know it could be correct..
what do you think?
@datakurre @MrTango i've added sizes="100vw" as default and make some improvements.
Now i think it's working much better than before..
Could you try it?
for sizes.. i think we cannot calculate them.. it depends on page layout.. i've added sizes as props to pass to the Image component and setted default to '100vw'
I think sizes as a prop is good start. Once someone is really building responsive theme with optimal sec-sets, that someone probably gets an idea, how this should be enhanced.
we didn't set the real size of image in attributes because we don't know the real size of image here, we only know the miniature size.. this is the big problem of this implementation and need to think about it..
while defining the srcset we give all the needed info's by appending the size like 800w of that scale.
The sizes attribute, is not only useful together with media queries, if you leave them of as i did in my example, you go just with the fallback size of 800px. This size is the size the image would take on the screen, no matter of the screen resolution.
On a 1x resolution screen it will be 800px as well as on a 2x resolution screen.
Think of it as the physical size of the image.
Another problem that we have here, is that srcset is injected in page only when image meets intersection observer.. it was a way to do the lazy loading of image... but i don't think it's the best way to do it, because this causes two image loading for one image:
the image set in src attribute of (/huge.jpg/@@images/image/listing)
and the needed image size from srcset.
i don't why you think you need this, if you add the loading="lazy" attribute, the browser should be able to load lazy. Right now chrome will still load to fast all images, firefox and safari will save more data and only load images when you are getting close to them.
So with that in mind, if you want the optimum control and have it working on chrome you have to do it with intersection observer.
For Plone 6 classic UI we will probably stick with the browser defaults and just use loading="lazy".
Chrome will eventually fix the behavior.
I guess when you add the srcset and the src attributes via the observer it should work, the browser can't load anything without either a src or a srcset.
datakurre:
src-set should really just list all the available images and their real pixel width
then
sizes attributes is there for telling the view port size; it supports responsive breakpoints and CSS math
Just with these in place, browser will pick whatever size it sees best fit. I didn't even see 2x or 3x src-set items to be really required, because browser was able know its pixel density and choose the right image, as long as one was available.
not quite, the sizes attribute with out a media query defines the Intrinsic Size of the image, the same you can archive by setting the width attribute on the img tag. But it is true that the Browser is able to do the math for us, by calculating Intrinsic Size * screen resolution >> needed image size.
With these sizes in place, I know that image scaling always scaled by width, src-set was giving correct information and browser was able choose the right size.
sizes attribute, of course, is more tricky, because its optimal value really depends on your theme.
it depends if you want to change the Intrinsic Size for different viewports, most of the time that not needed.
With my example above, the image is 800px on a Desktop. For mobile that resolution works too.
As we need a bit less than 400px usually * screen resolution (somewhere between 2x and 4x), leaves us with needed sizes of (800px up to 1600). Together with the general definition of img {max-with: 100%;} it only make sense when the image is smaller on Desktop than on a smaller screen.
For example if our image is 250px and we want it to be almost full screen on a mobile device.
Than something like this make sense:
sizes="(max-width: 768px) 90vw, 250px".
also one thing i noted while playing around with my example page is that we need to set the height when we are using lazy loading or everything jumps arround :dancers:
I hope we can get together tomorrow on discord for a chat.
volto copied to clipboard
plone
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
