Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: team_settings form error validation #3368

Merged
merged 5 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export const ButtonForm: React.FC<FormProps> = ({
color={formik.values.actionColour}
onChange={(color) => formik.setFieldValue("actionColour", color)}
label="Button colour"
errorMessage={formik.errors.actionColour}
/>
</InputRowItem>
</InputRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const TextLinkForm: React.FC<FormProps> = ({
color={formik.values.linkColour}
onChange={(color) => formik.setFieldValue("linkColour", color)}
label="Text link colour"
errorMessage={formik.errors.linkColour}
/>
</InputRowItem>
</InputRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const ThemeAndLogoForm: React.FC<FormProps> = ({
formik.setFieldValue("primaryColour", color)
}
label="Theme colour"
errorMessage={formik.errors.primaryColour}
/>
</InputRowItem>
</InputRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@ import { useStore } from "pages/FlowEditor/lib/store";
import React, { ChangeEvent } from "react";
import InputLabel from "ui/editor/InputLabel";
import Input from "ui/shared/Input";
import * as Yup from "yup";

import { SettingsForm } from "../shared/SettingsForm";
import { FormProps } from ".";

export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
const formSchema = Yup.object().shape({
boundaryUrl: Yup.string()
.url(
"Enter a boundary URL in the correct format, https://www.planning.data.gov.uk/",
)
.required("Enter a boundary URL"),
});

const formik = useFormik({
...formikConfig,
validationSchema: formSchema,
onSubmit: async (values, { resetForm }) => {
const isSuccess = await useStore.getState().updateTeamSettings({
boundaryUrl: values.boundaryUrl,
Expand Down Expand Up @@ -49,6 +59,7 @@ export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
<Input
name="boundary"
value={formik.values.boundaryUrl}
errorMessage={formik.errors.boundaryUrl}
onChange={(ev: ChangeEvent<HTMLInputElement>) => {
formik.setFieldValue("boundaryUrl", ev.target.value);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@ import { FormProps } from ".";
export default function ContactForm({ formikConfig, onSuccess }: FormProps) {
const formSchema = Yup.object().shape({
helpEmail: Yup.string()
.email("Please enter valid email")
.required("Help Email is required"),
helpPhone: Yup.string().required("Help Phone is required"),
helpOpeningHours: Yup.string().required(),
.email(
"Enter an email address in the correct format, like [email protected]",
)
.required("Enter a help email address"),
helpPhone: Yup.string().required("Enter a help phone number"),
helpOpeningHours: Yup.string().required("Enter your opening hours"),
homepage: Yup.string()
.url("Please enter a valid URL for the homepage")
.url(
"Enter a homepage URL in the correct format, like https://www.localauthority.gov.uk/",
)
.required("Enter a homepage"),
});

Expand Down Expand Up @@ -59,13 +63,15 @@ export default function ContactForm({ formikConfig, onSuccess }: FormProps) {
onChangeFn("homepage", event);
}}
value={formik.values.homepage}
errorMessage={formik.errors.homepage}
id="homepage"
/>
</InputLabel>
<InputLabel label="Contact email address" htmlFor="helpEmail">
<Input
name="helpEmail"
value={formik.values.helpEmail}
errorMessage={formik.errors.helpEmail}
onChange={(event) => {
onChangeFn("helpEmail", event);
}}
Expand All @@ -76,6 +82,7 @@ export default function ContactForm({ formikConfig, onSuccess }: FormProps) {
<Input
name="helpPhone"
value={formik.values.helpPhone}
errorMessage={formik.errors.helpPhone}
onChange={(event) => {
onChangeFn("helpPhone", event);
}}
Expand All @@ -87,6 +94,7 @@ export default function ContactForm({ formikConfig, onSuccess }: FormProps) {
multiline
name="helpOpeningHours"
value={formik.values.helpOpeningHours}
errorMessage={formik.errors.helpOpeningHours}
onChange={(event) => {
onChangeFn("helpOpeningHours", event);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,21 @@ export const SettingsForm = <TFormikValues,>({
{preview}
</Box>
)}

<ErrorWrapper
error={Object.values(formik.errors).join(", ")}
id="settings-error"
>
<Box>
<Button type="submit" variant="contained" disabled={!formik.dirty}>
Save
</Button>
<Button
onClick={() => formik.resetForm()}
type="reset"
variant="contained"
disabled={!formik.dirty}
color="secondary"
sx={{ ml: 1.5 }}
>
Reset changes
</Button>
</Box>
</ErrorWrapper>
<Box>
<Button type="submit" variant="contained" disabled={!formik.dirty}>
Save
</Button>
<Button
onClick={() => formik.resetForm()}
type="reset"
variant="contained"
disabled={!formik.dirty}
color="secondary"
sx={{ ml: 1.5 }}
>
Reset changes
</Button>
</Box>
</form>
</SettingsSection>
);
Expand Down
51 changes: 28 additions & 23 deletions editor.planx.uk/src/ui/editor/ColorPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ import Box, { BoxProps } from "@mui/material/Box";
import ButtonBase, { ButtonBaseProps } from "@mui/material/ButtonBase";
import { styled } from "@mui/material/styles";
import Typography from "@mui/material/Typography";
import { ErrorMessage } from "formik";
import React, { useState } from "react";
import { ChromePicker, ColorChangeHandler } from "react-color";
import ErrorWrapper from "ui/shared/ErrorWrapper";

export interface Props {
label?: string;
inline?: boolean;
color?: string;
errorMessage?: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
errorMessage?: string | undefined;
errorMessage?: string;

nit: This is the same thing but a bit more terse and matches our code style. The ? following the property name indicates that this property can be undefined.

onChange?: (newColor: string) => void;
}

Expand Down Expand Up @@ -93,28 +96,30 @@ export default function ColorPicker(props: Props): FCReturn {
};

return (
<Root inline={props.inline}>
<Typography mr={2} variant="body2" component="label">
{props.label || "Background colour"}:{" "}
</Typography>
<StyledButtonBase show={show} onClick={handleClick} disableRipple>
<Swatch sx={{ backgroundColor: props.color }} className="swatch" />
{props.color}
</StyledButtonBase>
{show ? (
<Popover className="popover">
<Cover
onClick={handleClose}
aria-label="Close Colour Picker"
disableRipple
/>
<ChromePicker
color={props.color}
disableAlpha={true}
onChange={handleChange}
/>
</Popover>
) : null}
</Root>
<ErrorWrapper error={props.errorMessage || undefined} id="settings-error">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a unique id here, for example colour-picker

<Root inline={props.inline}>
<Typography mr={2} variant="body2" component="label">
{props.label || "Background colour"}:{" "}
</Typography>
<StyledButtonBase show={show} onClick={handleClick} disableRipple>
<Swatch sx={{ backgroundColor: props.color }} className="swatch" />
{props.color}
</StyledButtonBase>
{show ? (
<Popover className="popover">
<Cover
onClick={handleClose}
aria-label="Close Colour Picker"
disableRipple
/>
<ChromePicker
color={props.color}
disableAlpha={true}
onChange={handleChange}
/>
</Popover>
) : null}
</Root>
</ErrorWrapper>
);
}
Loading