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 Editor Form UI Changes #3305

Merged
merged 22 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
575b581
init editor changes to plan ui additions
RODO94 Jun 20, 2024
4b6c683
continue layout and small form additions
RODO94 Jun 20, 2024
15e5e08
changes made to general settings
RODO94 Jun 21, 2024
26a7640
update SettingsForm to make formik optional and add inputs for contac…
RODO94 Jun 21, 2024
1df271e
add homepage and planning portal form
RODO94 Jun 21, 2024
3ccff1f
rename to general settings
RODO94 Jun 21, 2024
f29a19c
nit: change boundary input label
RODO94 Jun 21, 2024
ae06ade
initial wiring of formik config
RODO94 Jun 24, 2024
63ca171
add validation to isPlanningDataCollected bool
RODO94 Jun 24, 2024
3f980d6
remove optional operator from settingsForm type
RODO94 Jun 24, 2024
32c93e0
add error validation as schema to test formik error handling
RODO94 Jun 24, 2024
ca7ec39
nit: remove full width from input label and switch bool default value
RODO94 Jun 24, 2024
3cc93cc
delete temp settings form component
RODO94 Jun 24, 2024
329fec4
remove redundant variable
RODO94 Jun 24, 2024
52a56c0
made changes to comments from Daf@
RODO94 Jun 24, 2024
9c4b56d
remove optional operand from return comp of settingsForm
RODO94 Jun 24, 2024
2f58c27
remove optional operand from return comp of settingsForm
RODO94 Jun 24, 2024
97f6064
revert changes to settingsForm
RODO94 Jun 24, 2024
a0eaeaa
revert changes to store file
RODO94 Jun 24, 2024
0946197
revert changes to store file
RODO94 Jun 24, 2024
8b3c4b7
remove homepage and planning form and move homepage field to contact …
RODO94 Jun 24, 2024
6499098
remove commented out code in route view
RODO94 Jun 24, 2024
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
@@ -0,0 +1,59 @@
import { useFormik } from "formik";
import React, { ChangeEvent, useState } from "react";
import InputDescription from "ui/editor/InputDescription";
import Input from "ui/shared/Input";
import InputRow from "ui/shared/InputRow";
import InputRowLabel from "ui/shared/InputRowLabel";

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

export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
const formik = useFormik({
...formikConfig,
onSubmit(values, { resetForm }) {
onSuccess();
resetForm({ values });
},
});

