Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
responsive_image.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
1 May 2015 at 21:01 UTC
Updated:
14 Oct 2015 at 21:24 UTC
Jump to comment: Most recent, Most recent file
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 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 commentedMark sizes as required, default change to "100vw" to reflect the specs
Mark image styles as required as well
Comment #8
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 commented#10 Strange, I'll have another look
Comment #12
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 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 commented#16 Thanks for checking the wording, much better.
Comment #18
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!