Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The spec of picture has changed making sizes a required attribute when using width descriptor in sizes
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | interdiff-2481635-12-16.txt | 1.3 KB | RainbowArray |
| #16 | 2481635-16-sizes-validation.patch | 2.99 KB | RainbowArray |
Comments
Comment #1
RainbowArrayIs there anything we will need to change with this? Are there any situations where we would have provided a srcset with w suffixes without also having a sizes attribute?
Comment #2
attiks CreditAttribution: attiks commentedTwo places:
1/ The fallback image is using srcset, so we need to use sizes as well, unless we change the fallback top src, #2481637: Use src in fallback image
2/ If we add the UI, we need to make sizes a mandatory field with a default value of 100vw
Comment #3
attiks CreditAttribution: attiks commentedMark sizes as required, default change to "100vw" to reflect the specs
Mark image styles as required as well
Comment #8
attiks CreditAttribution: attiks commentedMoved the required to the states, so it works.
Comment #9
Jelle_SWorks great, thanks.
Comment #10
RainbowArrayManually tested this.
If sizes is selected, and you leave an empty sizes attribute, you are immediately prompted to enter a value for that attribute.
However, if you have a value in the sizes attribute, but don't select any image styles, then save, you get a message that the image style has saved, but you aren't returned to the list of image styles. If you then open up the breakpoint options, sizes is no longer selected. So that's confusing.
While there is a required indicator next to every image style (which is confusing: am I supposed to check every image style?), there is nothing that happens if you don't select at least one.
Some sort of onscreen validation on save would be nice, as well as a less confusing indictor for what is required.
Comment #11
attiks CreditAttribution: attiks at Attiks commented#10 Strange, I'll have another look
Comment #12
attiks CreditAttribution: attiks at Attiks commentedValidation added, the checkboxes after each image style is another bug, so I left it, if needed we can remove it.
Comment #13
attiks CreditAttribution: attiks at Attiks commentedAdding related issues for the #states problem:
#1017882: Required elements buggy with #states was supposed to fix it, but is only for D7
#1239930: states.js appends span.form-required to every label of a dependent field has a working patch and I marked it RTBC
Comment #14
Jelle_SRE #10: Wow. I totally missed that. Good to have an other pair of eyes :-)
Comment #15
Jelle_SBesides the states.js stuff (which is out of scope for this issue anyway) it all seems to work. So back to RTBC
Comment #16
RainbowArrayManually tested this, works much better now. Thanks!
Slight tweaks to the error messages.
Comment #17
attiks CreditAttribution: attiks at Attiks commented#16 Thanks for checking the wording, much better.
Comment #18
attiks CreditAttribution: attiks at Attiks commented#16 Thanks for checking the wording, much better.
Comment #19
webchickI was confused on why we needed to be this fancy when #required = TRUE does the job elsewhere.
Marc explained that sizes is only conditionally required if we're not using an image style, and that when we are using sizes we need at least one image style to act as the srcset value. Fair enough.
Committed and pushed to 8.0.x with small grammar fix. Thanks!