From dbb4fd784707139c07f90de2cf65987ef7e00331 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 13:37:48 -0700 Subject: [PATCH 1/6] [EuiAccordion] Add `role` prop and default to `group` (was previously `region` --- src/components/accordion/accordion.test.tsx | 2 +- src/components/accordion/accordion.tsx | 11 +++++++++++ .../accordion_children/accordion_children.tsx | 5 +++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/components/accordion/accordion.test.tsx b/src/components/accordion/accordion.test.tsx index bb02e3d6ac7..bcacffa6ed2 100644 --- a/src/components/accordion/accordion.test.tsx +++ b/src/components/accordion/accordion.test.tsx @@ -313,7 +313,7 @@ describe('EuiAccordion', () => { it('moves focus to the content when expanded', () => { const component = mount(); - 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); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index 80572d5babd..841b2bdc093 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -33,6 +33,14 @@ export type EuiAccordionProps = CommonProps & * 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['role']; /** * Class that will apply to the trigger for the accordion. */ @@ -124,6 +132,7 @@ export class EuiAccordionClass extends Component< isLoadingMessage: false, element: 'div' as const, buttonElement: 'button' as const, + role: 'group' as const, }; state = { @@ -184,6 +193,7 @@ export class EuiAccordionClass extends Component< children, className, id, + role, element: Element = 'div', buttonElement, buttonProps, @@ -239,6 +249,7 @@ export class EuiAccordionClass extends Component< /> & Pick< EuiAccordionProps, - 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage' + 'role' | 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage' > & { isOpen: boolean; accordionChildrenRef: Ref; @@ -37,6 +37,7 @@ type _EuiAccordionChildrenProps = HTMLAttributes & export const EuiAccordionChildren: FunctionComponent< _EuiAccordionChildrenProps > = ({ + role, children, accordionChildrenRef, paddingSize, @@ -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 From ac093b02a62c7666225007503c8f3d69129f92ae Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 13:37:57 -0700 Subject: [PATCH 2/6] Update tests/snapshots --- .../__snapshots__/accordion.test.tsx.snap | 46 +++++++++---------- src/components/accordion/accordion.test.tsx | 9 ++++ 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index af0032bd422..460bc19f21f 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > @@ -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" > diff --git a/src/components/accordion/accordion.test.tsx b/src/components/accordion/accordion.test.tsx index bcacffa6ed2..64599153f7c 100644 --- a/src/components/accordion/accordion.test.tsx +++ b/src/components/accordion/accordion.test.tsx @@ -41,6 +41,15 @@ describe('EuiAccordion', () => { }); }); + test('role', () => { + const { queryByRole } = render( + + ); + + expect(queryByRole('region')).toBeInTheDocument(); + expect(queryByRole('group')).not.toBeInTheDocument(); + }); + describe('buttonContentClassName', () => { it('is rendered', () => { const { container } = render( From d894a5bf480fd8b231f9bc2c52ba8d99df3813ec Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 13:43:21 -0700 Subject: [PATCH 3/6] Update downstream snapshots --- .../__snapshots__/collapsible_nav_group.test.tsx.snap | 4 ++-- .../__snapshots__/collapsible_nav_accordion.test.tsx.snap | 4 ++-- .../__snapshots__/collapsible_nav_item.test.tsx.snap | 2 +- .../__snapshots__/notification_event.test.tsx.snap | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) 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 9c7a4bfde42..d60f9fd81de 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 @@ -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" > @@ -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" > diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap index 293ba2b0a01..9a05ff15881 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap @@ -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" > @@ -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" > diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap index c176d6ddecc..54d33a5bab1 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap @@ -71,7 +71,7 @@ exports[`EuiCollapsibleNavItem renders a top level accordion if items exist 1`] class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed" id="generated-id" inert="" - role="region" + role="group" style="block-size: 0;" tabindex="-1" > diff --git a/src/components/notification/__snapshots__/notification_event.test.tsx.snap b/src/components/notification/__snapshots__/notification_event.test.tsx.snap index 31c0724e804..2d2a2009c54 100644 --- a/src/components/notification/__snapshots__/notification_event.test.tsx.snap +++ b/src/components/notification/__snapshots__/notification_event.test.tsx.snap @@ -516,7 +516,7 @@ exports[`EuiNotificationEvent props multiple messages are rendered 1`] = ` class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper-isClosed" id="euiNotificationEventMessagesAccordion_generated-id" inert="" - role="region" + role="group" style="block-size: 0;" tabindex="-1" > From d34ac51ef0d38742bd34e6351c8e020ff7e618ae Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 13:48:28 -0700 Subject: [PATCH 4/6] [EuiCollapsibleNavBeta] Update static group components to set a `role="group"` following the same DOM pattern as accordions, but not customizable (since the role is literally in the component name) --- .../collapsible_nav_group.test.tsx.snap | 3 +++ .../collapsible_nav_group/collapsible_nav_group.tsx | 12 ++++++++++-- .../collapsible_nav_group.test.tsx.snap | 6 ++++++ .../collapsible_nav_item/collapsible_nav_group.tsx | 7 ++++++- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/components/collapsible_nav_beta/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap index 04a39c8552e..64a81baf7b1 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap @@ -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" >
{isCollapsed && isPush ? ( @@ -72,10 +74,16 @@ export const EuiCollapsibleNavGroup: FunctionComponent< ) : ( <> - + )}
diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_group.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_group.test.tsx.snap index 55fa4548a06..1cd58f934fb 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_group.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_group.test.tsx.snap @@ -6,11 +6,14 @@ exports[`EuiCollapsibleNavGroup renders as a sub item 1`] = ` > Header
Header
); From 6e2599ca65e1471ffcdcd4571f65860a9c90651c Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 13:59:09 -0700 Subject: [PATCH 5/6] changelog --- upcoming_changelogs/7326.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 upcoming_changelogs/7326.md diff --git a/upcoming_changelogs/7326.md b/upcoming_changelogs/7326.md new file mode 100644 index 00000000000..cd8b7a759b2 --- /dev/null +++ b/upcoming_changelogs/7326.md @@ -0,0 +1,5 @@ +- Added a configurable `role` prop to `EuiAccordion` + +**Accessibility** + +- `EuiAccordion` now defaults to a less screenreader-noisy `group` role instead of `region`. If your accordion contains significant enough content to be a document landmark role, you may re-configure it back to `region`. From bb4f0aac929ac778136ae03e8ed80fb0934a12e4 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 27 Oct 2023 15:19:46 -0700 Subject: [PATCH 6/6] Fix props table not showing `role` prop --- src/components/accordion/accordion.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index 841b2bdc093..640576bc606 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -26,7 +26,7 @@ export const PADDING_SIZES = ['none', 'xs', 's', 'm', 'l', 'xl'] as const; export type EuiAccordionPaddingSize = (typeof PADDING_SIZES)[number]; export type EuiAccordionProps = CommonProps & - Omit, 'id'> & { + Omit, 'id' | 'role'> & { id: string; /** * Applied to the entire .euiAccordion wrapper.