return (
<SettingsForm
formik={formik}
legend="Boundary"
description={
<InputDescription>
The boundary URL is used to retrieve the outer boundary of your
council area. This can then help users define whether they are within
your council area.
<br />
<br />
The boundary should be given as a link from:{" "}
<a
href="https://www.planning.data.gov.uk/"
target="_blank"
rel="noopener noreferrer"
>
https://www.planning.data.gov.uk/
</a>
</InputDescription>
}
input={
<>
<InputRow>
<InputRowLabel>
Boundary URL
<Input
name="boundary"
value={formik.values.boundaryUrl}
onChange={(ev: ChangeEvent<HTMLInputElement>) => {
formik.setFieldValue("boundaryUrl", ev.target.value);
}}
/>
</InputRowLabel>
</InputRow>
</>
}
/>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { useFormik } from "formik";
import React, { ChangeEvent } from "react";
import InputDescription from "ui/editor/InputDescription";
import Input from "ui/shared/Input";
import InputRow from "ui/shared/InputRow";
import InputRowLabel from "ui/shared/InputRowLabel";
import * as Yup from "yup";

import { SettingsForm } from "../shared/SettingsForm";
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(),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DafyddLlyr maybe this is something we pick up with @ianjon3s as well, but in terms of UX copy and tone of voice in message handling, is there a particular way to write these error messages?

To ensure consistency in tone, I know people do like to manage how these a written

Copy link
Contributor

@DafyddLlyr DafyddLlyr Jun 24, 2024

Choose a reason for hiding this comment

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

We don't have standard copy for Editor-facing errors, but generally try to get input from the content team (+ Nomensa URs) on public-facing error messages.

I'm pretty sure most Editor-facing inputs just currently use .required() and would output whatever Yup outputs which would be something like ${fieldName} is required"helpEmail is required"

I think what you have here is great (${labelName} is required"Help email is required")

Copy link
Contributor Author

Choose a reason for hiding this comment

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


const formik = useFormik({
...formikConfig,
validationSchema: formSchema,
onSubmit(values, { resetForm }) {
onSuccess();
resetForm({ values });
},
});

const onChangeFn = (type: string, event: ChangeEvent<HTMLInputElement>) =>
formik.setFieldValue(type, event.target.value);

return (
<SettingsForm
legend="Contact Information"
formik={formik}
description={
<InputDescription>
Details to help direct different messages, feedback, and enquiries
from users.
</InputDescription>
}
input={
<>
<InputRow>
<InputRowLabel>
Help Email
<Input
name="helpEmail"
onChange={(event) => {
onChangeFn("helpEmail", event);
}}
/>
</InputRowLabel>
</InputRow>
<InputRow>
<InputRowLabel>
Help Phone
<Input
name="helpPhone"
onChange={(event) => {
onChangeFn("helpPhone", event);
}}
/>
</InputRowLabel>
</InputRow>
<InputRow>
<InputRowLabel>
Help Opening Hours
Copy link
Contributor

Choose a reason for hiding this comment

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

nit(ui/ux): I think we can remove the "Help..." prefixes from these labels - "Phone number", "Opening hours" and "Contact email address" feel like clearer / simpler labels.

There's always room for the data model and it's representation to the user to be different 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool!

What do you think about keeping the data model fields with the "help" prefix to group and indicate use?

Copy link
Contributor

@DafyddLlyr DafyddLlyr Jun 24, 2024

Choose a reason for hiding this comment

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

Yep, I'd keep that internally, and just change the labels displayed to users 👍

<Input
multiline
name="helpOpeningHours"
onChange={(event) => {
onChangeFn("helpOpeningHours", event);
}}
/>
</InputRowLabel>
</InputRow>
</>
}
/>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import RadioGroup from "@mui/material/RadioGroup";
import BasicRadio from "@planx/components/shared/Radio/BasicRadio";
import { useFormik } from "formik";
import React, { ChangeEvent, useState } from "react";
import InputDescription from "ui/editor/InputDescription";
import Input from "ui/shared/Input";
import InputRow from "ui/shared/InputRow";
import InputRowLabel from "ui/shared/InputRowLabel";

import { SettingsForm } from "../shared/SettingsForm";
import { FormProps } from ".";
export default function HomepagePlanningForm({
formikConfig,
onSuccess,
}: FormProps) {
const [showPlanningInputs, setShowPlanningInputs] = useState(false);

const formik = useFormik({
...formikConfig,
onSubmit(values, { resetForm }) {
onSuccess();
resetForm({ values });
},
});

const onChangeFn = (type: string, event: ChangeEvent<HTMLInputElement>) =>
formik.setFieldValue(type, event.target.value);

const boolTransform = (event: ChangeEvent<HTMLInputElement>) => {
if (event.target.value === "yes") {
setShowPlanningInputs(true);
return true;
} else if (event.target.value === "no") {
setShowPlanningInputs(false);
return false;
}
};
RODO94 marked this conversation as resolved.
Show resolved Hide resolved

return (
<SettingsForm
legend="Homepage and Planning Portal"
formik={formik}
description={
<InputDescription>
A link to your homepage displayed publicly to your users to help
navigate your council services and a link to your Planning Portal to
connect your planning data with our outputs.
</InputDescription>
}
input={
<>
<InputRow>
<InputRowLabel>
Homepage URL
<Input
name="homepage"
onChange={(event) => {
onChangeFn("homepage", event);
}}
/>
</InputRowLabel>
</InputRow>
<InputRow>
<InputRowLabel>
Do you collect Planning data?
<RadioGroup
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
name="isPlanningDataCollected"
defaultValue={
formik.values.isPlanningDataCollected === true ? "yes" : "no"
}
onChange={(event) => {
formik.setFieldValue(
"isPlanningDataCollected",
boolTransform(event),
);
}}
>
<BasicRadio
title="Yes"
variant="compact"
id="yes"
value="yes"
onChange={() => {}}
/>
<BasicRadio
title="No"
variant="compact"
id="no"
value="no"
onChange={() => {}}
/>
</RadioGroup>
</InputRowLabel>
</InputRow>
<InputRow>
Copy link
Contributor

Choose a reason for hiding this comment

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

So actually I think we can remove this whole block! When we added this feature, we thought councils may want to have customisable URLs here, but so far this hasn't proved to be the case.

Here's my recommendation -

  • We keep these columns on the team_settings table, so if we need to customise this in future we easily can
  • We don't show this as a feature to the user, and see if we get any feedback
  • In future, we can easily add these form filed, or remove the db columns, based on feedback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DafyddLlyr so remove this who file?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry - should have been clearer! Just the inputs for Planning Portal name and Planning Portal URL. We've found that nobody uses custom values here currently 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay! but keep the Boolean field and the homepage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asking because, as I am thinking, if it's just the Homepage + the Bool field, then what use is the bool field?

If it is just the Homepage, could I just incorporate that into the Contact Form?

Copy link
Contributor

Choose a reason for hiding this comment

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

Homepage should stay, and could move into contact 👍

Just taken a closer look at team_settings.has_planning_data - this looks like it's actually unused. We've recently moved this across to team_integrations (PR here - #2776)

What this means is -

  • We can remove the the form field for hasPlanningData - this is an integration, not a setting, and doesn't need to be handled here
  • We can remove the team_settings.has_planning_data column (in another PR)

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for not spotting this issue with has_planning_data sooner!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sick! All makes sense

<InputRowLabel>
Planning Portal Name
<Input
name="portalName"
disabled={showPlanningInputs ? false : true}
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
onChange={(event) => {
onChangeFn("portalName", event);
}}
/>
</InputRowLabel>
</InputRow>
<InputRow>
<InputRowLabel>
Planning Portal URL
<Input
name="portalUrl"
disabled={showPlanningInputs ? false : true}
onChange={(event) => {
onChangeFn("portalUrl", event);
}}
/>
</InputRowLabel>
</InputRow>
</>
}
/>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import Alert from "@mui/material/Alert";
import Box from "@mui/material/Box";
import Snackbar from "@mui/material/Snackbar";
import { styled } from "@mui/material/styles";
import Typography from "@mui/material/Typography";
import { TeamTheme } from "@opensystemslab/planx-core/types";
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
import { FormikConfig } from "formik";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect, useState } from "react";
import EditorRow from "ui/editor/EditorRow";

import BoundaryForm from "./BoundaryForm";
import ContactForm from "./ContactForm";
import HomepagePlanningForm from "./HomepagePlanningForm";

export const DesignPreview = styled(Box)(({ theme }) => ({
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
border: `2px solid ${theme.palette.border.input}`,
padding: theme.spacing(2),
boxShadow: "4px 4px 0px rgba(150, 150, 150, 0.5)",
}));

export const EXAMPLE_COLOUR = "#007078";
RODO94 marked this conversation as resolved.
Show resolved Hide resolved

export interface GeneralSettings {
boundaryUrl: string;
helpEmail: string;
helpPhone: string;
helpOpeningHours: string;
homepage: string;
isPlanningDataCollected: boolean;
portalName: string;
portalUrl: string;
}

export interface FormProps {
formikConfig: FormikConfig<GeneralSettings>;
onSuccess: () => void;
}

const GeneralSettings: React.FC = () => {
const [formikConfig, setFormikConfig] = useState<
FormikConfig<GeneralSettings> | undefined
>(undefined);

const initialValues = {
boundaryUrl: "",
helpEmail: "",
helpPhone: "",
helpOpeningHours: "",
homepage: "",
isPlanningDataCollected: true,
portalName: "",
portalUrl: "",
};

useEffect(() => {
const fetchTeam = async () => {
try {
setFormikConfig({
initialValues: initialValues,
onSubmit: () => {},
validateOnBlur: false,
validateOnChange: false,
enableReinitialize: true,
});
} catch (error) {
console.error("Error fetching team:", error);
}
};

fetchTeam();
}, []);

const [open, setOpen] = useState(true);
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
const [updateMessage, setUpdateMessage] = useState("Setting Updated");

const handleClose = (
_event?: React.SyntheticEvent | Event,
reason?: string,
) => {
if (reason === "clickaway") {
return;
}

setOpen(false);
};

const onSuccess = () => setOpen(true);

return (
<Box maxWidth="formWrap" mx="auto">
<EditorRow>
<Typography variant="h2" component="h3" gutterBottom>
General
</Typography>
<Typography variant="body1">
Important links and settings for how your users connect with you
</Typography>
</EditorRow>
{formikConfig && (
<>
<ContactForm formikConfig={formikConfig} onSuccess={onSuccess} />
<HomepagePlanningForm
formikConfig={formikConfig}
onSuccess={onSuccess}
/>
<BoundaryForm formikConfig={formikConfig} onSuccess={onSuccess} />
</>
)}
<Snackbar open={open} autoHideDuration={6000} onClose={handleClose}>
<Alert onClose={handleClose} severity="success" sx={{ width: "100%" }}>
{updateMessage}
</Alert>
</Snackbar>
</Box>
);
};

export default GeneralSettings;
Loading
Loading