Add a method to the @@images view to render a img tag with srcset #170
Conversation
|
@erral thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
@jenkins-plone-org please run jobs |
erral
changed the base branch from
master
to
config-with-default-template-999269c3
|
@jenkins-plone-org please run jobs |
There was a problem hiding this comment.
LGTM. Two small suggestions about word choice.
@MrTango Could you have a look as well please?
Co-authored-by: Maurits van Rees <maurits@py76.be>
Co-authored-by: Maurits van Rees <maurits@py76.be>
Co-authored-by: Maurits van Rees <maurits@py76.be>
Co-authored-by: Maurits van Rees <maurits@py76.be>
|
@jenkins-plone-org please run jobs |
|
@plone/classicui-team could we get a review from you on this one? I stumbled across this PR and remembered recent work on the Image handling documentation with |
… it will effectively be created when rendering the image
|
I've made a small change. When running the This means that the image will be created only when requested by the browser, which should create a better user experience when loading a page with a lot of images, not needing to create all the scales, but only the required ones. |
|
I have added some documentation both here and also in a PR in the documentation project plone/documentation#1951 |
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
|
Thanks @stevepiercy |
|
@jenkins-plone-org please run jobs |
|
@erral actually, let's kill the docs here, and put a link in the |
|
Then, I will check whether the documentation here is correctly added in the other place or not |
|
Yup, let's merge |
|
@stevepiercy we shouldn't kill the whole file, because it works as a doctest of the package. At least I will purge the "just documentation" part and leave the doctest part as is. |
| @@ -739,6 +739,71 @@ def picture( | |||
| fieldname=fieldname, | |||
| ).prettify() | |||
|
|
|||
| def srcset( | |||
There was a problem hiding this comment.
I see the tag method having a srcset_attribute, I wonder why adding a new method rather than changing the tag one.
There was a problem hiding this comment.
The point is that I didn't want to add a breaking change in the tag method, I want it to keep it working as it is and offer a way for the user who wants to render the srcset automatically, to get that.
I could do the changes to get that also in the tag method, but that would mean to change the semantics of it, so I discarded it.
Co-authored-by: Alessandro Pisa <alessandro.pisa@gmail.com>
|
Thanks for your review @ale-rt ! |
|
@jenkins-plone-org please run jobs |
|
@jenkins-plone-org please run jobs |
Apart from
<picture>tag with several<source>tags that we already provide (according to the configuration set in the images controlpanel) images can also be rendered using asrcsetattribute in theimgtag.In the
srcsetwe can provide an arbitrary number of images, and then we provide asizesattribute to let the browser know how much space it has. With these data, the browser decides which image from thesrcsetto render.This is the approach that Volto has adopted, and I think that providing it as an option for the developers to use is really nice. The developer will just need to call the view with the size they require.