From e8d73babaf80b645fccc87639495d9d5bd97610f Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Mon, 22 Apr 2019 17:15:32 -0400
Subject: [PATCH] Rebase and incorporates feedback

---
 __tests__/components/AddResourceRole.test.jsx | 124 ++++++++----------
 __tests__/components/CheckboxCard.test.jsx    |   5 +-
 .../components/SelectResourceStep.test.jsx    | 120 ++++++++---------
 __tests__/components/SelectRoleStep.test.jsx  |   4 +-
 __tests__/components/SelectableCard.test.jsx  |   6 +-
 src/components/AddRole/AddResourceRole.jsx    |  23 +++-
 src/components/AddRole/SelectResourceStep.jsx |  39 +++---
 src/components/AddRole/styles.scss            |   2 +-
 src/components/SelectedList/SelectedList.jsx  |  12 +-
 9 files changed, 161 insertions(+), 174 deletions(-)

diff --git a/__tests__/components/AddResourceRole.test.jsx b/__tests__/components/AddResourceRole.test.jsx
index 68a3b03fad..f439a320d3 100644
--- a/__tests__/components/AddResourceRole.test.jsx
+++ b/__tests__/components/AddResourceRole.test.jsx
@@ -1,6 +1,6 @@
 import React from 'react';
-import { mount } from 'enzyme';
-import { I18nProvider } from '@lingui/react';
+import { shallow } from 'enzyme';
+import { mountWithContexts } from '../enzymeHelpers';
 import AddResourceRole from '../../src/components/AddRole/AddResourceRole';
 
 describe('<SelectResourceStep />', () => {
@@ -30,28 +30,24 @@ describe('<SelectResourceStep />', () => {
     }
   };
   test('initially renders without crashing', () => {
-    mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          roles={roles}
-        />
-      </I18nProvider>
+    shallow(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={() => {}}
+        roles={roles}
+      />
     );
   });
   test('handleRoleCheckboxClick properly updates state', () => {
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          roles={roles}
-        />
-      </I18nProvider>
-    ).find('AddResourceRole');
+    const wrapper = shallow(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={() => {}}
+        roles={roles}
+      />
+    );
     wrapper.setState({
       selectedRoleRows: [
         {
@@ -79,16 +75,14 @@ describe('<SelectResourceStep />', () => {
     }]);
   });
   test('handleResourceCheckboxClick properly updates state', () => {
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          roles={roles}
-        />
-      </I18nProvider>
-    ).find('AddResourceRole');
+    const wrapper = shallow(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={() => {}}
+        roles={roles}
+      />
+    );
     wrapper.setState({
       selectedResourceRows: [
         {
@@ -113,15 +107,13 @@ describe('<SelectResourceStep />', () => {
   });
   test('clicking user/team cards updates state', () => {
     const spy = jest.spyOn(AddResourceRole.prototype, 'handleResourceSelect');
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          api={api}
-          roles={roles}
-        />
-      </I18nProvider>
+    const wrapper = mountWithContexts(
+      <AddResourceRole
+        onClose={() => {}}
+        onSave={() => {}}
+        api={api}
+        roles={roles}
+      />
     ).find('AddResourceRole');
     const selectableCardWrapper = wrapper.find('SelectableCard');
     expect(selectableCardWrapper.length).toBe(2);
@@ -133,16 +125,14 @@ describe('<SelectResourceStep />', () => {
     expect(wrapper.state('selectedResource')).toBe('teams');
   });
   test('readUsers and readTeams call out to corresponding api functions', () => {
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          roles={roles}
-        />
-      </I18nProvider>
-    ).find('AddResourceRole');
+    const wrapper = shallow(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={() => {}}
+        roles={roles}
+      />
+    );
     wrapper.instance().readUsers({
       foo: 'bar'
     });
@@ -158,16 +148,14 @@ describe('<SelectResourceStep />', () => {
     });
   });
   test('handleResourceSelect clears out selected lists and sets selectedResource', () => {
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={jest.fn()}
-          roles={roles}
-        />
-      </I18nProvider>
-    ).find('AddResourceRole');
+    const wrapper = shallow(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={() => {}}
+        roles={roles}
+      />
+    );
     wrapper.setState({
       selectedResource: 'teams',
       selectedResourceRows: [
@@ -199,15 +187,13 @@ describe('<SelectResourceStep />', () => {
   });
   test('handleWizardSave makes correct api calls, calls onSave when done', async () => {
     const handleSave = jest.fn();
-    const wrapper = mount(
-      <I18nProvider>
-        <AddResourceRole
-          api={api}
-          onClose={jest.fn()}
-          onSave={handleSave}
-          roles={roles}
-        />
-      </I18nProvider>
+    const wrapper = mountWithContexts(
+      <AddResourceRole
+        api={api}
+        onClose={() => {}}
+        onSave={handleSave}
+        roles={roles}
+      />
     ).find('AddResourceRole');
     wrapper.setState({
       selectedResource: 'users',
diff --git a/__tests__/components/CheckboxCard.test.jsx b/__tests__/components/CheckboxCard.test.jsx
index de02dca288..7b689e0dd9 100644
--- a/__tests__/components/CheckboxCard.test.jsx
+++ b/__tests__/components/CheckboxCard.test.jsx
@@ -1,17 +1,16 @@
 import React from 'react';
-import { mount } from 'enzyme';
+import { shallow } from 'enzyme';
 import CheckboxCard from '../../src/components/AddRole/CheckboxCard';
 
 describe('<CheckboxCard />', () => {
   let wrapper;
   test('initially renders without crashing', () => {
-    wrapper = mount(
+    wrapper = shallow(
       <CheckboxCard
         name="Foobar"
         itemId={5}
       />
     );
     expect(wrapper.length).toBe(1);
-    wrapper.unmount();
   });
 });
diff --git a/__tests__/components/SelectResourceStep.test.jsx b/__tests__/components/SelectResourceStep.test.jsx
index 5097a5ce03..0c207b871e 100644
--- a/__tests__/components/SelectResourceStep.test.jsx
+++ b/__tests__/components/SelectResourceStep.test.jsx
@@ -1,6 +1,6 @@
 import React from 'react';
-import { mount } from 'enzyme';
-import { I18nProvider } from '@lingui/react';
+import { shallow } from 'enzyme';
+import { mountWithContexts } from '../enzymeHelpers';
 import SelectResourceStep from '../../src/components/AddRole/SelectResourceStep';
 
 describe('<SelectResourceStep />', () => {
@@ -11,16 +11,14 @@ describe('<SelectResourceStep />', () => {
     jest.restoreAllMocks();
   });
   test('initially renders without crashing', () => {
-    mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={jest.fn()}
-          onSearch={jest.fn()}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    shallow(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={() => {}}
+        onSearch={() => {}}
+        sortedColumnKey="username"
+      />
     );
   });
   test('fetches resources on mount', async () => {
@@ -33,16 +31,14 @@ describe('<SelectResourceStep />', () => {
         ]
       }
     });
-    mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={jest.fn()}
-          onSearch={handleSearch}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    mountWithContexts(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={() => {}}
+        onSearch={handleSearch}
+        sortedColumnKey="username"
+      />
     );
     expect(handleSearch).toHaveBeenCalledWith({
       order_by: 'username',
@@ -66,17 +62,15 @@ describe('<SelectResourceStep />', () => {
         ]
       }
     });
-    const wrapper = await mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={jest.fn()}
-          onSearch={handleSearch}
-          selectedResourceRows={selectedResourceRows}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    const wrapper = await mountWithContexts(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={() => {}}
+        onSearch={handleSearch}
+        selectedResourceRows={selectedResourceRows}
+        sortedColumnKey="username"
+      />
     ).find('SelectResourceStep');
     await wrapper.instance().readResourceList({
       page: 1,
@@ -93,17 +87,15 @@ describe('<SelectResourceStep />', () => {
   });
   test('handleSetPage calls readResourceList with correct params', () => {
     const spy = jest.spyOn(SelectResourceStep.prototype, 'readResourceList');
-    const wrapper = mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={jest.fn()}
-          onSearch={jest.fn()}
-          selectedResourceRows={[]}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    const wrapper = mountWithContexts(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={() => {}}
+        onSearch={() => {}}
+        selectedResourceRows={[]}
+        sortedColumnKey="username"
+      />
     ).find('SelectResourceStep');
     wrapper.setState({ sortOrder: 'descending' });
     wrapper.instance().handleSetPage(2);
@@ -114,17 +106,15 @@ describe('<SelectResourceStep />', () => {
   });
   test('handleSort calls readResourceList with correct params', () => {
     const spy = jest.spyOn(SelectResourceStep.prototype, 'readResourceList');
-    const wrapper = mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={jest.fn()}
-          onSearch={jest.fn()}
-          selectedResourceRows={[]}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    const wrapper = mountWithContexts(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={() => {}}
+        onSearch={() => {}}
+        selectedResourceRows={[]}
+        sortedColumnKey="username"
+      />
     ).find('SelectResourceStep');
     wrapper.instance().handleSort('username', 'descending');
     expect(spy).toHaveBeenCalledWith({ page: 1, page_size: 5, order_by: '-username' });
@@ -133,17 +123,15 @@ describe('<SelectResourceStep />', () => {
   });
   test('clicking on row fires callback with correct params', () => {
     const handleRowClick = jest.fn();
-    const wrapper = mount(
-      <I18nProvider>
-        <SelectResourceStep
-          columns={columns}
-          displayKey="username"
-          onRowClick={handleRowClick}
-          onSearch={jest.fn()}
-          selectedResourceRows={[]}
-          sortedColumnKey="username"
-        />
-      </I18nProvider>
+    const wrapper = mountWithContexts(
+      <SelectResourceStep
+        columns={columns}
+        displayKey="username"
+        onRowClick={handleRowClick}
+        onSearch={() => {}}
+        selectedResourceRows={[]}
+        sortedColumnKey="username"
+      />
     );
     const selectResourceStepWrapper = wrapper.find('SelectResourceStep');
     selectResourceStepWrapper.setState({
diff --git a/__tests__/components/SelectRoleStep.test.jsx b/__tests__/components/SelectRoleStep.test.jsx
index 857ec0a9f6..6e323e2936 100644
--- a/__tests__/components/SelectRoleStep.test.jsx
+++ b/__tests__/components/SelectRoleStep.test.jsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { mount } from 'enzyme';
+import { mount, shallow } from 'enzyme';
 import SelectRoleStep from '../../src/components/AddRole/SelectRoleStep';
 
 describe('<SelectRoleStep />', () => {
@@ -30,7 +30,7 @@ describe('<SelectRoleStep />', () => {
     }
   ];
   test('initially renders without crashing', () => {
-    wrapper = mount(
+    wrapper = shallow(
       <SelectRoleStep
         roles={roles}
         selectedResourceRows={selectedResourceRows}
diff --git a/__tests__/components/SelectableCard.test.jsx b/__tests__/components/SelectableCard.test.jsx
index b9608f6333..fe123f8e8c 100644
--- a/__tests__/components/SelectableCard.test.jsx
+++ b/__tests__/components/SelectableCard.test.jsx
@@ -1,12 +1,12 @@
 import React from 'react';
-import { mount } from 'enzyme';
+import { shallow } from 'enzyme';
 import SelectableCard from '../../src/components/AddRole/SelectableCard';
 
 describe('<SelectableCard />', () => {
   let wrapper;
   const onClick = jest.fn();
   test('initially renders without crashing when not selected', () => {
-    wrapper = mount(
+    wrapper = shallow(
       <SelectableCard
         onClick={onClick}
       />
@@ -15,7 +15,7 @@ describe('<SelectableCard />', () => {
     wrapper.unmount();
   });
   test('initially renders without crashing when selected', () => {
-    wrapper = mount(
+    wrapper = shallow(
       <SelectableCard
         isSelected
         onClick={onClick}
diff --git a/src/components/AddRole/AddResourceRole.jsx b/src/components/AddRole/AddResourceRole.jsx
index 1221cb6e9d..5b702b8f27 100644
--- a/src/components/AddRole/AddResourceRole.jsx
+++ b/src/components/AddRole/AddResourceRole.jsx
@@ -142,6 +142,19 @@ class AddResourceRole extends React.Component {
       { name: i18nMark('Name'), key: 'name', isSortable: true }
     ];
 
+    let wizardTitle = '';
+
+    switch (selectedResource) {
+      case 'users':
+        wizardTitle = i18nMark('Add User Roles');
+        break;
+      case 'teams':
+        wizardTitle = i18nMark('Add Team Roles');
+        break;
+      default:
+        wizardTitle = i18nMark('Add Roles');
+    }
+
     const steps = [
       {
         name: i18nMark('Select Users Or Teams'),
@@ -179,10 +192,9 @@ class AddResourceRole extends React.Component {
                     emptyListTitle={i18n._(t`No Users Found`)}
                     onRowClick={this.handleResourceCheckboxClick}
                     onSearch={this.readUsers}
-                    selectedLabel={i18n._(t`Selected Users`)}
+                    selectedLabel={i18n._(t`Selected`)}
                     selectedResourceRows={selectedResourceRows}
                     sortedColumnKey="username"
-                    title={i18n._(t`Users`)}
                   />
                 )}
                 {selectedResource === 'teams' && (
@@ -192,9 +204,8 @@ class AddResourceRole extends React.Component {
                     emptyListTitle={i18n._(t`No Teams Found`)}
                     onRowClick={this.handleResourceCheckboxClick}
                     onSearch={this.readTeams}
-                    selectedLabel={i18n._(t`Selected Teams`)}
+                    selectedLabel={i18n._(t`Selected`)}
                     selectedResourceRows={selectedResourceRows}
-                    title={i18n._(t`Teams`)}
                   />
                 )}
               </Fragment>
@@ -212,7 +223,7 @@ class AddResourceRole extends React.Component {
                 onRolesClick={this.handleRoleCheckboxClick}
                 roles={roles}
                 selectedListKey={selectedResource === 'users' ? 'username' : 'name'}
-                selectedListLabel={selectedResource === 'users' ? i18n._(t`Selected Users`) : i18n._(t`Selected Teams`)}
+                selectedListLabel={i18n._(t`Selected`)}
                 selectedResourceRows={selectedResourceRows}
                 selectedRoleRows={selectedRoleRows}
               />
@@ -234,7 +245,7 @@ class AddResourceRole extends React.Component {
             onClose={onClose}
             onSave={this.handleWizardSave}
             steps={steps}
-            title={i18n._(t`Add Roles`)}
+            title={wizardTitle}
           />
         )}
       </I18n>
diff --git a/src/components/AddRole/SelectResourceStep.jsx b/src/components/AddRole/SelectResourceStep.jsx
index 9f15478631..f921913005 100644
--- a/src/components/AddRole/SelectResourceStep.jsx
+++ b/src/components/AddRole/SelectResourceStep.jsx
@@ -17,6 +17,14 @@ import DataListToolbar from '../DataListToolbar';
 import Pagination from '../Pagination';
 import SelectedList from '../SelectedList';
 
+const paginationStyling = {
+  paddingLeft: '0',
+  justifyContent: 'flex-end',
+  borderRight: '1px solid #ebebeb',
+  borderBottom: '1px solid #ebebeb',
+  borderTop: '0'
+};
+
 class SelectResourceStep extends React.Component {
   constructor (props) {
     super(props);
@@ -119,8 +127,7 @@ class SelectResourceStep extends React.Component {
       emptyListTitle,
       onRowClick,
       selectedLabel,
-      selectedResourceRows,
-      title
+      selectedResourceRows
     } = this.props;
 
     return (
@@ -138,9 +145,15 @@ class SelectResourceStep extends React.Component {
             </EmptyState>
           ) : (
             <Fragment>
-              <Title size="lg">
-                {title}
-              </Title>
+              {selectedResourceRows.length > 0 && (
+                <SelectedList
+                  displayKey={displayKey}
+                  label={selectedLabel}
+                  onRemove={onRowClick}
+                  selected={selectedResourceRows}
+                  showOverflowAfter={5}
+                />
+              )}
               <DataListToolbar
                 columns={columns}
                 noLeftMargin
@@ -168,19 +181,11 @@ class SelectResourceStep extends React.Component {
                 pageSizeOptions={null}
                 page_size={page_size}
                 showPageSizeOptions={false}
+                style={paginationStyling}
               />
             </Fragment>
           )}
         </Fragment>
-        {selectedResourceRows.length > 0 && (
-          <SelectedList
-            displayKey={displayKey}
-            label={selectedLabel}
-            onRemove={onRowClick}
-            selected={selectedResourceRows}
-            showOverflowAfter={5}
-          />
-        )}
         { error ? <div>error</div> : '' }
       </Fragment>
     );
@@ -196,8 +201,7 @@ SelectResourceStep.propTypes = {
   onSearch: PropTypes.func.isRequired,
   selectedLabel: PropTypes.string,
   selectedResourceRows: PropTypes.arrayOf(PropTypes.object),
-  sortedColumnKey: PropTypes.string,
-  title: PropTypes.string
+  sortedColumnKey: PropTypes.string
 };
 
 SelectResourceStep.defaultProps = {
@@ -207,8 +211,7 @@ SelectResourceStep.defaultProps = {
   onRowClick: () => {},
   selectedLabel: i18nMark('Selected Items'),
   selectedResourceRows: [],
-  sortedColumnKey: 'name',
-  title: ''
+  sortedColumnKey: 'name'
 };
 
 export default SelectResourceStep;
diff --git a/src/components/AddRole/styles.scss b/src/components/AddRole/styles.scss
index 21a8a73989..6349d03421 100644
--- a/src/components/AddRole/styles.scss
+++ b/src/components/AddRole/styles.scss
@@ -8,7 +8,7 @@
 
   .awx-selectableCard__indicator {
     display: flex;
-    flex: 0 0 10px;
+    flex: 0 0 5px;
   }
 
   .awx-selectableCard__label {
diff --git a/src/components/SelectedList/SelectedList.jsx b/src/components/SelectedList/SelectedList.jsx
index bf3b295d6d..485eaa615c 100644
--- a/src/components/SelectedList/SelectedList.jsx
+++ b/src/components/SelectedList/SelectedList.jsx
@@ -45,6 +45,7 @@ class SelectedList extends Component {
       isReadOnly
     } = this.props;
     const { showOverflow } = this.state;
+    const visibleItems = selected.slice(0, showOverflow ? selected.length : showOverflowAfter);
     return (
       <div className="awx-selectedList">
         <div className="pf-l-split" style={selectedRowStyling}>
@@ -56,20 +57,19 @@ class SelectedList extends Component {
             <div className="pf-c-chip-group">
               {isReadOnly ? (
                 <Fragment>
-                  {selected
-                    .slice(0, showOverflow ? selected.length : showOverflowAfter)
+                  {visibleItems
                     .map(selectedItem => (
                       <BasicChip
                         key={selectedItem.id}
-                        text={selectedItem[displayKey]}
-                      />
+                      >
+                        {selectedItem[displayKey]}
+                      </BasicChip>
                     ))
                   }
                 </Fragment>
               ) : (
                 <Fragment>
-                  {selected
-                    .slice(0, showOverflow ? selected.length : showOverflowAfter)
+                  {visibleItems
                     .map(selectedItem => (
                       <Chip
                         key={selectedItem.id}