feat: Team Settings Editor Form UI Changes #3305

RODO94 · 2024-06-20T14:18:49Z

What is this PR?

Following on from previous PR: #3289 -- which added a new team_settings table to the database, this PR makes the relevant UI and Editor changes to interact with the new table. In addition, a new settings form is made so users can manage their own team settings.

Remove "commented out" validation on TeamSettings route view

RODO94 · 2024-06-20T14:25:06Z

@DafyddLlyr Tried to update my branch to the latest main and accidentally pulled commits from you and Ian, is there a way to undo this?

I done a pull on main locally, ran a git merge main on feature branch, then git push...

It has worked previously.

Also, if it's not a problem, I'll do a git rebase main next time

DafyddLlyr · 2024-06-20T14:26:44Z

@RODO94 I think running a rebase on this branch, then force pushing, should drop those commits 👍

If not, the simplest approach to fix this may be a new branch and cherry-picking your commits.

github-actions · 2024-06-20T14:43:24Z

Removed vultr server and associated DNS entries

…t form

RODO94 · 2024-06-24T10:27:36Z

!! Worth noting that I have commented out the path validation check on the route to remove the permissions error, mostly for my mental health. This should be undone prior to squash + merge

RODO94 · 2024-06-24T12:36:25Z

Update: Altered PR title to focus only on Form UI components

Updated the PR title as I will break down the Editor changes into:

UI Components
Validation
Data Fetching
Error Handling + Component Changes

DafyddLlyr

A few small comments and suggestions, no show stoppers - looking good!

DafyddLlyr · 2024-06-24T13:02:42Z

editor.planx.uk/src/ui/shared/InputRowLabel.tsx

@@ -5,6 +5,7 @@ import React, { ReactNode } from "react";
 const Label = styled(Typography)(({ theme }) => ({
  flexShrink: 1,
  flexGrow: 0,
+  width: "100%",


It looks like this change has some unintended consequences in Design Settings where this component is also used.

@ianjon3s is going to take a wider review of these components and labels, so for now we could (temporarily) keep them inline, or use a repeated <Typography component="label" variant="subtitle2">

Another example I just hit -

editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts

editor.planx.uk/src/pages/FlowEditor/components/Settings/shared/SettingsForm.tsx

editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/newSettingsForm.tsx

editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/index.tsx

...r.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/HomepagePlanningForm.tsx

DafyddLlyr · 2024-06-24T13:20:18Z

...r.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/HomepagePlanningForm.tsx

+              </RadioGroup>
+            </InputRowLabel>
+          </InputRow>
+          <InputRow>


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

@DafyddLlyr so remove this who file?

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 👍

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

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?

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)

Apologies for not spotting this issue with has_planning_data sooner!

Sick! All makes sense

DafyddLlyr · 2024-06-24T13:22:40Z

editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/ContactForm.tsx

+          </InputRow>
+          <InputRow>
+            <InputRowLabel>
+              Help Opening Hours


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 👍

Cool!

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

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

DafyddLlyr · 2024-06-24T13:23:13Z

editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/ContactForm.tsx

+  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(),
+  });


@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

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")

@DafyddLlyr @ianjon3s worth noting the advice here: https://opensystemslab.notion.site/How-to-write-good-planning-service-content-30313a55fc5b4499846e32e88b1cf76a#b02887d8bd1d4cbe8e4d6d842a3a398b

…ofrm

DafyddLlyr

Perfect! Much smaller and neater with all those changes, thanks for working through them all.

RODO94 · 2024-07-05T08:44:12Z

Create team_settings table

RODO94 force-pushed the rory/team-settings-editor branch from bdc928c to a6f4edd Compare June 20, 2024 14:28

RODO94 mentioned this pull request Jun 20, 2024

chore: Make SettingsForm shared and generic #3308

Merged

RODO94 added 9 commits June 24, 2024 11:23

init editor changes to plan ui additions

575b581

continue layout and small form additions

4b6c683

changes made to general settings

15e5e08

update SettingsForm to make formik optional and add inputs for contac…

26a7640

…t form

add homepage and planning portal form

1df271e

