Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve accessibility - associate label and fields #22361

Merged
merged 1 commit into from
Jan 4, 2022

Conversation

braders
Copy link
Contributor

@braders braders commented Jan 3, 2022

Overview

The primary purpose of this change was to associate the labels on the event location form with the relevant inputs:

Screenshot 2022-01-03 at 17 41 14

As part of this change, I have also updated the contact forms so that the "ext" text becomes a label for the relevant telephone extension fields.

Before

The labels "Email 1:", "Phone 1:", "Ext" did not have a for attribute, and so were not associated with the input elements. This wasn't the end of the world as there is aria-label attributes in place, but adding the associations improves consistency, and means that mouse users can click on the labels to focus the relevant fields.

After

The labels have for attributes and so are semantically linked with the relevant fields.

Technical Details

I did consider just adding a hardcoded for attribute to the elements within the templates. However this seemed more brittle than using the style {$form.email.1.email.label}. This does mean that CRM_Contact_Form_Edit_Email::buildQuickForm and CRM_Contact_Form_Edit_Phone::buildQuickForm now set labels which may not be relevant to all scenarios where these methods are used. However I considered this ok, and any other contexts will simply ignore the labels unless they are explicitely rendered.

@civibot
Copy link

civibot bot commented Jan 3, 2022

(Standard links)

@civibot civibot bot added the master label Jan 3, 2022
@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Jan 3, 2022
@eileenmcnaughton
Copy link
Contributor

I tried this out on the demo sites & it seems to work fine. I haven't been terribly involved in the accessibility work so gave it merge ready rather than merging in case anyone else wants to weigh in.

@yashodha
Copy link
Contributor

yashodha commented Jan 4, 2022

@braders @eileenmcnaughton looks good, merging this.

@yashodha yashodha merged commit ea5ce20 into civicrm:master Jan 4, 2022
@eileenmcnaughton
Copy link
Contributor

thanks @yashodha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants