diff --git a/changelogs/upcoming/7696.md b/changelogs/upcoming/7696.md new file mode 100644 index 00000000000..4daea0fa436 --- /dev/null +++ b/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 diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index 460bc19f21f..94a560bdfc6 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/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" >
/// -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(); 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(); - cy.get('.euiAccordion__arrow').realClick(); - expectChildrenIsFocused(); - }); - - it('focuses the accordion content when the button is clicked', () => { - cy.realMount(); - 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(); - - 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 ( - <> - - - - ); - }; - cy.realMount(); - - 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 ( - setAccordionOpen(open)} - forceState={accordionOpen ? 'open' : 'closed'} - /> - ); - }; - cy.realMount(); - - 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(); + cy.get('.euiAccordion__button').realClick(); + cy.focused().should('have.class', 'euiAccordion__button'); }); }); diff --git a/src/components/accordion/accordion.test.tsx b/src/components/accordion/accordion.test.tsx index 64599153f7c..b7b8258a419 100644 --- a/src/components/accordion/accordion.test.tsx +++ b/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(); - 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); - }); }); }); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index 640576bc606..3ceb0af6bd4 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -152,16 +152,6 @@ 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) => ({ @@ -169,23 +159,11 @@ export class EuiAccordionClass extends Component< }), () => { 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} diff --git a/src/components/accordion/accordion_children/accordion_children.tsx b/src/components/accordion/accordion_children/accordion_children.tsx index 62a2d7ffad8..d119d9b725b 100644 --- a/src/components/accordion/accordion_children/accordion_children.tsx +++ b/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 & 'role' | 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage' > & { isOpen: boolean; - accordionChildrenRef: Ref; }; 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 > diff --git a/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap b/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap index d60f9fd81de..7f6277f2a14 100644 --- a/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap +++ b/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap @@ -256,7 +256,6 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true accepts accordion pro inert="" role="group" style="block-size: 0;" - tabindex="-1" >