feat: Fetch team_settings data for Editor forms and adding Update fn
#3366
Conversation
|
Removed vultr server and associated DNS entries |
There was a problem hiding this comment.
Looking good - a lot simpler and cleaner following those naming updates 👍
There's a few outstanding issues to address, please see comments.
Also, I'd take a look at co-locating errors alongside their inputs. This will lead to a better user experience, and is required for accessibility reasons. This can be addressed here or in a follow up PR - whatever works best for you.
| { | ||
| name: "General", | ||
| route: "general", | ||
| Component: GeneralSettings, | ||
| }, |
There was a problem hiding this comment.
nit: I think I'd reorder this array to be General → Design → Team
There was a problem hiding this comment.
@DafyddLlyr do you think it is worth doing these types of changes as a final PR when error handling is all sorted?
There was a problem hiding this comment.
Thought about it again, and tit doesn't really make it difference, so I've added it to this PR
| const isSuccess = await useStore.getState().updateTeamSettings({ | ||
| helpEmail: values.helpEmail, | ||
| helpOpeningHours: values.helpOpeningHours, | ||
| helpPhone: values.helpPhone, |
There was a problem hiding this comment.
homepage is missing here which means that changes to the URL are not persisted.
| onSubmit(values, { resetForm }) { | ||
| onSuccess(); | ||
| resetForm({ values }); | ||
| onSubmit: async (values, { resetForm }) => { |
There was a problem hiding this comment.
We're still missing some basic validation on this field. I appreciate there's still work to be done on the boundary field, but if we're going to unhide the menu and allow Editors to access it we should either put this in place first, or hide this field.
There was a problem hiding this comment.
I'll do the error handling in another PR, hopefully make it a lot cleaner. Current ones were just to test my understanding of formik and yup
There was a problem hiding this comment.
I have added basic validation now though
Maybe I should move that last commit to a new PR then? @DafyddLlyr |
|
Sorry for not being clear, I think this should merge as-is, and we can tackle per-field validation next in a follow up PR. |
1 similar comment
What does this PR do?
This is a cleaner branch from the work undertaken on this previous branch: #3319
Continuing work on the
team_settingsimplementation, this PR pulls in data from the database with a graphQL query and populates the corresponding form inputs.This PR also uses the update mutation, similar to
team_themesto update the form fields and database.This PR aligns with a planx-core change where the requests and types for a Team were altered to incorporate the new
team_settingstable.Reference to the planx-core changes