fix(vertical nav) improve accessibility support

[jgiardino]

Description

Update css to apply hover styles on menu items with focus
Update css to prevent keyboard Tab key from shiftin focus to hidden elements
Update html test page example to include aria attributes for better screenreader support

Also includes additional text for screen readers for the following elements:

• the arrow toggle that displays in submenus
• indicating the active menu items
• identifying the menu level (e.g. "Primary Navigation Menu" vs "Secondary Navigation Menu")

The following updates are needed but not included in this PR:

• JS updates
  • to capture click events when a nav item with a submenu has focus and Enter key is pressed (NOTE: click events are currently captured for the menu that displays for mobile devices)
  • to modify aria attributes like aria-hidden and aria-expanded
• providing visual indication to the hamburger when it has focus
• enabling keyboard focus on the menus in the masthead

Jira stories:
https://patternfly.atlassian.net/browse/PTNFLY-2537
https://patternfly.atlassian.net/browse/PTNFLY-2611

Changes

• CSS updates:
  • removed underline from nav item text on focus
  • applied hover styles to nav item on focus
  • modified collapse-toggle arrow icon so that if not visible, it cannot receive focus via keyboard or take up space
  • set display none to elements that are hidden, so that they cannot receive focus via keyboard
• HTML updates:
  Updates are mostly modeled after Adobe's accessible mega menu:
  • Role attributes for menu are not added. After researching this, I based this decision on the following reasons:
    • Standard markup is accessible by default, as noted in this article about aria-select. Changing default roles for elements can alter how assistive technologies interact with the controls and ultimately result in worse UX if you don't know what you're doing.
    • The w3c example for menus recommends using the arrow keys for navigating the menu items. Since we are not using arrow keys, and instead only using Tab and Enter to navigate menus, then we should not partially follow this example.
    • The menu role is not intended for navigation, as noted here: https://webaim.org/blog/three-things-voiceover/
  • Role attributes for list are being added. Even though this is redundant, there were a couple of issues I noticed that were resolved by including the default role to <ul> and <li> elements:
    • aria-label attributes were ignored if a role was not explicitly defined
    • In voiceover on mac, simple nested lists are described as expected without role, but with the additional DOM, this seemed to affect the ability of Voiceover to understand the list structure. However, the results of Voiceover were inconsistent.
  • Aria attributes are defined as shown in the sample html below. For these, I'm mostly following the pattern shown for Adobe's accessible mega menu
    • The adobe example uses aria-expanded with aria-controls. I did not include this as I noticed no benefit when testing with Chromevox or Voiceover, and my concerns that that screen readers that support the aria attribute have expectations about what key commands are enabled for the widget.
      • aria-expanded is applied to the element that controls the display of the submenu, but not to the submenu itself.
      • aria-hidden is applied to the submenu
    • aria-selected is not supported for links. The best solution I could find for indicating the current active item is to use <span class="sr-only">(Active Menu Item)</span>
    • aria-label is applied to the <ul> of the submenu

NOTE: only aria attributes are included in this example

<div> 
  <ul> 
    <li>
      <a aria-expanded="false"> 
         ...
      </a>
      <div aria-hidden="true"> <!-- submenu -->
        <div>...</div> <!-- submenu header and collapse-toggle -->
        <ul>...</ul>
    </li>
  </ul>
</div>  

Link to rawgit and/or image

To be provided

PR checklist (if relevant)

• Cross browser: works in IE9
• Cross browser: works in IE10
• Cross browser: works in IE11
• Cross browser: works in Edge
• Cross browser: works in Chrome
• Cross browser: works in Firefox
• Cross browser: works in Safari
• Cross browser: works in Opera
• Responsive: works in extra small, small, medium and large view ports.
• Preview the PR: An image or a URL for designer to preview this PR is provided.

[jgiardino]

I have several questions/comments about these updates, and didn't want to note all of them above...

@andresgalante , these are some of the html/css related questions/comments:

• I removed the underline from the primary nav menu items on focus, but it looks like the underline for submenus is intentional, so I left that.
• When the primary nav is not visible (e.g. when a submenu is pinned), the menu items can still have focus via the keyboard. I'm not sure what the right solution for this is. Should we do something like .collapsed-tertiary-nav-pf > .list-group > .list-group-item > a, [and similar path to secondary list items] {display: none} in the case where a tertiary menu is visible? I'd need to test this, and also look at the classes for the mobile case.

@jeff-phillips-18, can you clarify what the purpose of data-target is in the vertical nav?

[andresgalante]

Closes #723

[jeff-phillips-18]

@jgiardino I don't believe the data-target is used any more, it was originally used to determine what submenu to open.

[spadgett]

@andresgalante Is there any chance to reconsider the href="#0"? It breaks the browser back button among other problems.

See patternfly/angular-patternfly#641

[andresgalante]

Yeap, will an empty herf have tab index?

[jgiardino]

I tested href="" in the browser that PatternFly supports, and it does receive keyboard focus in all of them.

[andresgalante]

@LHinson we are waiting to do the JS part of this PR to merge it. Is there anyone able to do it?

[LHinson]

per @jgiardino:

I think the CSS is in good shape. I think the html could just be scrapped except for cases where we’re enhancing what’s there but not changing the overall structure. So mostly just a review of what’s there, and pruning out what we don’t want. I don’t see it as a huge effort.

So, we are going to close this PR and @jgiardino will create a new one after completing the above work + addressing #839