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

Conversation

RODO94
Copy link
Contributor

@RODO94 RODO94 commented Jul 3, 2024

What does this PR do?

Following previous work done on team_settings form in the Editor: #3366

This PR will introduce the Error Handling while making changes to shared SettingsForm to handle error messaging on an input level rather than form level.

The only change to an Input Component will be to the <ColourPicker> which is used in the Theme settings forms. The prop errorMessage has been added to the <ColourPicker> component to enable this input level error messaging

Copy link

github-actions bot commented Jul 3, 2024

Removed vultr server and associated DNS entries

@RODO94
Copy link
Contributor Author

RODO94 commented Jul 5, 2024

The errorMessage Prop in Colour Picker

The prop errorMessage in the <ColourPicker> Component has been added as optional due to it being used in the Editor in components like Notice.

@RODO94
Copy link
Contributor Author

RODO94 commented Jul 5, 2024

Error Message Guidance

The tone of voice guiding the error messaging follows information here: How to write good planning service content

@RODO94 RODO94 marked this pull request as ready for review July 5, 2024 08:41
@RODO94 RODO94 requested a review from a team July 5, 2024 08:41
@RODO94
Copy link
Contributor Author

RODO94 commented Jul 5, 2024

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Happy to merge following a few (very very minor!) fixes.


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.

</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

@RODO94 RODO94 merged commit 6d8c06b into main Jul 5, 2024
12 checks passed
@RODO94 RODO94 deleted the rory/team-settings-error-validation branch July 5, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants