Merge pull request #9924 from AlexSCorey/6464-SurveyMultipleChoiceRedesign

Redesign survey multiple choice 

SUMMARY
Addresses #6464.
This new design improves UI and reduces the risk to having mismatching choices and default values.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

UI

AWX VERSION
ADDITIONAL INFORMATION

Reviewed-by: Kersom <None>
Reviewed-by: Marliana Lara <marliana.lara@gmail.com>
Reviewed-by: Alex Corey <Alex.swansboro@gmail.com>
Reviewed-by: Sarah Akus <sarah.akus@gmail.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-05-07 17:37:23 +00:00 committed by GitHub
commit d60014987f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 2113 additions and 1969 deletions

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,155 @@
import React from 'react';
import { useField } from 'formik';
import { t } from '@lingui/macro';
import {
FormGroup,
TextInput,
Button,
InputGroup as PFInputGroup,
} from '@patternfly/react-core';
import PFCheckIcon from '@patternfly/react-icons/dist/js/icons/check-icon';
import styled from 'styled-components';
import Popover from '../../../components/Popover';
const InputGroup = styled(PFInputGroup)`
padding-bottom: 5px;
`;
const CheckIcon = styled(PFCheckIcon)`
color: var(--pf-c-button--m-plain--disabled--Color);
${props =>
props.selected && `color: var(--pf-c-button--m-secondary--active--Color)`};
`;
const validate = () => {
return value => {
let message;
const hasValue = value.find(({ choice }) =>
choice.trim().length > 0 ? choice : undefined
);
if (!hasValue) {
message = t`There must be a value in at least one input`;
}
return message;
};
};
function MultipleChoiceField({ label, tooltip }) {
const [
formattedChoicesField,
formattedChoicesMeta,
formattedChoicesHelpers,
] = useField({
name: 'formattedChoices',
validate: validate(),
});
const [typeField] = useField('type');
const isValid = !(formattedChoicesMeta.touched && formattedChoicesMeta.error);
return (
<FormGroup
label={label}
isRequired
name="formattedChoices"
id="formattedChoices"
helperText={
!formattedChoicesField.value[0].choice.trim().length
? t`Type answer then click checkbox on right to select answer as default.`
: t`Press 'Enter' to add more answer choices. One answer choice per line. `
}
helperTextInvalid={formattedChoicesMeta.error}
onBlur={e => {
if (!e.currentTarget.contains(e.relatedTarget)) {
formattedChoicesHelpers.setTouched();
}
}}
validated={isValid ? 'default' : 'error'}
labelIcon={<Popover content={tooltip} />}
>
{formattedChoicesField.value.map(({ choice, isDefault, id }, i) => (
<InputGroup key={id}>
<TextInput
data-cy={choice ? `${choice}-input` : 'new-choice-input'}
aria-label={choice || t`new choice`}
onKeyUp={e => {
if (
e.key === 'Enter' &&
choice.trim().length > 0 &&
i === formattedChoicesField.value.length - 1
) {
formattedChoicesHelpers.setValue(
formattedChoicesField.value.concat({
choice: '',
isDefault: false,
id: i + 1,
})
);
}
if (
e.key === 'Backspace' &&
!choice.trim() &&
formattedChoicesField.value.length > 1
) {
const removeEmptyField = formattedChoicesField.value.filter(
(c, index) => index !== i
);
formattedChoicesHelpers.setValue(removeEmptyField);
}
}}
value={choice}
onChange={value => {
const newValues = formattedChoicesField.value.map(
(choiceField, index) =>
i === index
? { choice: value, isDefault: false, id: choiceField.id }
: choiceField
);
formattedChoicesHelpers.setValue(newValues);
}}
/>
<Button
variant="control"
aria-label={t`Click to toggle default value`}
ouiaId={choice ? `${choice}-button` : 'new-choice-button'}
isDisabled={!choice.trim()}
onClick={() => {
const newValues = formattedChoicesField.value.map(
(choiceField, index) =>
i === index
? {
choice: choiceField.choice,
isDefault: !choiceField.isDefault,
id: choiceField.id,
}
: choiceField
);
const singleSelectValues = formattedChoicesField.value.map(
(choiceField, index) =>
i === index
? {
choice: choiceField.choice,
isDefault: !choiceField.isDefault,
id: choiceField.id,
}
: {
choice: choiceField.choice,
isDefault: false,
id: choiceField.id,
}
);
return typeField.value === 'multiplechoice'
? formattedChoicesHelpers.setValue(singleSelectValues)
: formattedChoicesHelpers.setValue(newValues);
}}
>
<CheckIcon selected={isDefault} />
</Button>
</InputGroup>
))}
</FormGroup>
);
}
export default MultipleChoiceField;

