set sizes for picture-tags, fixes #179 #180
Conversation
|
@MrTango 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! |
MrTango
force-pushed
the
mrtango-image-sizes
branch
2 times, most recently
from
affa538 to
4e707b9
Compare
|
@jenkins-plone-org please run jobs |
MrTango
force-pushed
the
mrtango-image-sizes
branch
from
4e707b9 to
6e5cd31
Compare
| scale_url = self.update_src_scale(src=src, scale=scale) | ||
| source_srcset.append(f"{scale_url} {scale_width}w") | ||
| source_tag = soup.new_tag("source", srcset=",\n".join(source_srcset)) | ||
| if not sizes: | ||
| sizes = f"(min-width: 576px) {target_width}px, 98vw" |
There was a problem hiding this comment.
There was a problem hiding this comment.
this fallback should be the same value as here https://github.com/plone/plone.base/pull/80/files#diff-935b88b2d094796541820b59f37748d9c86c288326d234e94b20de4ff09da231R1777
| scale_url = self.update_src_scale(src=src, scale=scale) | ||
| source_srcset.append(f"{scale_url} {scale_width}w") | ||
| source_tag = soup.new_tag("source", srcset=",\n".join(source_srcset)) | ||
| if not sizes: | ||
| sizes = f"(min-width: 576px) {target_width}px, 98vw" |
There was a problem hiding this comment.
this fallback should be the same value as here https://github.com/plone/plone.base/pull/80/files#diff-935b88b2d094796541820b59f37748d9c86c288326d234e94b20de4ff09da231R1777
There was a problem hiding this comment.
The only thing I see here is that if I request to create the picture tag without requesting any picture_variant but I request an explicit sizes, the generated img tag lacks the srcset and returns the original image in its src.
I understand not requesting any picture_variant and passing an explicit sizes as a way to render the full picture tag with the source with its srcset but passing a custom sizes instead of relying on predefined picture variants, which gives the template developer full control over the rendered HTML code.
This is what I wanted to achieve also in #170 but rendering a single img instead of a full picture.
|
Moreover, I have a question: If after these changes we are just rendering one This way the code is much simpler and also is the configuration of picture variants. |
|
The idea is, that the picture variants are defines, as they needed by a designer/integrator or Developer. Calling the picture method without a picture variant does not make sense in my world. Maybe let's have a short call these days on discord. |
|
I'll give a talk about the picture variants at the German Plone-Tagung and will probably also hold again in English, maybe at PloneConf if i can make it. |
I see your point, but that requires developers to predefine the picture variants in the controlpanel, while, as I see, just calling the method and passing the required Anyway, we can talk durinng the Beethoven sprint also :) |
also: allow to set lazy to false, to suppress the loading="lazy" attribute
this needs to be merged together:
plone/plone.outputfilters#86
plone/plone.base#80