feat(modal): deprecate iconDescription (#9828)

sstrubberg · dakahn · web-flow · commit a9c13fffe27b · 2021-10-21T12:21:41.000-05:00
* feat(modal): deprecate iconDescription new tests * feat(modal): update test co-authored: @aledavila * feat(modal): updated snappy * Update packages/react/src/components/Modal/Modal-test.js Co-authored-by: DAK <40970507+dakahn@users.noreply.github.com> * feat(modal): added test * feat(modal): updated snappy * feat(Modal): aria lable on the close icon * feat(Modal): updated wrapper test Co-authored-by: DAK <40970507+dakahn@users.noreply.github.com>
diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
@@ -3811,7 +3811,6 @@ Map {
   "Modal" => Object {
     "defaultProps": Object {
       "hasScrollingContent": false,
-      "iconDescription": "Close",
       "modalHeading": "",
       "modalLabel": "",
       "onKeyDown": [Function],
@@ -3841,9 +3840,7 @@ Map {
       "hasScrollingContent": Object {
         "type": "bool",
       },
-      "iconDescription": Object {
-        "type": "string",
-      },
+      "iconDescription": [Function],
       "id": Object {
         "type": "string",
       },
diff --git a/packages/react/src/components/Modal/Modal-story.js b/packages/react/src/components/Modal/Modal-story.js
@@ -68,7 +68,6 @@ const props = {
       '[data-modal-primary-focus]'
     ),
     size: select('Size (size)', sizes, 'md'),
-    iconDescription: text('Close icon description (iconDescription)', 'Close'),
     onBlur: action('onBlur'),
     onClick: action('onClick'),
     onFocus: action('onFocus'),
diff --git a/packages/react/src/components/Modal/Modal-test.js b/packages/react/src/components/Modal/Modal-test.js
@@ -47,19 +47,14 @@ describe('Modal', () => {
       expect(getModal(modal).props().id).toEqual('modal-1');
     });
 
-    it('has the expected default iconDescription', () => {
-      expect(mounted.props().iconDescription).toEqual('Close');
+    it('should not place the svg icon in the accessibility tree', () => {
+      const ariaHidden = mounted.find(Close20).props()['aria-hidden'];
+      expect(ariaHidden).toEqual('true');
     });
 
-    it('adds new iconDescription when passed via props', () => {
-      mounted.setProps({ iconDescription: 'new description' });
-      expect(mounted.props().iconDescription).toEqual('new description');
-    });
-
-    it('should have iconDescription match Icon component description prop', () => {
-      const description = mounted.find(Close20).props()['aria-label'];
-      const matches = mounted.props().iconDescription === description;
-      expect(matches).toEqual(true);
+    it("icon isn't a focusable tab stop", () => {
+      const icon = mounted.find(Close20).props().tabIndex;
+      expect(icon).toEqual('-1');
     });
 
     it('enables primary button by default', () => {
diff --git a/packages/react/src/components/Modal/Modal.js b/packages/react/src/components/Modal/Modal.js
@@ -80,7 +80,10 @@ export default class Modal extends Component {
     /**
      * Provide a description for "close" icon that can be read by screen readers
      */
-    iconDescription: PropTypes.string,
+    iconDescription: deprecate(
+      PropTypes.string,
+      'The iconDescription prop is no longer needed and can be safely removed. This prop will be removed in the next major release of Carbon.'
+    ),
 
     /**
      * Specify the DOM element ID of the top-level node.
@@ -220,7 +223,6 @@ export default class Modal extends Component {
     primaryButtonDisabled: false,
     onKeyDown: () => {},
     passiveModal: false,
-    iconDescription: 'Close',
     modalHeading: '',
     modalLabel: '',
     preventCloseOnClickOutside: false,
@@ -417,11 +419,13 @@ export default class Modal extends Component {
         className={this.modalCloseButtonClass}
         type="button"
         onClick={onRequestClose}
-        title={iconDescription}
-        aria-label={iconDescription}
+        title={ariaLabel ? ariaLabel : iconDescription}
+        aria-label={ariaLabel ? ariaLabel : iconDescription}
         ref={this.button}>
         <Close20
-          aria-label={iconDescription}
+          aria-hidden="true"
+          aria-label="close"
+          tabIndex="-1"
           className={`${this.modalCloseButtonClass}__icon`}
         />
       </button>
diff --git a/packages/react/src/components/ModalWrapper/__snapshots__/ModalWrapper-test.js.snap b/packages/react/src/components/ModalWrapper/__snapshots__/ModalWrapper-test.js.snap
@@ -56,7 +56,6 @@ exports[`ModalWrapper should render 1`] = `
     <Modal
       aria-label="test modal"
       hasScrollingContent={false}
-      iconDescription="Close"
       id="modal"
       modalHeading="Transactional Modal"
       modalLabel="Test Modal Label"
@@ -110,34 +109,38 @@ exports[`ModalWrapper should render 1`] = `
               Transactional Modal
             </h3>
             <button
-              aria-label="Close"
               className="bx--modal-close"
               onClick={[Function]}
-              title="Close"
               type="button"
             >
               <ForwardRef(Close20)
-                aria-label="Close"
+                aria-hidden="true"
+                aria-label="close"
                 className="bx--modal-close__icon"
+                tabIndex="-1"
               >
                 <Icon
-                  aria-label="Close"
+                  aria-hidden="true"
+                  aria-label="close"
                   className="bx--modal-close__icon"
                   fill="currentColor"
                   height={20}
                   preserveAspectRatio="xMidYMid meet"
+                  tabIndex="-1"
                   viewBox="0 0 32 32"
                   width={20}
                   xmlns="http://www.w3.org/2000/svg"
                 >
                   <svg
-                    aria-label="Close"
+                    aria-hidden="true"
+                    aria-label="close"
                     className="bx--modal-close__icon"
                     fill="currentColor"
-                    focusable="false"
+                    focusable="true"
                     height={20}
                     preserveAspectRatio="xMidYMid meet"
                     role="img"
+                    tabIndex="-1"
                     viewBox="0 0 32 32"
                     width={20}
                     xmlns="http://www.w3.org/2000/svg"
