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.
Currently when #title_display is set to 'invisible' the label generated for the form element comes after the form element in markup. This is confusing for screen-reader users (by which I mean myself). It is also not an intuitive way for a form to be rendered.
I'm not sure why we decided this was a good idea, but it's not, so let's reverse the order so that the invisible label is output prior to the form element.
Comment | File | Size | Author |
---|---|---|---|
#6 | 917220-2.patch | 2.79 KB | mgifford |
#1 | 917220-1.patch | 787 bytes | Everett Zufelt |
Comments
Comment #1
Everett Zufelt CreditAttribution: Everett Zufelt commentedPatch.
Let me know if the indentation is off, it seemed odd to me in the original
Comment #2
Everett Zufelt CreditAttribution: Everett Zufelt commentedComment #4
Everett Zufelt CreditAttribution: Everett Zufelt commentedIf someone would be able to rewrite the test case for this I would be greatly appreciative. Looks like we actually have a test looking for a label after the element, and it needs to look before.
Comment #5
mgiffordI think this addresses why the patch failed.
I don't know how critical this is, but it's probably more standard to have it above.
Comment #6
mgiffordack, forgot to attach the file. Sorry.
Comment #8
mgiffordSimilar failed test as in #123103: Retain #anchors during path alias -> normal path saving
Comment #9
Everett Zufelt CreditAttribution: Everett Zufelt commented#6: 917220-2.patch queued for re-testing.
Comment #10
bowersox CreditAttribution: bowersox commented+1 for this change. @Everett, I agree that this order for the label is much more usable. I reviewed the patch and it looks good to me.
Comment #11
Everett Zufelt CreditAttribution: Everett Zufelt commentedThankfully it passed all the tests this time. @brandonojc can you please RTBC?
Comment #12
bowersox CreditAttribution: bowersox commentedOkay, I tested this and it works well. I can see the different on a page like /admin/people/permissions where it is much more helpful to have the labels precede the checkboxes. This patch appears to work correctly. Nice one!
Comment #13
sun#6: 917220-2.patch queued for re-testing.
Comment #14
sunAlthough badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #15
Everett Zufelt CreditAttribution: Everett Zufelt commentedNot disagreeing. But, if you bump one of my issues provide clear documentation to support your claim.
Comment #16
mgiffordThis patch also seems to be pretty low-risk. I don't see how when a patch improves a user experience & has almost no risk of impacting anyone else why it shouldn't come into core. I can see this being one that shouldn't be brought in with a maintenance/security release as that could break existing tests. However, at this stage, it already passes the bots & improves the experience. It's also been marked RTBC already.
Comment #17
Everett Zufelt CreditAttribution: Everett Zufelt commentedTagging
Comment #18
webchickAre there visual changes associated with this patch? If so, could we have a before/after screenshot? Or is this purely dealing with where on the page the invisible text shows up?
Comment #19
Everett Zufelt CreditAttribution: Everett Zufelt commented@webchick
Absolutely no visual changes, just changes markup.
When #title_display is set to invisible the markup for the invisible label is currently being generated after the form field, this patch moves the markup for the invisible label to before the form field. In either case the label is invisible, hidden with .element-invisible.
Thanks.
Comment #20
mgiffordThere are no visual changes. I can provide a screenshot if you like, but it is just dealing with the invisible text.
Comment #21
webchickOk cool, thanks!
Committed to HEAD!