From 680d153a14ef50995001cabac9143019018b22f9 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 11 Feb 2019 18:10:27 -0500 Subject: [PATCH 1/9] add pagination to instance groups lookup modal --- __tests__/components/Lookup.test.jsx | 70 +++++-- .../screens/OrganizationAdd.test.jsx | 101 +++++----- src/api.js | 4 +- src/app.scss | 16 +- .../AnsibleSelect/AnsibleSelect.jsx | 6 +- src/components/DataListToolbar/styles.scss | 2 +- src/components/FormActionGroup.jsx | 39 ++++ src/components/Lookup/Lookup.jsx | 184 ++++++++++++------ src/components/Pagination/Pagination.jsx | 63 +++--- src/components/Pagination/styles.scss | 2 +- src/components/SelectedList/SelectedList.jsx | 16 +- src/components/SelectedList/styles.scss | 2 +- .../Organizations/screens/OrganizationAdd.jsx | 121 ++++-------- 13 files changed, 369 insertions(+), 257 deletions(-) create mode 100644 src/components/FormActionGroup.jsx diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index d5623aad55..bec3e1369a 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -7,14 +7,35 @@ let mockData = [{ name: 'foo', id: 1, isChecked: false }]; describe('', () => { test('initially renders succesfully', () => { mount( - { }} - data={mockData} - selected={[]} - /> + + { }} + getEndpoint={() => { }} + /> + ); }); + test('API response is formatted properly', (done) => { + const wrapper = mount( + + { }} + getEndpoint={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} + /> + + ).find('Lookup'); + + setImmediate(() => { + expect(wrapper.state().results).toEqual([{ id: 1, name: 'test instance' }]); + done(); + }); + }); test('Opens modal when search icon is clicked', () => { const spy = jest.spyOn(Lookup.prototype, 'handleModalToggle'); const mockSelected = [{ name: 'foo', id: 1 }]; @@ -22,9 +43,10 @@ describe('', () => { { }} - data={mockData} - selected={mockSelected} + getEndpoint={() => { }} /> ).find('Lookup'); @@ -39,34 +61,39 @@ describe('', () => { }]); expect(wrapper.state('isModalOpen')).toEqual(true); }); - test('calls "toggleSelected" when a user changes a checkbox', () => { + test('calls "toggleSelected" when a user changes a checkbox', (done) => { const spy = jest.spyOn(Lookup.prototype, 'toggleSelected'); + const mockSelected = [{ name: 'foo', id: 1 }]; const wrapper = mount( { }} - data={mockData} - selected={[]} + getEndpoint={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} /> ); - const searchItem = wrapper.find('.pf-c-input-group__text#search'); - searchItem.first().simulate('click'); - wrapper.find('input[type="checkbox"]').simulate('change'); - expect(spy).toHaveBeenCalled(); + setImmediate(() => { + const searchItem = wrapper.find('.pf-c-input-group__text#search'); + searchItem.first().simulate('click'); + wrapper.find('input[type="checkbox"]').simulate('change'); + expect(spy).toHaveBeenCalled(); + done(); + }); }); 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 }]; - const mockSelected = [{ name: 'foo', id: 1 }, { name: 'bar', id: 2 }]; const wrapper = mount( { }} - data={mockData} - selected={mockSelected} + getEndpoint={() => { }} /> ); @@ -124,9 +151,10 @@ describe('', () => { { }} /> ).find('Lookup'); @@ -142,6 +170,6 @@ describe('', () => { expect(onLookupSaveFn).toHaveBeenCalledWith([{ id: 1, name: 'foo' - }]); + }], 'fooBar'); }); }); diff --git a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx index 27e0b7b07b..6a41d1d1ff 100644 --- a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx +++ b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx @@ -1,27 +1,32 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; +import { I18nProvider } from '@lingui/react'; import OrganizationAdd from '../../../../src/pages/Organizations/screens/OrganizationAdd'; describe('', () => { test('initially renders succesfully', () => { mount( - + + + ); }); - test('calls "handleChange" when input values change', () => { - const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'handleChange'); + test('calls "onFieldChange" when input values change', () => { + const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onFieldChange'); const wrapper = mount( - + + + ); expect(spy).not.toHaveBeenCalled(); @@ -33,79 +38,69 @@ describe('', () => { const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onSubmit'); const wrapper = mount( - + + + ); expect(spy).not.toHaveBeenCalled(); - wrapper.find('button.at-C-SubmitButton').prop('onClick')(); + wrapper.find('button[aria-label="Save"]').prop('onClick')(); expect(spy).toBeCalled(); }); test('calls "onCancel" when Cancel button is clicked', () => { const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onCancel'); const wrapper = mount( - + + + ); expect(spy).not.toHaveBeenCalled(); - wrapper.find('button.at-C-CancelButton').prop('onClick')(); + wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); expect(spy).toBeCalled(); }); - test('API response is formatted properly', (done) => { - const mockedResp = { data: { results: [{ name: 'test instance', id: 1 }] } }; - const api = { getInstanceGroups: jest.fn().mockResolvedValue(mockedResp) }; - const wrapper = mount( - - - - ); - - setImmediate(() => { - const orgAddElem = wrapper.find('OrganizationAdd'); - expect([{ id: 1, isChecked: false, name: 'test instance' }]).toEqual(orgAddElem.state().results); - done(); - }); - }); - test('Successful form submission triggers redirect', (done) => { const onSuccess = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onSuccess'); - const resetForm = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'resetForm'); const mockedResp = { data: { id: 1, related: { instance_groups: '/bar' } } }; const api = { createOrganization: jest.fn().mockResolvedValue(mockedResp), createInstanceGroups: jest.fn().mockResolvedValue('done') }; const wrapper = mount( - + + + ); wrapper.find('input#add-org-form-name').simulate('change', { target: { value: 'foo' } }); - wrapper.find('button.at-C-SubmitButton').prop('onClick')(); + wrapper.find('button[aria-label="Save"]').prop('onClick')(); setImmediate(() => { expect(onSuccess).toHaveBeenCalled(); - expect(resetForm).toHaveBeenCalled(); done(); }); }); - test('updateSelectedInstanceGroups successfully sets selectedInstanceGroups state', () => { + test('onLookupSave successfully sets instanceGroups state', () => { const wrapper = mount( - + + + ).find('OrganizationAdd'); - wrapper.instance().updateSelectedInstanceGroups([ + wrapper.instance().onLookupSave([ { id: 1, name: 'foo' } - ]); - expect(wrapper.state('selectedInstanceGroups')).toEqual([ + ], 'instanceGroups'); + expect(wrapper.state('instanceGroups')).toEqual([ { id: 1, name: 'foo' @@ -113,14 +108,16 @@ describe('', () => { ]); }); - test('onSelectChange successfully sets custom_virtualenv state', () => { + test('onFieldChange successfully sets custom_virtualenv state', () => { const wrapper = mount( - + + + ).find('OrganizationAdd'); - wrapper.instance().onSelectChange('foobar'); - expect(wrapper.state('custom_virtualenv')).toBe('foobar'); + wrapper.instance().onFieldChange('fooBar', { target: { name: 'custom_virtualenv' } }); + expect(wrapper.state('custom_virtualenv')).toBe('fooBar'); }); test('onSubmit posts instance groups from selectedInstanceGroups', async () => { @@ -140,12 +137,14 @@ describe('', () => { }; const wrapper = mount( - + + + ).find('OrganizationAdd'); wrapper.setState({ name: 'mock org', - selectedInstanceGroups: [{ + instanceGroups: [{ id: 1, name: 'foo' }] diff --git a/src/api.js b/src/api.js index 028959b186..f0d3c866b8 100644 --- a/src/api.js +++ b/src/api.js @@ -106,8 +106,8 @@ class APIClient { return this.http.post(endpoint, data); } - getInstanceGroups () { - return this.http.get(API_INSTANCE_GROUPS); + getInstanceGroups (params) { + return this.http.get(API_INSTANCE_GROUPS, { params }); } createInstanceGroups (url, id) { diff --git a/src/app.scss b/src/app.scss index 555c87ecbc..393c1ac3f6 100644 --- a/src/app.scss +++ b/src/app.scss @@ -157,8 +157,11 @@ // .pf-c-modal-box__footer { - --pf-c-modal-box__footer--PaddingTop: 0; - --pf-c-modal-box__footer--PaddingBottom: 0; + --pf-c-modal-box__footer--PaddingTop: 20px; + --pf-c-modal-box__footer--PaddingRight: 20px; + --pf-c-modal-box__footer--PaddingBottom: 20px; + --pf-c-modal-box__footer--PaddingLeft: 20px; + justify-content: flex-end; } .pf-c-modal-box__header { @@ -171,6 +174,7 @@ .pf-c-modal-box__body { --pf-c-modal-box__body--PaddingLeft: 20px; --pf-c-modal-box__body--PaddingRight: 20px; + --pf-c-modal-box__body--PaddingBottom: 5px; } // @@ -215,12 +219,6 @@ } } -.at-align-right { - display: flex; - flex-direction: row; - justify-content: flex-end; -} - .awx-c-list { border-top: 1px solid #d7d7d7; border-bottom: 1px solid #d7d7d7; @@ -240,4 +238,4 @@ --pf-c-card__footer--PaddingY: 0; --pf-c-card__body--PaddingX: 0; --pf-c-card__body--PaddingY: 0; -} \ No newline at end of file +} diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx index ffaae0461a..a5038efa6f 100644 --- a/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -32,12 +32,12 @@ class AnsibleSelect extends React.Component { render () { const { count } = this.state; - const { labelName, selected, data } = this.props; + const { labelName, selected, data, fieldId } = this.props; let elem; if (count > 1) { elem = ( - - {data.map((datum) => ( ))} diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 7c17f50d56..1a31982629 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -1,6 +1,6 @@ .awx-toolbar { --awx-toolbar--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-toolbar--BorderColor: var(--pf-global--Color--light-200); + --awx-toolbar--BorderColor: #d1d1d1; --awx-toolbar--BorderWidth: var(--pf-global--BorderWidth--sm); border-bottom: var(--awx-toolbar--BorderWidth) solid var(--awx-toolbar--BorderColor); diff --git a/src/components/FormActionGroup.jsx b/src/components/FormActionGroup.jsx new file mode 100644 index 0000000000..682bad5ccd --- /dev/null +++ b/src/components/FormActionGroup.jsx @@ -0,0 +1,39 @@ +import React from 'react'; + +import { I18n } from '@lingui/react'; +import { t } from '@lingui/macro'; + +import { + ActionGroup, + Toolbar, + ToolbarGroup, + Button +} from '@patternfly/react-core'; + +const formActionGroupStyle = { + display: 'flex', + flexDirection: 'row', + justifyContent: 'flex-end', + marginTop: '10px' +}; + +const buttonGroupStyle = { + marginRight: '20px' +}; + +export default ({ onSubmit, isDisabled, onCancel }) => ( + + {({ i18n }) => ( + + + + + + + + + + + )} + +); diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index c4ffe97030..acf0f2ec2e 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -1,33 +1,82 @@ -import React from 'react'; +import React, { Fragment } from 'react'; -import { SearchIcon } from '@patternfly/react-icons'; +import { SearchIcon, CubesIcon } from '@patternfly/react-icons'; import { Modal, Button, - ActionGroup, - Toolbar, - ToolbarGroup, + EmptyState, + EmptyStateIcon, + EmptyStateBody, + Title } from '@patternfly/react-core'; import { I18n } from '@lingui/react'; -import { t, Trans } from '@lingui/macro'; +import { Trans, t } from '@lingui/macro'; import CheckboxListItem from '../ListItem'; - import SelectedList from '../SelectedList'; +import Pagination from '../Pagination'; + +const paginationStyling = { + paddingLeft: '0', + justifyContent: 'flex-end', + borderRight: '1px solid #d1d1d1', + borderBottom: '1px solid #d1d1d1', + borderTop: '0' +}; class Lookup extends React.Component { constructor (props) { super(props); this.state = { isModalOpen: false, - lookupSelectedItems: [] + lookupSelectedItems: [], + results: [], + count: 0, + page: 1, + page_size: 5, + error: null }; + this.onSetPage = this.onSetPage.bind(this); this.handleModalToggle = this.handleModalToggle.bind(this); this.wrapTags = this.wrapTags.bind(this); this.toggleSelected = this.toggleSelected.bind(this); this.saveModal = this.saveModal.bind(this); + this.getData = this.getData.bind(this); } + componentDidMount () { + const { page_size, page } = this.state; + this.getData({ page_size, page }); + } + + async getData (queryParams) { + const { getEndpoint } = this.props; + const { page } = queryParams; + + this.setState({ error: false }); + + try { + const { data } = await getEndpoint(queryParams); + const { results, count } = data; + + const stateToUpdate = { + page, + results, + count + }; + + this.setState(stateToUpdate); + } catch (err) { + this.setState({ error: true }); + } + } + + onSetPage = async (pageNumber, pageSize) => { + const page = parseInt(pageNumber, 10); + const page_size = parseInt(pageSize, 10); + this.getData({ page_size, page }); + }; + toggleSelected (row) { const { lookupSelectedItems } = this.state; const selectedIndex = lookupSelectedItems @@ -44,12 +93,12 @@ class Lookup extends React.Component { handleModalToggle () { const { isModalOpen } = this.state; - const { selected } = this.props; + const { value } = this.props; // Resets the selected items from parent state whenever modal is opened // This handles the case where the user closes/cancels the modal and // opens it again if (!isModalOpen) { - this.setState({ lookupSelectedItems: [...selected] }); + this.setState({ lookupSelectedItems: [...value] }); } this.setState((prevState) => ({ isModalOpen: !prevState.isModalOpen, @@ -57,13 +106,13 @@ class Lookup extends React.Component { } saveModal () { - const { onLookupSave } = this.props; + const { onLookupSave, name } = this.props; const { lookupSelectedItems } = this.state; - onLookupSave(lookupSelectedItems); + onLookupSave(lookupSelectedItems, name); this.handleModalToggle(); } - wrapTags (tags) { + wrapTags (tags = []) { return tags.map(tag => ( {tag.name} @@ -75,34 +124,66 @@ class Lookup extends React.Component { } render () { - const { isModalOpen, lookupSelectedItems } = this.state; - const { data, lookupHeader, selected } = this.props; + const { isModalOpen, lookupSelectedItems, error, results, count, page, page_size } = this.state; + const { value } = this.props; + let { lookupHeader } = this.props; + + if (!lookupHeader) { + lookupHeader = 'items'; + } + return ( -
- -
{this.wrapTags(selected)}
- -
    - {data.map(i => ( - item.id === i.id)} - onSelect={() => this.toggleSelected(i)} - /> - ))} -
- {lookupSelectedItems.length > 0 && ( - - {({ i18n }) => ( + + {({ i18n }) => ( +
+ +
{this.wrapTags(value)}
+ {i18n._(t`Save`)}, + + ]} + > + {(results.length === 0) && ( + + + + <Trans>{`No ${lookupHeader} Found`}</Trans> + + + {`Please add ${lookupHeader.toLowerCase()} to populate this list`} + + + ) || ( + +
    + {results.map(i => ( + item.id === i.id)} + onSelect={() => this.toggleSelected(i)} + /> + ))} +
+ + + )} + {lookupSelectedItems.length > 0 && ( )} - - )} - - - - - - - - - - - -
+ { error ?
error
: '' } + +
+ )} + ); } } diff --git a/src/components/Pagination/Pagination.jsx b/src/components/Pagination/Pagination.jsx index 8bbfe6360d..c96e9c94a1 100644 --- a/src/components/Pagination/Pagination.jsx +++ b/src/components/Pagination/Pagination.jsx @@ -105,10 +105,13 @@ class Pagination extends Component { pageCount, page_size, pageSizeOptions, + style } = this.props; const { value, isOpen } = this.state; - - const opts = pageSizeOptions.slice().reverse().filter(o => o !== page_size); + let opts; + if (pageSizeOptions) { + opts = pageSizeOptions.slice().reverse().filter(o => o !== page_size); + } const isOnFirst = page === 1; const isOnLast = page === pageCount; @@ -119,33 +122,35 @@ class Pagination extends Component { return ( {({ i18n }) => ( -
-
- Items Per Page - - {page_size} - - )} - > - {opts.map(option => ( - - {option} - - ))} - -
+
+ {opts && ( +
+ Items Per Page + + {page_size} + + )} + > + {opts.map(option => ( + + {option} + + ))} + +
+ )}
{`Items ${itemMin} - ${itemMax} of ${count}`} diff --git a/src/components/Pagination/styles.scss b/src/components/Pagination/styles.scss index 3da41d5c7b..8042bcf8fb 100644 --- a/src/components/Pagination/styles.scss +++ b/src/components/Pagination/styles.scss @@ -1,6 +1,6 @@ .awx-pagination { --awx-pagination--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-pagination--BorderColor: var(--pf-global--BackgroundColor--light-300); + --awx-pagination--BorderColor: #d1d1d1; --awx-pagination--disabled-BackgroundColor: #f2f2f2; --awx-pagination--disabled-Color: #C2C2CA; diff --git a/src/components/SelectedList/SelectedList.jsx b/src/components/SelectedList/SelectedList.jsx index 3e11549024..1a82e45ef9 100644 --- a/src/components/SelectedList/SelectedList.jsx +++ b/src/components/SelectedList/SelectedList.jsx @@ -3,6 +3,18 @@ import { Chip } from '@patternfly/react-core'; +const selectedRowStyling = { + paddingTop: '15px', + paddingBottom: '5px', + borderLeft: '0', + borderRight: '0' +}; + +const selectedLabelStyling = { + fontSize: '14px', + fontWeight: 'bold' +}; + class SelectedList extends Component { constructor (props) { super(props); @@ -23,8 +35,8 @@ class SelectedList extends Component { const { showOverflow } = this.state; return (
-
-
+
+
{label}
diff --git a/src/components/SelectedList/styles.scss b/src/components/SelectedList/styles.scss index dbf385d251..95f36094d6 100644 --- a/src/components/SelectedList/styles.scss +++ b/src/components/SelectedList/styles.scss @@ -1,6 +1,6 @@ .awx-selectedList { --awx-selectedList--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-selectedList--BorderColor: #d7d7d7; + --awx-selectedList--BorderColor: #d1d1d1; --awx-selectedList--BorderWidth: var(--pf-global--BorderWidth--sm); --awx-selectedList--FontSize: var(--pf-c-chip__text--FontSize); diff --git a/src/pages/Organizations/screens/OrganizationAdd.jsx b/src/pages/Organizations/screens/OrganizationAdd.jsx index cadd5d7c9e..fda1b0fe3f 100644 --- a/src/pages/Organizations/screens/OrganizationAdd.jsx +++ b/src/pages/Organizations/screens/OrganizationAdd.jsx @@ -6,10 +6,6 @@ import { Form, FormGroup, TextInput, - ActionGroup, - Toolbar, - ToolbarGroup, - Button, Gallery, Card, CardBody, @@ -18,75 +14,58 @@ import { import { ConfigContext } from '../../../context'; import Lookup from '../../../components/Lookup'; import AnsibleSelect from '../../../components/AnsibleSelect'; +import FormActionGroup from '../../../components/FormActionGroup'; -const format = (data) => { - const results = data.results.map((result) => ({ - id: result.id, - name: result.name, - isChecked: false - })); - return results; +const initialFormState = { + name: '', + description: '', + custom_virtualenv: '', + instanceGroups: [], + error: '', }; class OrganizationAdd extends React.Component { constructor (props) { super(props); - this.handleChange = this.handleChange.bind(this); - this.onSelectChange = this.onSelectChange.bind(this); + this.getInstanceGroups = this.getInstanceGroups.bind(this); + this.onFieldChange = this.onFieldChange.bind(this); + this.onLookupSave = this.onLookupSave.bind(this); this.onSubmit = this.onSubmit.bind(this); - this.resetForm = this.resetForm.bind(this); - this.onSuccess = this.onSuccess.bind(this); this.onCancel = this.onCancel.bind(this); - this.updateSelectedInstanceGroups = this.updateSelectedInstanceGroups.bind(this); + this.onSuccess = this.onSuccess.bind(this); } - state = { - name: '', - description: '', - results: [], - custom_virtualenv: '', - error: '', - selectedInstanceGroups: [] - }; + state = initialFormState; - async componentDidMount () { - const { api } = this.props; - try { - const { data } = await api.getInstanceGroups(); - const results = format(data); - this.setState({ results }); - } catch (error) { - this.setState({ error }); - } + onFieldChange (val, evt) { + this.setState({ [evt.target.name]: val || evt.target.value }); } - onSelectChange (value) { - this.setState({ custom_virtualenv: value }); + onLookupSave (val, targetName) { + this.setState({ [targetName]: val }); } async onSubmit () { const { api } = this.props; - const { name, description, custom_virtualenv } = this.state; + const { name, description, custom_virtualenv, instanceGroups } = this.state; const data = { name, description, custom_virtualenv }; - const { selectedInstanceGroups } = this.state; try { const { data: response } = await api.createOrganization(data); - const url = response.related.instance_groups; + const instanceGroupsUrl = response.related.instance_groups; try { - if (selectedInstanceGroups.length > 0) { - selectedInstanceGroups.forEach(async (select) => { - await api.createInstanceGroups(url, select.id); + if (instanceGroups.length > 0) { + instanceGroups.forEach(async (select) => { + await api.createInstanceGroups(instanceGroupsUrl, select.id); }); } } catch (err) { this.setState({ error: err }); } finally { - this.resetForm(); this.onSuccess(response.id); } } catch (err) { @@ -104,31 +83,18 @@ class OrganizationAdd extends React.Component { history.push(`/organizations/${id}`); } - updateSelectedInstanceGroups (selectedInstanceGroups) { - this.setState({ selectedInstanceGroups }); - } - - handleChange (_, evt) { - this.setState({ [evt.target.name]: evt.target.value }); - } - - resetForm () { - this.setState({ - name: '', - description: '', - }); - const { results } = this.state; - const reset = results.map((result) => ({ id: result.id, name: result.name, isChecked: false })); - this.setState({ results: reset }); + async getInstanceGroups (params) { + const { api } = this.props; + const data = await api.getInstanceGroups(params); + return data; } render () { const { name, - results, description, custom_virtualenv, - selectedInstanceGroups, + instanceGroups, error } = this.state; const enabled = name.length > 0; // TODO: add better form validation @@ -146,11 +112,10 @@ class OrganizationAdd extends React.Component { > @@ -158,38 +123,36 @@ class OrganizationAdd extends React.Component { id="add-org-form-description" name="description" value={description} - onChange={this.handleChange} + onChange={this.onFieldChange} /> - + {({ custom_virtualenvs }) => ( )} - - - - - - - - - - + { error ?
error
: '' } From c67088628f8b5eea24304e630a9ffcaf757b591c Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 12 Feb 2019 09:05:22 -0500 Subject: [PATCH 2/9] fix border color --- src/components/DataListToolbar/styles.scss | 2 +- src/components/Lookup/Lookup.jsx | 4 ++-- src/components/Pagination/styles.scss | 2 +- src/components/SelectedList/styles.scss | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 1a31982629..535d6aacaf 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -1,6 +1,6 @@ .awx-toolbar { --awx-toolbar--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-toolbar--BorderColor: #d1d1d1; + --awx-toolbar--BorderColor: #ebebeb; --awx-toolbar--BorderWidth: var(--pf-global--BorderWidth--sm); border-bottom: var(--awx-toolbar--BorderWidth) solid var(--awx-toolbar--BorderColor); diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index acf0f2ec2e..6a1c8962ad 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -19,8 +19,8 @@ import Pagination from '../Pagination'; const paginationStyling = { paddingLeft: '0', justifyContent: 'flex-end', - borderRight: '1px solid #d1d1d1', - borderBottom: '1px solid #d1d1d1', + borderRight: '1px solid #ebebeb', + borderBottom: '1px solid #ebebeb', borderTop: '0' }; diff --git a/src/components/Pagination/styles.scss b/src/components/Pagination/styles.scss index 8042bcf8fb..78141870ba 100644 --- a/src/components/Pagination/styles.scss +++ b/src/components/Pagination/styles.scss @@ -1,6 +1,6 @@ .awx-pagination { --awx-pagination--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-pagination--BorderColor: #d1d1d1; + --awx-pagination--BorderColor: #dbdbdb; --awx-pagination--disabled-BackgroundColor: #f2f2f2; --awx-pagination--disabled-Color: #C2C2CA; diff --git a/src/components/SelectedList/styles.scss b/src/components/SelectedList/styles.scss index 95f36094d6..4bf49ecfa7 100644 --- a/src/components/SelectedList/styles.scss +++ b/src/components/SelectedList/styles.scss @@ -1,6 +1,6 @@ .awx-selectedList { --awx-selectedList--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-selectedList--BorderColor: #d1d1d1; + --awx-selectedList--BorderColor: #ebebeb; --awx-selectedList--BorderWidth: var(--pf-global--BorderWidth--sm); --awx-selectedList--FontSize: var(--pf-c-chip__text--FontSize); From 6431ec603f3434bb7e38f21f8235353a112e47e8 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 12 Feb 2019 11:53:22 -0500 Subject: [PATCH 3/9] fix AnsibleSelect passing onChange callback --- src/components/AnsibleSelect/AnsibleSelect.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx index a5038efa6f..c0e2627e65 100644 --- a/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -25,9 +25,9 @@ class AnsibleSelect extends React.Component { return null; } - onSelectChange (val) { - const { selectChange } = this.props; - selectChange(val); + onSelectChange (val, event) { + const { onChange } = this.props; + onChange(val, event); } render () { From c1381f7b98599f2108834af51fa0f235702f8d18 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 12 Feb 2019 11:59:49 -0500 Subject: [PATCH 4/9] incorporate feedback on instance groups pagination --- __tests__/components/Lookup.test.jsx | 12 +++++------ src/components/Lookup/Lookup.jsx | 8 ++++---- .../Organizations/screens/OrganizationAdd.jsx | 20 +++++++++---------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index bec3e1369a..5b1d5233cf 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -13,7 +13,7 @@ describe('', () => { name="fooBar" value={mockData} onLookupSave={() => { }} - getEndpoint={() => { }} + getItems={() => { }} /> ); @@ -26,7 +26,7 @@ describe('', () => { name="fooBar" value={mockData} onLookupSave={() => { }} - getEndpoint={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} + getItems={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} /> ).find('Lookup'); @@ -46,7 +46,7 @@ describe('', () => { name="fooBar" value={mockSelected} onLookupSave={() => { }} - getEndpoint={() => { }} + getItems={() => { }} /> ).find('Lookup'); @@ -71,7 +71,7 @@ describe('', () => { name="fooBar" value={mockSelected} onLookupSave={() => { }} - getEndpoint={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} + getItems={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} /> ); @@ -93,7 +93,7 @@ describe('', () => { name="fooBar" value={mockData} onLookupSave={() => { }} - getEndpoint={() => { }} + getItems={() => { }} /> ); @@ -154,7 +154,7 @@ describe('', () => { name="fooBar" value={mockData} onLookupSave={onLookupSaveFn} - getEndpoint={() => { }} + getItems={() => { }} /> ).find('Lookup'); diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index 6a1c8962ad..bfa2c1e656 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -50,13 +50,13 @@ class Lookup extends React.Component { } async getData (queryParams) { - const { getEndpoint } = this.props; + const { getItems } = this.props; const { page } = queryParams; this.setState({ error: false }); try { - const { data } = await getEndpoint(queryParams); + const { data } = await getItems(queryParams); const { results, count } = data; const stateToUpdate = { @@ -150,7 +150,7 @@ class Lookup extends React.Component { ]} > - {(results.length === 0) && ( + {(results.length === 0) ? ( @@ -160,7 +160,7 @@ class Lookup extends React.Component { <Trans>{`Please add ${lookupHeader.toLowerCase()} to populate this list`}</Trans> </EmptyStateBody> </EmptyState> - ) || ( + ) : ( <Fragment> <ul className="pf-c-data-list awx-c-list"> {results.map(i => ( diff --git a/src/pages/Organizations/screens/OrganizationAdd.jsx b/src/pages/Organizations/screens/OrganizationAdd.jsx index fda1b0fe3f..6a8ad53559 100644 --- a/src/pages/Organizations/screens/OrganizationAdd.jsx +++ b/src/pages/Organizations/screens/OrganizationAdd.jsx @@ -16,14 +16,6 @@ import Lookup from '../../../components/Lookup'; import AnsibleSelect from '../../../components/AnsibleSelect'; import FormActionGroup from '../../../components/FormActionGroup'; -const initialFormState = { - name: '', - description: '', - custom_virtualenv: '', - instanceGroups: [], - error: '', -}; - class OrganizationAdd extends React.Component { constructor (props) { super(props); @@ -34,9 +26,15 @@ class OrganizationAdd extends React.Component { this.onSubmit = this.onSubmit.bind(this); this.onCancel = this.onCancel.bind(this); this.onSuccess = this.onSuccess.bind(this); - } - state = initialFormState; + this.state = { + name: '', + description: '', + custom_virtualenv: '', + instanceGroups: [], + error: '', + }; + } onFieldChange (val, evt) { this.setState({ [evt.target.name]: val || evt.target.value }); @@ -132,7 +130,7 @@ class OrganizationAdd extends React.Component { name="instanceGroups" value={instanceGroups} onLookupSave={this.onLookupSave} - getEndpoint={this.getInstanceGroups} + getItems={this.getInstanceGroups} /> </FormGroup> <ConfigContext.Consumer> From cbc1ae88759a6d09621ab5ae6724d888ef851238 Mon Sep 17 00:00:00 2001 From: John Mitchell <jlmitch5@ncsu.edu> Date: Wed, 13 Feb 2019 10:34:32 -0500 Subject: [PATCH 5/9] fix tests based on AnsibleSelect fn name change --- __tests__/components/AnsibleSelect.test.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/components/AnsibleSelect.test.jsx b/__tests__/components/AnsibleSelect.test.jsx index 6b72ed5cc2..27f9bcc5ee 100644 --- a/__tests__/components/AnsibleSelect.test.jsx +++ b/__tests__/components/AnsibleSelect.test.jsx @@ -9,7 +9,7 @@ describe('<AnsibleSelect />', () => { mount( <AnsibleSelect selected="foo" - selectChange={() => { }} + onChange={() => { }} labelName={label} data={mockData} /> @@ -20,7 +20,7 @@ describe('<AnsibleSelect />', () => { const wrapper = mount( <AnsibleSelect selected="foo" - selectChange={() => { }} + onChange={() => { }} labelName={label} data={mockData} /> @@ -33,7 +33,7 @@ describe('<AnsibleSelect />', () => { const wrapper = mount( <AnsibleSelect selected="foo" - selectChange={() => { }} + onChange={() => { }} labelName={label} data={null} /> From de658939c59c8adf78bc434c537e44142f297062 Mon Sep 17 00:00:00 2001 From: John Mitchell <jlmitch5@ncsu.edu> Date: Wed, 13 Feb 2019 10:50:04 -0500 Subject: [PATCH 6/9] add default value to props destructure statement --- src/components/Lookup/Lookup.jsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index bfa2c1e656..729325192f 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -125,12 +125,7 @@ class Lookup extends React.Component { render () { const { isModalOpen, lookupSelectedItems, error, results, count, page, page_size } = this.state; - const { value } = this.props; - let { lookupHeader } = this.props; - - if (!lookupHeader) { - lookupHeader = 'items'; - } + const { lookupHeader = 'items', value } = this.props; return ( <I18n> From 35c94e9cd83dd9c4b4bfb7fd5af38eda21ff111c Mon Sep 17 00:00:00 2001 From: John Mitchell <jlmitch5@ncsu.edu> Date: Wed, 13 Feb 2019 14:52:11 -0500 Subject: [PATCH 7/9] update selected to value in AnsibleSelect --- __tests__/components/AnsibleSelect.test.jsx | 11 ++++++++--- src/components/AnsibleSelect/AnsibleSelect.jsx | 7 ++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/__tests__/components/AnsibleSelect.test.jsx b/__tests__/components/AnsibleSelect.test.jsx index 27f9bcc5ee..a5c65b7d2e 100644 --- a/__tests__/components/AnsibleSelect.test.jsx +++ b/__tests__/components/AnsibleSelect.test.jsx @@ -8,18 +8,21 @@ describe('<AnsibleSelect />', () => { test('initially renders succesfully', async () => { mount( <AnsibleSelect - selected="foo" + value="foo" + name="bar" onChange={() => { }} labelName={label} data={mockData} /> ); }); + test('calls "onSelectChange" on dropdown select change', () => { const spy = jest.spyOn(AnsibleSelect.prototype, 'onSelectChange'); const wrapper = mount( <AnsibleSelect - selected="foo" + value="foo" + name="bar" onChange={() => { }} labelName={label} data={mockData} @@ -29,10 +32,12 @@ describe('<AnsibleSelect />', () => { wrapper.find('select').simulate('change'); expect(spy).toHaveBeenCalled(); }); + test('content not rendered when data property is falsey', () => { const wrapper = mount( <AnsibleSelect - selected="foo" + value="foo" + name="bar" onChange={() => { }} labelName={label} data={null} diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx index c0e2627e65..61c319639e 100644 --- a/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -26,18 +26,19 @@ class AnsibleSelect extends React.Component { } onSelectChange (val, event) { - const { onChange } = this.props; + const { onChange, name } = this.props; + event.target.name = name; onChange(val, event); } render () { const { count } = this.state; - const { labelName, selected, data, fieldId } = this.props; + const { labelName, value, data, fieldId } = this.props; let elem; if (count > 1) { elem = ( <FormGroup label={labelName} fieldId={fieldId || 'ansible-select'}> - <Select value={selected} id={`select-${fieldId}` || 'ansible-select-element'} onChange={this.onSelectChange} aria-label="Select Input"> + <Select value={value} id={`select-${fieldId}` || 'ansible-select-element'} onChange={this.onSelectChange} aria-label="Select Input"> {data.map((datum) => ( <SelectOption isDisabled={datum.disabled} key={datum} value={datum} label={datum} /> ))} From f2ab7f62b97648e0649fe81051b77cd6bd8f6987 Mon Sep 17 00:00:00 2001 From: John Mitchell <jlmitch5@ncsu.edu> Date: Wed, 13 Feb 2019 15:09:14 -0500 Subject: [PATCH 8/9] fix disabled of submit button --- src/components/FormActionGroup.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/FormActionGroup.jsx b/src/components/FormActionGroup.jsx index 682bad5ccd..3da293fe62 100644 --- a/src/components/FormActionGroup.jsx +++ b/src/components/FormActionGroup.jsx @@ -21,13 +21,13 @@ const buttonGroupStyle = { marginRight: '20px' }; -export default ({ onSubmit, isDisabled, onCancel }) => ( +export default ({ onSubmit, submitDisabled, onCancel }) => ( <I18n> {({ i18n }) => ( <ActionGroup style={formActionGroupStyle}> <Toolbar> <ToolbarGroup style={buttonGroupStyle}> - <Button aria-label={i18n._(t`Save`)} variant="primary" onClick={onSubmit} isDisabled={isDisabled}>{i18n._(t`Save`)}</Button> + <Button aria-label={i18n._(t`Save`)} variant="primary" onClick={onSubmit} isDisabled={submitDisabled}>{i18n._(t`Save`)}</Button> </ToolbarGroup> <ToolbarGroup> <Button aria-label={i18n._(t`Cancel`)} variant="secondary" onClick={onCancel}>{i18n._(t`Cancel`)}</Button> From b4007c7e043404563c71bdced968580b50ab05fe Mon Sep 17 00:00:00 2001 From: John Mitchell <jlmitch5@ncsu.edu> Date: Wed, 13 Feb 2019 15:40:46 -0500 Subject: [PATCH 9/9] put FormGroup component in form instead of in AnsibleSelect component --- src/components/AnsibleSelect/AnsibleSelect.jsx | 15 ++++++--------- .../Organizations/screens/OrganizationAdd.jsx | 16 ++++++++-------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx index 61c319639e..c8b7f142dd 100644 --- a/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -1,7 +1,6 @@ import React from 'react'; import { - FormGroup, Select, SelectOption, } from '@patternfly/react-core'; @@ -33,17 +32,15 @@ class AnsibleSelect extends React.Component { render () { const { count } = this.state; - const { labelName, value, data, fieldId } = this.props; + const { value, data } = this.props; let elem; if (count > 1) { elem = ( - <FormGroup label={labelName} fieldId={fieldId || 'ansible-select'}> - <Select value={value} id={`select-${fieldId}` || 'ansible-select-element'} onChange={this.onSelectChange} aria-label="Select Input"> - {data.map((datum) => ( - <SelectOption isDisabled={datum.disabled} key={datum} value={datum} label={datum} /> - ))} - </Select> - </FormGroup> + <Select value={value} onChange={this.onSelectChange} aria-label="Select Input"> + {data.map((datum) => ( + <SelectOption isDisabled={datum.disabled} key={datum} value={datum} label={datum} /> + ))} + </Select> ); } else { elem = null; diff --git a/src/pages/Organizations/screens/OrganizationAdd.jsx b/src/pages/Organizations/screens/OrganizationAdd.jsx index 6a8ad53559..d1d02b9d41 100644 --- a/src/pages/Organizations/screens/OrganizationAdd.jsx +++ b/src/pages/Organizations/screens/OrganizationAdd.jsx @@ -135,14 +135,14 @@ class OrganizationAdd extends React.Component { </FormGroup> <ConfigContext.Consumer> {({ custom_virtualenvs }) => ( - <AnsibleSelect - fieldId="add-org-form-virtual-env" - labelName="Ansible Environment" - name="custom_virtualenv" - value={custom_virtualenv} - onChange={this.onFieldChange} - data={custom_virtualenvs} - /> + <FormGroup label="Ansible Environment" fieldId="add-org-form-custom-virtualenv"> + <AnsibleSelect + name="custom_virtualenv" + value={custom_virtualenv} + onChange={this.onFieldChange} + data={custom_virtualenvs} + /> + </FormGroup> )} </ConfigContext.Consumer> </Gallery>