Fixed Test Coverage for Popover component and other small lint issues

Author: vinkumar2

config/jest.config.js

@@ -11,6 +11,7 @@ module.exports = {
     '**lib/core/**/*.js',
     '!**lib/components/**/*.mock.js',
     '!**lib/components/**/*.story.js',
+    '!**lib/components/**/*.styles.js',
     '!**lib/styles/**/*.js',
     '!**/node_modules/**',
   ],

lib/components/molecules/Popover/Popover.js

@@ -6,17 +6,15 @@
  */
 import React, { PureComponent, createRef } from 'react';
 import styled from 'styled-components';
+import classnames from 'classnames';
 
 import styles from './Popover.style';
 import type { Props, State } from './types';
 
 class Popover extends PureComponent<Props, State> {
   static defaultProps = {
-    popOverBody: '',
-    popOverHeader: '',
     isVisible: false,
     hidePopoverCloseBtn: false,
-    trigger: '',
   };
 
   /**
@@ -36,23 +34,26 @@ class Popover extends PureComponent<Props, State> {
 
   /**
    * @componentDidMount
-   * Add eventlistner to handle click outside the wrapper to that popover
+   * Add event listener to handle click outside the wrapper to that popover
    * gets closed and setting showPopover state on the basis of prop if passed
    */
   componentDidMount() {
-    document.addEventListener('mousedown', this.handleClickOutside);
+    document.addEventListener('mousedown', this.handleClickOutside, false);
+
     const { isVisible } = this.props;
-    this.setState({
-      showPopover: isVisible,
-    });
+    if (isVisible) {
+      this.setState({
+        showPopover: isVisible,
+      });
+    }
   }
 
   /**
    * @componentWillUnmount
-   * Remove all event listener on component unmounting
+   * Remove all event listener on component un-mounting
    */
   componentWillUnmount() {
-    document.removeEventListener('mousedown', this.handleClickOutside);
+    document.removeEventListener('mousedown', this.handleClickOutside, false);
   }
 
   /**
@@ -109,7 +110,11 @@ class Popover extends PureComponent<Props, State> {
     const element = trigger && React.cloneElement(trigger, { onClick: this.handleClick });
 
     return (
-      <div role="presentation" ref={this.wrapperRef} className={className}>
+      <div
+        role="presentation"
+        ref={this.wrapperRef}
+        className={classnames('popover-wrap', className)}
+      >
         {element}
         {showPopover && (
           <div role="dialog" className="popover">
@@ -118,9 +123,7 @@ class Popover extends PureComponent<Props, State> {
                 X
               </button>
             )}
-            <header className="popover__header">
-              <h3>{popOverHeader}</h3>
-            </header>
+            {popOverHeader && <h3 className="popover__header">{popOverHeader}</h3>}
             <div className="popover__body">{children}</div>
           </div>
         )}

lib/components/molecules/Popover/Popover.mock.js

@@ -1,25 +1,25 @@
 const defaultVisiblePopover = {
-  className: `example`,
+  className: 'example',
   popOverHeader: `This is a popover header text`,
   trigger: <button>Click to Open</button>,
   isVisible: true,
 };
 
 const hiddenPopover = {
-  className: `example`,
+  className: 'example',
   popOverHeader: `This is a popover header text`,
   trigger: <button>Click to Open</button>,
   isVisible: false,
 };
 
 const ListPopover = {
-  className: `example`,
+  className: 'example',
   popOverHeader: `This is a popover header text`,
   trigger: <button>Click to Open</button>,
 };
 
 const noCloseBtnPopover = {
-  className: `example`,
+  className: 'example',
   popOverHeader: `This is a popover header text`,
   trigger: <button>Click to Open</button>,
   isVisible: false,

lib/components/molecules/Popover/Popover.story.js

@@ -36,7 +36,7 @@ storiesOf('Molecules', module).addWithChapters('Popover', {
       title: 'Popover Variations',
       sections: [
         {
-          title: '[isVisible=true] variation',
+          title: 'By default Visible variation',
           sectionFn: () => (
             <Popover {...defaultVisiblePopover}>
               Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
@@ -45,7 +45,7 @@ storiesOf('Molecules', module).addWithChapters('Popover', {
           ),
         },
         {
-          title: '[isVisible=false] variation',
+          title: 'By default Invisible variation',
           sectionFn: () => (
             <Popover {...hiddenPopover}>
               Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
@@ -54,7 +54,7 @@ storiesOf('Molecules', module).addWithChapters('Popover', {
           ),
         },
         {
-          title: 'List(Component) variation',
+          title: 'List Component as child variation',
           sectionFn: () => (
             <Popover {...ListPopover}>
               <div>

lib/components/molecules/Popover/tests/Popover.test.js

@@ -1,15 +1,63 @@
-import { shallow } from 'enzyme';
-// import renderer from 'react-test-renderer';
+import { shallow, mount } from 'enzyme';
+import { PopoverVanilla } from '../index';
 
-import Popover from '../index';
+const interactivePopover = {
+  className: 'example',
+  popOverHeader: 'This is a popover header text',
+  trigger: <button id="popover-trigger">Click to Open</button>,
+};
 
-describe('<Popover />', () => {
-  let PopoverComponent = '';
+describe('<Popover /> Rendering', () => {
+  let PopoverComponent;
   beforeEach(() => {
-    PopoverComponent = shallow(<Popover>Test</Popover>);
+    PopoverComponent = shallow(<PopoverVanilla>Test</PopoverVanilla>);
   });
 
   test('should render correctly', () => {
     expect(PopoverComponent).toMatchSnapshot();
   });
 });
+
+describe('Popover functional behavior', () => {
+  let PopoverComponent;
+
+  test('Should have Close Button By Default and can be closed', () => {
+    PopoverComponent = mount(
+      <PopoverVanilla {...interactivePopover} isVisible>
+        Test
+      </PopoverVanilla>
+    );
+    expect(PopoverComponent.find('.popover__close').length).toEqual(1);
+    PopoverComponent = mount(
+      <PopoverVanilla {...interactivePopover} hidePopoverCloseBtn>
+        Test
+      </PopoverVanilla>
+    );
+    expect(PopoverComponent.find('.popover__close').length).toEqual(0);
+  });
+
+  test('Should open By Default when isVisible set to true', () => {
+    PopoverComponent = mount(
+      <PopoverVanilla {...interactivePopover} isVisible>
+        Test
+      </PopoverVanilla>
+    );
+    expect(PopoverComponent.find('.popover__header').length).toEqual(1);
+  });
+
+  test('Should open and close on Trigger and X button click', () => {
+    PopoverComponent = mount(
+      <div>
+        <p className="test">Test Content</p>
+        <PopoverVanilla {...interactivePopover}>Test</PopoverVanilla>
+      </div>
+    );
+    // Test Default Open through trigger
+    PopoverComponent.find('#popover-trigger').simulate('click');
+    expect(PopoverComponent.find('.popover__header').length).toEqual(1);
+    // Test Close Popover when clicking outside through trigger the popover
+    // Test Close through X button
+    PopoverComponent.find('.popover__close').simulate('click');
+    expect(PopoverComponent.find('.popover__header').length).toEqual(0);
+  });
+});

lib/components/molecules/Popover/tests/__snapshots__/Popover.test.js.snap

@@ -1,14 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`<Popover /> should render correctly 1`] = `
-<Popover
-  className="Popover-sc-1xo3w5k-0 cPkpcj"
-  hidePopoverCloseBtn={false}
-  isVisible={false}
-  popOverBody=""
-  popOverHeader=""
-  trigger=""
->
-  Test
-</Popover>
+exports[`<Popover /> Rendering should render correctly 1`] = `
+<div
+  className="popover-wrap"
+  role="presentation"
+/>
 `;

lib/components/molecules/Popover/types/index.js

@@ -2,11 +2,11 @@
 import type { Node } from 'react';
 
 export type Props = {
-  popOverHeader: Node | string,
+  popOverHeader?: Node | string,
   children: Node | string,
-  isVisible: boolean,
-  hidePopoverCloseBtn: boolean,
-  className: string,
+  isVisible?: boolean,
+  hidePopoverCloseBtn?: boolean,
+  className?: string,
   trigger: React$Element<*>,
 };
 

package.json

@@ -13,7 +13,7 @@
   "scripts": {
     "test": "cross-env NODE_ENV=test jest --config ./config/jest.config.js",
     "test:updateSnapshot": "yarn run test --updateSnapshot",
-    "test:watch": "yarn run test --watch",
+    "test:watch": "yarn run test --watch --verbose",
     "audit": "yarn run build -- --report",
     "clean:dist": "rimraf dist",
     "build": "yarn run clean:dist && cross-env BABEL_ENV=production rollup --config rollup.config.js",