[EuiAccordion] Update component logic to retain focus on <button> w…

…hen clicked (#7696)

Co-authored-by: Cee Chen <[email protected]>

changelogs/upcoming/7696.md

@@ -0,0 +1,3 @@
+**Accessibility**
+
+- Updated `EuiAccordion` to keep focus on accordion trigger instead of moving to content on click/keypress

src/components/accordion/__snapshots__/accordion.test.tsx.snap

@@ -41,7 +41,6 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -97,7 +96,6 @@ exports[`EuiAccordion behavior does not open when isDisabled 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -150,7 +148,6 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
     id="22"
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -203,7 +200,6 @@ exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`]
     id="24"
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -259,7 +255,6 @@ exports[`EuiAccordion is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -311,7 +306,6 @@ exports[`EuiAccordion isDisabled is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -346,7 +340,6 @@ exports[`EuiAccordion props arrowDisplay none is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -396,7 +389,6 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -448,7 +440,6 @@ exports[`EuiAccordion props arrowProps is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -502,7 +493,6 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -552,7 +542,6 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -600,7 +589,6 @@ exports[`EuiAccordion props buttonElement is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -652,7 +640,6 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -703,7 +690,6 @@ exports[`EuiAccordion props buttonProps paddingSize l 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -754,7 +740,6 @@ exports[`EuiAccordion props buttonProps paddingSize m 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -805,7 +790,6 @@ exports[`EuiAccordion props buttonProps paddingSize s 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -853,7 +837,6 @@ exports[`EuiAccordion props element is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -910,7 +893,6 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -960,7 +942,6 @@ exports[`EuiAccordion props forceState closed is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -1013,7 +994,6 @@ exports[`EuiAccordion props forceState open is rendered 1`] = `
     id="17"
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -1066,7 +1046,6 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
     id="12"
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children emotion-euiAccordion__children"
@@ -1129,7 +1108,6 @@ exports[`EuiAccordion props isLoading is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children euiAccordion__children-isLoading emotion-euiAccordion__children-isLoading"
@@ -1188,7 +1166,6 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = `
     inert=""
     role="group"
     style="block-size: 0;"
-    tabindex="-1"
   >
     <div
       class="euiAccordion__children euiAccordion__children-isLoading emotion-euiAccordion__children-isLoading"

src/components/accordion/accordion.spec.tsx

@@ -10,7 +10,7 @@
 /// <reference types="cypress-real-events" />
 /// <reference types="../../../cypress/support" />
 
-import React, { useState } from 'react';
+import React from 'react';
 import { EuiAccordion, EuiAccordionProps } from './index';
 
 const sharedProps: EuiAccordionProps = {
@@ -47,88 +47,18 @@ describe('EuiAccordion', () => {
       cy.realMount(<EuiAccordion {...sharedProps} />);
       cy.realPress('Tab');
       cy.realPress('Enter');
-      cy.realPress(['Shift', 'Tab']);
       cy.focused().invoke('attr', 'aria-expanded').should('equal', 'true');
       cy.realPress('Space');
       cy.focused().invoke('attr', 'aria-expanded').should('equal', 'false');
     });
   });
 
-  describe('focus management', () => {
-    const expectChildrenIsFocused = () => {
-      cy.focused()
-        .should('have.class', 'euiAccordion__childWrapper')
-        .should('have.attr', 'tabindex', '-1');
-    };
-
-    it('focuses the accordion content when the arrow is clicked', () => {
-      cy.realMount(<EuiAccordion {...sharedProps} />);
-      cy.get('.euiAccordion__arrow').realClick();
-      expectChildrenIsFocused();
-    });
-
-    it('focuses the accordion content when the button is clicked', () => {
-      cy.realMount(<EuiAccordion {...sharedProps} />);
-      cy.realPress('Tab');
-      cy.focused().contains('Click me to toggle').realPress('Enter');
-      expectChildrenIsFocused();
-    });
-
-    describe('forceState', () => {
-      it('does not focus the accordion when `forceState` prevents the accordion from opening', () => {
-        cy.realMount(<EuiAccordion {...sharedProps} forceState="closed" />);
-
-        cy.contains('Click me to toggle').realClick();
-        // cy.focused() is flaky here and doesn't always return an element, so use document.activeElement instead
-        cy.then(() => {
-          expect(document.activeElement).not.to.have.class(
-            'euiAccordion__childWrapper'
-          );
-        });
-      });
-
-      it('does not focus the accordion when programmatically toggled from outside the accordion', () => {
-        const ControlledComponent = () => {
-          const [accordionOpen, setAccordionOpen] = useState(false);
-          return (
-            <>
-              <button
-                data-test-subj="toggleForceState"
-                onClick={() => setAccordionOpen(!accordionOpen)}
-              >
-                Control accordion
-              </button>
-              <EuiAccordion
-                {...sharedProps}
-                forceState={accordionOpen ? 'open' : 'closed'}
-              />
-            </>
-          );
-        };
-        cy.realMount(<ControlledComponent />);
-
-        cy.get('[data-test-subj="toggleForceState"]').realClick();
-        cy.focused()
-          .should('not.have.class', 'euiAccordion__childWrapper')
-          .should('have.attr', 'data-test-subj', 'toggleForceState');
-      });
-
-      it('attempts to focus the accordion children when `onToggle` controls `forceState`', () => {
-        const ControlledComponent = () => {
-          const [accordionOpen, setAccordionOpen] = useState(false);
-          return (
-            <EuiAccordion
-              {...sharedProps}
-              onToggle={(open) => setAccordionOpen(open)}
-              forceState={accordionOpen ? 'open' : 'closed'}
-            />
-          );
-        };
-        cy.realMount(<ControlledComponent />);
-
-        cy.contains('Click me to toggle').realClick();
-        expectChildrenIsFocused();
-      });
-    });
+  // Note: we used to move focus automatically to the accordion contents on open/
+  // toggle button click, but we no longer do so after receiving a11y audit feedback
+  // that the change of context was confusing/not helpful
+  it('maintains focus on the toggle button when opened', () => {
+    cy.realMount(<EuiAccordion {...sharedProps} />);
+    cy.get('.euiAccordion__button').realClick();
+    cy.focused().should('have.class', 'euiAccordion__button');
   });
 });

src/components/accordion/accordion.test.tsx

@@ -319,18 +319,5 @@ describe('EuiAccordion', () => {
       expect(onToggleHandler).toBeCalled();
       expect(onToggleHandler).toBeCalledWith(false);
     });
-
-    it('moves focus to the content when expanded', () => {
-      const component = mount(<EuiAccordion id={getId()} />);
-      const childWrapper = component.find('div[role="group"]').getDOMNode();
-
-      expect(childWrapper).not.toBeFalsy();
-      expect(childWrapper).not.toBe(document.activeElement);
-
-      // click the button
-      component.find('button').at(0).simulate('click');
-
-      expect(childWrapper).toBe(document.activeElement);
-    });
   });
 });

src/components/accordion/accordion.tsx

@@ -152,40 +152,18 @@ export class EuiAccordionClass extends Component<
     if (forceState) {
       const nextState = !this.isOpen;
       this.props.onToggle?.(nextState);
-
-      // If the accordion should theoretically be opened, wait a tick (allows
-      // consumer state to update) and attempt to focus the child content.
-      // NOTE: Even if the accordion does not actually open, this is fine -
-      // the `inert` property on the hidden children will prevent focus
-      if (nextState === true) {
-        requestAnimationFrame(() => {
-          this.accordionChildrenEl?.focus();
-        });
-      }
     } else {
       this.setState(
         (prevState) => ({
           isOpen: !prevState.isOpen,
         }),
         () => {
           this.props.onToggle?.(this.state.isOpen);
-
-          // If the accordion is open, programmatically move focus
-          // from the accordion trigger to the child content
-          if (this.state.isOpen) {
-            this.accordionChildrenEl?.focus();
-          }
         }
       );
     }
   };
 
-  // Used to focus the accordion children on user trigger click only (vs controlled/programmatic open)
-  accordionChildrenEl: HTMLDivElement | null = null;
-  accordionChildrenRef = (node: HTMLDivElement | null) => {
-    this.accordionChildrenEl = node;
-  };
-
   generatedId = htmlIdGenerator()();
 
   render() {
@@ -256,7 +234,6 @@ export class EuiAccordionClass extends Component<
           isLoading={isLoading}
           isLoadingMessage={isLoadingMessage}
           isOpen={this.isOpen}
-          accordionChildrenRef={this.accordionChildrenRef}
         >
           {children}
         </EuiAccordionChildren>

src/components/accordion/accordion_children/accordion_children.tsx

@@ -9,7 +9,6 @@
 import React, {
   FunctionComponent,
   HTMLAttributes,
-  Ref,
   useCallback,
   useMemo,
   useState,
@@ -32,14 +31,12 @@ type _EuiAccordionChildrenProps = HTMLAttributes<HTMLDivElement> &
     'role' | 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage'
   > & {
     isOpen: boolean;
-    accordionChildrenRef: Ref<HTMLDivElement>;
   };
 export const EuiAccordionChildren: FunctionComponent<
   _EuiAccordionChildrenProps
 > = ({
   role,
   children,
-  accordionChildrenRef,
   paddingSize,
   isLoading,
   isLoadingMessage,
@@ -90,9 +87,7 @@ export const EuiAccordionChildren: FunctionComponent<
       className="euiAccordion__childWrapper"
       css={wrapperCssStyles}
       style={heightInlineStyle}
-      ref={accordionChildrenRef}
       role={role}
-      tabIndex={-1}
       // @ts-expect-error - inert property not yet available in React TS defs. TODO: Remove this once https://github.com/DefinitelyTyped/DefinitelyTyped/pull/60822 is merged
       inert={!isOpen ? '' : undefined} // Can't pass a boolean currently, Jest throws errors
     >