From 860764b56a16434b8083251dd467c47dc85e98c4 Mon Sep 17 00:00:00 2001 From: Trevor Pierce <1Copenut@users.noreply.github.com> Date: Thu, 18 Apr 2024 17:12:15 -0500 Subject: [PATCH] [EuiAccordion] Update component logic to retain focus on ` - - - ); - }; - 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" >