feat: improve role form validation (#5548)

https://linear.app/unleash/issue/2-1717/improve-the-ux-when-all-the-required-fields-are-not-filled-in Improves role form validation behavior. We may want to look into a form validation library, like [react-hook-form](https://react-hook-form.com/), for future implementations.
2025-07-31 13:47:02 +02:00 · 2023-12-05 12:39:30 +00:00 · 2023-12-05 12:39:30 +00:00 · f348acb3b9
commit f348acb3b9
parent fa9d38fc22
3 changed files with 146 additions and 44 deletions
--- a/frontend/src/component/admin/roles/RoleForm/RoleForm.tsx
+++ b/frontend/src/component/admin/roles/RoleForm/RoleForm.tsx
@ -1,4 +1,4 @@
-import { styled } from '@mui/material';
+import { Alert, styled } from '@mui/material';
 import Input from 'component/common/Input/Input';
 import { PermissionAccordion } from './PermissionAccordion/PermissionAccordion';
 import {
@ -23,6 +23,7 @@ import {
    PROJECT_ROLE_TYPES,
    ROOT_ROLE_TYPE,
 } from '@server/util/constants';
+import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';

 const StyledInputDescription = styled('p')(({ theme }) => ({
    display: 'flex',
@ -38,28 +39,38 @@ const StyledInput = styled(Input)(({ theme }) => ({
    maxWidth: theme.spacing(50),
 }));

+const StyledInputFullWidth = styled(Input)({
+    width: '100%',
+});
+
 interface IRoleFormProps {
    type?: PredefinedRoleType;
    name: string;
-    onSetName: (name: string) => void;
+    setName: React.Dispatch<React.SetStateAction<string>>;
+    validateName: (name: string) => boolean;
    description: string;
    setDescription: React.Dispatch<React.SetStateAction<string>>;
+    validateDescription: (description: string) => boolean;
    checkedPermissions: ICheckedPermissions;
    setCheckedPermissions: React.Dispatch<
        React.SetStateAction<ICheckedPermissions>
    >;
    errors: IRoleFormErrors;
+    showErrors: boolean;
 }

 export const RoleForm = ({
    type = ROOT_ROLE_TYPE,
    name,
-    onSetName,
+    setName,
    description,
    setDescription,
    checkedPermissions,
    setCheckedPermissions,
    errors,
+    showErrors,
+    validateName,
+    validateDescription,
 }: IRoleFormProps) => {
    const { permissions } = usePermissions({
        revalidateIfStale: false,
@ -95,6 +106,10 @@ export const RoleForm = ({
        setCheckedPermissions(newCheckedPermissions);
    };

+    const handleOnBlur = (callback: Function) => {
+        setTimeout(() => callback(), 300);
+    };
+
    return (
        <div>
            <StyledInputDescription>
@ -102,27 +117,34 @@ export const RoleForm = ({
            </StyledInputDescription>
            <StyledInput
                autoFocus
-                label='Role name'
+                label='Role name *'
                error={Boolean(errors.name)}
                errorText={errors.name}
                value={name}
-                onChange={(e) => onSetName(e.target.value)}
+                onChange={(e) => setName(e.target.value)}
+                onBlur={(e) => handleOnBlur(() => validateName(e.target.value))}
                autoComplete='off'
-                required
            />
            <StyledInputDescription>
                What is your new role description?
            </StyledInputDescription>
-            <StyledInput
-                label='Role description'
+            <StyledInputFullWidth
+                label='Role description *'
+                error={Boolean(errors.description)}
+                errorText={errors.description}
                value={description}
                onChange={(e) => setDescription(e.target.value)}
+                onBlur={(e) =>
+                    handleOnBlur(() => validateDescription(e.target.value))
+                }
                autoComplete='off'
-                required
            />
            <StyledInputDescription>
                What is your role allowed to do?
            </StyledInputDescription>
+            <Alert severity='info'>
+                You must select at least one permission.
+            </Alert>
            {categories.map(({ label, type, permissions }) => (
                <PermissionAccordion
                    key={label}
@ -145,6 +167,20 @@ export const RoleForm = ({
                    onCheckAll={() => onCheckAll(permissions)}
                />
            ))}
+            <ConditionallyRender
+                condition={showErrors}
+                show={() => (
+                    <Alert severity='error' icon={false}>
+                        <ul>
+                            {Object.values(errors)
+                                .filter(Boolean)
+                                .map((error) => (
+                                    <li key={error}>{error}</li>
+                                ))}
+                        </ul>
+                    </Alert>
+                )}
+            />
        </div>
    );
 };
--- a/frontend/src/component/admin/roles/RoleForm/useRoleForm.ts
+++ b/frontend/src/component/admin/roles/RoleForm/useRoleForm.ts
@ -7,11 +7,17 @@ import { ROOT_ROLE_TYPE } from '@server/util/constants';

 enum ErrorField {
    NAME = 'name',
+    DESCRIPTION = 'description',
+    PERMISSIONS = 'permissions',
 }

-export interface IRoleFormErrors {
-    [ErrorField.NAME]?: string;
-}
+const DEFAULT_ERRORS = {
+    [ErrorField.NAME]: undefined,
+    [ErrorField.DESCRIPTION]: undefined,
+    [ErrorField.PERMISSIONS]: undefined,
+};
+
+export type IRoleFormErrors = Record<ErrorField, string | undefined>;

 export const useRoleForm = (
    initialName = '',
@ -24,7 +30,8 @@ export const useRoleForm = (
    const [description, setDescription] = useState(initialDescription);
    const [checkedPermissions, setCheckedPermissions] =
        useState<ICheckedPermissions>({});
-    const [errors, setErrors] = useState<IRoleFormErrors>({});
+    const [errors, setErrors] = useState<IRoleFormErrors>(DEFAULT_ERRORS);
+    const [validated, setValidated] = useState(false);

    useEffect(() => {
        setName(initialName);
@ -40,6 +47,28 @@ export const useRoleForm = (
        setCheckedPermissions(newCheckedPermissions);
    }, [initialPermissions.length]);

+    useEffect(() => {
+        if (name !== '') {
+            validateName(name);
+        } else {
+            clearError(ErrorField.NAME);
+        }
+    }, [name]);
+
+    useEffect(() => {
+        if (description !== '') {
+            validateDescription(description);
+        } else {
+            clearError(ErrorField.DESCRIPTION);
+        }
+    }, [description]);
+
+    useEffect(() => {
+        if (validated) {
+            validatePermissions(checkedPermissions);
+        }
+    }, [checkedPermissions]);
+
    const getRolePayload = (type: PredefinedRoleType = ROOT_ROLE_TYPE) => ({
        name,
        description,
@ -70,29 +99,79 @@ export const useRoleForm = (
        setErrors((errors) => ({ ...errors, [field]: error }));
    };

+    const validateName = (name: string) => {
+        if (!isNotEmpty(name)) {
+            setError(ErrorField.NAME, 'Name is required.');
+            return false;
+        }
+
+        if (!isNameUnique(name)) {
+            setError(ErrorField.NAME, 'Name must be unique.');
+            return false;
+        }
+
+        clearError(ErrorField.NAME);
+        return true;
+    };
+
+    const validateDescription = (description: string) => {
+        if (!isNotEmpty(description)) {
+            setError(ErrorField.DESCRIPTION, 'Description is required.');
+            return false;
+        }
+
+        clearError(ErrorField.DESCRIPTION);
+        return true;
+    };
+
+    const validatePermissions = (permissions: ICheckedPermissions) => {
+        if (!hasPermissions(permissions)) {
+            setError(
+                ErrorField.PERMISSIONS,
+                'You must select at least one permission.',
+            );
+            return false;
+        }
+
+        clearError(ErrorField.PERMISSIONS);
+        return true;
+    };
+
+    const validate = () => {
+        const validName = validateName(name);
+        const validDescription = validateDescription(description);
+        const validPermissions = validatePermissions(checkedPermissions);
+
+        setValidated(true);
+
+        return validName && validDescription && validPermissions;
+    };
+
+    const showErrors = validated && Object.values(errors).some(Boolean);
+
    const reload = () => {
        setName(initialName);
        setDescription(initialDescription);
        setCheckedPermissions(
            permissionsToCheckedPermissions(initialPermissions),
        );
+        setValidated(false);
+        setErrors(DEFAULT_ERRORS);
    };

    return {
        name,
-        description,
-        checkedPermissions,
-        errors,
        setName,
+        validateName,
+        description,
        setDescription,
+        validateDescription,
+        checkedPermissions,
        setCheckedPermissions,
        getRolePayload,
-        clearError,
-        setError,
-        isNameUnique,
-        isNotEmpty,
-        hasPermissions,
-        ErrorField,
+        errors,
+        showErrors,
+        validate,
        reload,
    };
 };
--- a/frontend/src/component/admin/roles/RoleModal/RoleModal.tsx
+++ b/frontend/src/component/admin/roles/RoleModal/RoleModal.tsx
@ -48,18 +48,16 @@ export const RoleModal = ({
    const {
        name,
        setName,
+        validateName,
        description,
        setDescription,
+        validateDescription,
        checkedPermissions,
        setCheckedPermissions,
        getRolePayload,
-        isNameUnique,
-        isNotEmpty,
-        hasPermissions,
        errors,
-        setError,
-        clearError,
-        ErrorField,
+        showErrors,
+        validate,
        reload: reloadForm,
    } = useRoleForm(role?.name, role?.description, role?.permissions);
    const { refetch: refetchRoles } = useRoles();
@ -68,11 +66,6 @@ export const RoleModal = ({
    const { uiConfig } = useUiConfig();

    const editing = role !== undefined;
-    const isValid =
-        isNameUnique(name) &&
-        isNotEmpty(name) &&
-        isNotEmpty(description) &&
-        hasPermissions(checkedPermissions);

    const payload = getRolePayload(type);

@ -85,14 +78,6 @@ export const RoleModal = ({
    --data-raw '${JSON.stringify(payload, undefined, 2)}'`;
    };

-    const onSetName = (name: string) => {
-        clearError(ErrorField.NAME);
-        if (!isNameUnique(name)) {
-            setError(ErrorField.NAME, 'A role with that name already exists.');
-        }
-        setName(name);
-    };
-
    const refetch = () => {
        refetchRoles();
        refetchRole();
@ -101,7 +86,7 @@ export const RoleModal = ({
    const onSubmit = async (e: FormEvent<HTMLFormElement>) => {
        e.preventDefault();

-        if (!isValid) return;
+        if (!validate()) return;

        try {
            if (editing) {
@ -151,19 +136,21 @@ export const RoleModal = ({
                    <RoleForm
                        type={type}
                        name={name}
-                        onSetName={onSetName}
+                        setName={setName}
+                        validateName={validateName}
                        description={description}
                        setDescription={setDescription}
+                        validateDescription={validateDescription}
                        checkedPermissions={checkedPermissions}
                        setCheckedPermissions={setCheckedPermissions}
                        errors={errors}
+                        showErrors={showErrors}
                    />
                    <StyledButtonContainer>
                        <Button
                            type='submit'
                            variant='contained'
                            color='primary'
-                            disabled={!isValid}
                        >
                            {editing ? 'Save' : 'Add'} role
                        </Button>