Merge pull request #109 from ansible/add-prop-checking

Add proptypes to shared components

CONTRIBUTING.md

@@ -15,6 +15,7 @@ Have questions about this document or anything not covered here? Feel free to re
 * [Working with React](#working-with-react)
   * [Class constructors vs Class properties](#class-constructors-vs-class-properties)
   * [Binding](#binding)
+  * [Typechecking with PropTypes](#typechecking-with-proptypes)
 * [Testing](#testing)
   * [Jest](#jest)
   * [Enzyme](#enzyme)
@@ -90,35 +91,25 @@ It is good practice to bind our class methods within our class constructor metho
   2. [Performance advantages](https://stackoverflow.com/a/44844916).
   3. Ease of [testing](https://github.com/airbnb/enzyme/issues/365).
 
-### Component Lifecycle
+### Typechecking with PropTypes
+Shared components should have their prop values typechecked. This will help catch bugs when components get refactored/renamed.
+```javascript
+About.propTypes = {
+  ansible_version: PropTypes.string,
+  isOpen: PropTypes.bool,
+  onClose: PropTypes.func.isRequired,
+  version: PropTypes.string,
+};
 
-A React Component has various [lifecylce methods](http://projects.wojtekmaj.pl/react-lifecycle-methods-diagram/). Understanding the basic  lifecycle of a Component will help you determine which method to utilize for your specific need. Below are some general guidelines:
+About.defaultProps = {
+  ansible_version: null,
+  isOpen: false,
+  version: null,
+};
+```
 
-BAD:
-  Use `render` method to make asynchronous calls.
-  ```javascript
-      render () {
-        const { data } = await api.get(API_CONFIG);
 
-        return(<div>`Hello, ${data.usename}!`</div>);
-      }
-  ```
-GOOD:
-  Use `componentDidMount()` method to make asynchronous calls to retrieve data a Componenet may need.
-  ```javascript
-      async componentDidMount () {
-        try {
-          const { data } = await api.get(API_CONFIG);
-          this.setState({ data });
-        } catch (error) {
-          this.setState({ error });
-        }
-      }
 
-      render() {
-        return(<div>`Hello, ${this.state.data.usename}!`</div>)
-      }
-  ```
 ## Testing
 All code, new or otherwise, should have at least 80% test coverage.
 ### Jest

__tests__/components/About.test.jsx

@@ -6,11 +6,11 @@ import About from '../../src/components/About';
 describe('<About />', () => {
   let aboutWrapper;
   let closeButton;
-
+  const onClose = jest.fn();
   test('initially renders without crashing', () => {
     aboutWrapper = mount(
       <I18nProvider>
-        <About isOpen />
+        <About isOpen onClose={onClose} />
       </I18nProvider>
     );
     expect(aboutWrapper.length).toBe(1);
@@ -18,7 +18,6 @@ describe('<About />', () => {
   });
 
   test('close button calls onClose handler', () => {
-    const onClose = jest.fn();
     aboutWrapper = mount(
       <I18nProvider>
         <About isOpen onClose={onClose} />

__tests__/components/AnsibleSelect.test.jsx

@@ -33,19 +33,6 @@ describe('<AnsibleSelect />', () => {
     expect(spy).toHaveBeenCalled();
   });
 
-  test('content not rendered when data property is falsey', () => {
-    const wrapper = mount(
-      <AnsibleSelect
-        value="foo"
-        name="bar"
-        onChange={() => { }}
-        label={label}
-        data={null}
-      />
-    );
-    expect(wrapper.find('FormGroup')).toHaveLength(0);
-    expect(wrapper.find('Select')).toHaveLength(0);
-  });
   test('Returns correct select options if defaultSelected props is passed', () => {
     const wrapper = mount(
       <AnsibleSelect

__tests__/components/Lookup.test.jsx

@@ -3,7 +3,7 @@ import { mount } from 'enzyme';
 import { I18nProvider } from '@lingui/react';
 import Lookup from '../../src/components/Lookup';
 
-let mockData = [{ name: 'foo', id: 1, isChecked: false }];
+let mockData = [{ name: 'foo', id: 1 }];
 describe('<Lookup />', () => {
   test('initially renders succesfully', () => {
     mount(
@@ -85,7 +85,7 @@ describe('<Lookup />', () => {
   });
   test('calls "toggleSelected" when remove icon is clicked', () => {
     const spy = jest.spyOn(Lookup.prototype, 'toggleSelected');
-    mockData = [{ name: 'foo', id: 1, isChecked: false }, { name: 'bar', id: 2, isChecked: true }];
+    mockData = [{ name: 'foo', id: 1 }, { name: 'bar', id: 2 }];
     const wrapper = mount(
       <I18nProvider>
         <Lookup
@@ -103,20 +103,21 @@ describe('<Lookup />', () => {
   });
   test('"wrapTags" method properly handles data', () => {
     const spy = jest.spyOn(Lookup.prototype, 'wrapTags');
-    mockData = [{ name: 'foo', id: 0, isChecked: false }, { name: 'bar', id: 1, isChecked: false }];
+    mockData = [{ name: 'foo', id: 0 }, { name: 'bar', id: 1 }];
     const wrapper = mount(
       <I18nProvider>
         <Lookup
           lookup_header="Foo Bar"
           onLookupSave={() => { }}
-          data={mockData}
+          value={mockData}
           selected={[]}
+          getItems={() => { }}
         />
       </I18nProvider>
     );
     expect(spy).toHaveBeenCalled();
     const pill = wrapper.find('span.awx-c-tag--pill');
-    expect(pill).toHaveLength(0);
+    expect(pill).toHaveLength(2);
   });
   test('toggleSelected successfully adds/removes row from lookupSelectedItems state', () => {
     mockData = [{ name: 'foo', id: 1 }];
@@ -125,8 +126,9 @@ describe('<Lookup />', () => {
         <Lookup
           lookup_header="Foo Bar"
           onLookupSave={() => { }}
-          data={mockData}
+          value={mockData}
           selected={[]}
+          getItems={() => { }}
         />
       </I18nProvider>
     ).find('Lookup');

__tests__/components/NotificationList.test.jsx

@@ -112,9 +112,9 @@ describe('<Notifications />', () => {
     const getNotificationsFn = jest.fn().mockResolvedValue({
       data: {
         results: [
-          { id: 1 },
+          { id: 1, notification_type: 'slack' },
-          { id: 2 },
+          { id: 2, notification_type: 'email' },
-          { id: 3 }
+          { id: 3, notification_type: 'github' }
         ]
       }
     });

__tests__/components/NotificationListItem.test.jsx

@@ -6,6 +6,7 @@ import NotificationListItem from '../../src/components/NotificationsList/Notific
 
 describe('<NotificationListItem />', () => {
   let wrapper;
+  const toggleNotification = jest.fn();
 
   afterEach(() => {
     if (wrapper) {
@@ -18,7 +19,12 @@ describe('<NotificationListItem />', () => {
     wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
-          <NotificationListItem />
+          <NotificationListItem
+            itemId={9000}
+            toggleNotification={toggleNotification}
+            detailUrl="/foo"
+            notificationType="slack"
+          />
         </MemoryRouter>
       </I18nProvider>
     );
@@ -26,7 +32,6 @@ describe('<NotificationListItem />', () => {
   });
 
   test('handles success click when toggle is on', () => {
-    const toggleNotification = jest.fn();
     wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -34,6 +39,8 @@ describe('<NotificationListItem />', () => {
             itemId={9000}
             successTurnedOn
             toggleNotification={toggleNotification}
+            detailUrl="/foo"
+            notificationType="slack"
           />
         </MemoryRouter>
       </I18nProvider>
@@ -43,7 +50,6 @@ describe('<NotificationListItem />', () => {
   });
 
   test('handles success click when toggle is off', () => {
-    const toggleNotification = jest.fn();
     wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -51,6 +57,8 @@ describe('<NotificationListItem />', () => {
             itemId={9000}
             successTurnedOn={false}
             toggleNotification={toggleNotification}
+            detailUrl="/foo"
+            notificationType="slack"
           />
         </MemoryRouter>
       </I18nProvider>
@@ -60,7 +68,6 @@ describe('<NotificationListItem />', () => {
   });
 
   test('handles error click when toggle is on', () => {
-    const toggleNotification = jest.fn();
     wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -68,6 +75,8 @@ describe('<NotificationListItem />', () => {
             itemId={9000}
             errorTurnedOn
             toggleNotification={toggleNotification}
+            detailUrl="/foo"
+            notificationType="slack"
           />
         </MemoryRouter>
       </I18nProvider>
@@ -77,7 +86,6 @@ describe('<NotificationListItem />', () => {
   });
 
   test('handles error click when toggle is off', () => {
-    const toggleNotification = jest.fn();
     wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -85,6 +93,8 @@ describe('<NotificationListItem />', () => {
             itemId={9000}
             errorTurnedOn={false}
             toggleNotification={toggleNotification}
+            detailUrl="/foo"
+            notificationType="slack"
           />
         </MemoryRouter>
       </I18nProvider>

__tests__/components/PageHeaderToolbar.test.jsx

@@ -8,17 +8,24 @@ import PageHeaderToolbar from '../../src/components/PageHeaderToolbar';
 describe('PageHeaderToolbar', () => {
   const pageHelpDropdownSelector = 'Dropdown QuestionCircleIcon';
   const pageUserDropdownSelector = 'Dropdown UserIcon';
+  const onAboutClick = jest.fn();
+  const onLogoutClick = jest.fn();
 
   test('expected content is rendered on initialization', () => {
-    const wrapper = mount(<I18nProvider><PageHeaderToolbar /></I18nProvider>);
+    const wrapper = mount(
+      <I18nProvider>
+        <PageHeaderToolbar
+          onAboutClick={onAboutClick}
+          onLogoutClick={onLogoutClick}
+        />
+      </I18nProvider>
+    );
 
     expect(wrapper.find(pageHelpDropdownSelector)).toHaveLength(1);
     expect(wrapper.find(pageUserDropdownSelector)).toHaveLength(1);
   });
 
   test('dropdowns have expected items and callbacks', () => {
-    const onAboutClick = jest.fn();
-    const onLogoutClick = jest.fn();
     const wrapper = mount(
       <MemoryRouter>
         <I18nProvider>

__tests__/components/Tooltip.test.jsx

@@ -7,14 +7,24 @@ describe('<Tooltip />', () => {
   let content;
   let mouseOverHandler;
   let mouseOutHandler;
+  const child = (<span>foo</span>);
+  const message = 'hi';
 
   test('initially renders without crashing', () => {
-    elem = mount(<Tooltip />);
+    elem = mount(
+      <Tooltip message={message}>
+        {child}
+      </Tooltip>
+    );
     expect(elem.length).toBe(1);
   });
 
   test('shows/hides with mouse over and leave', () => {
-    elem = mount(<Tooltip />);
+    elem = mount(
+      <Tooltip message={message}>
+        {child}
+      </Tooltip>
+    );
     mouseOverHandler = elem.find('.mouseOverHandler');
     mouseOutHandler = elem.find('.mouseOutHandler');
     expect(elem.state().isDisplayed).toBe(false);
@@ -34,7 +44,11 @@ describe('<Tooltip />', () => {
   });
 
   test('shows/hides with focus and blur', () => {
-    elem = mount(<Tooltip />);
+    elem = mount(
+      <Tooltip message={message}>
+        {child}
+      </Tooltip>
+    );
     mouseOverHandler = elem.find('.mouseOverHandler');
     mouseOutHandler = elem.find('.mouseOutHandler');
     expect(elem.state().isDisplayed).toBe(false);

__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx

@@ -2,6 +2,7 @@ import React from 'react';
 import { mount } from 'enzyme';
 import { MemoryRouter } from 'react-router-dom';
 import { I18nProvider } from '@lingui/react';
+import { ConfigContext } from '../../../../src/context';
 import OrganizationAdd from '../../../../src/pages/Organizations/screens/OrganizationAdd';
 
 describe('<OrganizationAdd />', () => {
@@ -157,4 +158,37 @@ describe('<OrganizationAdd />', () => {
     });
     expect(createInstanceGroupsFn).toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 1);
   });
+
+  test('AnsibleSelect component renders if there are virtual environments', () => {
+    const config = {
+      custom_virtualenvs: ['foo', 'bar'],
+    };
+    const wrapper = mount(
+      <MemoryRouter>
+        <I18nProvider>
+          <ConfigContext.Provider value={config}>
+            <OrganizationAdd api={{}} />
+          </ConfigContext.Provider>
+        </I18nProvider>
+      </MemoryRouter>
+    ).find('OrganizationAdd').find('AnsibleSelect');
+    expect(wrapper.find('Select')).toHaveLength(1);
+    expect(wrapper.find('SelectOption')).toHaveLength(2);
+  });
+
+  test('AnsibleSelect component does not render if there are 0 virtual environments', () => {
+    const config = {
+      custom_virtualenvs: [],
+    };
+    const wrapper = mount(
+      <MemoryRouter>
+        <I18nProvider>
+          <ConfigContext.Provider value={config}>
+            <OrganizationAdd api={{}} />
+          </ConfigContext.Provider>
+        </I18nProvider>
+      </MemoryRouter>
+    ).find('OrganizationAdd').find('AnsibleSelect');
+    expect(wrapper.find('Select')).toHaveLength(0);
+  });
 });

src/components/About.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { I18n } from '@lingui/react';
 import { Trans, t } from '@lingui/macro';
 import {
@@ -86,4 +87,17 @@ class About extends React.Component {
   }
 }
 
+About.propTypes = {
+  ansible_version: PropTypes.string,
+  isOpen: PropTypes.bool,
+  onClose: PropTypes.func.isRequired,
+  version: PropTypes.string,
+};
+
+About.defaultProps = {
+  ansible_version: null,
+  isOpen: false,
+  version: null,
+};
+
 export default About;

src/components/AnsibleSelect/AnsibleSelect.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 
 import {
   Select,
@@ -11,19 +12,6 @@ class AnsibleSelect extends React.Component {
     this.onSelectChange = this.onSelectChange.bind(this);
   }
 
-  state = {
-    count: 1,
-  }
-
-  static getDerivedStateFromProps (nexProps) {
-    if (nexProps.data) {
-      return {
-        count: nexProps.data.length,
-      };
-    }
-    return null;
-  }
-
   onSelectChange (val, event) {
     const { onChange, name } = this.props;
     event.target.name = name;
@@ -31,21 +19,30 @@ class AnsibleSelect extends React.Component {
   }
 
   render () {
-    const { count } = this.state;
+    const { label, value, data, defaultSelected } = this.props;
-    const { label = '', value, data, defaultSelected } = this.props;
+    return (
-    let elem;
+      <Select value={value} onChange={this.onSelectChange} aria-label="Select Input">
-    if (count > 1) {
+        {data.map((datum) => (datum === defaultSelected
-      elem = (
+          ? (<SelectOption key="" value="" label={`Use Default ${label}`} />) : (<SelectOption key={datum} value={datum} label={datum} />)))
-        <Select value={value} onChange={this.onSelectChange} aria-label="Select Input">
+        }
-          {data.map((datum) => (datum === defaultSelected
+      </Select>
-            ? (<SelectOption key="" value="" label={`Use Default ${label}`} />) : (<SelectOption key={datum} value={datum} label={datum} />)))
+    );
-          }
-        </Select>
-      );
-    } else {
-      elem = null;
-    }
-    return elem;
   }
 }
+
+AnsibleSelect.defaultProps = {
+  data: [],
+  label: 'Ansible Select',
+  defaultSelected: null,
+};
+
+AnsibleSelect.propTypes = {
+  data: PropTypes.arrayOf(PropTypes.string),
+  defaultSelected: PropTypes.string,
+  label: PropTypes.string,
+  name: PropTypes.string.isRequired,
+  onChange: PropTypes.func.isRequired,
+  value: PropTypes.string.isRequired,
+};
+
 export default AnsibleSelect;

src/components/BasicChip/BasicChip.jsx

@@ -1,4 +1,6 @@
 import React from 'react';
+import PropTypes from 'prop-types';
+
 import { Chip } from '@patternfly/react-core';
 import './basicChip.scss';
 
@@ -10,4 +12,8 @@ const BasicChip = ({ text }) => (
   </Chip>
 );
 
+BasicChip.propTypes = {
+  text: PropTypes.string.isRequired,
+};
+
 export default BasicChip;

src/components/Breadcrumbs/Breadcrumbs.jsx

@@ -1,4 +1,5 @@
 import React, { Fragment } from 'react';
+import PropTypes from 'prop-types';
 import {
   PageSection,
   PageSectionVariants,
@@ -68,4 +69,12 @@ const Crumb = ({ breadcrumbConfig, match }) => {
   );
 };
 
+Breadcrumbs.propTypes = {
+  breadcrumbConfig: PropTypes.objectOf(PropTypes.string).isRequired,
+};
+
+Crumb.propTypes = {
+  breadcrumbConfig: PropTypes.objectOf(PropTypes.string).isRequired,
+};
+
 export default withRouter(Breadcrumbs);

src/components/DataListToolbar/DataListToolbar.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { I18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import {
@@ -36,7 +37,6 @@ class DataListToolbar extends React.Component {
     super(props);
 
     const { sortedColumnKey } = this.props;
-
     this.state = {
       isSearchDropdownOpen: false,
       isSortDropdownOpen: false,
@@ -282,4 +282,29 @@ class DataListToolbar extends React.Component {
   }
 }
 
+DataListToolbar.propTypes = {
+  addUrl: PropTypes.string,
+  columns: PropTypes.arrayOf(PropTypes.object).isRequired,
+  isAllSelected: PropTypes.bool,
+  onSearch: PropTypes.func,
+  onSelectAll: PropTypes.func,
+  onSort: PropTypes.func,
+  showDelete: PropTypes.bool,
+  showSelectAll: PropTypes.bool,
+  sortOrder: PropTypes.string,
+  sortedColumnKey: PropTypes.string,
+};
+
+DataListToolbar.defaultProps = {
+  addUrl: null,
+  onSearch: null,
+  onSelectAll: null,
+  onSort: null,
+  showDelete: false,
+  showSelectAll: false,
+  sortOrder: 'ascending',
+  sortedColumnKey: 'name',
+  isAllSelected: false,
+};
+
 export default DataListToolbar;

src/components/FormActionGroup.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 
 import { I18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -21,7 +22,7 @@ const buttonGroupStyle = {
   marginRight: '20px'
 };
 
-export default ({ onSubmit, submitDisabled, onCancel }) => (
+const FormActionGroup = ({ onSubmit, submitDisabled, onCancel }) => (
   <I18n>
     {({ i18n }) => (
       <ActionGroup style={formActionGroupStyle}>
@@ -37,3 +38,15 @@ export default ({ onSubmit, submitDisabled, onCancel }) => (
     )}
   </I18n>
 );
+
+FormActionGroup.propTypes = {
+  onCancel: PropTypes.func.isRequired,
+  onSubmit: PropTypes.func.isRequired,
+  submitDisabled: PropTypes.bool,
+};
+
+FormActionGroup.defaultProps = {
+  submitDisabled: false,
+};
+
+export default FormActionGroup;

src/components/ListItem/CheckboxListItem.jsx

@@ -1,11 +1,12 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { I18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import {
   Checkbox,
 } from '@patternfly/react-core';
 
-export default ({
+const CheckboxListItem = ({
   itemId,
   name,
   isSelected,
@@ -32,3 +33,12 @@ export default ({
     </div>
   </li>
 );
+
+CheckboxListItem.propTypes = {
+  itemId: PropTypes.number.isRequired,
+  name: PropTypes.string.isRequired,
+  isSelected: PropTypes.bool.isRequired,
+  onSelect: PropTypes.func.isRequired,
+};
+
+export default CheckboxListItem;

src/components/Lookup/Lookup.jsx

@@ -1,5 +1,5 @@
 import React, { Fragment } from 'react';
-
+import PropTypes from 'prop-types';
 import { SearchIcon, CubesIcon } from '@patternfly/react-icons';
 import {
   Modal,
@@ -125,7 +125,7 @@ class Lookup extends React.Component {
 
   render () {
     const { isModalOpen, lookupSelectedItems, error, results, count, page, page_size } = this.state;
-    const { lookupHeader = 'items', value } = this.props;
+    const { lookupHeader, value } = this.props;
 
     return (
       <I18n>
@@ -174,6 +174,7 @@ class Lookup extends React.Component {
                     pageCount={Math.ceil(count / page_size)}
                     page_size={page_size}
                     onSetPage={this.onSetPage}
+                    pageSizeOptions={null}
                     style={paginationStyling}
                   />
                 </Fragment>
@@ -194,4 +195,18 @@ class Lookup extends React.Component {
     );
   }
 }
+
+Lookup.propTypes = {
+  getItems: PropTypes.func.isRequired,
+  lookupHeader: PropTypes.string,
+  name: PropTypes.string,
+  onLookupSave: PropTypes.func.isRequired,
+  value: PropTypes.arrayOf(PropTypes.object).isRequired,
+};
+
+Lookup.defaultProps = {
+  lookupHeader: 'items',
+  name: null,
+};
+
 export default Lookup;

src/components/NavExpandableGroup.jsx

@@ -1,4 +1,5 @@
 import React, { Component } from 'react';
+import PropTypes from 'prop-types';
 import {
   withRouter
 } from 'react-router-dom';
@@ -55,4 +56,10 @@ class NavExpandableGroup extends Component {
   }
 }
 
+NavExpandableGroup.propTypes = {
+  groupId: PropTypes.string.isRequired,
+  groupTitle: PropTypes.string.isRequired,
+  routes: PropTypes.arrayOf(PropTypes.object).isRequired,
+};
+
 export default withRouter(NavExpandableGroup);

src/components/NotificationsList/NotificationListItem.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { I18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import {
@@ -20,7 +21,6 @@ class NotificationListItem extends React.Component {
       errorTurnedOn,
       toggleNotification
     } = this.props;
-
     const capText = {
       textTransform: 'capitalize'
     };
@@ -69,5 +69,21 @@ class NotificationListItem extends React.Component {
   }
 }
 
+NotificationListItem.propTypes = {
+  detailUrl: PropTypes.string.isRequired,
+  errorTurnedOn: PropTypes.bool,
+  itemId: PropTypes.number.isRequired,
+  name: PropTypes.string,
+  notificationType: PropTypes.string.isRequired,
+  successTurnedOn: PropTypes.bool,
+  toggleNotification: PropTypes.func.isRequired,
+};
+
+NotificationListItem.defaultProps = {
+  errorTurnedOn: false,
+  name: null,
+  successTurnedOn: false,
+};
+
 export default NotificationListItem;
 

src/components/NotificationsList/Notifications.list.jsx

@@ -2,6 +2,7 @@ import React, {
   Component,
   Fragment
 } from 'react';
+import PropTypes from 'prop-types';
 import { Title, EmptyState, EmptyStateIcon, EmptyStateBody } from '@patternfly/react-core';
 import { CubesIcon } from '@patternfly/react-icons';
 import { I18n, i18nMark } from '@lingui/react';
@@ -29,8 +30,6 @@ class Notifications extends Component {
     order_by: 'name',
   };
 
-  pageSizeOptions = [5, 10, 25, 50];
-
   constructor (props) {
     super(props);
 
@@ -55,7 +54,6 @@ class Notifications extends Component {
     this.onSort = this.onSort.bind(this);
     this.onSetPage = this.onSetPage.bind(this);
     this.onSelectAll = this.onSelectAll.bind(this);
-    this.onSelect = this.onSelect.bind(this);
     this.toggleNotification = this.toggleNotification.bind(this);
     this.updateUrl = this.updateUrl.bind(this);
     this.postToError = this.postToError.bind(this);
@@ -114,18 +112,6 @@ class Notifications extends Component {
     this.setState({ selected });
   };
 
-  onSelect = id => {
-    const { selected } = this.state;
-
-    const isSelected = selected.includes(id);
-
-    if (isSelected) {
-      this.setState({ selected: selected.filter(s => s !== id) });
-    } else {
-      this.setState({ selected: selected.concat(id) });
-    }
-  };
-
   toggleNotification = (id, isCurrentlyOn, status) => {
     if (status === 'success') {
       this.postToSuccess(id, isCurrentlyOn);
@@ -286,7 +272,6 @@ class Notifications extends Component {
       successTemplateIds,
       errorTemplateIds
     } = this.state;
-
     return (
       <Fragment>
         {noInitialResults && (
@@ -326,8 +311,6 @@ class Notifications extends Component {
                       name={o.name}
                       notificationType={o.notification_type}
                       detailUrl={`/notifications/${o.id}`}
-                      isSelected={selected.includes(o.id)}
-                      onSelect={() => this.onSelect(o.id)}
                       toggleNotification={this.toggleNotification}
                       errorTurnedOn={errorTemplateIds.includes(o.id)}
                       successTurnedOn={successTemplateIds.includes(o.id)}
@@ -341,7 +324,6 @@ class Notifications extends Component {
               page={page}
               pageCount={pageCount}
               page_size={page_size}
-              pageSizeOptions={this.pageSizeOptions}
               onSetPage={this.onSetPage}
             />
           </Fragment>
@@ -353,4 +335,12 @@ class Notifications extends Component {
   }
 }
 
+Notifications.propType = {
+  getError: PropTypes.func.isRequired,
+  getNotifications: PropTypes.func.isRequired,
+  getSuccess: PropTypes.func.isRequired,
+  postError: PropTypes.func.isRequired,
+  postSuccess: PropTypes.func.isRequired,
+};
+
 export default Notifications;

src/components/PageHeaderToolbar.jsx

@@ -1,4 +1,5 @@
 import React, { Component } from 'react';
+import PropTypes from 'prop-types';
 import { Link } from 'react-router-dom';
 
 import { t } from '@lingui/macro';
@@ -126,4 +127,14 @@ class PageHeaderToolbar extends Component {
   }
 }
 
+PageHeaderToolbar.propTypes = {
+  isAboutDisabled: PropTypes.bool,
+  onAboutClick: PropTypes.func.isRequired,
+  onLogoutClick: PropTypes.func.isRequired,
+};
+
+PageHeaderToolbar.defaultProps = {
+  isAboutDisabled: false,
+};
+
 export default PageHeaderToolbar;

src/components/Pagination/Pagination.jsx

@@ -1,4 +1,5 @@
 import React, { Component } from 'react';
+import PropTypes from 'prop-types';
 import { I18n } from '@lingui/react';
 import { Trans, t } from '@lingui/macro';
 import {
@@ -224,4 +225,19 @@ class Pagination extends Component {
   }
 }
 
+Pagination.propTypes = {
+  count: PropTypes.number,
+  onSetPage: PropTypes.func.isRequired,
+  page: PropTypes.number.isRequired,
+  pageCount: PropTypes.number,
+  pageSizeOptions: PropTypes.arrayOf(PropTypes.number),
+  page_size: PropTypes.number.isRequired,
+};
+
+Pagination.defaultProps = {
+  count: null,
+  pageCount: null,
+  pageSizeOptions: [5, 10, 25, 50],
+};
+
 export default Pagination;

src/components/SelectedList/SelectedList.jsx

@@ -1,4 +1,5 @@
 import React, { Component } from 'react';
+import PropTypes from 'prop-types';
 import {
   Chip
 } from '@patternfly/react-core';
@@ -70,4 +71,16 @@ class SelectedList extends Component {
   }
 }
 
+SelectedList.propTypes = {
+  label: PropTypes.string,
+  onRemove: PropTypes.func.isRequired,
+  selected: PropTypes.arrayOf(PropTypes.object).isRequired,
+  showOverflowAfter: PropTypes.number,
+};
+
+SelectedList.defaultProps = {
+  label: 'Selected',
+  showOverflowAfter: 5,
+};
+
 export default SelectedList;

src/components/Tabs/Tab.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { NavLink } from 'react-router-dom';
 import './tabs.scss';
 
@@ -15,4 +16,18 @@ const Tab = ({ children, link, replace }) => (
   </li>
 );
 
+Tab.propTypes = {
+  children: PropTypes.oneOfType([
+    PropTypes.arrayOf(PropTypes.node),
+    PropTypes.node
+  ]).isRequired,
+  link: PropTypes.string,
+  replace: PropTypes.bool,
+};
+
+Tab.defaultProps = {
+  link: null,
+  replace: false,
+};
+
 export default Tab;

src/components/Tabs/Tabs.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 import { Link } from 'react-router-dom';
 import { Button } from '@patternfly/react-core';
 import { TimesIcon } from '@patternfly/react-icons';
@@ -35,4 +36,21 @@ const Tabs = ({ children, labelText, closeButton }) => (
   </div>
 );
 
+Tabs.propTypes = {
+  children: PropTypes.oneOfType([
+    PropTypes.arrayOf(PropTypes.node),
+    PropTypes.node
+  ]).isRequired,
+  labelText: PropTypes.string,
+  closeButton: PropTypes.shape({
+    text: PropTypes.string,
+    link: PropTypes.string,
+  }),
+};
+
+Tabs.defaultProps = {
+  labelText: null,
+  closeButton: null,
+};
+
 export default Tabs;

src/components/Tooltip/Tooltip.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 
 class Tooltip extends React.Component {
   transforms = {
@@ -74,4 +75,14 @@ class Tooltip extends React.Component {
   }
 }
 
+Tooltip.propTypes = {
+  children: PropTypes.element.isRequired,
+  message: PropTypes.string.isRequired,
+  position: PropTypes.string,
+};
+
+Tooltip.defaultProps = {
+  position: 'top',
+};
+
 export default Tooltip;

src/components/TowerLogo/TowerLogo.jsx

@@ -1,4 +1,5 @@
 import React, { Component } from 'react';
+import PropTypes from 'prop-types';
 import { withRouter } from 'react-router-dom';
 import { I18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -58,4 +59,12 @@ class TowerLogo extends Component {
   }
 }
 
+TowerLogo.propTypes = {
+  linkTo: PropTypes.string,
+};
+
+TowerLogo.defaultProps = {
+  linkTo: null,
+};
+
 export default withRouter(TowerLogo);

src/pages/Organizations/screens/OrganizationAdd.jsx

@@ -137,16 +137,18 @@ class OrganizationAdd extends React.Component {
                 </FormGroup>
                 <ConfigContext.Consumer>
                   {({ custom_virtualenvs }) => (
-                    <FormGroup label="Ansible Environment" fieldId="add-org-form-custom-virtualenv">
+                    custom_virtualenvs && custom_virtualenvs.length > 1 && (
-                      <AnsibleSelect
+                      <FormGroup label="Ansible Environment" fieldId="add-org-custom-virtualenv">
-                        label="Ansible Environment"
+                        <AnsibleSelect
-                        name="custom_virtualenv"
+                          label="Ansible Environment"
-                        value={custom_virtualenv}
+                          name="custom_virtualenv"
-                        onChange={this.onFieldChange}
+                          value={custom_virtualenv}
-                        data={custom_virtualenvs}
+                          onChange={this.onFieldChange}
-                        defaultSelected={defaultEnv}
+                          data={custom_virtualenvs}
-                      />
+                          defaultSelected={defaultEnv}
-                    </FormGroup>
+                        />
+                      </FormGroup>
+                    )
                   )}
                 </ConfigContext.Consumer>
               </Gallery>

src/pages/Organizations/screens/OrganizationsList.jsx

@@ -39,8 +39,6 @@ class OrganizationsList extends Component {
     order_by: 'name',
   };
 
-  pageSizeOptions = [5, 10, 25, 50];
-
   constructor (props) {
     super(props);
 
@@ -207,7 +205,6 @@ class OrganizationsList extends Component {
       selected,
     } = this.state;
     const { match } = this.props;
-
     return (
       <PageSection variant={medium}>
         <Card>
@@ -258,7 +255,6 @@ class OrganizationsList extends Component {
                 page={page}
                 pageCount={pageCount}
                 page_size={page_size}
-                pageSizeOptions={this.pageSizeOptions}
                 onSetPage={this.onSetPage}
               />
               { loading ? <div>loading...</div> : '' }