Ensure sidenav is only tabable when in view

[bethcollins92]

Done

• Added functionality using tabindex to ensure the side navigation is only tab-able when it's in view (aria-hidden can't be used on focusable elements)
• Ensure on page load depending on screen size the side is either tab-able or not
• Update the tab index when the screen size changes and the drawer is hidden or in view
• Update tab index when the toggle side nav button is clicked
• Added accessibility notes

Fixes #4134

QA

• Open demo
• First, load the page on a screen size bigger than 1035 when the side nav is visible by default
• Check you can tab through the links
• Then change the screen size so the side nav is hidden. Check when you tab through the page it ignores the side nav links
• Make the screen bigger again to reveal the side nav and check you can tab through the links
• Make it smaller again, and use the "Toggle side navigation" button to open the side nav drawer. Check you can tab through the links
• Close the side nav with the button, and then make sure the links return to being non tab-able.
• When opening and closing toggle drawer button using the keyboard, make sure the focus lands on the "toggle side navigation" button inside and outside the drawer

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

• PR should have one of the following labels to automatically categorise it in release notes:
  • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
• Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
  • if CSS class names are not changed it can be bugfix relesase (x.x.X)
  • if CSS class names are changed/added/removed it should be minor version (x.X.0)
  • see the wiki for more details
• Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.
• Documentation side navigation should be updated with the relevant labels.

[webteam-app]

Demo starting at https://vanilla-framework-4371.demos.haus

[bartaz]

I wonder if we should do it another way around. Keep the side navigation focusable, but make sure it shows up on the screen when it's focused? Not sure which one would be a better accessibility practice.

[anthonydillon]

If it helps, I would expect it to be a well labelled (using aria-controls and aria-expanded) user action to open and enter the side navigation. Then programmaticly focus on the first navigation item.

[bethcollins92]

We will discuss in the Accessibility guild to get some more opinions before moving forward

[sowasred2012]

@bartaz we talked about it in the guild meeting after you left (I promise we weren't waiting for you to leave so we could make decisions without you 😉 )

We thought it's a better experience if the user makes the decision to open the nav or not - they could be tabbing through the page to get to something further down the page, and it'd be frustrating if they first have to open then close the navigation. The demo seems to work well, and Beth mentioned she'd update it so that (as Ant mentioned) focus is set to the first item when it's opened, and returned to the CTA when it's closed.

[bethcollins92]

I've updated the PR to ensure the focus lands on the CTA/first element in the side nav. I couldn't decide, when the drawer opens, whether the focus should land on the first element (which is the toggle side navigation button) or the "Get started" list item, what do you think @sowasred2012 @bartaz?

[sowasred2012]

I think that's spot on!

[bartaz]

Something strange is happening when tabbing.
When focus is on "Toggle button" on the page (the one that opens the nav) and you hit Tab focus "disappears" somewhere offscreen and only the next Tab lands on some link after the "Toggle button".

Interestingly when the focus is "away" hitting Enter opens the navigation. So it seems after tabbing out of "Toggle button" the focus is still on it? Or focus moves to "Toggle button" inside the navigation?

[bethcollins92]

@bartaz Ahh yes I forgot about this little bug, I'll need to remove the tab focus from the button as well as all the links

[bartaz]

Would be nice to have ESC button close the side nav as well.

[bethcollins92]

I'm a few steps closer to the end! The reason transitionend event listener was being flakey and not working on the drawer element is because it's an animation not a transition. I've checked it here and I think it's good https://caniuse.com/?search=animationend ? (don't really know what I'm looking at though).

Also, there is a lot of js missing in the _toggle_script.js file which exists in script.js relating to the side nav. The throttle stuff which we need to fix the display of the drawer when the screen is resized. I feel like this is a lot of JS to add to the example but equally we need it to make sure it works on screen resize?

Could you give it a QA @bartaz

[bartaz]

With new class name added and other renamed we should update the docs. "Class reference" and "Javascript Functionaliy" sections need to be updated with new class names. We should add information about deprecated class names to "What's new" page and maybe in "Class reference" as well?

[bethcollins92]

@bartaz I've updated the class names in the javascript section and the class reference table. I've also added a notification at the bottom of the class reference mentioning the deprecated class names. As I am updating the what's new table in the other PR I will add this there as not to cause more conflicts

[bartaz]

LGTM, lets get this thing merged! 🍾 🎆 🎉