Add ARIA Support and Error Messaging for Custom Link Validation in Admin Menu #7130
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
joedolson
left a comment
joedolson
left a comment
There was a problem hiding this comment.
There are a few improvements that could still be made here.
src/js/_enqueues/lib/nav-menu.js
Outdated
| urlWrap.addClass('has-error'); | ||
|
|
||
| // Announce error message via screen reader | ||
| wp.a11y.speak('Invalid URL. Please enter a valid URL.', 'assertive'); |
There was a problem hiding this comment.
This needs to be a translatable string. Also, the text doesn't match the actual validation rule - which is really only checking whether the value is logically empty. (Either blank, or a protocol with no values.)
The message should probably be something more like "No content entered. Please enter valid link." I don't want to say URL, because you can enter values that aren't URLs - e.g., #content could be used to define a skip link, or #open-modal could be used to open a modal on the page. Because these fields have historically allowed almost any content, we really can't expect a URL now, and the message should reflect that.
src/js/_enqueues/lib/nav-menu.js
Outdated
| // Announce error message via screen reader | ||
| wp.a11y.speak('Invalid URL. Please enter a valid URL.', 'assertive'); | ||
| } else { | ||
| urlInput.removeClass('form-invalid') |
There was a problem hiding this comment.
While it makes sense for the error message to fire on submission (since the validation cases pretty much only cover the case where the input field was ignored), the error clearing state should fire on keyup, so the error is cleared as soon as any value that isn't http:// or https:// is entered.
src/wp-admin/includes/nav-menu.php
Outdated
| type="text"<?php wp_nav_menu_disabled_check( $nav_menu_selected_id ); ?> | ||
| class="code menu-item-textbox form-required" placeholder="https://" | ||
| /> | ||
| <span id="custom-url-error" class="error-message" style="display: none;">Invalid URL.</span> |
There was a problem hiding this comment.
This also needs to be a translatable string; and, similarly, should use text that better reflects the real validation.
Preferably, the error message reported to screen readers should be the same as the one displayed in the input.
2 participants
Changes done in PR:
aria-invalid,aria-describedby) to the URL input field.wp.a11y.speak.Screen.Recording.2024-08-03.at.3.23.01.AM.mov
Trac ticket: https://core.trac.wordpress.org/ticket/60969
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.