From a8fa816165b4e50d7873307836512caf4456ed72 Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 12 Feb 2020 13:49:57 -0500 Subject: [PATCH 1/4] Adds FieldWithPrompt component to handle fields that are also promptable --- .../FieldWithPrompt/FieldWithPrompt.jsx | 91 +++++++++++++++++++ .../src/components/FieldWithPrompt/index.js | 1 + .../Template/shared/JobTemplateForm.jsx | 44 ++++----- 3 files changed, 112 insertions(+), 24 deletions(-) create mode 100644 awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx create mode 100644 awx/ui_next/src/components/FieldWithPrompt/index.js diff --git a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx new file mode 100644 index 0000000000..26a6dac3f2 --- /dev/null +++ b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx @@ -0,0 +1,91 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import { Tooltip } from '@patternfly/react-core'; +import { CheckboxField } from '@components/FormField'; +import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; +import styled from 'styled-components'; + +const QuestionCircleIcon = styled(PFQuestionCircleIcon)` + margin-left: 10px; +`; + +const FieldHeader = styled.div` + display: flex; + justify-content: space-between; + padding-bottom: var(--pf-c-form__label--PaddingBottom); + + label { + --pf-c-form__label--PaddingBottom: 0px; + } +`; + +const StyledCheckboxField = styled(CheckboxField)` + --pf-c-check__label--FontSize: var(--pf-c-form__label--FontSize); +`; + +function FieldWithPrompt({ + children, + fieldId, + i18n, + isRequired, + label, + promptId, + promptName, + promptValidate, + tooltip, + tooltipMaxWidth, +}) { + return ( +
+ +
+ + {tooltip && ( + + + + )} +
+ + + {children} +
+ ); +} + +FieldWithPrompt.propTypes = { + fieldId: PropTypes.string.isRequired, + isRequired: PropTypes.bool, + label: PropTypes.string.isRequired, + promptId: PropTypes.string.isRequired, + promptValidate: PropTypes.func, + tooltip: PropTypes.node, + tooltipMaxWidth: PropTypes.string, +}; + +FieldWithPrompt.defaultProps = { + isRequired: false, + promptValidate: () => {}, + tooltip: null, + tooltipMaxWidth: '', +}; + +export default withI18n()(FieldWithPrompt); diff --git a/awx/ui_next/src/components/FieldWithPrompt/index.js b/awx/ui_next/src/components/FieldWithPrompt/index.js new file mode 100644 index 0000000000..77c1d5bda6 --- /dev/null +++ b/awx/ui_next/src/components/FieldWithPrompt/index.js @@ -0,0 +1 @@ +export { default } from './FieldWithPrompt'; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 326ae5f389..5f48522dfb 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -21,6 +21,7 @@ import FormField, { FieldTooltip, FormSubmitError, } from '@components/FormField'; +import FieldWithPrompt from '@components/FieldWithPrompt'; import FormRow from '@components/FormRow'; import CollapsibleSection from '@components/CollapsibleSection'; import { required } from '@util/validators'; @@ -227,37 +228,31 @@ class JobTemplateForm extends Component { type="text" label={i18n._(t`Description`)} /> - - {({ form, field }) => { - const isValid = !form.touched.job_type || !form.errors.job_type; - return ( - - + + {({ form, field }) => { + const isValid = !form.touched.job_type || !form.errors.job_type; + return ( - - ); - }} - + ); + }} + + Date: Thu, 13 Feb 2020 09:06:35 -0500 Subject: [PATCH 2/4] Adds tooltip to run type that was previously removed. Fixes unit test failures by adding ask_job_type_on_launch to mock data. --- .../JobTemplateAdd/JobTemplateAdd.test.jsx | 31 ++++++++-------- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 35 ++++++++++--------- .../Template/shared/JobTemplateForm.jsx | 6 +++- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx index 508b85de82..748b71dbcf 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -8,25 +8,26 @@ import { JobTemplatesAPI, LabelsAPI } from '@api'; jest.mock('@api'); const jobTemplateData = { - name: 'Foo', - description: 'Baz', - job_type: 'run', - inventory: 1, - project: 2, - playbook: 'Bar', - forks: 0, - limit: '', - verbosity: '0', - job_slice_count: 1, - timeout: 0, - job_tags: '', - skip_tags: '', - diff_mode: false, allow_callbacks: false, allow_simultaneous: false, - use_fact_cache: false, + ask_job_type_on_launch: false, + description: 'Baz', + diff_mode: false, + forks: 0, host_config_key: '', + inventory: 1, + job_slice_count: 1, + job_tags: '', + job_type: 'run', + limit: '', + name: 'Foo', + playbook: 'Bar', + project: 2, scm_branch: '', + skip_tags: '', + timeout: 0, + use_fact_cache: false, + verbosity: '0', }; describe('', () => { diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index d00cc655a0..8798249169 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -9,27 +9,24 @@ import JobTemplateEdit from './JobTemplateEdit'; jest.mock('@api'); const mockJobTemplate = { - id: 1, - name: 'Foo', - description: 'Bar', - job_type: 'run', - inventory: 2, - project: 3, - playbook: 'Baz', - type: 'job_template', - forks: 0, - limit: '', - verbosity: '0', - job_slice_count: 1, - timeout: 0, - job_tags: '', - skip_tags: '', - diff_mode: false, allow_callbacks: false, allow_simultaneous: false, - use_fact_cache: false, + ask_job_type_on_launch: false, + description: 'Bar', + diff_mode: false, + forks: 0, host_config_key: '', + id: 1, + inventory: 2, + job_slice_count: 1, + job_tags: '', + job_type: 'run', + limit: '', + name: 'Foo', + playbook: 'Baz', + project: 3, scm_branch: '', + skip_tags: '', summary_fields: { user_capabilities: { edit: true, @@ -50,6 +47,10 @@ const mockJobTemplate = { name: 'Boo', }, }, + timeout: 0, + type: 'job_template', + use_fact_cache: false, + verbosity: '0', }; const mockRelatedCredentials = { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 5f48522dfb..1e3dd2caed 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -230,10 +230,14 @@ class JobTemplateForm extends Component { /> Date: Thu, 13 Feb 2020 09:42:09 -0500 Subject: [PATCH 3/4] Adds basic unit test coverage for the FieldWithPrompt component --- .../FieldWithPrompt/FieldWithPrompt.jsx | 17 ++--- .../FieldWithPrompt/FieldWithPrompt.test.jsx | 66 +++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.test.jsx diff --git a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx index 26a6dac3f2..6328c9f26a 100644 --- a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx +++ b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import PropTypes from 'prop-types'; +import { bool, func, node, string } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Tooltip } from '@patternfly/react-core'; @@ -72,13 +72,14 @@ function FieldWithPrompt({ } FieldWithPrompt.propTypes = { - fieldId: PropTypes.string.isRequired, - isRequired: PropTypes.bool, - label: PropTypes.string.isRequired, - promptId: PropTypes.string.isRequired, - promptValidate: PropTypes.func, - tooltip: PropTypes.node, - tooltipMaxWidth: PropTypes.string, + fieldId: string.isRequired, + isRequired: bool, + label: string.isRequired, + promptId: string.isRequired, + promptName: string.isRequired, + promptValidate: func, + tooltip: node, + tooltipMaxWidth: string, }; FieldWithPrompt.defaultProps = { diff --git a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.test.jsx b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.test.jsx new file mode 100644 index 0000000000..4358737144 --- /dev/null +++ b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.test.jsx @@ -0,0 +1,66 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import { Field, Formik } from 'formik'; +import FieldWithPrompt from './FieldWithPrompt'; + +describe('FieldWithPrompt', () => { + let wrapper; + + afterEach(() => { + wrapper.unmount(); + }); + + test('Required asterisk and Tooltip hidden when not required and tooltip not provided', () => { + wrapper = mountWithContexts( + + {() => ( + + + {() => } + + + )} + + ); + expect(wrapper.find('.pf-c-form__label-required')).toHaveLength(0); + expect(wrapper.find('Tooltip')).toHaveLength(0); + }); + + test('Required asterisk and Tooltip shown when required and tooltip provided', () => { + wrapper = mountWithContexts( + + {() => ( + + + {() => } + + + )} + + ); + expect(wrapper.find('.pf-c-form__label-required')).toHaveLength(1); + expect(wrapper.find('Tooltip')).toHaveLength(1); + }); +}); From 4e811c744afee90c433e29e6c9fbc342c2ec0b8e Mon Sep 17 00:00:00 2001 From: mabashian Date: Fri, 14 Feb 2020 10:56:11 -0500 Subject: [PATCH 4/4] Use FieldTooltip instead of Tooltip component. Remove promptValidate prop from FieldWithPrompt. This checkbox shouldn't ever need a custom validator function. --- .../FieldWithPrompt/FieldWithPrompt.jsx | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx index 6328c9f26a..8932c7cbe4 100644 --- a/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx +++ b/awx/ui_next/src/components/FieldWithPrompt/FieldWithPrompt.jsx @@ -1,16 +1,10 @@ import React from 'react'; -import { bool, func, node, string } from 'prop-types'; +import { bool, node, string } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Tooltip } from '@patternfly/react-core'; -import { CheckboxField } from '@components/FormField'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; +import { CheckboxField, FieldTooltip } from '@components/FormField'; import styled from 'styled-components'; -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; - const FieldHeader = styled.div` display: flex; justify-content: space-between; @@ -33,9 +27,7 @@ function FieldWithPrompt({ label, promptId, promptName, - promptValidate, tooltip, - tooltipMaxWidth, }) { return (
@@ -49,21 +41,12 @@ function FieldWithPrompt({ )} - {tooltip && ( - - - - )} + {tooltip && }
{children} @@ -77,16 +60,12 @@ FieldWithPrompt.propTypes = { label: string.isRequired, promptId: string.isRequired, promptName: string.isRequired, - promptValidate: func, tooltip: node, - tooltipMaxWidth: string, }; FieldWithPrompt.defaultProps = { isRequired: false, - promptValidate: () => {}, tooltip: null, - tooltipMaxWidth: '', }; export default withI18n()(FieldWithPrompt);