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

refactor: [M3-8986] - Refactor VPCEditDrawer and SubnetEditDrawer to use react-hook-form #11393

Merged
merged 3 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Refactor VPCEditDrawer and SubnetEditDrawer to use `react-hook-form` instead of `formik` ([#11393](https://github.com/linode/manager/pull/11393))
80 changes: 50 additions & 30 deletions packages/manager/src/features/VPCs/VPCDetail/SubnetEditDrawer.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { yupResolver } from '@hookform/resolvers/yup';
import { Notice, TextField } from '@linode/ui';
import { useFormik } from 'formik';
import { modifySubnetSchema } from '@linode/validation';
import * as React from 'react';
import { Controller, useForm } from 'react-hook-form';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
import { useGrants, useProfile } from 'src/queries/profile/profile';
import { useUpdateSubnetMutation } from 'src/queries/vpcs/vpcs';
import { getErrorMap } from 'src/utilities/errorUtils';

import type { ModifySubnetPayload, Subnet } from '@linode/api-v4';

Expand All @@ -24,29 +25,41 @@ export const SubnetEditDrawer = (props: Props) => {
const { onClose, open, subnet, vpcId } = props;

const {
error,
isPending,
mutateAsync: updateSubnet,
reset,
reset: resetMutation,
} = useUpdateSubnetMutation(vpcId, subnet?.id ?? -1);

const form = useFormik<ModifySubnetPayload>({
enableReinitialize: true,
initialValues: {
const {
control,
formState: { errors, isDirty, isSubmitting },
handleSubmit,
reset: resetForm,
setError,
} = useForm<ModifySubnetPayload>({
mode: 'onBlur',
resolver: yupResolver(modifySubnetSchema),
values: {
label: subnet?.label ?? '',
},
async onSubmit(values) {
await updateSubnet(values);
onClose();
},
});

React.useEffect(() => {
if (open) {
form.resetForm();
reset();
const handleDrawerClose = () => {
onClose();
resetForm();
resetMutation();
};

const onSubmit = async (values: ModifySubnetPayload) => {
try {
await updateSubnet(values);
handleDrawerClose();
} catch (errors) {
for (const error of errors) {
setError(error?.field ?? 'root', { message: error.reason });
}
}
}, [open]);
};

const { data: profile } = useProfile();
const { data: grants } = useGrants();
Expand All @@ -59,26 +72,33 @@ export const SubnetEditDrawer = (props: Props) => {
Boolean(profile?.restricted) &&
(vpcPermissions?.permissions === 'read_only' || grants?.vpc.length === 0);

const errorMap = getErrorMap(['label'], error);

return (
<Drawer onClose={onClose} open={open} title="Edit Subnet">
{errorMap.none && <Notice text={errorMap.none} variant="error" />}
<Drawer onClose={handleDrawerClose} open={open} title="Edit Subnet">
{errors.root?.message && (
<Notice text={errors.root.message} variant="error" />
)}
{readOnly && (
<Notice
important
text={`You don't have permissions to edit ${subnet?.label}. Please contact an account administrator for details.`}
variant="error"
/>
)}
<form onSubmit={form.handleSubmit}>
<TextField
disabled={readOnly}
errorText={errorMap.label}
label="Label"
<form onSubmit={handleSubmit(onSubmit)}>
<Controller
render={({ field, fieldState }) => (
<TextField
disabled={readOnly}
errorText={fieldState.error?.message}
label="Label"
name="label"
onBlur={field.onBlur}
onChange={field.onChange}
value={field.value}
/>
)}
control={control}
name="label"
onChange={form.handleChange}
value={form.values.label}
/>
<TextField
disabled
Expand All @@ -89,12 +109,12 @@ export const SubnetEditDrawer = (props: Props) => {
<ActionsPanel
primaryButtonProps={{
'data-testid': 'save-button',
disabled: !form.dirty,
disabled: !isDirty || readOnly,
label: 'Save',
loading: isPending,
loading: isPending || isSubmitting,
type: 'submit',
}}
secondaryButtonProps={{ label: 'Cancel', onClick: onClose }}
secondaryButtonProps={{ label: 'Cancel', onClick: handleDrawerClose }}
/>
</form>
</Drawer>
Expand Down
122 changes: 63 additions & 59 deletions packages/manager/src/features/VPCs/VPCLanding/VPCEditDrawer.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import { yupResolver } from '@hookform/resolvers/yup';
import { Notice, TextField } from '@linode/ui';
import { updateVPCSchema } from '@linode/validation/lib/vpcs.schema';
import { useFormik } from 'formik';
import { updateVPCSchema } from '@linode/validation';
import * as React from 'react';
import { Controller, useForm } from 'react-hook-form';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
import { RegionSelect } from 'src/components/RegionSelect/RegionSelect';
import { useGrants, useProfile } from 'src/queries/profile/profile';
import { useRegionsQuery } from 'src/queries/regions/regions';
import { useUpdateVPCMutation } from 'src/queries/vpcs/vpcs';
import { getErrorMap } from 'src/utilities/errorUtils';

import type { UpdateVPCPayload, VPC } from '@linode/api-v4/lib/vpcs/types';
import type { UpdateVPCPayload, VPC } from '@linode/api-v4';

interface Props {
onClose: () => void;
Expand All @@ -36,84 +36,88 @@ export const VPCEditDrawer = (props: Props) => {
(vpcPermissions?.permissions === 'read_only' || grants?.vpc.length === 0);

const {
error,
isPending,
mutateAsync: updateVPC,
reset,
reset: resetMutation,
} = useUpdateVPCMutation(vpc?.id ?? -1);

interface UpdateVPCPayloadWithNone extends UpdateVPCPayload {
none?: string;
}

const form = useFormik<UpdateVPCPayloadWithNone>({
enableReinitialize: true,
initialValues: {
const {
control,
formState: { errors, isDirty, isSubmitting },
handleSubmit,
reset: resetForm,
setError,
} = useForm<UpdateVPCPayload>({
mode: 'onBlur',
resolver: yupResolver(updateVPCSchema),
values: {
description: vpc?.description,
label: vpc?.label,
},
async onSubmit(values) {
await updateVPC(values);
onClose();
},
validateOnChange: false,
validationSchema: updateVPCSchema,
});

const handleFieldChange = (field: string, value: string) => {
form.setFieldValue(field, value);
if (form.errors[field as keyof UpdateVPCPayloadWithNone]) {
form.setFieldError(field, undefined);
}
const handleDrawerClose = () => {
onClose();
resetForm();
resetMutation();
};

React.useEffect(() => {
if (open) {
form.resetForm();
reset();
}
}, [open]);

// If there's an error, sync it with formik
React.useEffect(() => {
if (error) {
const errorMap = getErrorMap(['label', 'description'], error);
for (const [field, reason] of Object.entries(errorMap)) {
form.setFieldError(field, reason);
const onSubmit = async (values: UpdateVPCPayload) => {
try {
await updateVPC(values);
handleDrawerClose();
} catch (errors) {
for (const error of errors) {
setError(error?.field ?? 'root', { message: error.reason });
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [error]);
};

const { data: regionsData, error: regionsError } = useRegionsQuery();

return (
<Drawer onClose={onClose} open={open} title="Edit VPC">
{form.errors.none && <Notice text={form.errors.none} variant="error" />}
<Drawer onClose={handleDrawerClose} open={open} title="Edit VPC">
{errors.root?.message && (
<Notice text={errors.root.message} variant="error" />
)}
{readOnly && (
<Notice
important
text={`You don't have permissions to edit ${vpc?.label}. Please contact an account administrator for details.`}
variant="error"
/>
)}
<form onSubmit={form.handleSubmit}>
<TextField
disabled={readOnly}
errorText={form.errors.label}
label="Label"
<form onSubmit={handleSubmit(onSubmit)}>
<Controller
render={({ field, fieldState }) => (
<TextField
disabled={readOnly}
errorText={fieldState.error?.message}
label="Label"
name="label"
onBlur={field.onBlur}
onChange={field.onChange}
value={field.value}
/>
)}
control={control}
name="label"
onChange={(e) => handleFieldChange('label', e.target.value)}
value={form.values.label}
/>
<TextField
disabled={readOnly}
errorText={form.errors.description}
label="Description"
multiline
onChange={(e) => handleFieldChange('description', e.target.value)}
rows={1}
value={form.values.description}
<Controller
render={({ field, fieldState }) => (
<TextField
disabled={readOnly}
errorText={fieldState.error?.message}
label="Description"
multiline
onBlur={field.onBlur}
onChange={field.onChange}
rows={1}
value={field.value}
/>
)}
control={control}
name="description"
/>
{regionsData && (
<RegionSelect
Expand All @@ -129,12 +133,12 @@ export const VPCEditDrawer = (props: Props) => {
<ActionsPanel
primaryButtonProps={{
'data-testid': 'save-button',
disabled: !form.dirty || readOnly,
disabled: !isDirty || readOnly,
label: 'Save',
loading: isPending,
loading: isPending || isSubmitting,
type: 'submit',
}}
secondaryButtonProps={{ label: 'Cancel', onClick: onClose }}
secondaryButtonProps={{ label: 'Cancel', onClick: handleDrawerClose }}
/>
</form>
</Drawer>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/validation": Changed
---

Update VPC label validation schema punctuation, fix label validation regex ([#11393](https://github.com/linode/manager/pull/11393))
12 changes: 6 additions & 6 deletions packages/validation/src/vpcs.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import ipaddr from 'ipaddr.js';
import { array, lazy, object, string } from 'yup';

const LABEL_MESSAGE = 'Label must be between 1 and 64 characters.';
const LABEL_REQUIRED = 'Label is required';
const LABEL_REQUIRED = 'Label is required.';
const LABEL_REQUIREMENTS =
'Must include only ASCII letters, numbers, and dashes';
'Label must include only ASCII letters, numbers, and dashes.';

const labelTestDetails = {
testName: 'no two dashes in a row',
testMessage: 'Must not contain two dashes in a row',
testMessage: 'Label must not contain two dashes in a row.',
};

const IP_EITHER_BOTH_NOT_NEITHER =
Expand Down Expand Up @@ -116,11 +116,11 @@ const labelValidation = string()
)
.min(1, LABEL_MESSAGE)
.max(64, LABEL_MESSAGE)
.matches(/[a-zA-Z0-9-]+/, LABEL_REQUIREMENTS);
.matches(/^[a-zA-Z0-9-]*$/, LABEL_REQUIREMENTS);

export const updateVPCSchema = object({
label: labelValidation.notRequired(),
description: string().notRequired(),
label: labelValidation,
description: string(),
Comment on lines 118 to +123
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • fixes regex so that it checks against all characters, not just the first one, for non letter/number characters (besides dashes)
  • removed the notRequired() since that led to type errors when using the schema: Type 'Maybe<string | undefined>' is not assignable to type 'string | undefined'. Type 'null' is not assignable to type 'string | undefined'. Removing this makes the schema align better with the payload too, so I don't think this will be an issue

});

export const createSubnetSchema = object().shape(
Expand Down
Loading