View File

@ -0,0 +1,160 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { Formik } from 'formik';
import { mountWithContexts } from '../../../../testUtils/enzymeHelpers';
import MultipleChoiceField from './MultipleChoiceField';
describe('<MultipleChoiceField/>', () => {
test('should activate default values, multiselect', async () => {
let wrapper;
act(() => {
wrapper = mountWithContexts(
<Formik
initialValues={{
formattedChoices: [
{ choice: 'apollo', isDefault: true },
{ choice: 'alex', isDefault: true },
{ choice: 'athena', isDefault: false },
],
type: 'multiselect',
}}
>
<MultipleChoiceField id="question-options" name="choices" />
</Formik>
);
});
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onKeyUp')({ key: 'Enter' })
);
wrapper.update();
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
3
);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(2)
.prop('onChange')('spencer')
);
wrapper.update();
await act(() =>
wrapper.find('Button[ouiaId="spencer-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="spencer-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
});
test('should select default, multiplechoice', async () => {
let wrapper;
act(() => {
wrapper = mountWithContexts(
<Formik
initialValues={{
formattedChoices: [
{ choice: 'alex', isDefault: true, id: 1 },
{ choice: 'apollo', isDefault: false, id: 2 },
{ choice: 'athena', isDefault: false, id: 3 },
],
type: 'multiplechoice',
}}
>
<MultipleChoiceField id="question-options" name="choices" />
</Formik>
);
});
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
3
);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onKeyUp')({ key: 'Enter' })
);
wrapper.update();
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(2)
.prop('onChange')('spencer')
);
wrapper.update();
await act(() =>
wrapper.find('Button[ouiaId="spencer-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="spencer-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
});
});

View File