rename to general settings

3ccff1f

nit: change boundary input label

f29a19c

initial wiring of formik config

ae06ade

add validation to isPlanningDataCollected bool

63ca171

RODO94 force-pushed the rory/team-settings-editor branch from 41805bc to 63ca171 Compare June 24, 2024 10:24

RODO94 added 2 commits June 24, 2024 11:44

remove optional operator from settingsForm type

3f980d6

add error validation as schema to test formik error handling

32c93e0

RODO94 changed the title ~~feat: Team Settings Editor Changes~~ feat: Team Settings Editor Form UI Changes Jun 24, 2024

RODO94 marked this pull request as ready for review June 24, 2024 12:37

DafyddLlyr reviewed Jun 24, 2024

View reviewed changes

RODO94 added 8 commits June 24, 2024 14:39

nit: remove full width from input label and switch bool default value

ca7ec39

delete temp settings form component

3cc93cc

remove redundant variable

329fec4

made changes to comments from Daf@

52a56c0

remove optional operand from return comp of settingsForm

9c4b56d

remove optional operand from return comp of settingsForm

2f58c27

revert changes to settingsForm

97f6064

revert changes to store file

a0eaeaa

RODO94 added 3 commits June 24, 2024 15:20

revert changes to store file

0946197

remove homepage and planning form and move homepage field to contact …

8b3c4b7

…ofrm

remove commented out code in route view

6499098

RODO94 requested a review from DafyddLlyr June 24, 2024 14:46

DafyddLlyr approved these changes Jun 24, 2024

View reviewed changes

RODO94 merged commit 9020589 into main Jun 25, 2024
12 checks passed

RODO94 deleted the rory/team-settings-editor branch June 25, 2024 09:14

This was referenced Jun 25, 2024

chore: Remove has_planning_data from team_settings table #3314

Merged

Production deploy #3317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Team Settings Editor Form UI Changes #3305

feat: Team Settings Editor Form UI Changes #3305

RODO94 commented Jun 20, 2024 •

edited

Loading

RODO94 commented Jun 20, 2024

DafyddLlyr commented Jun 20, 2024

github-actions bot commented Jun 20, 2024 •

edited

Loading

RODO94 commented Jun 24, 2024

RODO94 commented Jun 24, 2024 •

edited

Loading

DafyddLlyr left a comment

DafyddLlyr Jun 24, 2024

DafyddLlyr Jun 24, 2024

DafyddLlyr Jun 24, 2024

RODO94 Jun 24, 2024

DafyddLlyr Jun 24, 2024

RODO94 Jun 24, 2024

RODO94 Jun 24, 2024

DafyddLlyr Jun 24, 2024

DafyddLlyr Jun 24, 2024

RODO94 Jun 24, 2024

DafyddLlyr Jun 24, 2024

RODO94 Jun 24, 2024

DafyddLlyr Jun 24, 2024 •

edited

Loading

DafyddLlyr Jun 24, 2024

RODO94 Jun 24, 2024

DafyddLlyr Jun 24, 2024 •

edited

Loading

RODO94 Jun 25, 2024

DafyddLlyr left a comment

RODO94 commented Jul 5, 2024

feat: Team Settings Editor Form UI Changes #3305

feat: Team Settings Editor Form UI Changes #3305

Conversation

RODO94 commented Jun 20, 2024 • edited Loading

What is this PR?

RODO94 commented Jun 20, 2024

DafyddLlyr commented Jun 20, 2024

github-actions bot commented Jun 20, 2024 • edited Loading

RODO94 commented Jun 24, 2024

RODO94 commented Jun 24, 2024 • edited Loading

Update: Altered PR title to focus only on Form UI components

DafyddLlyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr left a comment

Choose a reason for hiding this comment

RODO94 commented Jul 5, 2024

RODO94 commented Jun 20, 2024 •

edited

Loading

github-actions bot commented Jun 20, 2024 •

edited

Loading

RODO94 commented Jun 24, 2024 •

edited

Loading

DafyddLlyr Jun 24, 2024 •

edited

Loading

DafyddLlyr Jun 24, 2024 •

edited

Loading