fix(ExpandableSection): added ARIA attributes (patternfly#9303)

* fix(ExpandableSection): added ARIA attributes * Updated failing snapshots due to mismatching generated ID
tlabaj · Jul 31, 2023 · 1e1fa7d · 1e1fa7d
1 parent f37548e
commit 1e1fa7d
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 38 deletions.
diff --git a/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx b/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx
@@ -4,7 +4,7 @@ import { css } from '@patternfly/react-styles';
 import lineClamp from '@patternfly/react-tokens/dist/esm/c_expandable_section_m_truncate__content_LineClamp';
 import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon';
 import { PickOptional } from '../../helpers/typeUtils';
-import { debounce } from '../../helpers/util';
+import { debounce, getUniqueId } from '../../helpers/util';
 import { getResizeObserver } from '../../helpers/resizeObserver';
 
 export enum ExpandableSectionVariant {
@@ -23,6 +23,12 @@ export interface ExpandableSectionProps extends React.HTMLProps<HTMLDivElement>
    * property's value should match the contenId property of the expandable section toggle sub-component.
    */
   contentId?: string;
+  /** Id of the toggle of the expandable section, which provides an accessible name to the
+   * expandable section content via the aria-labelledby attribute. When the isDetached property
+   * is also passed in, the value of this property must match the toggleId property of the
+   * expandable section toggle sub-component.
+   */
+  toggleId?: string;
   /** Display size variant. Set to "lg" for disclosure styling. */
   displaySize?: 'default' | 'lg';
   /** Forces active state. */
@@ -102,7 +108,6 @@ export class ExpandableSection extends React.Component<ExpandableSectionProps, E
     displaySize: 'default',
     isWidthLimited: false,
     isIndented: false,
-    contentId: '',
     variant: 'default'
   };
 
@@ -191,13 +196,24 @@ export class ExpandableSection extends React.Component<ExpandableSectionProps, E
       isWidthLimited,
       isIndented,
       contentId,
+      toggleId,
       variant,
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
       truncateMaxLines,
       ...props
     } = this.props;
+
+    if (isDetached && !toggleId) {
+      /* eslint-disable no-console */
+      console.warn(
+        'ExpandableSection: The toggleId value must be passed in and must match the toggleId of the ExpandableSectionToggle.'
+      );
+    }
+
     let onToggle = onToggleProp;
     let propOrStateIsExpanded = isExpanded;
