Insert link accessibility #1933

mehigh · 2017-07-18T08:39:20Z

Description

Fixes #694 by improving the screen reader accessibility of the element.
My updates apply to the add link and edit link pop-out elements,
handling the following aspects:

The buttons (Apply, Edit, Remove) now have the aria-label attribute
added in for accessibility purposes. There is no visual change for them.
The URL input now has an ID that makes use of the withInstanceId
feature from components
The URL input now has a screen-reader-text label which uses the ID on
the for attribute to improve the screen reader accessibility of the
element. This label is not visible.

How Has This Been Tested?

I used the browser's inspector (screenshots reflect that) to confirm
the presence of the a11y attributes and screen-reader-text elements on
both the add link and edit link forms.

The updates done in blocks/editable/format-toolbar.js were visible in
the add link / edit existing link workflow in the text block, just like
the screenshots show.

I wasn't able to figure out where is the
"blocks/library/button/index.js" actually used. It has in the edit
function a form with a class of editable-format-toolbar__link-modal,
just like the one generated with the insert/edit URL.

Locally only I tried adding an extra class on the
form.editable-format-toolbar__link-modal and trying to hunt it in the
browser, but I wasn't able to stumble upon it myself. That's why the
changes done on that file are minimal (added the button aria-label and
added a and added an ID to the URL input element itself,
without making use of withInstanceId - because there is no export
default I could add the withInstanceId call around).

Screenshots:

Add link form:

Edit link form:

Types of changes

New feature (non-breaking change which adds functionality).

Checklist:

My code is tested.
My code follows the WordPress code style.

## Description Fixes WordPress#694 by improving the screen reader accessibility of the element. My updates apply to the add link and edit link pop-out elements, handling the following aspects: - The buttons (Apply, Edit, Remove) now have the aria-label attribute added in for accessibility purposes. There is no visual change for them. - The URL input now has an ID that makes use of the withInstanceId feature from components - The URL input now has a screen-reader-text label which uses the ID on the for attribute to improve the screen reader accessibility of the element. This label is not visible. ## How Has This Been Tested? I used the browser's inspector (screenshots reflect that) to confirm the presence of the a11y attributes and screen-reader-text elements on both the add link and edit link forms. The updates done in blocks/editable/format-toolbar.js were visible in the add link / edit existing link workflow in the text block, just like the screenshots show. I wasn't able to figure out where is the "blocks/library/button/index.js" actually used. It has in the edit function a form with a class of editable-format-toolbar__link-modal, just like the one generated with the insert/edit URL. Locally only I tried adding an extra class on the form.editable-format-toolbar__link-modal and trying to hunt it in the browser, but I wasn't able to stumble upon it myself. That's why the changes done on that file are minimal (added the button aria-label and added a <label for> and added an ID to the URL input element itself, without making use of withInstanceId - because there is no export default I could add the withInstanceId call around). ## Screenshots: Add link form: ![Add link](http://files.urldocs.com/share/add-link/add-link.jpg "Add link form") Edit link form: ![Edit link](http://files.urldocs.com/share/edit-link/edit-link.jpg "Edit link form") ## Types of changes New feature (non-breaking change which adds functionality). ## Checklist: - [x] My code is tested. - [x] My code follows the WordPress code style.

mehigh · 2017-07-18T08:41:38Z

@aduth
This is the exact code diff as in
#1575
but in a clean PR to avoid all of the merge commits.

I did this clean as this reached a point of completion for the requested feature.

aduth · 2017-07-18T12:47:17Z

blocks/editable/format-toolbar.js

