search

philbuchanan / Accordion-Shortcodes

A WordPress plugin that adds a few shortcodes to allow for accordion dropdowns.
https://wordpress.org/plugins/accordion-shortcodes/
16 stars 10 forks source link

updated code for a11y, removed improper ARIA #77

Closed CarlyGerard closed 5 years ago

CarlyGerard commented 5 years ago

We plan on using Accordion Shortcodes in our WP instance, but I noticed there is some ARIA that doesn't seem to fit the accordion design pattern. The current version uses tablists, with tabs and tabpanel items. According to ARIA 1.1, these are not recommended roles and attributes for an accordion: https://www.w3.org/TR/wai-aria-practices-1.1/#accordion

The following link is an example of how ARIA interprets the tablist widget interface: https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-1/tabs.html

These are very different interfaces and require different types of interaction. Therefore, the templates here require adjustment to properly match the ARIA recommended design pattern. Feedback is appreciated.

CarlyGerard commented 5 years ago

@philbuchanan I made some additional changes as I've been working through the module, mainly adding the click event to the element with aria-expanded, now the mock button, instead of the heading. The NVDA screen reader tends to not announce an accordion button as expanded if the click event is on the heading vs. the div role="button" in my changes. Tested in Mac and Windows and it seems to still work the way it should. Would still appreciate a review to make sure that's not going to break any other functionality though.

CarlyGerard commented 5 years ago

Made changes you recommended, let me know if there's anything else I'm missing. Thank you @philbuchanan!

philbuchanan commented 5 years ago

I'll likely make some slight modifications to the code arrangement before publishing the plugin, but I'm accepting the PR now.

CarlyGerard commented 5 years ago

Perfect, thank you.

CarlyGerard commented 5 years ago

Follow up question: our plugins are managed by a WP vendor, I'm guessing they will need to somehow pull the new code?

philbuchanan commented 5 years ago

Follow up question: our plugins are managed by a WP vendor, I'm guessing they will need to somehow pull the new code?

This plugin is hosted on the Wordpress plugin repo. Once published, users in Wordpress should see the plugin needs an update.

I also discovered another issue I should have caught. <div> tags inside an <h#> tag won't validate.

I'm thinking reverting the nested <div> of adding an additional setting that lets users optionally wrap the title in an actually <button> tag. This will keep backwards compatibility, but will allows users to opt into better accessibility.

CarlyGerard commented 5 years ago

@philbuchanan Sounds great to me. Is this something you're going to add, or should I work on it? I'll probably have to adjust some code anyway on our end since it could be a full-fledged button (yay) that could affect our styles (whoops).

philbuchanan commented 5 years ago

It is something I'll add. Just need to find the time over the next few days.