Provide a medium size image in whitehall image definitions #2534
Conversation
| type: "string", | ||
| }, | ||
| }, | ||
| "$ref": "#/definitions/image", |
There was a problem hiding this comment.
I'm a JSON schema amateur, so just checking my understanding of this.
- Image is NOT a required property in either the shared definition or in the topical event (before this change)
- If the shared definition is included, image can still be optional, but if the image object is present, then
urlis required - Not all topical event have images, which is fine because we handle that in the presenter
Can you deploy this to integration and try to publish a topical event with an without an image to make sure?
There was a problem hiding this comment.
yes i think you're understanding is correct.
totally, i'll deploy to integration and test it
There was a problem hiding this comment.
tested and nothing breaks, but i need to deploy this pr in order to actually see values for the new images alphagov/whitehall#8432
| type: "string", | ||
| }, | ||
| }, | ||
| "$ref": "#/definitions/image", |
There was a problem hiding this comment.
Could this change to topical events go in a separate commit?
| @@ -35,6 +35,11 @@ | |||
| type: "string", | |||
| format: "uri", | |||
| }, | |||
| medium_resolution_url: { | |||
There was a problem hiding this comment.
Is this definitely applicable/useful to all Whitehall schema types that include this definition?
There was a problem hiding this comment.
I had a chat with some frontendy people - Kuba, Max - and they clearly expressed desire to provide srcset attribute in the ui so that different devices could choose the most appropriate image size. For example the Coronation page was difficult to make look nice because the image in its small resolution looked pixelated.
I think this is useful for all types because with this change, in the future, all types could publish at least 3 sizes, and then rendering world could utilize those 3 sizes to create a srcset attribute.
Please see this pr for an example: alphagov/collections#3426
tunylund
force-pushed
the
provide-more-image-resolutions-for-topical-events
branch
from
39dcd91 to
4d22e87
Compare
.idea/.gitignore
Outdated
| @@ -0,0 +1,8 @@ | |||
| # Default ignored files | |||
There was a problem hiding this comment.
A bunch of IntelliJ files have snuck in here.
Idk whether we should choose to add them to the .gitignore, or whether everyone should do https://sebastiandedeyne.com/setting-up-a-global-gitignore-file
There was a problem hiding this comment.
oh gosh how embarassing
Public pages look better when they display images with various options for different devices. For example if an img element provides a srcset attribute, high pdi phones can opt to use the higher resolution image instead of a smaller image. Atm Topical Event pages use this. It is desireable to have more pages use this feature. Whitehall needs to publish each image resolution and that requires this schema change.
Topical Event pages use srcset attribute to provide different image resolutions for different devices. This changes allows Whitehall to publish 3 different image resolutions so that topical events can then use them.
tunylund
force-pushed
the
provide-more-image-resolutions-for-topical-events
branch
from
4d22e87 to
878e8d2
Compare
tunylund
deleted the
provide-more-image-resolutions-for-topical-events
branch
Public pages look better when they display images with various options for different devices. For example if an img element provides a srcset attribute, high pdi phones can opt to use the higher resolution image instead of a smaller image. Atm Topical Event pages use this. It is desireable to have more pages use this feature.
Whitehall needs to publish each image resolution and that requires this schema change.
Trello