@@ -151,17 +152,24 @@ class FormatToolbar extends Component {
 						className="editable-format-toolbar__link-modal"
 						style={ linkStyle }
 						onSubmit={ this.submitLink }>
+						<label


Is there any difference between a visually-hidden label for screen reader purposes, vs. using the aria-label attribute on the input itself?

@afercia Would you have insight here?

It really depends on a case by case basis. For form elements, a proper <label> element is always preferable. I would say a proper visible label 🙂 (so they can also be clicked). On the other hand, screen-reader-text is basically a hack, the only advantage is that it's understandable by any software reading the page source, even if the software doesn't understand ARIA. Still a hack though. There are historical reasons for screen-reader-text, since a few years ago not all the major screen readers fully supported ARIA. Today, aria-label is more standard, but one of the first rule of ARIA is: do not use unless you really really have to.
Both a <label> element and an aria-label element contribute to the accessible name calculation and are equivalent in this regard. Overall, in this specific case, given the design constraints, I'd say go with aria-label.

I have't found a resource to list what the current assistive technology software is supporting, and whether aria-label functions in a similar way to the visually hidden label.

The aria is the "W3C" way to handle such a scenario.

I've just tested VoiceOver on the mac and it reads out loud the aria-label, just like it does with the hidden label.

I've looked at a survey from 2 years ago:
http://webaim.org/projects/screenreadersurvey6/
and the charts were lead by commercial screen reader software applications - Jaws / ZoomText and Window Eyes.
Free solutions like NVDA and VoiceOver had a lower usage percentage.

And if this worked in VoiceOver, I would say that the commercial ones should support this just as well.

Really good input on this, @aduth !

@afercia
My opinion is that if JAWS (the screen reader which used to be the only one in town) supports aria-label, we should go ahead and use that instead of the visually hidden ones.
Is there any way we can find out?

I've asked a tester and the overall input was that ARIA is well supported amongst the popular screen readers.

@mehigh there's no need to ask around, we know the stuff 😉 what I've said is that a few years ago ARIA was not fully supported yet, today it is. We should also consider that a percentage of users might still use old screen reader versions since, especially the commercial ones like JAWS, they are pretty expensive. However, I'm OK with using aria-label but if you want to learn more you should really read the 5 rules of ARIA usage:
https://www.w3.org/TR/using-aria/
First one:

If you can use a native HTML element [HTML51] or attribute with the semantics and behaviour you require already built in, instead of re-purposing an element and adding an ARIA role, state or property to make it accessible, then do so.

That'e because ARIA shouldn't ever be used as something to "magically" fix things: always prefer native HTML instead.

@afercia
I think it makes sense to use aria-label in this specific environment because it simplifies the implementation by quite a bit.
My only hope is that JAWS has support for aria, just like how the other popular screen readers do.

Latest JAWS version support ARIA, since a few years.

because it simplifies the implementation by quite a bit

This is really the latest reason why one should use ARIA 🙂 and that's what I was trying to explain above.

@afercia I could easily plug those things in, they were well implemented. I thought the overhead of the instances ids might not worth the much more complex flow of having the labels separate.

So whatever you decide @afercia / @aduth , let me know and I'll let this ready for merging.

aduth · 2017-07-18T12:47:55Z

blocks/library/button/index.js

@@ -60,20 +61,27 @@ registerBlockType( 'core/button', {
 					<form
 						className="editable-format-toolbar__link-modal"
 						onSubmit={ ( event ) => event.preventDefault() }>
+						<label


Same question as above. Might avoid need for instanceId altogether.

…he input

mehigh · 2017-07-18T13:46:49Z

I've added the aria-label. Thanks for your reference, afercia. I'll read it through.

aduth · 2017-07-18T19:15:40Z

blocks/editable/format-toolbar.js

@@ -155,13 +155,17 @@ class FormatToolbar extends Component {
 							autoFocus
 							className="editable-format-toolbar__link-input"
 							type="url"
+							aria-label={
+								/* translators: accessibility text */


I don't know that translator comment is necessary here, because unlike something like "(opens in new window)", "URL" is not a string that needs clarification by its context here.

I just discarded the translator comment.

aduth

The back-and-forth is a bit unclear, but my impression is that we're okay to move forward with aria-label, in which case this looks good to me.

* Update ref * Update ref to point to gutenberg master

mehigh mentioned this pull request Jul 18, 2017

Insert link accessibility #1575

Closed

2 tasks

694 - Fix formatting

9f03b8e

aduth reviewed Jul 18, 2017

View reviewed changes

694 - Removed visually hidden labels, added aria-label attribute on t…

97de6c4

…he input

694 - Reverted the edit function declaration

3d9f396

aduth reviewed Jul 18, 2017

View reviewed changes

694 - Dropped accessibility text comment

2841b37

aduth approved these changes Jul 18, 2017

View reviewed changes

aduth merged commit 66a865b into WordPress:master Jul 18, 2017

ceyhun pushed a commit that referenced this pull request Apr 22, 2020

Add Buttons block (#1933)

55ac1bd

* Update ref * Update ref to point to gutenberg master

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Insert link accessibility #1933

Insert link accessibility #1933

mehigh commented Jul 18, 2017 •

edited

mehigh commented Jul 18, 2017

aduth Jul 18, 2017

afercia Jul 18, 2017

mehigh Jul 18, 2017

mehigh Jul 18, 2017

mehigh Jul 18, 2017

afercia Jul 18, 2017

mehigh Jul 18, 2017

afercia Jul 18, 2017

mehigh Jul 18, 2017

aduth Jul 18, 2017

mehigh commented Jul 18, 2017

aduth Jul 18, 2017

mehigh Jul 18, 2017

aduth left a comment

Insert link accessibility #1933

Insert link accessibility #1933

Conversation

mehigh commented Jul 18, 2017 • edited

Description

How Has This Been Tested?

Screenshots:

Types of changes

Checklist:

mehigh commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehigh commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth left a comment

Choose a reason for hiding this comment

mehigh commented Jul 18, 2017 •

edited