refactor(forms): simplify form handling and button behavior (#10441)

Removed redundant handleSave and handleSubmit props in domain settings. Integrated form submission logic directly into form components, ensuring consistent behavior and reducing complexity. Updated button components to explicitly support the "type" attribute for improved accessibility and functionality. --------- Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
2025-02-25 11:37:36 +01:00
parent bfc92fc00d
commit 7abc67c905
14 changed files with 199 additions and 167 deletions
--- a/packages/twenty-front/src/modules/settings/components/SaveAndCancelButtons/SaveButton.tsx
+++ b/packages/twenty-front/src/modules/settings/components/SaveAndCancelButtons/SaveButton.tsx
@ -15,6 +15,7 @@ export const SaveButton = ({ onSave, disabled }: SaveButtonProps) => {
      accent="blue"
      disabled={disabled}
      onClick={onSave}
+      type="submit"
      Icon={IconDeviceFloppy}
    />
  );
--- a/packages/twenty-front/src/modules/settings/components/SettingsRadioCard.tsx
+++ b/packages/twenty-front/src/modules/settings/components/SettingsRadioCard.tsx
@ -34,16 +34,18 @@ const StyledDescription = styled.div`

 type SettingsRadioCardProps = {
  value: string;
-  handleClick: (value: string) => void;
+  handleSelect: (value: string) => void;
  isSelected: boolean;
  title: string;
  description?: string;
  Icon?: IconComponent;
+  role?: string;
+  ariaChecked?: boolean;
 };

 export const SettingsRadioCard = ({
  value,
-  handleClick,
+  handleSelect,
  title,
  description,
  isSelected,
@ -51,8 +53,10 @@ export const SettingsRadioCard = ({
 }: SettingsRadioCardProps) => {
  const theme = useTheme();

+  const onClick = () => handleSelect(value);
+
  return (
-    <StyledRadioCardContent onClick={() => handleClick(value)}>
+    <StyledRadioCardContent tabIndex={0} onClick={onClick}>
      {Icon && <Icon size={theme.icon.size.xl} color={theme.color.gray50} />}
      <span>
        {title && <StyledTitle>{title}</StyledTitle>}
--- a/packages/twenty-front/src/modules/settings/components/SettingsRadioCardContainer.tsx
+++ b/packages/twenty-front/src/modules/settings/components/SettingsRadioCardContainer.tsx
@ -25,16 +25,18 @@ export const SettingsRadioCardContainer = ({
  onChange,
 }: SettingsRadioCardContainerProps) => {
  return (
-    <StyledRadioCardContainer>
+    <StyledRadioCardContainer role="radiogroup">
      {options.map((option) => (
        <SettingsRadioCard
          key={option.value}
+          role="radio"
          value={option.value}
          isSelected={value === option.value}
-          handleClick={onChange}
+          handleSelect={onChange}
          title={option.title}
          description={option.description}
          Icon={option.Icon}
+          ariaChecked={value === option.value}
        />
      ))}
    </StyledRadioCardContainer>
--- a/packages/twenty-front/src/modules/settings/security/components/SSO/SettingsSSOOIDCForm.tsx
+++ b/packages/twenty-front/src/modules/settings/security/components/SSO/SettingsSSOOIDCForm.tsx
@ -30,6 +30,7 @@ const StyledLinkContainer = styled.div`
 const StyledButtonCopy = styled.div`
  align-items: end;
  display: flex;
+  margin-bottom: ${({ theme }) => theme.spacing(1)};
 `;

 export const SettingsSSOOIDCForm = () => {
@ -70,6 +71,7 @@ export const SettingsSSOOIDCForm = () => {
                  });
                  navigator.clipboard.writeText(authorizedUrl);
                }}
+                type="button"
              />
            </StyledButtonCopy>
          </StyledContainer>
@ -94,6 +96,7 @@ export const SettingsSSOOIDCForm = () => {
                  });
                  navigator.clipboard.writeText(redirectionUrl);
                }}
+                type="button"
              />
            </StyledButtonCopy>
          </StyledContainer>
