Fix/categories block label missing #10912
Conversation
…element for the dropdown element using the unique instance ID. Applied Categories in the i18n helper for the label. Refactored a few props references to clean up the code
| <label htmlFor={ `${ className }-${ instanceId }` } className="screen-reader-text"> | ||
| { __( 'Categories' ) } | ||
| </label> | ||
| <select id={ `${ className }-${ instanceId }` } className={ `${ className }__dropdown` }> |
There was a problem hiding this comment.
I think id can be statically set in here to something like:
const selectId = `category-categories-select-${ instanceId }`;
...
<label htmlFor={ selectId } className="screen-reader-text">
....
<select id={ selectId } className={ `${ className }__dropdown` }>There was a problem hiding this comment.
Thanks for the feedback, I'll make this change and push a commit when I find some time later today.
| @@ -1,11 +1,16 @@ | |||
| /** | |||
There was a problem hiding this comment.
I noticed that loadash was located in the wrong section of imports. It's fixed now.
There was a problem hiding this comment.
Thanks! I wasn't sure what order to put it in I'll make sure to check out the documentation for the JS standards
| const parentId = showHierarchy ? 0 : null; | ||
| const categories = this.getCategories( parentId ); | ||
|
|
||
| const selectId = `blocks-category-select-${ instanceId }`; |
There was a problem hiding this comment.
I updated this id myself to align with other similar ids added in different places.
|
I can confirm it works as expected. |
gziolo
added
the
[Focus] Accessibility (a11y)
@tofumatt - what's your take on that? |
* added withInstanceId HoC to pass in unique instance. Created a label element for the dropdown element using the unique instance ID. Applied Categories in the i18n helper for the label. Refactored a few props references to clean up the code * switch to id instead of name * statically set class name in constant * Move lodash to external dependencies section * Change the name of the select id
|
The issue also mentions that we should show this label on the frontend which is not the case with the PR. Any taker for a follow-up PR for the PHP side of things? |
|
This seems to have been lost in the current trunk; there is no longer a label associated with the category block dropdown. |
|
@joedolson would you mind opening a new issue for this in order for us to debug it properly. |
|
Looking at the diff above, this adds the label in the editor but not in the frontend. I guess it was never fixed in the frontend. |
Description
The categories block does not have a label when it is set to "Display as dropdown" as mentioned in #10871. This updates the markup in the admin to render a visually hidden label for assistive technology to provide context to the dropdown.
Also mentioned in the related issue above is that the labels should possibly be visual by default. Once merged, another PR can be opened to address that.
One thing to note is that various screen readers will respond differently to select dropdowns based on the type of label used (implicit vs explicit) so even though in my testing it never read the label because I was using VoiceOver and Safari, other browsers/screen readers will respond differently
(http://www.davidmacd.com/blog/select-box-label.html)
How has this been tested?
Tested on OSX in Chrome, Safari, Firefox, Edge, and IE11 using Browserstack. I used Voice Over + Safari to test that the label is properly read.
Screenshots
(My category name was test, which I now realize should probably be something a little more contextual)
Types of changes
Bug fix
Checklist: