fix: PF4 About Modal: make trademark, logoImageSrc and logoImageAlt p… (

patternfly#1082) * fix: PF4 About Modal: make trademark, logoImageSrc and logoImageAlt props optional * fix typos
redallen · Dec 19, 2018 · dbc8b06 · dbc8b06
1 parent 12c3661
commit dbc8b06
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 6 deletions.
diff --git a/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.d.ts b/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.d.ts
@@ -5,11 +5,11 @@ export interface AboutModalProps extends HTMLProps<HTMLDivElement> {
   isOpen?: boolean;
   onClose?: Function;
   productName: string;
-  trademark: string;
+  trademark?: string;
   brandImageSrc: string;
   brandImageAlt: string;
-  logoImageSrc: string;
-  logoImageAlt: string;
+  logoImageSrc?: string;
+  logoImageAlt?: string;
   heroImageSrc: string;
   heroImageAlt?: string;
 }

diff --git a/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.js b/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.js
@@ -19,15 +19,20 @@ const propTypes = {
   /** Product Name */
   productName: PropTypes.string.isRequired,
   /** Trademark information */
-  trademark: PropTypes.string.isRequired,
+  trademark: PropTypes.string,
   /** the URL of the image for the Brand. */
   brandImageSrc: PropTypes.string.isRequired,
   /** the alternate text of the Brand image. */
   brandImageAlt: PropTypes.string.isRequired,
   /** the URL of the image for the Logo. */
-  logoImageSrc: PropTypes.string.isRequired,
+  logoImageSrc: PropTypes.string,
   /** the alternate text of the Logo image. */
-  logoImageAlt: PropTypes.string.isRequired,
+  logoImageAlt: props => {
+    if (props.logoImageSrc && !props.logoImageAlt) {
+      return new Error('logoImageAlt is required when a logoImageSrc is specified');
+    }
+    return null;
+  },
   /** the URL of the image for the Hero. */
   heroImageSrc: PropTypes.string.isRequired,
   /** the alternate text of the Hero image. */
@@ -38,6 +43,9 @@ const defaultProps = {
   className: '',
   isOpen: false,
   onClose: () => undefined,
+  trademark: '',
+  logoImageSrc: '',
+  logoImageAlt: '',
   heroImageAlt: ''
 };
 

diff --git a/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.test.js b/packages/patternfly-4/react-core/src/components/AboutModal/AboutModal.test.js
@@ -55,3 +55,20 @@ test('Each modal is given new ariaDescribedById and ariaLablledbyId', () => {
   expect(first.props().ariaLabelledbyId).not.toBe(second.props().ariaLabelledbyId);
   expect(first.props().ariaDescribedById).not.toBe(second.props().ariaDescribedById);
 });
+
+test('Console error is generated when the logoImageSrc is provided without logoImageAlt', () => {
+  const noImgAltrops = {
+    onClose: jest.fn(),
+    children: 'modal content',
+    productName: 'Product Name',
+    trademark: 'Trademark and copyright information here',
+    brandImageSrc: 'brandImg...',
+    brandImageAlt: 'Brand Image',
+    logoImageSrc: 'logoImg...',
+    heroImageSrc: 'heroImg...'
+  };
+  const myMock = jest.fn();
+  global.console = { error: myMock };
+  shallow(<AboutModal {...noImgAltrops}> Test About Modal </AboutModal>);
+  expect(myMock).toBeCalled();
+});