--- a/packages/twenty-front/src/modules/settings/security/components/SSO/SettingsSSOSAMLForm.tsx
+++ b/packages/twenty-front/src/modules/settings/security/components/SSO/SettingsSSOSAMLForm.tsx
@ -52,6 +52,7 @@ const StyledLinkContainer = styled.div`
 const StyledButtonCopy = styled.div`
  align-items: end;
  display: flex;
+  margin-bottom: ${({ theme }) => theme.spacing(1)};
 `;

 export const SettingsSSOSAMLForm = () => {
@ -136,6 +137,7 @@ export const SettingsSSOSAMLForm = () => {
            Icon={IconUpload}
            onClick={handleUploadFileClick}
            title={t`Upload file`}
+            type="button"
          ></Button>
          {isXMLMetadataValid() && (
            <IconCheck
@ -157,7 +159,8 @@ export const SettingsSSOSAMLForm = () => {
              Icon={IconDownload}
              onClick={downloadMetadata}
              title={t`Download file`}
-            ></Button>
+              type="button"
+            />
          </StyledContainer>
          <HorizontalSeparator text={'Or'} />
          <StyledContainer>
@ -181,6 +184,7 @@ export const SettingsSSOSAMLForm = () => {
                  });
                  navigator.clipboard.writeText(acsUrl);
                }}
+                type="button"
              />
            </StyledButtonCopy>
          </StyledContainer>
@ -205,6 +209,7 @@ export const SettingsSSOSAMLForm = () => {
                  });
                  navigator.clipboard.writeText(entityID);
                }}
+                type="button"
              />
            </StyledButtonCopy>
          </StyledContainer>
--- a/packages/twenty-front/src/pages/settings/security/SettingsSecurityApprovedAccessDomain.tsx
+++ b/packages/twenty-front/src/pages/settings/security/SettingsSecurityApprovedAccessDomain.tsx
@ -23,7 +23,7 @@ export const SettingsSecurityApprovedAccessDomain = () => {

  const [createApprovedAccessDomain] = useCreateApprovedAccessDomainMutation();

-  const formConfig = useForm<{ domain: string; email: string }>({
+  const form = useForm<{ domain: string; email: string }>({
    mode: 'onSubmit',
    resolver: zodResolver(
      z
@ -49,21 +49,18 @@ export const SettingsSecurityApprovedAccessDomain = () => {
    },
  });

-  const domain = formConfig.watch('domain');
+  const domain = form.watch('domain');

  const handleSave = async () => {
    try {
-      if (!formConfig.formState.isValid) {
+      if (!form.formState.isValid || !form.formState.isSubmitting) {
        return;
      }
      createApprovedAccessDomain({
        variables: {
          input: {
-            domain: formConfig.getValues('domain'),
-            email:
-              formConfig.getValues('email') +
-              '@' +
-              formConfig.getValues('domain'),
+            domain: form.getValues('domain'),
+            email: form.getValues('email') + '@' + form.getValues('domain'),
          },
        },
        onCompleted: () => {
@ -86,67 +83,78 @@ export const SettingsSecurityApprovedAccessDomain = () => {
  };

  return (
-    <SubMenuTopBarContainer
-      title="New Approved Access Domain"
-      actionButton={
-        <SaveAndCancelButtons
-          onCancel={() => navigate(SettingsPath.Security)}
-          onSave={formConfig.handleSubmit(handleSave)}
-        />
-      }
-      links={[
-        {
-          children: <Trans>Workspace</Trans>,
-          href: getSettingsPath(SettingsPath.Workspace),
-        },
-        {
-          children: <Trans>Security</Trans>,
-          href: getSettingsPath(SettingsPath.Security),
-        },
-        { children: <Trans>New Approved Access Domain</Trans> },
-      ]}
-    >
-      <SettingsPageContainer>
-        <Section>
-          <H2Title title={t`Domain`} description={t`The name of your Domain`} />
-          <Controller
-            name="domain"
-            control={formConfig.control}
-            render={({ field: { onChange, value }, fieldState: { error } }) => (
-              <TextInput
-                autoComplete="off"
-                value={value}
-                onChange={(domain: string) => {
-                  onChange(domain);
-                }}
-                fullWidth
-                placeholder="yourdomain.com"
-                error={error?.message}
-              />
-            )}
+    <form onSubmit={form.handleSubmit(handleSave)}>
+      <SubMenuTopBarContainer
+        title="New Approved Access Domain"
+        actionButton={
+          <SaveAndCancelButtons
+            onCancel={() => navigate(SettingsPath.Security)}
+            isSaveDisabled={form.formState.isSubmitting}
          />
-        </Section>
-        <Section>
-          <H2Title
-            title={t`Email verification`}
-            description={t`We will send your a link to verify domain ownership`}
-          />
-          <Controller
-            name="email"
-            control={formConfig.control}
-            render={({ field: { onChange, value }, fieldState: { error } }) => (
-              <TextInput
-                autoComplete="off"
-                value={value.split('@')[0]}
-                onChange={onChange}
-                fullWidth
-                error={error?.message}
-              />
-            )}
-          />
-          {domain}
-        </Section>
-      </SettingsPageContainer>
-    </SubMenuTopBarContainer>
+        }
+        links={[
+          {
+            children: <Trans>Workspace</Trans>,
+            href: getSettingsPath(SettingsPath.Workspace),
+          },
+          {
+            children: <Trans>Security</Trans>,
+            href: getSettingsPath(SettingsPath.Security),
+          },
+          { children: <Trans>New Approved Access Domain</Trans> },
+        ]}
+      >
+        <SettingsPageContainer>
+          <Section>
+            <H2Title
+              title={t`Domain`}
+              description={t`The name of your Domain`}
+            />
+            <Controller
+              name="domain"
+              control={form.control}
+              render={({
+                field: { onChange, value },
+                fieldState: { error },
+              }) => (
+                <TextInput
+                  autoComplete="off"
+                  value={value}
+                  onChange={(domain: string) => {
+                    onChange(domain);
+                  }}
+                  fullWidth
+                  placeholder="yourdomain.com"
+                  error={error?.message}
+                />
+              )}
+            />
+          </Section>
+          <Section>
+            <H2Title
+              title={t`Email verification`}
+              description={t`We will send your a link to verify domain ownership`}
+            />
+            <Controller
+              name="email"
+              control={form.control}
+              render={({
+                field: { onChange, value },
+                fieldState: { error },
+              }) => (
+                <TextInput
+                  autoComplete="off"
+                  value={value.split('@')[0]}
+                  onChange={onChange}
+                  fullWidth
+                  error={error?.message}
+                />
+              )}
+            />
+            {domain}
+          </Section>
+        </SettingsPageContainer>
+      </SubMenuTopBarContainer>
+    </form>
  );
 };
--- a/packages/twenty-front/src/pages/settings/security/SettingsSecuritySSOIdentifyProvider.tsx
+++ b/packages/twenty-front/src/pages/settings/security/SettingsSecuritySSOIdentifyProvider.tsx
@ -24,8 +24,8 @@ export const SettingsSecuritySSOIdentifyProvider = () => {
  const { enqueueSnackBar } = useSnackBar();
  const { createSSOIdentityProvider } = useCreateSSOIdentityProvider();

-  const formConfig = useForm<SettingSecurityNewSSOIdentityFormValues>({
-    mode: 'onChange',
+  const form = useForm<SettingSecurityNewSSOIdentityFormValues>({
+    mode: 'onSubmit',
    resolver: zodResolver(SSOIdentitiesProvidersParamsSchema),
    defaultValues: Object.values(sSOIdentityProviderDefaultValues).reduce(
      (acc, fn) => ({ ...acc, ...fn() }),
@ -35,12 +35,12 @@ export const SettingsSecuritySSOIdentifyProvider = () => {

  const handleSave = async () => {
    try {
-      const type = formConfig.getValues('type');
+      const type = form.getValues('type');

      await createSSOIdentityProvider(
        SSOIdentitiesProvidersParamsSchema.parse(
          pick(
-            formConfig.getValues(),
+            form.getValues(),
            Object.keys(sSOIdentityProviderDefaultValues[type]()),
          ),
        ),
@ -55,33 +55,34 @@ export const SettingsSecuritySSOIdentifyProvider = () => {
  };

  return (
-    <SubMenuTopBarContainer
-      title={t`New SSO Configuration`}
-      actionButton={
-        <SaveAndCancelButtons
-          isSaveDisabled={!formConfig.formState.isValid}
-          onCancel={() => navigate(SettingsPath.Security)}
-          onSave={handleSave}
-        />
-      }
-      links={[
-        {
-          children: <Trans>Workspace</Trans>,
-          href: getSettingsPath(SettingsPath.Workspace),
-        },
-        {
-          children: <Trans>Security</Trans>,
-          href: getSettingsPath(SettingsPath.Security),
-        },
-        { children: <Trans>New SSO provider</Trans> },
-      ]}
-    >
+    <form onSubmit={form.handleSubmit(handleSave)}>
      <FormProvider
        // eslint-disable-next-line react/jsx-props-no-spreading
-        {...formConfig}
+        {...form}
      >
-        <SettingsSSOIdentitiesProvidersForm />
+        <SubMenuTopBarContainer
+          title={t`New SSO Configuration`}
+          actionButton={
+            <SaveAndCancelButtons
+              onCancel={() => navigate(SettingsPath.Security)}
+              isSaveDisabled={form.formState.isSubmitting}
+            />
+          }
+          links={[
+            {
+              children: <Trans>Workspace</Trans>,
+              href: getSettingsPath(SettingsPath.Workspace),
+            },
+            {
+              children: <Trans>Security</Trans>,
+              href: getSettingsPath(SettingsPath.Security),
+            },
+            { children: <Trans>New SSO provider</Trans> },
+          ]}
+        >
+          <SettingsSSOIdentitiesProvidersForm />
+        </SubMenuTopBarContainer>
      </FormProvider>
-    </SubMenuTopBarContainer>
+    </form>
  );
 };
--- a/packages/twenty-front/src/pages/settings/workspace/SettingsCustomDomain.tsx
+++ b/packages/twenty-front/src/pages/settings/workspace/SettingsCustomDomain.tsx
@ -23,11 +23,7 @@ const StyledRecordsWrapper = styled.div`
  }
 `;

-export const SettingsCustomDomain = ({
-  handleSave,
-}: {
-  handleSave: () => void;
-}) => {
+export const SettingsCustomDomain = () => {
  const { customDomainRecords, loading } = useRecoilValue(
    customDomainRecordsState,
  );
@ -36,7 +32,7 @@ export const SettingsCustomDomain = ({

  const { t } = useLingui();

-  const { control, handleSubmit } = useFormContext<{
+  const { control } = useFormContext<{
    customDomain: string;
  }>();

@ -57,11 +53,6 @@ export const SettingsCustomDomain = ({
              onChange={onChange}
              placeholder="crm.yourdomain.com"
              error={error?.message}
-              onKeyDown={(e) => {
-                if (e.key === 'Enter') {
-                  handleSubmit(handleSave);
-                }
-              }}
              loading={!!loading}
              fullWidth
            />
--- a/packages/twenty-front/src/pages/settings/workspace/SettingsCustomDomainRecords.tsx
+++ b/packages/twenty-front/src/pages/settings/workspace/SettingsCustomDomainRecords.tsx
@ -7,9 +7,10 @@ import { TableHeader } from '@/ui/layout/table/components/TableHeader';
 import { TableRow } from '@/ui/layout/table/components/TableRow';
 import styled from '@emotion/styled';
 import { useLingui } from '@lingui/react/macro';
-import { Button } from 'twenty-ui';
+import { Button, IconCopy } from 'twenty-ui';
 import { useDebouncedCallback } from 'use-debounce';
 import { CustomDomainValidRecords } from '~/generated/graphql';
+import { useTheme } from '@emotion/react';

 const StyledTable = styled(Table)`
  border-bottom: 1px solid ${({ theme }) => theme.border.color.light};
