[Bug] When navigating through the accordion panels with the arrow keys the panels activate automatically

[kelsS]

Description of the issue

When you navigate through the accordion with assistive tech and the keyboard, pressing the arrow keys to switch to different accordion panels is also activating the panels. The keyboard pattern for accordions is that the arrow keys ONLY shift the focus to the next or previous panel.

Impacted component(s)

• pfe-accordion

Steps to reproduce

1. Go to https://patternflyelements.org/components/accordion/
2. Use the tab key to navigate to the first accordion panel on the page
3. Press the left, right, up, or down arrow keys to navigate through the panels
4. You will see that the panels open automatically when they receive focus via the arrow keys

Expected behavior

The expected behavior is that the arrow keys only allow users to navigate through the accordion panels. The enter key and spacebar key are the only methods used to activate the panels and display the content. This method must be activated by the user expressly and should not happen automatically.

See

• WCAG Accordion Example
• Carnegie Museums Accordion Example

Screenshots

[bennypowers]

Thank you for the issue, great catch @kelsS

If we're willing to wait a bit, I'm happy to fix this on #1804

[bennypowers]

cc @kylebuch8 for prioritization vis a vis code freeze

[kelsS]

Hi @bennypowers! I reviewed the accordion changes merged into #1804. The keyboard testing and Lighthouse audit looked good. Looks like these two critical keyboard accordion issues are fixed now! #1779 and #1813. Thank you so much!

[kelsS]

@kylebuch8, @bennypowers fixed this issue in ticket 1804. Any idea when the fixes for those two issues can be merged into the production branch? Those keyboard issues block this Jira issue for the A-Z products page.

[bennypowers]

@kelsS @Djfaucette Can you try putting this script on an affected page and LMK if it solves the problem?

<script>
customElements.whenDefined('pfe-accordion').then(() => {
  const PfeAccordion = customElements.get('pfe-accordion');
      
  if (PfeAccordion.version.startsWith('1')) {
    
    // TODO: recurse through shadowRoots to get all instances
    for (const el of document.querySelectorAll('pfe-accordion')) {
      // remove event listeners
      el.disconnectedCallback();
    }
    
    // Commence evil prototype hacking
    PfeAccordion.prototype._keydownHandler = function(evt) {
      const currentHeader = evt.target;

      if (!(currentHeader instanceof customElements.get('pfe-accordion-header'))) {
        return;
      }

      let newHeader;

      switch (evt.key) {
        case 'ArrowDown':
          evt.preventDefault();
          newHeader = this._nextHeader();
          break;
        case 'ArrowUp':
          evt.preventDefault();
          newHeader = this._previousHeader();
          break;
        case 'Home':
          evt.preventDefault();
          newHeader = this._firstHeader();
          break;
        case 'End':
          evt.preventDefault();
          newHeader = this._lastHeader();
          break;
        default:
          return;
      }
      
      newHeader?.focus?.();
      // v1 doesn't delegateFocus 
      newHeader.shadowRoot.querySelector("button").focus();
    }
    
    // TODO: recurse through shadowRoots to get all instances
    for (const el of document.querySelectorAll('pfe-accordion')) {
      el.connectedCallback();
    }
  }
    
})
</script>

[kelsS]

Hi @bennypowers and @Djfaucette! This fix solves the arrow key issue when navigating through the accordion panels.

However, the second issue still remains that you can tab into panels that are not active. To solve this issue, tabindex="-1" needs to be added to the pfe-accordion-panel tags when they are closed. When a panel is activated by the user the tabindex property needs to be removed from the currently active panel.

Along with the -1 tabindex attribute, the panels also need to have aria-hidden="true" on the pfe-accordion-panel tags if the panels are not active. This is because the show/hide CSS is not using visibility or display to show and hide the content. Adding the aria-hidden true attribute ensures that the panels are removed from the Accessibility tree until the user activates the panel. Completely remove the tabindex and aria-hidden attribute from the currently active panel once the user has activated it with the panel button. This adds the active panel back to the tab sequence and back into the Accessibility tree.

Here is more information on controlling the tabbing within components with Roving Index and here is more information on the aria-hidden attribute.

Inactive State

Active State

Happy to help any way I can, just let me know. Thanks, y'all, this is a critical accessibility fix and I appreciate your efforts to address the issues. 🙂

[bennypowers]

@kelsS Thank you for the thorough explanation.

I've added a whole bunch of tests on the 2.0 branch, I believe they should cover everything you brought up. Would you mind giving them a look to make sure it didn't miss anything?

