Add image srcset definition and json schema #5

MrTango · 2022-04-08T16:42:41Z

needs an upgrade step in plone.app.upgrades to load the config

Sorted the attributes alphabetically.

tisto · 2022-04-21T14:16:16Z

Just for the record. We are adding a new srcset configuration option for Classic here and it totally unclear how this would relate to the srcset handling and configuration of Volto. If we continue to move in that direction, we risk that we will end up with two srcset configurations in the Plone control panel (one for Classic and one for Volto).

If we continue to treat control panels in Plone 6 as "Classic only" and ignore Volto in the picture I honestly don't see how we could keep both together in the long run.

We plan to address the handling of control panels in Volto in general during Beethoven Sprint in the first week of May. Adding new options does not make our job easier, especially not if they are conflicting with settings that we plan to have in Volto as well.

I don't want to hold back the development of Classic here and srcsets are for sure a useful feature. Though, if we don't find a solution for these kinds of problems, my fear is that we will end up with a fork of Volto and Classic at some point.

The bottom line is this: if our plan is to continue to add new features to Plone Classic and we don't have the time or energy to sync our efforts, we should say this clearly right away and then live (and plan) with the consequences.

It is essential IMHO that the Plone community understands that the @plone/volto-team is always on the "weak end" of things here. We rely on Plone Core/Base/Classic and we need to take any change into account and make sure things work and do not affect Volto in a negative way.

If we treat Plone 6 (Volto) as a product, we must not ship a control panel setting that has no effect whatsoever (I am aware that we already have lots of those anyways and that we need a solution to this in general).

@plone/framework-team I'd like to put this on the agenda for our next meeting.

jensens · 2022-04-21T14:45:41Z

It might be a good idea to tackle this at the connected Bonn/Bukarest sprint?

sneridagh · 2022-04-21T15:58:24Z

My 5c:

@nzambello worked his ass off in:
plone/volto#2103

This PR is still not merged, but it would be amazing if we syncronize the efforts and do not come up with different implementations/solutions. Also, I do not see why it's needed so many "config" in the panel... (in fact, a full control panel at all). Nicola didn't need that much, I think he's only working with the current scales and calculating CSS from that.

Also, if we rely on integrators to come up with the right CSS for each available scale, instead of guiding them, the most probable thing that will happen is that any won't work at the end.

We will hopefully finish the PR during the upcoming Beethoven sprint.

giuliaghisini · 2022-04-22T06:56:47Z

Completely agree with @sneridagh.
There's that implementation of srcset from @nzambello that we have been using in our Redturtle's site for a while now...
It doesn't need particular configuration, only a definition of sizes in config.settings, but i think it could be improved reading that data from plone images controlpanel...

src/plone/base/interfaces/controlpanel.py

ale-rt · 2022-04-22T07:13:56Z

src/plone/base/interfaces/controlpanel.py

