[FOOTER] At mobile menus change disclosure button elements from <h4> to <button>

[mherchel]

Resolves #4475.

Description

This PR fixes some accessibility issues in the big footer variation. At mobile widths the footer menus collapse into menus. However the menu disclosure button is actually a <h4> element, which is not navigable by keyboard, and has a host of other accessibility issues (for example, a space or enter won't always activate this.

This PR fixes the issue by actually swapping out the <h4> for a <button> at mobile widths (and vice-versa at larger widths).

In addition to the JavaScript changes, I had to make some changes to the styling so that button styles don't leak through, and to the tests, so the tests dynamically query for the selector instead of storing it as a variable.

• Follow the 18F Front End Coding Style Guide and Accessibility Guide.
• Run npm test and make sure the tests for the files you have changed have passed.
• Run your code through HTML_CodeSniffer and make sure it’s error free.
• Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

[mahoneycm]

Great job! Everything works as expected across Chrome, Firefox, and Edge.

We did notice the focus border is hidden on both sides and the bottom when collapsed

Is there a way we can get at least the top and bottom to appear while collapsed to avoid confusion on which button is being selected? Currently, it appears correctly when not collapsed

It also looks like the first secondary link is pushed right up against the button. Could you add some space between them when in the smallest mobile view for a little bit of separation?

Thank you for your contribution to USWDS!

[mherchel]

Good catch! Just pushed up a fix. Here's a gif of the new focus states in action. Note that the left edge is cut out of the gif, but it looks perfect in action.

[mherchel]

It also looks like the first secondary link is pushed right up against the button. Could you add some space between them when in the smallest mobile view for a little bit of separation?

Just realized that I missed that comment. This is now fixed in the commit that I just pushed.

[mahoneycm]

Both updates look great! Loving these gif examples

[mahoneycm]

Thanks for your work on this Mike!

[amyleadem]

Thank you for this contribution @mherchel ! This is a very helpful improvement to the accessibility of our footer.

On review, I discovered a couple items that need 

to be addressed prior to merge:

Issue 1 - aria-expanded attribute doesn't always accurately reflect current state

Items affected: footer__primary-link

Window width: < mobile-lg

Issue: When an expanded item is collapsed via the user opening a second item, the aria-expanded attribute on the now-collapsed item remains set to "true".

Expected: Collapsing an expanded item should update the aria-expanded attribute to false.


Issue 2 - Cursor displays with a pointer on non-clickable headers




Items affected: footer__primary-link




Window width: > mobile-lg




Issue: Cursor displays as a pointer despite being a non-actionable item.





Expected: Cursor should be set to auto on non-actionable items

Issue 3 - Inaccessible link items at transition width

Items affected: footer list--unstyled
Window width: 480px
Issue: The user cannot access secondary link list items at exactly 480px. The footer structure changes but the list items remain hidden with no mechanism for exposing the links.


Expected: The link list items should be accessible to the user at all window widths.

Thanks again for all your work on this. Please let me know if you have any questions.

[mherchel]

Thanks for the review! I'll be able to get back into this next week at the earliest. Thanks again,

[thisisdano]

Any update on this one?

[mherchel]

Not yet, although it's still on my @todo list. I should have some time this upcoming sprint though :)

[mherchel]

Alrighty...

Issue 1 - aria-expanded attribute doesn't always accurately reflect current state

Good catch on this! This was happening when another menu opened (which in turn closes the original menu item).

This is fixed. To prevent this from happening again, and to make the code a bit more resilient, I refactored the CSS to style against the aria-expanded state of the <button> element, instead of the hidden CSS class of the parent <section> (which seemed weird to me TBH).


Issue 2 - Cursor displays with a pointer on non-clickable headers

This is actually a pre-existing issue (see https://designsystem.digital.gov/components/footer/#big-footer). However, this is a simple fix and I've taken care of it in this PR.

Issue 3 - Inaccessible link items at transition width

I can't reproduce this one! I've tried both Chrome and FF. Can you still reproduce this? If so, can you give me more info on how to do so?

Animated GIF

showing what I've done:

[amyleadem]

Thank you, @mherchel! I apologize for my delayed response. Everything is looking great.

One request - can you update your branch with the latest updates from the develop branch? We merged some tweaks in PR #4551 related to Issue 3 and I wanted to make sure they are included.

Other than that, all looks good to me.

Apologies again for the delay. We appreciate your efforts!

[mherchel]

Thanks!

can you update your branch with the latest updates from the develop branch?

Just updated my fork. Tests are still passing on my local so 🤞

[amyleadem]

@mherchel Thanks so much for the quick reply. All tests are passing on my end as well. @mejiaj and @thisisdano, passing it to you!

[mejiaj]

Thank you for your contribution, and even more for your patience! It's very much appreciated.

Overall it looks great, added some comments. In the past we've had a11y issues pop up for using aria-expanded without an aria-controls on the same element. Adding those IDs and then referencing them would help improve a11y.

Besides that this PR is in good shape.

[mherchel]

Thanks! These changes are pretty straightforward. I'm kinda slammed this week, so it'll likely be next week when I get to them :)

[mherchel]

This should be good to go! Let me know if there are any more changes requested!

[mejiaj]

@mherchel just a small issue, otherwise LGTM!

[mherchel]

All changes are complete. Please re-review!

[mejiaj]

Looks good, thanks for making those changes!

[mahoneycm]

Looks great!

Might be able to get rid of the !important tag by setting the $chevron-icon-defaults "color" to buttonText in the future

[mherchel]