Test Cases

 <pfe-accordion>
   ✓ should upgrade pfe-accordion
   ✓ should expand a panel when a header is selected
   ✓ should collapse a panel when an already expanded header is selected
   ✓ should randomly generate ids for aria use
   ✓ should use the ids that are provided instead of generating new ones
   ✓ should toggle a panel when toggle is called
   ✓ should expand a panel when expand is called
   ✓ should collapse a panel when collapse is called
   ✓ should expand all panels when expandAll is called
   ✓ should collapse all panels when collapseAll is called
   ✓ should fire a pfe-accordion:change event when a header is clicked
   ✓ should open the items listed in the expanded-index attribute
   ✓ should add a warning in the console if a pfe-accordion-header lightdom is not a heading level tag
   ✓ should render as disclosure if there is only one header in an accordion
   ✓ should not render as a disclosure if the disclosure attribute is set to false and there is only one header
   ✓ should switch from an accordion to a disclosure if the disclosure attribute switches from false to true
   ✓ should switch to a disclosure if an accordion loses children and only one header is left
   ✓ should switch to an accordion from a disclosure if the accordion gains more than one header
   ✓ should properly initialize any dynamically added headers and panels
  for assistive technology
   with all panels closed
     ✓ applies aria-hidden="true" to all panels
   with all panels open
     ✓ applies aria-hidden="false" to all panels
   when focus is on the first header
    Space
      ✓ expands the first panel
      ✓ applies aria-hidden="false" to the first panel
    Enter
      ✓ expands the first panel
    Tab
      ✓ moves focus to the second header
    Shift+Tab
      ✓ moves focus to the body
    ArrowDown
      ✓ moves focus to the second header
      ✓ does not open panels
    ArrowUp
      ✓ moves focus to the last header
      ✓ does not open panels
    Home
      ✓ moves focus to the first header
      ✓ does not open panels
    End
      ✓ moves focus to the last header
      ✓ does not open panels
   when focus is on the middle header
    Space
      ✓ expands the middle panel
    Enter
      ✓ expands the middle panel
    Tab
      ✓ moves focus to the last header
    ArrowDown
      ✓ moves focus to the last header
    ArrowUp
      ✓ moves focus to the first header
    Home
      ✓ moves focus to the first header
    End
      ✓ moves focus to the last header
   when focus is on the last header
    Space
      ✓ expands the last panel
     then Space
       ✓ collapses the last panel
    Enter
      ✓ expands the last panel
     then Enter
       ✓ collapses the last panel
    Shift+Tab
      ✓ moves focus to the link in middle header
    ArrowDown
      ✓ moves focus to the first header
    ArrowUp
      ✓ moves focus to the middle header
    Home
      ✓ moves focus to the first header
    End
      ✓ moves focus to the last header
   when the first panel is expanded
    and focus is on the first header
     Space
       ✓ collapses the first panel
     Enter
       ✓ collapses the first panel
     Tab
       ✓ moves focus to the link in the first panel
      Tab
        ✓ moves focus to the second header
       Shift+Tab
         ✓ keeps focus on the link in the first panel
      Space
       ArrowDown
         ✓ keeps focus on the link in the first panel
         ✓ does not open other panels
       ArrowUp
         ✓ keeps focus on the link in the first panel
         ✓ does not open other panels
       Home
         ✓ keeps focus on the link in the first panel
         ✓ does not open other panels
       End
         ✓ keeps focus on the link in the first panel
         ✓ does not open other panels
     Shift+Tab
       ✓ moves focus to the body
     ArrowDown
       ✓ moves focus to the second header
       ✓ does not open other panels
     ArrowUp
       ✓ moves focus to the last header
       ✓ does not open other panels
     Home
       ✓ moves focus to the first header
       ✓ does not open other panels
     End
       ✓ moves focus to the last header
       ✓ does not open other panels
    and focus is on the middle header
     Shift+Tab
       ✓ moves focus to the link in first panel
   when the middle panel is expanded
     ✓ applies aria-hidden="false" to the middle panel
    and focus is on the middle header
     Space
       ✓ collapses the second panel
     Enter
       ✓ collapses the second panel
     Tab
       ✓ moves focus to the link in the second panel
     Shift+Tab
       ✓ moves focus to the first header
    and focus is on the last header
     Shift+Tab
       ✓ moves focus to the link in middle panel
   when the last panel is expanded
    when focus is on the last header
     Tab
       ✓ moves focus to the link in last panel
 <pfe-accordion-header>
   ✓ should upgrade pfe-accordion-header
   ✓ must have an id
   ✓ should add the aria-controls attribute corresponding to the header ID
 <pfe-accordion-panel>
   ✓ should upgrade pfe-accordion-panel
   ✓ must have an id
   ✓ should add the aria-labelledby attribute corresponding to the header ID
   ✓ should set the role attribute to "region"

I did not implement the roving tabindex, but I believe that what we have so far in these tests covers those cases. The guidelines do say that landing on the first focusable element in a panel is an acceptable convention. Does that suit you?

[kelsS]

Hi @bennypowers is there another demo link I can test with? Since the guidelines are only the bare minimum implementing the way I specified earlier would be the best user experience and the most accessible.

[kelsS]

Also when you say "selected" here in the test case "✓ should expand a panel when a header is selected" what do you mean by selected?

[kelsS]

Also instead of updating the aria-hidden attribute to false, it is best to remove the attribute entirely on the open panel.