@@ -57,13 +60,55 @@ def dump_json_to_text(obj):
 Disallow: /*view$
 """

+IMAGE_SRCSET_SCHEMA = json.dumps(


This isn't exactly the most beautiful thing to see...

Why? B/c it is on import time?

Do we need this variable at all?
This is just a string, why not having it the image_srcsets field?

I think it would look ugly in the field definition because there is so much indentation. Calling black on it could even make it worse due to the line length. And we then have two big dictionaries within the field.

ale-rt · 2022-04-22T07:15:26Z

src/plone/base/interfaces/controlpanel.py

+        default={
+                "large": {
+                    "title": "Large",
+                    "preview": "++theme++barceloneta/static/preview-image-large.png",


This mentions barceloneta, I am not really sure we want to have it in plone.base

Well. difficult. I think we can go with it, but it is a bit ugly to forward reference defaults.

Thinking further, maybe it would be better to not have any complex defaults in here and configure them in the XML in Products.CMFPlone?

I think that it would be better to have this setup by barceloneta/volto generic setup.

so far it's mor of an example, we can leave this property out of core anyway.
This is meant to have a preview in the UI.

maybe it is time for a site profile package in the core? So for classic we could have this and for volto there can be others? Usually it is done out of core for site installations but here we will have different setups. Worth it?

jensens · 2022-04-22T09:25:33Z

Also, I do not see why it's needed so many "config" in the panel... (in fact, a full control panel at all). Nicola didn't need that much, I think he's only working with the current scales and calculating CSS from that.

I think we need this. CSS is not enough to deliver different different images for different resolutions. You for sure can strip this down, but its not how modern web works. In our recent project I worked together with a young webdev who wondered why Plone is so limited when it comes to modern srcset and scaling, even why no webp is available. Here we are on multiple levels in the past and need to improve. I talked with @MrTango and others about this and he really looked into the current state of image delivery.

So: CSS is possible, but not enough. Really.

mauritsvanrees · 2022-04-22T10:44:14Z

From the looks of it, the PR from Nicola creates tags like this when you request an image for scale 'large':

<img src="foo/@@images/image/large"
  srcset="foo/@@images/image/listing 16w, foo/@@images/image/icon 32w, ... etc." />

So you get the requested scale, plus in the srcset you get url and width of all available scales.
That seems useful, and we could do this in plone.namedfile as well, perhaps as fallback for when the new setting in the control panel is not filled in.
(You don't get the cacheable uniques image urls this way, but maybe that is solved somewhere else already. Anyway, detail.)

It does not look like this would be in any way affected by the new control panel setting. Nor does this seem to need an extra setting of its own. So I think the Volto team should be fine with this. I hope so. I can certainly misinterpret things, I did not fully read the Volto PR.

If an imaging control panel in Volto reads and shows the IImagingSchema, then it should probably hide the new setting, because Volto does not use that. That might be the only extra work. I expect that there are lots of settings in Plone control panels that have no meaning for Volto, also in earlier Plone versions.

yurj · 2022-04-22T10:54:00Z

I remember when Plone was full of portal_skin folders and features were in the scripts/templates. More than classic/volto thing, this is a client/server. The srcset feature is server side or client side feature or 50/50? Settings are in the ZODB/zcml or in config files/command line parameters?

Co-authored-by: Alessandro Pisa <[email protected]>

tisto · 2022-04-25T06:38:28Z

Let's take @jensens recommendation and discuss this during Beethoven Sprint. Most folks involved in the discussion will be there. I guess this will be hybrid discussion/meeting anyways, so anyone who is interested can join us. I will post updates here.

move _createObjectByType as unrestricted_construct_instance to here

Show Collection, hide Image and File in Navigation

Sorted the attributes alphabetically.

Co-authored-by: Alessandro Pisa <[email protected]>

…ourcesets-settings' into mrtango-image-handling-sourcesets-settings

Co-authored-by: Alessandro Pisa <[email protected]>

…ourcesets-settings' into mrtango-image-handling-sourcesets-settings

MrTango mentioned this pull request Apr 8, 2022

Mrtango tinymce image srcset config WIP plone/mockup#1156

Merged

MrTango force-pushed the mrtango-image-handling-sourcesets-settings branch from 91724ae to 8dcbd0f Compare April 8, 2022 16:45

Show Collection, hide Image and File in Navigation

8c696f3

This was referenced Apr 13, 2022

Add image srcset's configuration to TinyMCE pattern settings plone/Products.CMFPlone#3477

Merged

Add image sourcesets filter plone/plone.outputfilters#49

Merged

MrTango added 2 commits April 19, 2022 17:26

Add image srcset definition and json schema

19a3efc

Allow picture tag and loading attributes in HTMLFilter

2799df3

MrTango force-pushed the mrtango-image-handling-sourcesets-settings branch from 8dcbd0f to 2799df3 Compare April 19, 2022 14:28

MrTango and others added 2 commits April 19, 2022 19:13

Add hideInEditor property in IMAGE_SRCSET_SCHEMA

b4ba912

Allow srcset attribute in HTMLFilter.

f1cfd24

Sorted the attributes alphabetically.

jensens approved these changes Apr 20, 2022

View reviewed changes

ale-rt reviewed Apr 22, 2022

View reviewed changes

src/plone/base/interfaces/controlpanel.py Outdated Show resolved Hide resolved

ale-rt reviewed Apr 22, 2022

View reviewed changes

src/plone/base/interfaces/controlpanel.py Show resolved Hide resolved

ale-rt reviewed Apr 22, 2022

View reviewed changes

Update src/plone/base/interfaces/controlpanel.py

566b2b0

Co-authored-by: Alessandro Pisa <[email protected]>

MrTango and others added 4 commits April 25, 2022 20:17

move IMAGE_SRCSET_SCHEMA into inteface defintion and simplify defaults

552be4f

move _createObjectByType as unrestricted_construct_instance to here

c84d32a

Merge pull request #8 from plone/unrestricted_construct_instance

01c8e58

move _createObjectByType as unrestricted_construct_instance to here

Merge pull request #4 from plone/change_displayed_types_defaults

16e5824

Show Collection, hide Image and File in Navigation

pnicolli mentioned this pull request May 4, 2022

Add Image for srcset and lazy loading plone/volto#2103

Closed

use additionalScales for every source

0e0881a

MrTango and others added 11 commits May 5, 2022 12:09

Add image srcset definition and json schema

5a4f2d4

Allow picture tag and loading attributes in HTMLFilter

33814e5

Add hideInEditor property in IMAGE_SRCSET_SCHEMA

c040156

Allow srcset attribute in HTMLFilter.

865f903

Sorted the attributes alphabetically.

Update src/plone/base/interfaces/controlpanel.py

bd91ba1

Co-authored-by: Alessandro Pisa <[email protected]>

move IMAGE_SRCSET_SCHEMA into inteface defintion and simplify defaults

98afe3f

use additionalScales for every source

0506332

Merge remote-tracking branch 'remotes/origin/mrtango-image-handling-s…

951e867

…ourcesets-settings' into mrtango-image-handling-sourcesets-settings

rename sourceset to source and remove exlucludeScales param

ff2de27

revert naming change of sourceset

71c957f

isort fix

95498f9

MrTango mentioned this pull request May 10, 2022

Image scales catalog metadata plone/Products.CMFPlone#3521

Merged

MrTango and others added 13 commits May 18, 2022 12:54

Add additionalScales in default settings

6205b85

Add image srcset definition and json schema

1ee0b76

Add hideInEditor property in IMAGE_SRCSET_SCHEMA

7b93e61

Update src/plone/base/interfaces/controlpanel.py

f32ef4e

Co-authored-by: Alessandro Pisa <[email protected]>

move IMAGE_SRCSET_SCHEMA into inteface defintion and simplify defaults

47b4304

rename sourceset to source and remove exlucludeScales param

efd1e06

revert naming change of sourceset

f92f1aa

isort fix

aa37bf4

Add additionalScales in default settings

18c55e1

Merge remote-tracking branch 'remotes/origin/mrtango-image-handling-s…

5439896

…ourcesets-settings' into mrtango-image-handling-sourcesets-settings

rename image_srcsets to picture_variants

09457a8

allow sizes attribute in hTML filter

fc2b7d9

remove duplicate scale for small

16dc0e1

mauritsvanrees merged commit a5243b8 into main Jun 16, 2022

mauritsvanrees deleted the mrtango-image-handling-sourcesets-settings branch June 16, 2022 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image srcset definition and json schema #5

Add image srcset definition and json schema #5

MrTango commented Apr 8, 2022 •

edited

Loading

tisto commented Apr 21, 2022

jensens commented Apr 21, 2022

sneridagh commented Apr 21, 2022

giuliaghisini commented Apr 22, 2022

ale-rt Apr 22, 2022

jensens Apr 22, 2022

ale-rt Apr 22, 2022

mauritsvanrees Apr 22, 2022

ale-rt Apr 22, 2022

jensens Apr 22, 2022

jensens Apr 22, 2022

ale-rt Apr 22, 2022

MrTango Apr 22, 2022

yurj Apr 23, 2022

jensens commented Apr 22, 2022

mauritsvanrees commented Apr 22, 2022

yurj commented Apr 22, 2022

tisto commented Apr 25, 2022

Add image srcset definition and json schema #5

Add image srcset definition and json schema #5

Conversation

MrTango commented Apr 8, 2022 • edited Loading

tisto commented Apr 21, 2022

jensens commented Apr 21, 2022

sneridagh commented Apr 21, 2022

giuliaghisini commented Apr 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jensens commented Apr 22, 2022

mauritsvanrees commented Apr 22, 2022

yurj commented Apr 22, 2022

tisto commented Apr 25, 2022

MrTango commented Apr 8, 2022 •

edited

Loading