Skip to content

Commit

Permalink
[EuiAccordion] Add configurable role prop and change default role t…
Browse files Browse the repository at this point in the history
…o `group` (#7326)
  • Loading branch information
cee-chen authored Oct 30, 2023
1 parent 4270450 commit 31f1a48
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 36 deletions.
46 changes: 23 additions & 23 deletions src/components/accordion/__snapshots__/accordion.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="25"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -95,7 +95,7 @@ exports[`EuiAccordion behavior does not open when isDisabled 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="23"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -148,7 +148,7 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
aria-labelledby="generated-id"
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isOpen"
id="22"
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -201,7 +201,7 @@ exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`]
aria-labelledby="generated-id"
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isOpen"
id="24"
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -257,7 +257,7 @@ exports[`EuiAccordion is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="0"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -309,7 +309,7 @@ exports[`EuiAccordion isDisabled is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="21"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -344,7 +344,7 @@ exports[`EuiAccordion props arrowDisplay none is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="14"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -394,7 +394,7 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="13"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -446,7 +446,7 @@ exports[`EuiAccordion props arrowProps is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="15"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -500,7 +500,7 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="3"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -550,7 +550,7 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="2"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -598,7 +598,7 @@ exports[`EuiAccordion props buttonElement is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="10"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -650,7 +650,7 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="4"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -701,7 +701,7 @@ exports[`EuiAccordion props buttonProps paddingSize l 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="7"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -752,7 +752,7 @@ exports[`EuiAccordion props buttonProps paddingSize m 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="6"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -803,7 +803,7 @@ exports[`EuiAccordion props buttonProps paddingSize s 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="5"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -851,7 +851,7 @@ exports[`EuiAccordion props element is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="1"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -908,7 +908,7 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="11"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -958,7 +958,7 @@ exports[`EuiAccordion props forceState closed is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="16"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -1011,7 +1011,7 @@ exports[`EuiAccordion props forceState open is rendered 1`] = `
aria-labelledby="generated-id"
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isOpen"
id="17"
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -1064,7 +1064,7 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
aria-labelledby="generated-id"
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isOpen"
id="12"
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -1127,7 +1127,7 @@ exports[`EuiAccordion props isLoading is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="19"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -1186,7 +1186,7 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="20"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down
11 changes: 10 additions & 1 deletion src/components/accordion/accordion.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ describe('EuiAccordion', () => {
});
});

test('role', () => {
const { queryByRole } = render(
<EuiAccordion id="role-region" role="region" />
);

expect(queryByRole('region')).toBeInTheDocument();
expect(queryByRole('group')).not.toBeInTheDocument();
});

describe('buttonContentClassName', () => {
it('is rendered', () => {
const { container } = render(
Expand Down Expand Up @@ -313,7 +322,7 @@ describe('EuiAccordion', () => {

it('moves focus to the content when expanded', () => {
const component = mount(<EuiAccordion id={getId()} />);
const childWrapper = component.find('div[role="region"]').getDOMNode();
const childWrapper = component.find('div[role="group"]').getDOMNode();

expect(childWrapper).not.toBeFalsy();
expect(childWrapper).not.toBe(document.activeElement);
Expand Down
13 changes: 12 additions & 1 deletion src/components/accordion/accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,21 @@ export const PADDING_SIZES = ['none', 'xs', 's', 'm', 'l', 'xl'] as const;
export type EuiAccordionPaddingSize = (typeof PADDING_SIZES)[number];

export type EuiAccordionProps = CommonProps &
Omit<HTMLAttributes<HTMLElement>, 'id'> & {
Omit<HTMLAttributes<HTMLElement>, 'id' | 'role'> & {
id: string;
/**
* Applied to the entire .euiAccordion wrapper.
* When using `fieldset`, it will enforce `buttonElement = legend` as well.
*/
element?: 'div' | 'fieldset';
/**
* Defaults to the [group role](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/group_role).
*
* If your accordion contains significant enough content to be a document
* [landmark role](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/region_role#accessibility_concerns), consider using the `region` role instead.
* @default group
*/
role?: HTMLAttributes<HTMLElement>['role'];
/**
* Class that will apply to the trigger for the accordion.
*/
Expand Down Expand Up @@ -124,6 +132,7 @@ export class EuiAccordionClass extends Component<
isLoadingMessage: false,
element: 'div' as const,
buttonElement: 'button' as const,
role: 'group' as const,
};

state = {
Expand Down Expand Up @@ -184,6 +193,7 @@ export class EuiAccordionClass extends Component<
children,
className,
id,
role,
element: Element = 'div',
buttonElement,
buttonProps,
Expand Down Expand Up @@ -239,6 +249,7 @@ export class EuiAccordionClass extends Component<
/>

<EuiAccordionChildren
role={role}
id={id}
aria-labelledby={buttonId}
paddingSize={paddingSize}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ import {
type _EuiAccordionChildrenProps = HTMLAttributes<HTMLDivElement> &
Pick<
EuiAccordionProps,
'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage'
'role' | 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage'
> & {
isOpen: boolean;
accordionChildrenRef: Ref<HTMLDivElement>;
};
export const EuiAccordionChildren: FunctionComponent<
_EuiAccordionChildrenProps
> = ({
role,
children,
accordionChildrenRef,
paddingSize,
Expand Down Expand Up @@ -90,7 +91,7 @@ export const EuiAccordionChildren: FunctionComponent<
css={wrapperCssStyles}
style={heightInlineStyle}
ref={accordionChildrenRef}
role="region"
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true accepts accordion pro
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="id"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -319,7 +319,7 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true will render an accord
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="id"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports[`EuiCollapsibleNavGroup renders 1`] = `
aria-label="aria-label"
class="euiCollapsibleNavLink euiCollapsibleNavItem emotion-euiCollapsibleNavLink-isTopItem-isNotAccordion-euiCollapsibleNavGroup__title"
data-test-subj="test subject string"
id="generated-id"
>
<span
data-euiicon-type="home"
Expand All @@ -19,7 +20,9 @@ exports[`EuiCollapsibleNavGroup renders 1`] = `
</span>
</span>
<div
aria-labelledby="generated-id"
class="euiCollapsibleNavItem__items emotion-euiCollapsibleNavItem__items-isGroup"
role="group"
>
<span
class="euiCollapsibleNavLink euiCollapsibleNavSubItem emotion-euiCollapsibleNavLink-isSubItem"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import React, { FunctionComponent, HTMLAttributes, useContext } from 'react';
import classNames from 'classnames';

import { useEuiTheme } from '../../../services';
import { useEuiTheme, useGeneratedHtmlId } from '../../../services';
import { CommonProps } from '../../common';

import { EuiCollapsibleNavContext } from '../context';
Expand Down Expand Up @@ -65,17 +65,25 @@ export const EuiCollapsibleNavGroup: FunctionComponent<
wrapperProps?.css,
];

const labelledById = useGeneratedHtmlId();

return (
<div {...wrapperProps} className={classes} css={cssStyles}>
{isCollapsed && isPush ? (
<EuiCollapsedNavPopover className={classes} items={items} {...props} />
) : (
<>
<EuiCollapsibleNavItem
id={labelledById}
{...props}
css={styles.euiCollapsibleNavGroup__title}
/>
<EuiCollapsibleNavSubItems items={items} isGroup />
<EuiCollapsibleNavSubItems
items={items}
isGroup
role="group"
aria-labelledby={props.id || labelledById}
/>
</>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="generated-id"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down Expand Up @@ -113,7 +113,7 @@ exports[`EuiCollapsibleNavAccordion renders as a top level item 1`] = `
class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed"
id="generated-id"
inert=""
role="region"
role="group"
style="block-size: 0;"
tabindex="-1"
>
Expand Down
Loading

0 comments on commit 31f1a48

Please sign in to comment.