@ -9,23 +9,41 @@ export default function SurveyQuestionAdd({ survey, updateSurvey }) {
const match = useRouteMatch();
const handleSubmit = async question => {
const formData = { ...question };
try {
if (survey.spec?.some(q => q.variable === question.variable)) {
if (survey.spec?.some(q => q.variable === formData.variable)) {
setFormError(
new Error(
`Survey already contains a question with variable named “${question.variable}`
`Survey already contains a question with variable named “${formData.variable}`
)
);
return;
}
if (question.type === 'multiselect') {
question.default = question.default
.split('\n')
.filter(v => v !== '' || '\n')
.map(v => v.trim())
.join('\n');
let choices = '';
let defaultAnswers = '';
if (
formData.type === 'multiselect' ||
formData.type === 'multiplechoice'
) {
formData.formattedChoices.forEach(({ choice, isDefault }, i) => {
choices =
i === formData.formattedChoices.length - 1
? choices.concat(`${choice}`)
: choices.concat(`${choice}\n`);
if (isDefault) {
defaultAnswers =
i === formData.formattedChoices.length - 1
? defaultAnswers.concat(`${choice}`)
: defaultAnswers.concat(`${choice}\n`);
}
});
formData.default = defaultAnswers.trim();
formData.choices = choices.trim();
}
const newSpec = survey.spec ? survey.spec.concat(question) : [question];
delete formData.formattedChoices;
const newSpec = survey.spec ? survey.spec.concat(formData) : [formData];
await updateSurvey(newSpec);
history.push(match.url.replace('/add', ''));
} catch (err) {

View File

@ -37,14 +37,15 @@ export default function SurveyQuestionEdit({ survey, updateSurvey }) {
};
const handleSubmit = async formData => {
const submittedData = { ...formData };
try {
if (
formData.variable !== question.variable &&
survey.spec.find(q => q.variable === formData.variable)
submittedData.variable !== question.variable &&
survey.spec.find(q => q.variable === submittedData.variable)
) {
setFormError(
new Error(
`Survey already contains a question with variable named “${formData.variable}`
`Survey already contains a question with variable named “${submittedData.variable}`
)
);
return;
@ -55,16 +56,31 @@ export default function SurveyQuestionEdit({ survey, updateSurvey }) {
if (questionIndex === -1) {
throw new Error('Question not found in spec');
}
if (formData.type === 'multiselect') {
formData.default = formData.default
.split('\n')
.filter(v => v !== '' || '\n')
.map(v => v.trim())
.join('\n');
let choices = '';
let defaultAnswers = '';
if (
submittedData.type === 'multiselect' ||
submittedData.type === 'multiplechoice'
) {
submittedData.formattedChoices.forEach(({ choice, isDefault }, i) => {
choices =
i === submittedData.formattedChoices.length - 1
? choices.concat(`${choice}`)
: choices.concat(`${choice}\n`);
if (isDefault) {
defaultAnswers =
i === submittedData.formattedChoices.length - 1
? defaultAnswers.concat(`${choice}`)
: defaultAnswers.concat(`${choice}\n`);
}
});
submittedData.default = defaultAnswers.trim();
submittedData.choices = choices.trim();
}
await updateSurvey([
...survey.spec.slice(0, questionIndex),
formData,
submittedData,
...survey.spec.slice(questionIndex + 1),
]);
navigateToList();

View File

@ -1,7 +1,6 @@
import React from 'react';
import { func, string, bool, number, shape } from 'prop-types';
import { Formik, useField } from 'formik';
import { t } from '@lingui/macro';
import { Form, FormGroup } from '@patternfly/react-core';
import { FormColumnLayout } from '../../../components/FormLayout';
@ -11,6 +10,8 @@ import FormField, {
PasswordField,
FormSubmitError,
} from '../../../components/FormField';
import { useConfig } from '../../../contexts/Config';
import getDocsBaseUrl from '../../../util/getDocsBaseUrl';
import AnsibleSelect from '../../../components/AnsibleSelect';
import Popover from '../../../components/Popover';
import {
@ -21,12 +22,22 @@ import {
integer,
number as numberValidator,
} from '../../../util/validators';
import MultipleChoiceField from './MultipleChoiceField';
function AnswerTypeField() {
const [field] = useField({
const [field, meta, helpers] = useField({
name: 'type',
validate: required(t`Select a value for this field`),
});
const [choicesField, choicesMeta, choicesHelpers] = useField(
'formattedChoices'
);
const singleDefault = choicesField.value.map((c, i) =>
i === 0
? { choice: c.choice, isDefault: true, id: c.id }
: { choice: c.choice, isDefault: false, id: c.id }
);
return (
<FormGroup
@ -44,6 +55,28 @@ function AnswerTypeField() {
<AnsibleSelect
id="question-type"
{...field}
onChange={(e, val) => {
helpers.setValue(val);
// Edit Mode: Makes the first choice the default value if
// the type switches from multiselect, to multiple choice
if (
val === 'multiplechoice' &&
['multiplechoice', 'multiselect'].includes(meta.initialValue) &&
val !== meta.initialValue
) {
choicesHelpers.setValue(singleDefault);
}
// Edit Mode: Resets Multiple choice or Multiselect values if the user move type
// back to one of those values
if (
['multiplechoice', 'multiselect'].includes(val) &&
val === meta.initialValue
) {
choicesHelpers.setValue(choicesMeta.initialValue);
}
}}
data={[
{ key: 'text', value: 'text', label: t`Text` },
{ key: 'textarea', value: 'textarea', label: t`Textarea` },
@ -72,33 +105,47 @@ function SurveyQuestionForm({
handleCancel,
submitError,
}) {
const defaultIsNotAvailable = choices => {
return defaultValue => {
let errorMessage;
const found = [...defaultValue].every(dA => choices.indexOf(dA) > -1);
const config = useConfig();
if (!found) {
errorMessage = t`Default choice must be answered from the choices listed.`;
}
return errorMessage;
};
let initialValues = {
question_name: question?.question_name || '',
question_description: question?.question_description || '',
required: question ? question?.required : true,
type: question?.type || 'text',
variable: question?.variable || '',
min: question?.min || 0,
max: question?.max || 1024,
default: question?.default || '',
choices: question?.choices || '',
formattedChoices: [{ choice: '', isDefault: false, id: 0 }],
new_question: !question,
};
if (question?.type === 'multiselect' || question?.type === 'multiplechoice') {
const newQuestions = question.choices.split('\n').map((c, i) => {
if (question.default.split('\n').includes(c)) {
return { choice: c, isDefault: true, id: i };
}
return { choice: c, isDefault: false, id: i };
});
initialValues = {
question_name: question?.question_name || '',
question_description: question?.question_description || '',
required: question ? question?.required : true,
type: question?.type || 'text',
variable: question?.variable || '',
min: question?.min || 0,
max: question?.max || 1024,
formattedChoices: newQuestions,
new_question: !question,
};
}
return (
<Formik
enableReinitialize
initialValues={{
question_name: question?.question_name || '',
question_description: question?.question_description || '',
required: question ? question?.required : true,
type: question?.type || 'text',
variable: question?.variable || '',
min: question?.min || 0,
max: question?.max || 1024,
default: question?.default || '',
choices: question?.choices || '',
new_question: !question,
}}
initialValues={initialValues}
onSubmit={handleSubmit}
>
{formik => (
@ -202,29 +249,24 @@ function SurveyQuestionForm({
/>
)}
{['multiplechoice', 'multiselect'].includes(formik.values.type) && (
<>
<FormField
id="question-options"
name="choices"
type="textarea"
label={t`Multiple Choice Options`}
validate={required(null)}
tooltip={t`Each answer choice must be on a separate line.`}
isRequired
rows="10"
/>
<FormField
id="question-default"
name="default"
validate={defaultIsNotAvailable(formik.values.choices)}
type={
formik.values.type === 'multiplechoice'
? 'text'
: 'textarea'
}
label={t`Default answer`}
/>
</>
<MultipleChoiceField
label={t`Multiple Choice Options`}
tooltip={
<>
<span>{t`Refer to the`} </span>
<a
href={`${getDocsBaseUrl(
config
)}/html/userguide/job_templates.html#surveys`}
target="_blank"
rel="noreferrer"
>
{t`documentation`}{' '}
</a>
{t`for more information.`}
</>
}
/>
)}
</FormColumnLayout>
<FormSubmitError error={submitError} />
@ -256,5 +298,4 @@ SurveyQuestionForm.defaultProps = {
question: null,
submitError: null,
};
export default SurveyQuestionForm;

View File

@ -17,12 +17,15 @@ const noop = () => {};
async function selectType(wrapper, type) {
await act(async () => {
wrapper.find('AnsibleSelect#question-type').invoke('onChange')({
target: {
name: 'type',
value: type,
wrapper.find('AnsibleSelect#question-type').invoke('onChange')(
{
target: {
name: 'type',
value: type,
},
},
});
type
);
});
wrapper.update();
}
@ -146,12 +149,15 @@ describe('<SurveyQuestionForm />', () => {
});
await selectType(wrapper, 'multiplechoice');
expect(wrapper.find('FormField#question-options').prop('type')).toEqual(
'textarea'
);
expect(wrapper.find('FormField#question-default').prop('type')).toEqual(
'text'
expect(wrapper.find('MultipleChoiceField').length).toBe(1);
expect(wrapper.find('MultipleChoiceField').find('TextInput').length).toBe(
1
);
expect(
wrapper
.find('MultipleChoiceField')
.find('Button[aria-label="Click to toggle default value"]').length
).toBe(1);
});
test('should provide fields for multi-select question', async () => {
@ -168,12 +174,15 @@ describe('<SurveyQuestionForm />', () => {
});
await selectType(wrapper, 'multiselect');
expect(wrapper.find('FormField#question-options').prop('type')).toEqual(
'textarea'
);
expect(wrapper.find('FormField#question-default').prop('type')).toEqual(
'textarea'
expect(wrapper.find('MultipleChoiceField').length).toBe(1);
expect(wrapper.find('MultipleChoiceField').find('TextInput').length).toBe(
1
);
expect(
wrapper
.find('MultipleChoiceField')
.find('Button[aria-label="Click to toggle default value"]').length
).toBe(1);
});
test('should provide fields for integer question', async () => {
@ -225,7 +234,7 @@ describe('<SurveyQuestionForm />', () => {
wrapper.find('FormField#question-default input').prop('type')
).toEqual('number');
});
test('should not throw validation error', async () => {
test('should activate default values, multiselect', async () => {
let wrapper;
act(() => {
@ -239,25 +248,75 @@ describe('<SurveyQuestionForm />', () => {
});
await selectType(wrapper, 'multiselect');
await act(async () =>
wrapper.find('textarea#question-options').simulate('change', {
target: { value: 'a \n b', name: 'choices' },
})
wrapper
.find('MultipleChoiceField')
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onChange')('alex')
);
await act(async () =>
wrapper.find('textarea#question-options').simulate('change', {
target: { value: 'b \n a', name: 'default' },
})
);
wrapper.find('FormField#question-default').prop('validate')('b \n a', {});
wrapper.update();
expect(
wrapper
.find('FormGroup[fieldId="question-default"]')
.prop('helperTextInvalid')
).toBe(undefined);
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onKeyUp')({ key: 'Enter' })
);
wrapper.update();
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
2
);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(1)
.prop('onChange')('spencer')
);
wrapper.update();
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
2
);
await act(() =>
wrapper.find('Button[ouiaId="spencer-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="spencer-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
});
test('should throw validation error', async () => {
test('should select default, multiplechoice', async () => {
let wrapper;
act(() => {
@ -269,23 +328,69 @@ describe('<SurveyQuestionForm />', () => {
/>
);
});
await selectType(wrapper, 'multiselect');
await selectType(wrapper, 'multiplechoice');
await act(async () =>
wrapper.find('textarea#question-options').simulate('change', {
target: { value: 'a \n b', name: 'choices' },
})
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onChange')('alex')
);
await act(async () =>
wrapper.find('textarea#question-default').simulate('change', {
target: { value: 'c', name: 'default' },
})
);
wrapper.find('FormField#question-default').prop('validate')('c', {});
wrapper.update();
expect(
wrapper
.find('FormGroup[fieldId="question-default"]')
.prop('helperTextInvalid')
).toBe('Default choice must be answered from the choices listed.');
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
await act(() =>
wrapper.find('Button[ouiaId="alex-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
1
);
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(0)
.prop('onKeyUp')({ key: 'Enter' })
);
wrapper.update();
await act(async () =>
wrapper
.find('MultipleChoiceField')
.find('TextInput')
.at(1)
.prop('onChange')('spencer')
);
wrapper.update();
expect(wrapper.find('MultipleChoiceField').find('InputGroup').length).toBe(
2
);
await act(() =>
wrapper.find('Button[ouiaId="spencer-button"]').prop('onClick')()
);
wrapper.update();
expect(
wrapper
.find('Button[ouiaId="spencer-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(true);
expect(
wrapper
.find('Button[ouiaId="alex-button"]')
.find('CheckIcon')
.prop('selected')
).toBe(false);
});
});