Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiAccordion] Fix focus behavior on user interaction open/close vs forceState open/close #7314

Merged
merged 4 commits into from
Oct 26, 2023
Merged

[EuiAccordion] Fix focus behavior on user interaction open/close vs forceState open/close #7314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56fe8eb
Fix accordion open focus behavior to only occur on user click
cee-chen Oct 25, 2023
b3ed776
Rewrite Accordion E2E tests to be more focused and cover new focus logic
cee-chen Oct 25, 2023
611c0ce
Attempt to preserve focus behavior on user click/toggle even when `fo…
cee-chen Oct 25, 2023
fdecfe2
changelog
cee-chen Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 87 additions & 94 deletions src/components/accordion/accordion.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,129 +10,122 @@
/// <reference types="cypress-real-events" />
/// <reference types="../../../cypress/support" />

import React from 'react';
import React, { useState } from 'react';
import { EuiAccordion, EuiAccordionProps } from './index';
import { EuiPanel } from '../../components/panel';
import { htmlIdGenerator } from '../../services';

const baseProps: EuiAccordionProps = {
const sharedProps: EuiAccordionProps = {
buttonContent: 'Click me to toggle',
id: htmlIdGenerator()(),
id: 'cypress-accordion',
initialIsOpen: false,
children: (
<>
Test accordion content.{' '}
<a data-test-subj="childLink" href="#">
Focusable link inside content
</a>
</>
),
};

const noArrow = { arrowDisplay: 'none' };
const noArrowProps: EuiAccordionProps = Object.assign(baseProps, noArrow);

describe('EuiAccordion', () => {
describe('Keyboard and screen reader accessibility', () => {
it('renders with required props', () => {
cy.realMount(
<EuiAccordion {...baseProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here.
</EuiPanel>
</EuiAccordion>
);
describe('keyboard and screen reader accessibility', () => {
it('does not tab to the arrow if the button is interactive', () => {
cy.realMount(<EuiAccordion {...sharedProps} initialIsOpen={true} />);
cy.realPress('Tab');
cy.focused().contains('Click me to toggle');
cy.realPress('Tab');
cy.focused().contains('Focusable link inside content');
});

it('opens and closes on ENTER keypress', () => {
cy.realMount(
<EuiAccordion {...baseProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here.
</EuiPanel>
</EuiAccordion>
);
it('does tab to the arrow if the button is not interactive', () => {
cy.realMount(<EuiAccordion {...sharedProps} buttonElement="div" />);
cy.realPress('Tab');
cy.focused().contains('Click me to toggle').realPress('Enter');
cy.realPress(['Shift', 'Tab']);
cy.focused().invoke('attr', 'aria-expanded').should('equal', 'true');
cy.realPress('Enter');
cy.focused().invoke('attr', 'aria-expanded').should('equal', 'false');
cy.focused().should('have.class', 'euiAccordion__arrow');
});

it('opens and closes on SPACE keypress', () => {
cy.realMount(
<EuiAccordion {...baseProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here.
</EuiPanel>
</EuiAccordion>
);
it('opens and closes the accordion on keypress', () => {
cy.realMount(<EuiAccordion {...sharedProps} />);
cy.realPress('Tab');
cy.focused().contains('Click me to toggle').realPress('Space');
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('Props and navigation', () => {
it('should not have an arrow', () => {
cy.realMount(
<EuiAccordion {...noArrowProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here.
</EuiPanel>
</EuiAccordion>
);
cy.get('.euiAccordion__arrow').should('not.exist');
});
describe('focus management', () => {
const expectChildrenIsFocused = () => {
cy.focused()
.should('have.class', 'euiAccordion__childWrapper')
.should('have.attr', 'tabindex', '-1');
};

it('manages focus when panel is clicked', () => {
cy.realMount(
<EuiAccordion {...noArrowProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here.
</EuiPanel>
</EuiAccordion>
);
cy.get('button').contains('Click me to toggle').realClick();
cy.focused().invoke('attr', 'tabindex').should('equal', '-1');
cy.focused().contains('Any content inside of EuiAccordion');
it('focuses the accordion content when the arrow is clicked', () => {
cy.realMount(<EuiAccordion {...sharedProps} />);
cy.get('.euiAccordion__arrow').realClick();
expectChildrenIsFocused();
});

it('manages focus when panel is opened by keyboard interaction', () => {
cy.realMount(
<EuiAccordion {...noArrowProps}>
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here. We will include <a href="#">a link</a> to confirm focus.
</EuiPanel>
</EuiAccordion>
);
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');
cy.focused().invoke('attr', 'tabindex').should('equal', '-1');
cy.focused().contains('Any content inside of EuiAccordion');
cy.realPress('Tab');
cy.focused().contains('a link');
expectChildrenIsFocused();
});

it('manages focus when forceState is open', () => {
cy.realMount(
<EuiAccordion {...noArrowProps} forceState="open">
<EuiPanel color="subdued">
Any content inside of <strong>EuiAccordion</strong> will appear
here. We will include <a href="#">a link</a> to confirm focus.
</EuiPanel>
</EuiAccordion>
);
cy.realPress('Tab');
cy.focused().contains('Click me to toggle');
cy.focused().invoke('attr', 'aria-expanded').should('equal', 'true');
cy.focused().invoke('attr', 'tabindex').should('not.exist');
cy.realPress('Tab');
cy.focused().contains('a link');
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()
.should('not.have.class', 'euiAccordion__childWrapper')
.contains('Click me to toggle');
Comment on lines +83 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this double assertion for certifying the forceState="closed" prevents focus being moved.

});

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();
});
});
});
});
26 changes: 25 additions & 1 deletion src/components/accordion/accordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,42 @@ export class EuiAccordionClass extends Component<
onToggle = () => {
const { forceState } = this.props;
if (forceState) {
this.props.onToggle?.(forceState === 'open' ? false : true);
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() {
Expand Down Expand Up @@ -222,6 +245,7 @@ export class EuiAccordionClass extends Component<
isLoading={isLoading}
isLoadingMessage={isLoadingMessage}
isOpen={this.isOpen}
accordionChildrenRef={this.accordionChildrenRef}
>
{children}
</EuiAccordionChildren>
Expand Down
18 changes: 5 additions & 13 deletions src/components/accordion/accordion_children/accordion_children.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
import React, {
FunctionComponent,
HTMLAttributes,
useRef,
Ref,
useCallback,
useMemo,
useState,
} from 'react';
import classNames from 'classnames';

import { useEuiTheme, useUpdateEffect } from '../../../services';
import { useEuiTheme } from '../../../services';
import { EuiResizeObserver } from '../../observer/resize_observer';

import { EuiAccordionProps } from '../accordion';
Expand All @@ -32,11 +32,13 @@ type _EuiAccordionChildrenProps = HTMLAttributes<HTMLDivElement> &
'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage'
> & {
isOpen: boolean;
accordionChildrenRef: Ref<HTMLDivElement>;
};
export const EuiAccordionChildren: FunctionComponent<
_EuiAccordionChildrenProps
> = ({
children,
accordionChildrenRef,
paddingSize,
isLoading,
isLoadingMessage,
Expand Down Expand Up @@ -67,8 +69,6 @@ export const EuiAccordionChildren: FunctionComponent<
isOpen ? wrapperStyles.isOpen : wrapperStyles.isClosed,
];

const wrapperRef = useRef<HTMLDivElement | null>(null);

/**
* Update the accordion wrapper height whenever the accordion opens, and also
* whenever the child content updates (which will change the height)
Expand All @@ -83,21 +83,13 @@ export const EuiAccordionChildren: FunctionComponent<
[isOpen, contentHeight]
);

/**
* Focus the children wrapper when the accordion is opened,
* but not if the accordion is initially open on mount
*/
useUpdateEffect(() => {
if (isOpen) wrapperRef.current?.focus();
}, [isOpen]);

return (
<div
{...rest}
className="euiAccordion__childWrapper"
css={wrapperCssStyles}
style={heightInlineStyle}
ref={wrapperRef}
ref={accordionChildrenRef}
role="region"
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
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/7314.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Accessibility**

- `EuiAccordion`s no longer attempt to focus child content when the accordion is externally opened via `forceState`, but will continue to focus expanded content when users click the toggle button.