+    const uniqueContentId = contentId || getUniqueId('expandable-section-content');
+    const uniqueToggleId = toggleId || getUniqueId('expandable-section-toggle');
 
     // uncontrolled
     if (isExpanded === undefined) {
@@ -219,6 +235,8 @@ export class ExpandableSection extends React.Component<ExpandableSectionProps, E
         className={css(styles.expandableSectionToggle)}
         type="button"
         aria-expanded={propOrStateIsExpanded}
+        aria-controls={uniqueContentId}
+        id={uniqueToggleId}
         onClick={(event) => onToggle(event, !propOrStateIsExpanded)}
       >
         {variant !== ExpandableSectionVariant.truncate && (
@@ -250,7 +268,9 @@ export class ExpandableSection extends React.Component<ExpandableSectionProps, E
           ref={this.expandableContentRef}
           className={css(styles.expandableSectionContent)}
           hidden={variant !== ExpandableSectionVariant.truncate && !propOrStateIsExpanded}
-          id={contentId}
+          id={uniqueContentId}
+          aria-labelledby={uniqueToggleId}
+          role="region"
         >
           {children}
         </div>

diff --git a/packages/react-core/src/components/ExpandableSection/ExpandableSectionToggle.tsx b/packages/react-core/src/components/ExpandableSection/ExpandableSectionToggle.tsx
@@ -16,6 +16,10 @@ export interface ExpandableSectionToggleProps extends React.HTMLProps<HTMLDivEle
    * property should match the contentId property of the main expandable section component.
    */
   contentId?: string;
+  /** Id of the toggle. The value passed into this property should match the aria-labelledby
+   * property of the main expandable section component.
+   */
+  toggleId?: string;
   /** Direction the toggle arrow should point when the expandable section is expanded. */
   direction?: 'up' | 'down';
   /** @beta Flag to determine toggle styling when the expandable content is truncated. */
@@ -32,6 +36,7 @@ export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionT
   isExpanded = false,
   onToggle,
   contentId,
+  toggleId,
   direction = 'down',
   hasTruncatedContent = false,
   ...props
@@ -52,6 +57,7 @@ export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionT
       aria-expanded={isExpanded}
       aria-controls={contentId}
       onClick={() => onToggle(!isExpanded)}
+      id={toggleId}
     >
       {!hasTruncatedContent && (
         <span

diff --git a/packages/react-core/src/components/ExpandableSection/__tests__/ExpandableSection.test.tsx b/packages/react-core/src/components/ExpandableSection/__tests__/ExpandableSection.test.tsx
@@ -6,15 +6,20 @@ import userEvent from '@testing-library/user-event';
 import { ExpandableSection, ExpandableSectionVariant } from '../ExpandableSection';
 import { ExpandableSectionToggle } from '../ExpandableSectionToggle';
 
-const props = {};
+const props = { contentId: 'content-id', toggleId: 'toggle-id' };
 
 test('ExpandableSection', () => {
   const { asFragment } = render(<ExpandableSection {...props}>test </ExpandableSection>);
   expect(asFragment()).toMatchSnapshot();
 });
 
 test('Renders ExpandableSection expanded', () => {
-  const { asFragment } = render(<ExpandableSection isExpanded> test </ExpandableSection>);
+  const { asFragment } = render(
+    <ExpandableSection {...props} isExpanded>
+      {' '}
+      test{' '}
+    </ExpandableSection>
+  );
   expect(asFragment()).toMatchSnapshot();
 });
 
@@ -29,17 +34,22 @@ test('ExpandableSection onToggle called', async () => {
 });
 
 test('Renders Uncontrolled ExpandableSection', () => {
-  const { asFragment } = render(<ExpandableSection toggleText="Show More"> test </ExpandableSection>);
+  const { asFragment } = render(
+    <ExpandableSection {...props} toggleText="Show More">
+      {' '}
+      test{' '}
+    </ExpandableSection>
+  );
   expect(asFragment()).toMatchSnapshot();
 });
 
 test('Detached ExpandableSection renders successfully', () => {
   const { asFragment } = render(
     <React.Fragment>
-      <ExpandableSection {...props} isExpanded isDetached contentId="test">
+      <ExpandableSection isExpanded isDetached {...props}>
         test
       </ExpandableSection>
-      <ExpandableSectionToggle isExpanded contentId="test" direction="up">
+      <ExpandableSectionToggle isExpanded direction="up" {...props}>
         Toggle text
       </ExpandableSectionToggle>
     </React.Fragment>
@@ -58,7 +68,7 @@ test('Disclosure ExpandableSection', () => {
 
 test('Renders ExpandableSection indented', () => {
   const { asFragment } = render(
-    <ExpandableSection isExpanded isIndented>
+    <ExpandableSection {...props} isExpanded isIndented>
       {' '}
       test{' '}
     </ExpandableSection>
@@ -67,27 +77,56 @@ test('Renders ExpandableSection indented', () => {
 });
 
 test('Does not render with pf-m-truncate class when variant is not passed', () => {
-  render(<ExpandableSection {...props}>test</ExpandableSection>);
+  render(<ExpandableSection>test</ExpandableSection>);
 
   expect(screen.getByText('test').parentElement).not.toHaveClass('pf-m-truncate');
 });
 
 test('Does not render with pf-m-truncate class when variant is not truncate', () => {
+  render(<ExpandableSection variant={ExpandableSectionVariant.default}>test</ExpandableSection>);
+
+  expect(screen.getByText('test').parentElement).not.toHaveClass('pf-m-truncate');
+});
+
+test('Renders with pf-m-truncate class when variant is truncate', () => {
+  render(<ExpandableSection variant={ExpandableSectionVariant.truncate}>test</ExpandableSection>);
+
+  expect(screen.getByText('test').parentElement).toHaveClass('pf-m-truncate');
+});
+
+test('Renders with value passed to contentId', () => {
   render(
-    <ExpandableSection variant={ExpandableSectionVariant.default} {...props}>
-      test
+    <ExpandableSection data-testid="test-id" contentId="custom-id">
+      Test
     </ExpandableSection>
   );
 
-  expect(screen.getByText('test').parentElement).not.toHaveClass('pf-m-truncate');
+  const wrapper = screen.getByTestId('test-id');
+  const content = wrapper.querySelector('#custom-id');
+  expect(content).toBeInTheDocument();
 });
 
-test('Renders with pf-m-truncate class when variant is truncate', () => {
+test('Renders with value passed to toggleId', () => {
   render(
-    <ExpandableSection variant={ExpandableSectionVariant.truncate} {...props}>
-      test
+    <ExpandableSection data-testid="test-id" toggleId="custom-id">
+      Test
     </ExpandableSection>
   );
 
-  expect(screen.getByText('test').parentElement).toHaveClass('pf-m-truncate');
+  const wrapper = screen.getByTestId('test-id');
+  const toggle = wrapper.querySelector('#custom-id');
+  expect(toggle).toBeVisible();
+});
+
+test('Renders with ARIA attributes when contentId and toggleId are passed', () => {
+  render(
+    <ExpandableSection data-testid="test-id" {...props}>
+      Test
+    </ExpandableSection>
+  );
+
+  const wrapper = screen.getByTestId('test-id');
+
+  expect(wrapper).toContainHTML('aria-labelledby="toggle-id"');
+  expect(wrapper).toContainHTML('aria-controls="content-id"');
 });
diff --git a/.../src/components/ExpandableSection/__tests__/__snapshots__/ExpandableSection.test.tsx.snap b/.../src/components/ExpandableSection/__tests__/__snapshots__/ExpandableSection.test.tsx.snap
@@ -6,8 +6,10 @@ exports[`Detached ExpandableSection renders successfully 1`] = `
     class="pf-v5-c-expandable-section pf-m-expanded pf-m-detached"
   >
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
-      id="test"
+      id="content-id"
+      role="region"
     >
       test
     </div>
@@ -16,9 +18,10 @@ exports[`Detached ExpandableSection renders successfully 1`] = `
     class="pf-v5-c-expandable-section pf-m-expanded pf-m-detached"
   >
     <button
-      aria-controls="test"
+      aria-controls="content-id"
       aria-expanded="true"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -54,7 +57,9 @@ exports[`Disclosure ExpandableSection 1`] = `
     class="pf-v5-c-expandable-section pf-m-display-lg pf-m-limit-width"
   >
     <button
+      aria-controls="content-id"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -79,9 +84,11 @@ exports[`Disclosure ExpandableSection 1`] = `
       />
     </button>
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
       hidden=""
-      id=""
+      id="content-id"
+      role="region"
     >
       test 
     </div>
@@ -95,7 +102,9 @@ exports[`ExpandableSection 1`] = `
     class="pf-v5-c-expandable-section"
   >
     <button
+      aria-controls="content-id"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -120,9 +129,11 @@ exports[`ExpandableSection 1`] = `
       />
     </button>
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
       hidden=""
-      id=""
+      id="content-id"
+      role="region"
     >
       test 
     </div>
@@ -136,8 +147,10 @@ exports[`Renders ExpandableSection expanded 1`] = `
     class="pf-v5-c-expandable-section pf-m-expanded"
   >
     <button
+      aria-controls="content-id"
       aria-expanded="true"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -162,8 +175,10 @@ exports[`Renders ExpandableSection expanded 1`] = `
       />
     </button>
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
-      id=""
+      id="content-id"
+      role="region"
     >
        test 
     </div>
@@ -177,8 +192,10 @@ exports[`Renders ExpandableSection indented 1`] = `
     class="pf-v5-c-expandable-section pf-m-expanded pf-m-indented"
   >
     <button
+      aria-controls="content-id"
       aria-expanded="true"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -203,8 +220,10 @@ exports[`Renders ExpandableSection indented 1`] = `
       />
     </button>
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
-      id=""
+      id="content-id"
+      role="region"
     >
        test 
     </div>
@@ -218,7 +237,9 @@ exports[`Renders Uncontrolled ExpandableSection 1`] = `
     class="pf-v5-c-expandable-section"
   >
     <button
+      aria-controls="content-id"
       class="pf-v5-c-expandable-section__toggle"
+      id="toggle-id"
       type="button"
     >
       <span
@@ -245,9 +266,11 @@ exports[`Renders Uncontrolled ExpandableSection 1`] = `
       </span>
     </button>
     <div
+      aria-labelledby="toggle-id"
       class="pf-v5-c-expandable-section__content"
       hidden=""
-      id=""
+      id="content-id"
+      role="region"
     >
        test 
     </div>