Skip to content

Commit

Permalink
Merge pull request #313 from NIAEFEUP/feature/explicit-offer-errors
Browse files Browse the repository at this point in the history
Form errors should be made more explicit
  • Loading branch information
Process-ing authored Mar 13, 2023
2 parents 6d13452 + b44a324 commit d47281c
Show file tree
Hide file tree
Showing 20 changed files with 85 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ApplyURLComponent = ({ disabled, errors, requestErrors, control, classes }
className={classes.applyURLInput}
value={value}
label="Application URL"
id="applyURL"
id={name}
error={!!errors?.applyURL || !!requestErrors.applyURL}
inputRef={ref}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const IsHiddenComponent = ({ disabled, control }) => (
checked={value}
onChange={onChange}
name={name}
id={name}
onBlur={onBlur}
disabled={disabled}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const IsPaidComponent = ({ disabled, errors, requestErrors, control, textFieldPr
<TextField
name={name}
fullWidth
id="is-paid"
id={name}
select
label="Compensation"
value={value}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const JobDurationComponent = ({ disabled, control }) => {
>
<Slider
name={name}
id={name}
value={value}
onChange={(_e, values) => onChange(values)}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const JobStartDateComponent = ({ disabled, errors, requestErrors, control }) =>
margin="dense"
value={value}
label="Job Start Date"
id="startDate-input"
id={name}
name={name}
onChange={(_, value) => (onChange(value))}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const JobTypeComponent = ({ disabled, errors, requestErrors, control, textFieldP
<TextField
name={name}
fullWidth
id="job_type"
id={name}
select
label="Job Type *"
value={value ? value : ""}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const LocationComponent = ({ disabled, errors, requestErrors, control }) => (
onChange={(_e, value) => onChange(value)}
onBlur={onBlur}
name={name}
id={name}
error={errors.location || requestErrors.location}
disabled={disabled}
/>
Expand Down
30 changes: 27 additions & 3 deletions src/components/Offers/Form/form-components/OfferForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
Collapse,
Button,
} from "@material-ui/core";
import React, { useState, useCallback, useContext } from "react";
import React, { useState, useCallback, useContext, useEffect } from "react";
import { Redirect } from "react-router-dom";
import PropTypes from "prop-types";
import MultiOptionTextField from "../../../utils/form/MultiOptionTextField";
Expand Down Expand Up @@ -43,6 +43,18 @@ export const PAID_OPTIONS = [
{ value: false, label: "Unpaid" },
];

const scrollToError = (errorArray) => {
if (Object.keys(errorArray).length !== 0) {
const element = document.getElementById(Object.keys(errorArray)[0]);
if (element?.scrollIntoView) {
element.scrollIntoView({ behavior: "smooth" });
}
if (element?.focus) {
element.focus();
}
}
};

const OfferForm = ({ context, title }) => {
const {
submit,
Expand Down Expand Up @@ -102,12 +114,20 @@ const OfferForm = ({ context, title }) => {
},
};

useEffect(() => {
scrollToError(errors);
}, [errors]);

useEffect(() => {
scrollToError(requestErrors);
}, [requestErrors]);

return (
success
? <Redirect to={`/offer/${offerId}`} push />
:
<div className={classes.formCard}>
<CardHeader title={!isMobile && title } />
<CardHeader title={!isMobile && title} />
<Content className={classes.formContent}>
<ConnectedLoginAlert
isLoggedIn={isLoggedIn}
Expand Down Expand Up @@ -164,10 +184,11 @@ const OfferForm = ({ context, title }) => {
<Controller
name="fields"
render={(
{ field: { onBlur, name } },
{ field: { onBlur, name } },
) => (
<MultiOptionAutocomplete
name={name}
id={name}
onBlur={onBlur}
error={errors.fields || requestErrors.fields}
disabled={formDisabled}
Expand All @@ -193,6 +214,7 @@ const OfferForm = ({ context, title }) => {
) => (
<MultiOptionAutocomplete
name={name}
id={name}
onBlur={onBlur}
error={errors.technologies || requestErrors.technologies}
disabled={formDisabled}
Expand Down Expand Up @@ -314,6 +336,7 @@ const OfferForm = ({ context, title }) => {
<MultiOptionTextField
values={contacts}
label="Contacts *"
id="contacts"
itemLabelPrefix="Contact #"
controllerName="contacts"
onAdd={appendContact}
Expand All @@ -329,6 +352,7 @@ const OfferForm = ({ context, title }) => {
<MultiOptionTextField
values={requirements}
label="Requirements *"
id="requirements"
itemLabelPrefix="Requirement #"
controllerName="requirements"
onAdd={appendRequirement}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const OwnerComponent = ({ disabled, errors, requestErrors, control }) => (
name={name}
value={value}
label="Owner ID *"
id="owner"
id={name}
error={!!errors.owner || !!requestErrors.owner}
inputRef={ref}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const PublicationDateComponent = ({ disabled, errors, requestErrors, control, da
margin="dense"
value={value}
label="Publication Date *"
id="publishDate-input"
id={name}
name={name}
disabled={disabled}
onChange={(_, value) => onChange(value)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const PublicationEndDateComponent = ({ fields, disabled, errors, requestErrors,
margin="dense"
value={value}
label="Publication End Date *"
id="publishEndDate-input"
id={name}
name={name}
disabled={disabled}
onChange={(_, value) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ const TextEditorComponent = ({ disabled, errors, requestErrors, control }) => (
<Controller
name="descriptionText"
render={(
{ field: { onChange: onChangeDescriptionText } },
{ field: { onChange: onChangeDescriptionText, name } },
) => (
<Controller
name="description"
render={(
{ field: { onChange: onChangeDescription, value } },
) => (
<TextEditor
id={name}
onChangeDescription={onChangeDescription}
onChangeDescriptionText={onChangeDescriptionText}
error={!!errors?.descriptionText || !!requestErrors?.descriptionText}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ const TitleComponent = ({ disabled, errors, requestErrors, control }) => (
) => (
<TextField
name={name}
id={name}
value={value}
label="Offer Title *"
id="title"
error={!!errors.title || !!requestErrors.title}
inputRef={ref}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const VacanciesComponent = ({ disabled, errors, requestErrors, control }) => (
name={name}
value={value}
label="Vacancies"
id="vacancies"
id={name}
disabled={disabled}
error={!!errors?.vacancies || !!requestErrors.vacancies}
helperText={
Expand Down
33 changes: 31 additions & 2 deletions src/components/Offers/New/CreateOfferForm.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ describe("Create Offer Form", () => {
);

// Should work with label but somehow it wasn't being targeted
const input = screen.getByTestId("tech-selector");
const input = screen.getByTestId("technologies");

await act(async () => {

Expand All @@ -405,7 +405,7 @@ describe("Create Offer Form", () => {
{ initialState, theme }
);

const input = screen.getByTestId("tech-selector");
const input = screen.getByTestId("technologies");
fireEvent.mouseDown(input);

fireEvent.click(screen.getByText("React"));
Expand Down Expand Up @@ -735,5 +735,34 @@ describe("Create Offer Form", () => {
expect(screen.queryByText("Publication End Date *")).toBeVisible();
expect(screen.queryByText("Hide offer")).toBeVisible();
});

it("should scroll when a field has an error", async () => {
useSession.mockImplementation(() => ({ isLoggedIn: true, data: { company: { name: "Company Name" } } }));

renderWithStoreAndTheme(
<BrowserRouter>
<MuiPickersUtilsProvider utils={DateFnsUtils}>
<CreateOfferWrapper>
<CreateOfferPage />
</CreateOfferWrapper>
</MuiPickersUtilsProvider>
</BrowserRouter>,
{ initialState, theme }
);

const scrollIntoViewMock = jest.fn();
window.HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;

const titleInput = screen.getByLabelText("Offer Title *");
const submitButton = screen.getByTestId("submit-offer");

await act(async () => {
await fireEvent.focus(titleInput);
await fireEvent.blur(titleInput);
await fireEvent.click(submitButton);
});

expect(scrollIntoViewMock).toBeCalledWith({ behavior: "smooth" });
});
});
});
5 changes: 3 additions & 2 deletions src/components/utils/LocationPicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const asyncThrottle = (func, wait) => {
});
};

const LocationPicker = ({ name, label, value, onChange, onBlur, error, disabled }) => {
const LocationPicker = ({ name, id, label, value, onChange, onBlur, error, disabled }) => {
const [inputValue, setInputValue] = useState("");
const [options, setOptions] = useState([]);
const [loading, setLoading] = useState(false);
Expand Down Expand Up @@ -106,6 +106,7 @@ const LocationPicker = ({ name, label, value, onChange, onBlur, error, disabled
value={value}
inputValue={inputValue}
name={name}
id={id}
disabled={disabled}
onBlur={onBlur}
onChange={(e, newValue) => {
Expand Down Expand Up @@ -149,12 +150,12 @@ const LocationPicker = ({ name, label, value, onChange, onBlur, error, disabled
LocationPicker.propTypes = {
name: PropTypes.string.isRequired,
label: PropTypes.string.isRequired,
id: PropTypes.string,
value: PropTypes.any,
onChange: PropTypes.func.isRequired,
onBlur: PropTypes.func.isRequired,
error: PropTypes.any,
disabled: PropTypes.bool,

};

export default LocationPicker;
5 changes: 3 additions & 2 deletions src/components/utils/TextEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ EditorToolbar.propTypes = {
disabled: PropTypes.bool,
};

const TextEditor = ({ content, onChangeDescription, onChangeDescriptionText, error, helperText: additionalHelperText, disabled }) => {
const TextEditor = ({ id, content, onChangeDescription, onChangeDescriptionText, error, helperText: additionalHelperText, disabled }) => {
const editor = useEditor({
extensions: [
StarterKit,
Expand Down Expand Up @@ -177,7 +177,7 @@ const TextEditor = ({ content, onChangeDescription, onChangeDescriptionText, err
{!!editor &&
<FormControl margin="dense" fullWidth>
<EditorToolbar editor={editor} disabled={disabled} />
<EditorContent editor={editor} />
<EditorContent editor={editor} id={id} />
<FormHelperText error={error}>
{helperText}
</FormHelperText>
Expand All @@ -188,6 +188,7 @@ const TextEditor = ({ content, onChangeDescription, onChangeDescriptionText, err
};

TextEditor.propTypes = {
id: PropTypes.string,
content: PropTypes.any.isRequired,
onChangeDescription: PropTypes.func.isRequired,
onChangeDescriptionText: PropTypes.func.isRequired,
Expand Down
4 changes: 3 additions & 1 deletion src/components/utils/form/MultiOptionTextField.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ const MultiOptionTextField = ({
disabled,
textFieldProps,
addEntryBtnTestId,
id,
}) => {
const classes = useMultiOptionTextFieldStyle();
return (
<>
<Typography variant="h6">
{label}
</Typography>
<Box display="flex" flexDirection="column">
<Box id={id} display="flex" flexDirection="column">
{values.map(({ id }, i) => (
<Controller
key={id}
Expand Down Expand Up @@ -114,6 +115,7 @@ MultiOptionTextField.propTypes = {
disabled: PropTypes.bool,
textFieldProps: PropTypes.object,
addEntryBtnTestId: PropTypes.string,
id: PropTypes.string,
};

export default MultiOptionTextField;
4 changes: 2 additions & 2 deletions src/components/utils/offers/useFieldSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ export default (value, setValues) => {
placeholder: "Fields",
multiple: true,
options: Object.keys(FIELD_OPTIONS),
id: "fields-selector",
id: "fields",
getOptionLabel: (option) => FIELD_OPTIONS[option],
onChange: useCallback((e, value) => value && setValues(value), [setValues]),
value: value,
inputProps: {
"data-testid": "fields-selector",
"data-testid": "fields",
},
};

Expand Down
4 changes: 2 additions & 2 deletions src/components/utils/offers/useTechSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ export default (value, setValues) => {
placeholder: "Technologies",
multiple: true,
options: Object.keys(TECH_OPTIONS),
id: "tech-selector",
id: "technologies",
getOptionLabel: (option) => TECH_OPTIONS[option],
onChange: useCallback((e, value) => value && setValues(value), [setValues]),
value: value,
inputProps: {
"data-testid": "tech-selector",
"data-testid": "technologies",
},
};

Expand Down

0 comments on commit d47281c

Please sign in to comment.