Skip to content
This repository has been archived by the owner on Mar 9, 2024. It is now read-only.

Improve feedback for accessibility when moving items in nav menu #210

Merged
merged 20 commits into from
Sep 21, 2023
Merged

Improve feedback for accessibility when moving items in nav menu #210

merged 20 commits into from
Sep 21, 2023

Conversation

KTS915
Copy link
Member

@KTS915 KTS915 commented Sep 15, 2023

Improve feedback for accessibility when moving items in nav menu, addressing Issue #181

This adds new polite feedback for screenreader users when they move an item within a nav-menu.

Description

The message re-uses the aria-labels already present in the DOM to provide the message. I have taken this approach to reduce confusion and to avoid internationalization/localization problems. This does mean, however, that the word "child", which was mentioned in Issue #181 is not used here, and the word "sub" is used instead because that's what CP currently uses.

It would be worth asking the person who originally asked for this whether that approach is acceptable. If not, I think we need to consider whether that means we should change "Sub item" to "Child item" throughout the nav-menu so as to avoid fragmentation of terminology and resultant confusion.

Motivation and context

Fixes Issue #181 .

How has this been tested?

Tested on my localhost test site.

To replicate, you need to open the browser inspector and scroll down to the bottom of the DOM to find <div id="a11y-speak-polite". Then drag and drop an element within the nav menu. You should see new text content added to the div. (You might need to expand the div to see it.)

Note that I have had to use a timeout of one second to get this to work because otherwise the new position in the menu is not recognized. I presume this is because the code waits to see if the move is completed first. So the message will not appear immediately.

Screenshots

Screenshot at 2023-09-15 09-50-58

Improve feedback for accessibility when moving items in nav menu
Adds new localized strings to `nav-menus.php` to make them available in `nav-menu.js`
@KTS915
Copy link
Member Author

KTS915 commented Sep 15, 2023

New commit overcomes the localization limitation by adding strings to the wp_localize_script function in nav-menus.php, so the new polite message uses the term "child" instead of "sub".

renovate bot and others added 3 commits September 16, 2023 13:15
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@mattyrob
Copy link
Collaborator

This looks like a good change, I've just pushed some tidy up to the JavaScript as per the implemented coding standards.

@KTS915
Copy link
Member Author

KTS915 commented Sep 16, 2023

Thanks, @mattyrob. I've taken note of what you've done so as to follow similar practices in future.

@mattyrob
Copy link
Collaborator

Thanks, @mattyrob. I've taken note of what you've done so as to follow similar practices in future.

@KTS915

Basically just use ES5 syntax, so vet instead of let or const and no 'modern' function declarations.

Also, I think the norm is to declare or scope variables at the top of blocks of code.

That's essentailly all I did structurally, and then added some white space or readability - the JavaScript files are all minified in the build steps so we can make them as human readable as possible with no impact on file sizes when used on production sites using the minified versions.

Removes CSS that's no longer needed because of previous commits, and adds new CSS to make the nav menu page more accessible.
@viktorix
Copy link
Member

Working on getting feedback.

renovate bot and others added 2 commits September 19, 2023 16:38
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Changes `focus-within` to use `outline` instead of `border`
@rkingett
Copy link

rkingett commented Sep 19, 2023

Just tested this with NVDA on my own website and it works in speech and Braille, so not sure why it isn't passing checks, but it works for all menu items including moving items in and out of child menus and or parent menus.

After some testing, I have found that merely adding `setTimeout` in itself creates enough of a delay to enable the correct message to be generated. There is no need to set a value greater than 0.
Copy link
Member

@viktorix viktorix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good now. The failing tests should be fixed in #218.

mattyrob and others added 10 commits September 20, 2023 08:22
In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process.

This caused a failure in some tests that set the `open_basedir` PHP directive to a value that did not include `sys_get_temp_dir()`.

This adds `sys_get_temp_dir()` to the `open_basedir` value set by the tests to ensure that permission is still granted for the temporary directory.

PHPUnit uses `sys_get_temp_dir()`. To ensure the result is the same, Core's `get_temp_dir()` function is not used.

References:
- sebastianbergmann/phpunit#5356

WP:Props desrosj, mukesh27, SergeyBiryukov, costdev.
Fixes https://core.trac.wordpress.org/ticket/59394.

---

Merges https://core.trac.wordpress.org/changeset/56622 / WordPress/wordpress-develop@7d96189ba1 to ClassicPress.

Co-authored-by: Colin Stewart <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Improve feedback for accessibility when moving items in nav menu
Adds new localized strings to `nav-menus.php` to make them available in `nav-menu.js`
Removes CSS that's no longer needed because of previous commits, and adds new CSS to make the nav menu page more accessible.
Changes `focus-within` to use `outline` instead of `border`
After some testing, I have found that merely adding `setTimeout` in itself creates enough of a delay to enable the correct message to be generated. There is no need to set a value greater than 0.
@mattyrob mattyrob merged commit f295a47 into ClassicPress:develop Sep 21, 2023
@KTS915 KTS915 deleted the nav-menu-feedback branch September 21, 2023 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants