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>

Image of text area with multiple values.

CommentFileSizeAuthor
#20 allowed_number_of-2273671-20.patch4.21 KBmgifford
#16 aria-decribedby-before.png51.27 KBmgifford
#16 aria-decribedby-after.png53.86 KBmgifford
#14 interdiff-2273671-11-14.txt896 bytesJeroenT
#14 allowed_number_of-2273671-14.patch4.19 KBJeroenT
#14 allowed_number_of-2273671-14-test-only.patch998 bytesJeroenT
#11 allowed_number_of-2273671-11.patch3.21 KBStryKaizer
#5 Selection_009.png88.76 KBmgifford
#4 datetime-2273671-4.patch1.88 KBTim Bozeman
Text-With-Unlimited-Values.png120.71 KBmgifford
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue summary: View changes
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue summary: View changes
Tim Bozeman’s picture

Tim Bozeman CreditAttribution: Tim Bozeman commented
Assigned: Unassigned » Tim Bozeman

:D Dibs!

Tim Bozeman’s picture

Tim Bozeman CreditAttribution: Tim Bozeman commented
Status: Active » Needs review
FileSize
datetime-2273671-4.patch1.88 KB

I added the aria-describedby to the table field-date-values and the ID to the description.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Reviewed & tested by the community
FileSize
Selection_009.png88.76 KB

Looks good to me!

Aria describedby in html for grouped fields.

EDIT: No UI change and removes hard baked CSS classes in the templates.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

I think we should add test coverage for this.

Tim Bozeman’s picture

Tim Bozeman CreditAttribution: Tim Bozeman commented

Where could we put a test for this? A new file in the forms tests?

mgifford queued 4: datetime-2273671-4.patch for re-testing.

Tim Bozeman’s picture

Tim Bozeman CreditAttribution: Tim Bozeman commented
Assigned: Tim Bozeman » Unassigned

D:

mgifford queued 4: datetime-2273671-4.patch for re-testing.

StryKaizer’s picture

StryKaizer
Primary language Dutch
Location Belgium
CreditAttribution: StryKaizer commented
FileSize
allowed_number_of-2273671-11.patch3.21 KB

Reroll 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.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs work » Needs review
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs review » Needs work

Bot 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.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
allowed_number_of-2273671-14-test-only.patch998 bytes
allowed_number_of-2273671-14.patch4.19 KB
interdiff-2273671-11-14.txt896 bytes

Created 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.

The last submitted patch, 14: allowed_number_of-2273671-14-test-only.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs review » Reviewed & tested by the community
FileSize
aria-decribedby-after.png53.86 KB
aria-decribedby-before.png51.27 KB

I'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.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: allowed_number_of-2273671-14.patch, failed testing.

mgifford queued 14: allowed_number_of-2273671-14.patch for re-testing.

The last submitted patch, 14: allowed_number_of-2273671-14.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs work » Needs review
FileSize
allowed_number_of-2273671-20.patch4.21 KB

This should be just the same once I got rid of the fuzz Hunk #2 succeeded at 24 with fuzz 2.

StryKaizer’s picture

StryKaizer
Primary language Dutch
Location Belgium
CreditAttribution: StryKaizer commented
Status: Needs review » Reviewed & tested by the community

Looks good, thanks all!!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 8a14b89 on 8.0.x 
    Issue #2273671 by JeroenT, mgifford, Tim Bozeman, StryKaizer, catch:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.