Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding src-set in image block #1553

Closed
wants to merge 4 commits into from
Closed

Adding src-set in image block #1553

wants to merge 4 commits into from

Conversation

iFlameing
Copy link
Member

Fixes #1428

Copy link
Contributor

@tiberiuichim tiberiuichim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image scales have been traditionally configurable. It would be nice if there was at least a setting to tweak in the config.settings, to set the available image size for that Plone backend.

@sneridagh
Copy link
Member

@tiberiuichim the idea is that using srcsets the browser decide which size to display, depending on the container current width, so the user don't have to set them anymore.

The proposed sizes range are thought to cover the major range of possibilities that these specific components will handle. Custom components will have to define new, if required, then use them (ideally using srcsets too).

We will go from choosing scales, to choose the size of the containers, and that's in the layout of each component. I am missing something or some use case in these components that should be configurable?

@tiberiuichim
Copy link
Contributor

@sneridagh thanks for the explanation, LGTM

@tisto
Copy link
Member

tisto commented Jun 14, 2020

@iFlameing could you please rebase your branch?

@mister-roboto
Copy link

@dependabot[bot] you need to sign the Plone Contributor Agreement in order to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html

@iFlameing
Copy link
Member Author

@tisto I rebased my pr :)

@iFlameing
Copy link
Member Author

iFlameing commented Jun 15, 2020

@datakurre I am trying to make the image responsive in Volto. But when I am checking with respImage lint it is falling. can you tell me why?
Screenshot from 2020-06-15 20-30-03

@datakurre
Copy link
Member

@iFlameing Have you also configured scales at Plone control panel as in the commit messages of datakurre/my-volto-app@89c86ba

<picture>
<source
srcSet={`
${flattenToAppURL(data.url)}/@@images/image/mini 200w,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe save flattenToAppURL(data.url) to an intermediary var? This way code is shorter and optimized

@tisto
Copy link
Member

tisto commented Jun 19, 2020

@datakurre @iFlameing I guess we have to add those new scalings to kitconcept.volto before we merge this PR. The question is what happens if those scales are not available in the backend? I'd guess the browser would just fail to load them which kind of sucks since we essentially break things for anybody who is not using kitconcept.volto in its latest version. Therefore we might have to consider waiting with this until we do another major Volto release. What do you think?

@datakurre
Copy link
Member

datakurre commented Jun 19, 2020

Only two of those sizes are completely missing: great and huge. But because those scales should also be available in REST API data, would it make image tiles too complex if they only render the sizes that are really available? But sure the experience would still be bad, because the original scales sizes don’t double well as x2 or x3 sizes compared to those in my example 🤔

The original issue, for sure, are the outdated default scales in Plone :(

@sneridagh
Copy link
Member

Superseded by #3337

@sneridagh sneridagh closed this Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PLIP: Implement srcset and sizes for images in Volto
6 participants