diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index 5a12e7444c6..1f606027299 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -218,6 +218,59 @@ exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`] `; +exports[`EuiAccordion behavior restores tabbable children on accordion open 1`] = ` +
+ + + + tabbable item three + +
+ tabbable item four +
+
+`; + +exports[`EuiAccordion behavior sets tabbable children to \`tabIndex={-1}\` when accordions are closed 1`] = ` +
+ + + + tabbable item three + +
+ tabbable item four +
+
+`; + exports[`EuiAccordion is rendered 1`] = `
{ expect(childWrapper).toBe(document.activeElement); }); + + it('sets tabbable children to `tabIndex={-1}` when accordions are closed', () => { + const { container } = render( + + + + tabbable item three +
tabbable item four
+
+ ); + const children = container.querySelector('.euiAccordion__children')!; + + expect(children.querySelectorAll('[tabindex="-1"]')).toHaveLength(4); + expect(children).toMatchSnapshot(); + }); + + it('restores tabbable children on accordion open', () => { + const { container, getByTestSubject } = render( + + + + tabbable item three +
tabbable item four
+
+ ); + fireEvent.click(getByTestSubject('trigger')); + const children = container.querySelector('.euiAccordion__children')!; + + expect(children.querySelectorAll('[tabindex="-1"]')).toHaveLength(0); + expect(children).toMatchSnapshot(); + }); }); }); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index dfea8f59dd3..8eb69ed73a5 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -8,6 +8,7 @@ import React, { Component, HTMLAttributes, ReactNode } from 'react'; import classNames from 'classnames'; +import { tabbable, FocusableElement } from 'tabbable'; import { CommonProps } from '../common'; @@ -108,9 +109,13 @@ export type EuiAccordionProps = CommonProps & isDisabled?: boolean; }; +type EuiAccordionState = { + isOpen: boolean; +}; + export class EuiAccordionClass extends Component< WithEuiThemeProps & EuiAccordionProps, - { isOpen: boolean } + EuiAccordionState > { static defaultProps = { initialIsOpen: false, @@ -125,6 +130,7 @@ export class EuiAccordionClass extends Component< childContent: HTMLDivElement | null = null; childWrapper: HTMLDivElement | null = null; + tabbableChildren: FocusableElement[] | null = null; state = { isOpen: this.props.forceState @@ -132,14 +138,16 @@ export class EuiAccordionClass extends Component< : this.props.initialIsOpen!, }; + get isOpen() { + return this.props.forceState + ? this.props.forceState === 'open' + : this.state.isOpen; + } + setChildContentHeight = () => { - const { forceState } = this.props; requestAnimationFrame(() => { const height = - this.childContent && - (forceState ? forceState === 'open' : this.state.isOpen) - ? this.childContent.clientHeight - : 0; + this.childContent && this.isOpen ? this.childContent.clientHeight : 0; this.childWrapper && this.childWrapper.setAttribute( 'style', @@ -150,17 +158,33 @@ export class EuiAccordionClass extends Component< componentDidMount() { this.setChildContentHeight(); + if (!this.isOpen) this.preventTabbing(); } - componentDidUpdate() { + componentDidUpdate( + prevProps: EuiAccordionProps, + prevState: EuiAccordionState + ) { this.setChildContentHeight(); + + if ( + (prevProps.forceState === 'open' && this.props.forceState === 'closed') || + (prevState.isOpen === true && this.state.isOpen === false) + ) { + this.preventTabbing(); + } + if ( + (prevProps.forceState === 'closed' && this.props.forceState === 'open') || + (prevState.isOpen === false && this.state.isOpen === true) + ) { + this.enableTabbing(); + } } onToggle = () => { const { forceState } = this.props; if (forceState) { - this.props.onToggle && - this.props.onToggle(forceState === 'open' ? false : true); + this.props.onToggle?.(forceState === 'open' ? false : true); } else { this.setState( (prevState) => ({ @@ -170,12 +194,49 @@ export class EuiAccordionClass extends Component< if (this.state.isOpen && this.childWrapper) { this.childWrapper.focus(); } - this.props.onToggle && this.props.onToggle(this.state.isOpen); + this.props.onToggle?.(this.state.isOpen); } ); } }; + // When accordions are closed, tabbable children should not be present in the tab order + preventTabbing = () => { + if (this.childContent) { + // Re-check for children on every close - content can change dynamically + this.tabbableChildren = tabbable(this.childContent); + + this.tabbableChildren.forEach((element) => { + // If the element has an existing `tabIndex` set, make sure we can restore it + const originalTabIndex = element.getAttribute('tabIndex'); + if (originalTabIndex) { + element.setAttribute('data-original-tabindex', originalTabIndex); + } + + element.setAttribute('tabIndex', '-1'); + }); + } + }; + + enableTabbing = () => { + // If no tabbable children were set, we don't need to re-enable anything + if (this.tabbableChildren) { + this.tabbableChildren.forEach((element) => { + const originalTabIndex = element.getAttribute('data-original-tabindex'); + if (originalTabIndex) { + // If the element originally had an existing `tabIndex` set, restore it + element.setAttribute('tabIndex', originalTabIndex); + element.removeAttribute('data-original-tabindex'); + } else { + // If not, remove the tabIndex property + element.removeAttribute('tabIndex'); + } + }); + // Cleanup - unset the list of children + this.tabbableChildren = null; + } + }; + setChildContentRef = (node: HTMLDivElement | null) => { this.childContent = node; }; @@ -214,8 +275,6 @@ export class EuiAccordionClass extends Component< ...rest } = this.props; - const isOpen = forceState ? forceState === 'open' : this.state.isOpen; - // Force button element to be a legend if the element is a fieldset const ButtonElement = Element === 'fieldset' ? 'legend' : _ButtonElement; const buttonElementIsFocusable = ButtonElement === 'button'; @@ -229,7 +288,7 @@ export class EuiAccordionClass extends Component< const classes = classNames( 'euiAccordion', { - 'euiAccordion-isOpen': isOpen, + 'euiAccordion-isOpen': this.isOpen, }, className ); @@ -252,7 +311,7 @@ export class EuiAccordionClass extends Component< const iconButtonClasses = classNames( 'euiAccordion__iconButton', { - 'euiAccordion__iconButton-isOpen': isOpen, + 'euiAccordion__iconButton-isOpen': this.isOpen, 'euiAccordion__iconButton--right': _arrowDisplay === 'right', }, arrowProps?.className @@ -276,13 +335,13 @@ export class EuiAccordionClass extends Component< const childWrapperStyles = euiAccordionChildWrapperStyles(theme); const cssChildWrapperStyles = [ childWrapperStyles.euiAccordion__childWrapper, - isOpen && childWrapperStyles.isOpen, + this.isOpen && childWrapperStyles.isOpen, ]; const iconButtonStyles = euiAccordionIconButtonStyles(theme); const cssIconButtonStyles = [ iconButtonStyles.euiAccordion__iconButton, - isOpen && iconButtonStyles.isOpen, + this.isOpen && iconButtonStyles.isOpen, _arrowDisplay === 'right' && iconButtonStyles.arrowRight, arrowProps?.css, ]; @@ -312,7 +371,7 @@ export class EuiAccordionClass extends Component< iconType="arrowRight" onClick={this.onToggle} aria-controls={id} - aria-expanded={isOpen} + aria-expanded={this.isOpen} aria-labelledby={buttonId} tabIndex={buttonElementIsFocusable ? -1 : 0} isDisabled={isDisabled} @@ -364,7 +423,7 @@ export class EuiAccordionClass extends Component< css={cssButtonStyles} aria-controls={id} // `aria-expanded` is only a valid attribute on interactive controls - axe-core throws a violation otherwise - aria-expanded={ButtonElement === 'button' ? isOpen : undefined} + aria-expanded={ButtonElement === 'button' ? this.isOpen : undefined} onClick={isDisabled ? undefined : this.onToggle} type={ButtonElement === 'button' ? 'button' : undefined} disabled={ButtonElement === 'button' ? isDisabled : undefined} diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx index 9735d9ed301..5a5de350df3 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx @@ -47,7 +47,9 @@ export const Playground: Story = { export const EdgeCaseTesting: Story = { render: ({ ...args }) => ( - + // Since we're not using EuiFlyoutBody/Footer, the flex display isn't needed + // and causes item vertical margins to not collapse +