Coming from #1918994: Improve Datetime and Daterange Widget accessibility we need to be able to associate description fields with titles in a semantic way.
Right now it's something like:
<table class="field-multiple-table responsive-enabled tabledrag-processed tableresponsive-processed" id="field-text-values--3">
...
</table>
<div class="description">This is amazing text. It should have ARIA.</div>And it should be more like:
<table class="field-multiple-table responsive-enabled tabledrag-processed tableresponsive-processed" id="field-text-values--3" aria-describedby="multiple-text-id">
...
</table>
<div class="description" id="multiple-text-id">This is amazing text. It should have ARIA.</div>
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | allowed_number_of-2273671-20.patch | 4.21 KB | mgifford |
| #16 | aria-decribedby-before.png | 51.27 KB | mgifford |
| #16 | aria-decribedby-after.png | 53.86 KB | mgifford |
| #14 | interdiff-2273671-11-14.txt | 896 bytes | jeroent |
| #14 | allowed_number_of-2273671-14.patch | 4.19 KB | jeroent |
Comments
Comment #1
mgiffordComment #2
mgiffordComment #3
tim bozeman commented:D Dibs!
Comment #4
tim bozeman commentedI added the aria-describedby to the table field-date-values and the ID to the description.
Comment #5
mgiffordLooks good to me!
EDIT: No UI change and removes hard baked CSS classes in the templates.
Comment #6
catchI think we should add test coverage for this.
Comment #7
tim bozeman commentedWhere could we put a test for this? A new file in the forms tests?
Comment #9
tim bozeman commentedD:
Comment #11
strykaizerReroll for current head attached
I adjusted the code a bit to match the style used in fieldset.html.twig and included the new twig markup in the classy theme too.
Comment #12
mgiffordComment #13
mgiffordBot likes it. Code looks good. An interdiff between 4 & 11 would be nice, but it is pretty simple.
This still needs tests though so moving it to Needs work.
Comment #14
jeroentCreated test that checks if the aria-describedby attribute is effectively set on multiple value widgets.
Patch allowed_number_of-2273671-14-test-only.patch is only the test so this one should fail.
Comment #16
mgiffordI'm ready to mark this RTBC. I've uploaded both the before/after images. This is a nice semantic improvement. Thanks for adding the tests @JeroenT.
Comment #20
mgiffordThis should be just the same once I got rid of the fuzz
Hunk #2 succeeded at 24 with fuzz 2.Comment #21
strykaizerLooks good, thanks all!!
Comment #22
webchickCommitted and pushed to 8.0.x. Thanks!