@ -42,12 +43,15 @@ export const SettingsCustomDomainRecords = ({
 }) => {
  const { enqueueSnackBar } = useSnackBar();

+  const theme = useTheme();
+
  const { t } = useLingui();

  const copyToClipboard = (value: string) => {
    navigator.clipboard.writeText(value);
    enqueueSnackBar(t`Copied to clipboard!`, {
      variant: SnackBarVariant.Success,
+      icon: <IconCopy size={theme.icon.size.md} />,
    });
  };

@ -67,6 +71,7 @@ export const SettingsCustomDomainRecords = ({
              <StyledButton
                title={record.key}
                onClick={() => copyToClipboardDebounced(record.key)}
+                type="button"
              />
            </StyledTableCell>
            <StyledTableCell>
@ -75,12 +80,14 @@ export const SettingsCustomDomainRecords = ({
                onClick={() =>
                  copyToClipboardDebounced(record.type.toUpperCase())
                }
+                type="button"
              />
            </StyledTableCell>
            <StyledTableCell>
              <StyledButton
                title={record.value}
                onClick={() => copyToClipboardDebounced(record.value)}
+                type="button"
              />
            </StyledTableCell>
          </TableRow>
--- a/packages/twenty-front/src/pages/settings/workspace/SettingsDomain.tsx
+++ b/packages/twenty-front/src/pages/settings/workspace/SettingsDomain.tsx
@ -107,6 +107,9 @@ export const SettingsDomain = () => {
          customDomain:
            customDomain && customDomain.length > 0 ? customDomain : null,
        });
+        enqueueSnackBar(t`Custom domain updated`, {
+          variant: SnackBarVariant.Success,
+        });
      },
      onError: (error) => {
        if (
@ -161,6 +164,10 @@ export const SettingsDomain = () => {
          subdomain,
        });

+        enqueueSnackBar(t`Subdomain updated`, {
+          variant: SnackBarVariant.Success,
+        });
+
        redirectToWorkspaceDomain(currentUrl.toString());
      },
    });
@ -169,12 +176,6 @@ export const SettingsDomain = () => {
  const handleSave = async () => {
    const values = form.getValues();

-    if (!values || !form.formState.isValid || !currentWorkspace) {
-      return enqueueSnackBar(t`Invalid form values`, {
-        variant: SnackBarVariant.Error,
-      });
-    }
-
    if (
      subdomainValue === currentWorkspace?.subdomain &&
      customDomainValue === currentWorkspace?.customDomain
@ -184,6 +185,12 @@ export const SettingsDomain = () => {
      });
    }

+    if (!values || !currentWorkspace) {
+      return enqueueSnackBar(t`Invalid form values`, {
+        variant: SnackBarVariant.Error,
+      });
+    }
+
    if (
      isDefined(values.subdomain) &&
      values.subdomain !== currentWorkspace.subdomain
@ -197,38 +204,40 @@ export const SettingsDomain = () => {
  };

  return (
-    <SubMenuTopBarContainer
-      title={t`Domain`}
-      links={[
-        {
-          children: <Trans>Workspace</Trans>,
-          href: getSettingsPath(SettingsPath.Workspace),
-        },
-        {
-          children: <Trans>General</Trans>,
-          href: getSettingsPath(SettingsPath.Workspace),
-        },
-        { children: <Trans>Domain</Trans> },
-      ]}
-      actionButton={
-        <SaveAndCancelButtons
-          onCancel={() => navigate(SettingsPath.Workspace)}
-          onSave={form.handleSubmit(handleSave)}
-        />
-      }
-    >
-      <SettingsPageContainer>
-        {/* eslint-disable-next-line react/jsx-props-no-spreading */}
-        <FormProvider {...form}>
-          <SettingsSubdomain handleSave={handleSave} />
-          {isCustomDomainEnabled && (
-            <>
-              <SettingsCustomDomainEffect />
-              <SettingsCustomDomain handleSave={handleSave} />
-            </>
-          )}
-        </FormProvider>
-      </SettingsPageContainer>
-    </SubMenuTopBarContainer>
+    <form onSubmit={form.handleSubmit(handleSave)}>
+      {/* eslint-disable-next-line react/jsx-props-no-spreading */}
+      <FormProvider {...form}>
+        <SubMenuTopBarContainer
+          title={t`Domain`}
+          links={[
+            {
+              children: <Trans>Workspace</Trans>,
+              href: getSettingsPath(SettingsPath.Workspace),
+            },
+            {
+              children: <Trans>General</Trans>,
+              href: getSettingsPath(SettingsPath.Workspace),
+            },
+            { children: <Trans>Domain</Trans> },
+          ]}
+          actionButton={
+            <SaveAndCancelButtons
+              onCancel={() => navigate(SettingsPath.Workspace)}
+              isSaveDisabled={form.formState.isSubmitting}
+            />
+          }
+        >
+          <SettingsPageContainer>
+            <SettingsSubdomain />
+            {isCustomDomainEnabled && (
+              <>
+                <SettingsCustomDomainEffect />
+                <SettingsCustomDomain />
+              </>
+            )}
+          </SettingsPageContainer>
+        </SubMenuTopBarContainer>
+      </FormProvider>
+    </form>
  );
 };
--- a/packages/twenty-front/src/pages/settings/workspace/SettingsSubdomain.tsx
+++ b/packages/twenty-front/src/pages/settings/workspace/SettingsSubdomain.tsx
@ -23,17 +23,13 @@ const StyledDomain = styled.h2`
  white-space: nowrap;
 `;

-export const SettingsSubdomain = ({
-  handleSave,
-}: {
-  handleSave: () => void;
-}) => {
+export const SettingsSubdomain = () => {
  const domainConfiguration = useRecoilValue(domainConfigurationState);
  const { t } = useLingui();

  const currentWorkspace = useRecoilValue(currentWorkspaceState);

-  const { control, handleSubmit } = useFormContext<{
+  const { control } = useFormContext<{
    subdomain: string;
  }>();

@ -56,11 +52,6 @@ export const SettingsSubdomain = ({
                error={error?.message}
                disabled={!!currentWorkspace?.customDomain}
                fullWidth
-                onKeyDown={(e) => {
-                  if (e.key === 'Enter') {
-                    handleSubmit(handleSave);
-                  }
-                }}
              />
              {isDefined(domainConfiguration.frontDomain) && (
                <StyledDomain>
--- a/packages/twenty-ui/src/input/button/components/Button.tsx
+++ b/packages/twenty-ui/src/input/button/components/Button.tsx
@ -408,18 +408,21 @@ export const Button = ({
  soon = false,
  disabled = false,
  justify = 'flex-start',
-  focus = false,
+  focus: propFocus = false,
  onClick,
  to,
  target,
  dataTestId,
  hotkeys,
  ariaLabel,
+  type,
 }: ButtonProps) => {
  const theme = useTheme();

  const isMobile = useIsMobile();

+  const [isFocused, setIsFocused] = React.useState(propFocus);
+
  return (
    <StyledButton
      fullWidth={fullWidth}
@ -428,7 +431,7 @@ export const Button = ({
      size={size}
      position={position}
      disabled={soon || disabled}
-      focus={focus}
+      focus={isFocused}
      justify={justify}
      accent={accent}
      className={className}
@ -438,6 +441,9 @@ export const Button = ({
      target={target}
      data-testid={dataTestId}
      aria-label={ariaLabel}
+      type={type}
+      onFocus={() => setIsFocused(true)}
+      onBlur={() => setIsFocused(false)}
    >
      {Icon && <Icon size={theme.icon.size.sm} />}
      {title}
--- a/packages/twenty-ui/src/input/button/components/LightButton.tsx
+++ b/packages/twenty-ui/src/input/button/components/LightButton.tsx
@ -14,6 +14,7 @@ export type LightButtonProps = {
  disabled?: boolean;
  focus?: boolean;
  onClick?: (event: MouseEvent<HTMLButtonElement>) => void;
+  type?: React.ComponentProps<'button'>['type'];
 };

 const StyledButton = styled.button<
@ -82,6 +83,7 @@ export const LightButton = ({
  accent = 'secondary',
  disabled = false,
  focus = false,
+  type = 'button',
  onClick,
 }: LightButtonProps) => {
  const theme = useTheme();
@ -91,6 +93,7 @@ export const LightButton = ({
      onClick={onClick}
      disabled={disabled}
      focus={focus && !disabled}
+      type={type}
      accent={accent}
      className={className}
      active={active}
--- a/packages/twenty-ui/src/input/components/Radio.tsx
+++ b/packages/twenty-ui/src/input/components/Radio.tsx
@ -141,6 +141,7 @@ export const Radio = ({
        id={optionId}
        name={name}
        data-testid="input-radio"
+        tabIndex={-1}
        checked={checked}
        value={value || label}
        radio-size={size}