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

Insert link accessibility #694

Closed
afercia opened this issue May 6, 2017 · 5 comments
Closed

Insert link accessibility #694

afercia opened this issue May 6, 2017 · 5 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@afercia
Copy link
Contributor

afercia commented May 6, 2017

See also #679

All form elements need a properly associated label. As per the WordPress accessibility standards, it must be an explicitly associated label (with for/id attributes, not wrapping the field).

The label can be visually hidden. Also, please consider the placeholder attribute is not a replacement for a label, see https://core.trac.wordpress.org/ticket/40331

Regarding the buttons, at the moment they don't have any text or label that can be announced by assistive technologies, they contain just SVG icons, see #528

These buttons need to be labeled, an aria-label attribute or a visually hidden text within the buttons would be appropriate.

when inserting a link:

link insert

when editing a link:

link edit

not sure what to do when editing a link, currently there's no form, just a not-editable link element with an empty href attribute. Waiting for development to complete for further review.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label May 6, 2017
@aduth aduth added Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take labels May 26, 2017
@afercia
Copy link
Contributor Author

afercia commented Jun 7, 2017

Looking back at this I see there's now a field to enter the link URL and seems almost ready (no search for now). However, the button to insert a link:

screen shot 2017-06-07 at 17 53 18

needs to be labeled (an aria-label would work) as well as the edit and unlink buttons:

screen shot 2017-06-07 at 20 32 52

as there's nothing to announce what these buttons are, they're announced just as "button".

To be 100% compliant, the input field itself needs to be labeled (explicitly associated label, nor wrapping, using for/id attributes) and the label can also be visually hidden (screen-reader-text) if a visible one isn't desirable.

Once a link is inserted in the content, and the cursor on the link makes the toolbar appear, I couldn't find a way to use the Edit and Unlink buttons with a keyboard, unless I'm missing something.

@mehigh
Copy link
Contributor

mehigh commented Jun 27, 2017

I'm interested in working on this feature and improving the accessibility of the add a link functionality.

@mehigh mehigh mentioned this issue Jun 29, 2017
2 tasks
@mehigh
Copy link
Contributor

mehigh commented Jun 29, 2017

Thank you for the suggestions, @afercia
I've handled the a11y concerns to improve the screen reader experience.

Also, I created a 30s screencast showing how one would arrive to the edit / unlink buttons with the keyboard:
Screencast showing add / edit link keyboard navigation functionality in Guttenberg

@afercia
Copy link
Contributor Author

afercia commented Jun 29, 2017

@mehigh thanks! I'd just consider to improve a bit the label text and change:

"Submit" to "Apply" or maybe "Insert link"
("Apply" is the text currently used in WordPress)

"Unlink" to "Remove link"
("Unlink" si difficult to translate in some languages, "Remove link" is the current text used in WP)

@mehigh
Copy link
Contributor

mehigh commented Jun 29, 2017

Thank you. I've updated my PR to include the wording you suggested.

I picked the common "Apply" to allow existing translations to pass through into this button properly.

mehigh added a commit to xwp/gutenberg that referenced this issue Jul 18, 2017
## 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 mehigh mentioned this issue Jul 18, 2017
2 tasks
aduth pushed a commit that referenced this issue Jul 18, 2017
* Insert link accessibility

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

* 694 - Fix formatting

* 694 - Removed visually hidden labels, added aria-label attribute on the input

* 694 - Reverted the edit function declaration

* 694 - Dropped accessibility text comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

3 participants