Merge 0c4afe9 into dcefc94

Workday · Apr 20, 2020 · c9b703e · c9b703e
2 parents dcefc94 + 0c4afe9
commit c9b703e
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 24 deletions.
diff --git a/cypress/integration/Modal.spec.ts b/cypress/integration/Modal.spec.ts
@@ -75,6 +75,10 @@ describe('Modal', () => {
             h.modal.get().should('have.attr', 'aria-labelledby');
           });
 
+          it('should have an aria-modal=true', () => {
+            h.modal.get().should('have.attr', 'aria-modal', 'true');
+          });
+
           it('should contain the title', () => {
             h.modal
               .get()
@@ -235,6 +239,10 @@ describe('Modal', () => {
           h.modal.get().should('have.attr', 'aria-labelledby');
         });
 
+        it('should have an aria-modal=true', () => {
+          h.modal.get().should('have.attr', 'aria-modal', 'true');
+        });
+
         it('should contain the title', () => {
           h.modal
             .get()
@@ -332,6 +340,10 @@ describe('Modal', () => {
           h.modal.get().should('have.attr', 'aria-labelledby');
         });
 
+        it('should have an aria-modal=true', () => {
+          h.modal.get().should('have.attr', 'aria-modal', 'true');
+        });
+
         it('should contain the title', () => {
           h.modal
             .get()

diff --git a/modules/modal/react/lib/ModalContent.tsx b/modules/modal/react/lib/ModalContent.tsx
@@ -8,6 +8,10 @@ import Popup, {PopupPadding} from '@workday/canvas-kit-react-popup';
 import {ModalWidth} from './Modal';
 
 export interface ModalContentProps extends React.HTMLAttributes<HTMLDivElement> {
+  /**
+   * Aria label will override aria-labelledby, it is used if there is no heading or we need custom label for popup
+   */
+  ariaLabel?: string;
   /**
    * The padding of the Modal. Accepts `zero`, `s`, or `l`.
    * @default PopupPadding.l
@@ -152,6 +156,7 @@ const useInitialFocus = (
 };
 
 const ModalContent = ({
+  ariaLabel,
   closeOnEscape = true,
   width = ModalWidth.s,
   padding = PopupPadding.l,
@@ -215,6 +220,8 @@ const ModalContent = ({
         handleClose={handleClose}
         padding={padding}
         transformOrigin={transformOrigin}
+        aria-modal={true}
+        ariaLabel={ariaLabel}
       >
         {children}
       </Popup>

diff --git a/modules/modal/react/spec/Modal.spec.tsx b/modules/modal/react/spec/Modal.spec.tsx
@@ -3,26 +3,29 @@ import {render, fireEvent} from '@testing-library/react';
 
 import Modal from '../lib/Modal';
 
+const renderModal = (props?: any) =>
+  render(
+    <Modal open={true} handleClose={jest.fn()} heading="Test" {...props}>
+      Hello World
+    </Modal>
+  );
+
 describe('Modal', () => {
   test('should call a callback function', async () => {
-    const cb = jest.fn();
-    const {findByLabelText} = render(
-      <Modal open={true} handleClose={cb} heading="Test">
-        Hello World
-      </Modal>
-    );
-
+    const handleClose = jest.fn();
+    const {findByLabelText} = renderModal({handleClose});
     fireEvent.click(await findByLabelText('Close'));
-
-    expect(cb).toHaveBeenCalledTimes(1);
+    expect(handleClose).toHaveBeenCalledTimes(1);
   });
 
-  test('Modal should spread extra props', async () => {
-    const cb = jest.fn();
-    const {getByRole} = render(
-      <Modal handleClose={cb} heading="Test" open={true} data-propspread="test" />
-    );
-
+  test('Modal should spread extra props', () => {
+    const {getByRole} = renderModal({['data-propspread']: 'test'});
     expect(getByRole('dialog').parentElement).toHaveAttribute('data-propspread', 'test');
   });
+
+  test('Modal should replace aria-labeldBy with custom aria-label', () => {
+    const customAriaLabel = 'custom aria label';
+    const {getByRole} = renderModal({ariaLabel: customAriaLabel});
+    expect(getByRole('dialog')).toHaveAttribute('aria-label', customAriaLabel);
+  });
 });
diff --git a/modules/popup/react/lib/Popup.tsx b/modules/popup/react/lib/Popup.tsx
@@ -21,6 +21,10 @@ export enum PopupPadding {
 }
 
 export interface PopupProps extends React.HTMLAttributes<HTMLDivElement> {
+  /**
+   * Aria label will override aria-labelledby, it is used if there is no heading or we need custom label for popup
+   */
+  ariaLabel?: string;
   /**
    * The padding of the Popup. Accepts `zero`, `s`, or `l`.
    * @default PopupPadding.l
@@ -98,6 +102,21 @@ const Container = styled('div', {
   })
 );
 
+const getHeadingId = (heading: React.ReactNode, id: string) => (heading ? id : undefined);
+
+const getAriaLabel = (
+  ariaLabel: string | undefined,
+  headingId: string | undefined
+): object | undefined => {
+  if (ariaLabel) {
+    return {['aria-label']: ariaLabel};
+  }
+  if (headingId) {
+    return {['aria-labelledby']: headingId};
+  }
+  return undefined;
+};
+
 const CloseIconContainer = styled('div')<Pick<PopupProps, 'closeIconSize'>>(
   {
     position: 'absolute',
@@ -128,16 +147,17 @@ export default class Popup extends React.Component<PopupProps> {
       width,
       heading,
       popupRef,
+      ariaLabel,
       ...elemProps
     } = this.props;
-
+    const headingId = getHeadingId(heading, this.id);
     return (
       <Container
         transformOrigin={transformOrigin}
         width={width}
         role="dialog"
-        aria-labelledby={heading ? this.id : undefined}
         ref={popupRef}
+        {...getAriaLabel(ariaLabel, headingId)}
         {...elemProps}
       >
         {handleClose && (
@@ -153,13 +173,7 @@ export default class Popup extends React.Component<PopupProps> {
             />
           </CloseIconContainer>
         )}
-        <Card
-          depth={depth}
-          heading={heading}
-          headingId={heading ? this.id : undefined}
-          width="100%"
-          padding={padding}
-        >
+        <Card depth={depth} heading={heading} headingId={headingId} width="100%" padding={padding}>
           {this.props.children}
         </Card>
       </Container>