chore: LoginForm a11y improvements (#30337)

RocketChat · Sep 9, 2023 · e9ef5d7 · e9ef5d7
1 parent d37503b
commit e9ef5d7
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 100 deletions.
diff --git a/apps/meteor/tests/e2e/login.spec.ts b/apps/meteor/tests/e2e/login.spec.ts
@@ -12,6 +12,11 @@ test.describe.parallel('Login', () => {
 		await page.goto('/home');
 	});
 
+	test('should not have any accessibility violations', async ({ makeAxeBuilder }) => {
+		const results = await makeAxeBuilder().analyze();
+		expect(results.violations).toEqual([]);
+	})
+
 	test('Login with invalid credentials', async () => {
 		await test.step('expect to have username and password marked as invalid', async () => {
 			await poRegistration.username.type(faker.internet.email());

diff --git a/apps/meteor/tests/e2e/register.spec.ts b/apps/meteor/tests/e2e/register.spec.ts
@@ -128,7 +128,7 @@ test.describe.serial('register', () => {
 			await page.goto('/home');
 			await poRegistration.goToRegister.click();
 
-			const results = await makeAxeBuilder().disableRules(['landmark-one-main', 'region']).analyze();
+			const results = await makeAxeBuilder().analyze();
 
 			expect(results.violations).toEqual([]);
 		});

diff --git a/packages/web-ui-registration/src/LoginForm.tsx b/packages/web-ui-registration/src/LoginForm.tsx
@@ -3,24 +3,47 @@ import { useUniqueId } from '@rocket.chat/fuselage-hooks';
 import { Form, ActionLink } from '@rocket.chat/layout';
 import { useLoginWithPassword, useSetting } from '@rocket.chat/ui-contexts';
 import { useMutation } from '@tanstack/react-query';
-import type { UseMutationResult } from '@tanstack/react-query';
 import type { ReactElement } from 'react';
-import { useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { useForm } from 'react-hook-form';
 import { Trans, useTranslation } from 'react-i18next';
 
 import EmailConfirmationForm from './EmailConfirmationForm';
 import LoginServices from './LoginServices';
 import type { DispatchLoginRouter } from './hooks/useLoginRouter';
 
-export type LoginErrors =
-	| 'error-user-is-not-activated'
-	| 'error-invalid-email'
-	| 'error-login-blocked-for-ip'
-	| 'error-login-blocked-for-user'
-	| 'error-license-user-limit-reached'
-	| 'user-not-found'
-	| 'error-app-user-is-not-allowed-to-login';
+const LOGIN_SUBMIT_ERRORS = {
+	'error-user-is-not-activated': {
+		type: 'warning',
+		i18n: 'registration.page.registration.waitActivationWarning',
+	},
+	'error-app-user-is-not-allowed-to-login': {
+		type: 'danger',
+		i18n: 'registration.page.login.errors.AppUserNotAllowedToLogin',
+	},
+	'user-not-found': {
+		type: 'danger',
+		i18n: 'registration.page.login.errors.wrongCredentials',
+	},
+	'error-login-blocked-for-ip': {
+		type: 'danger',
+		i18n: 'registration.page.login.errors.loginBlockedForIp',
+	},
+	'error-login-blocked-for-user': {
+		type: 'danger',
+		i18n: 'registration.page.login.errors.loginBlockedForUser',
+	},
+	'error-license-user-limit-reached': {
+		type: 'warning',
+		i18n: 'registration.page.login.errors.licenseUserLimitReached',
+	},
+	'error-invalid-email': {
+		type: 'danger',
+		i18n: 'registration.page.login.errors.invalidEmail',
+	},
+} as const;
+
+export type LoginErrors = keyof typeof LOGIN_SUBMIT_ERRORS;
 
 export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRouter }): ReactElement => {
 	const {
@@ -30,12 +53,8 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute
 		clearErrors,
 		getValues,
 		formState: { errors },
-	} = useForm<{
-		email?: string;
-		username: string;
-		password: string;
-	}>({
-		mode: 'onChange',
+	} = useForm<{ username: string; password: string }>({
+		mode: 'onBlur',
 	});
 
 	const { t } = useTranslation();
@@ -48,21 +67,13 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute
 	const usernameOrEmailPlaceholder = String(useSetting('Accounts_EmailOrUsernamePlaceholder'));
 	const passwordPlaceholder = String(useSetting('Accounts_PasswordPlaceholder'));
 
-	const loginMutation: UseMutationResult<
-		void,
-		Error,
-		{
-			username: string;
-			password: string;
-			email?: string;
-		}
-	> = useMutation({
-		mutationFn: (formData) => {
+	const loginMutation = useMutation({
+		mutationFn: (formData: { username: string; password: string }) => {
 			return login(formData.username, formData.password);
 		},
 		onError: (error: any) => {
 			if ([error.error, error.errorType].includes('error-invalid-email')) {
-				setError('email', { type: 'invalid-email', message: t('registration.page.login.errors.invalidEmail') });
+				setError('username', { type: 'invalid-email', message: t('registration.page.login.errors.invalidEmail') });
 			}
 
 			if ('error' in error && error.error !== 403) {
@@ -76,20 +87,32 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute
 		},
 	});
 
-	if (errors.email?.type === 'invalid-email') {
-		return <EmailConfirmationForm onBackToLogin={() => clearErrors('email')} email={getValues('email')} />;
+	const usernameId = useUniqueId();
+	const passwordId = useUniqueId();
+	const loginFormRef = useRef<HTMLElement>(null);
+
+	useEffect(() => {
+		if (loginFormRef.current) {
+			loginFormRef.current.focus();
+		}
+	}, []);
+
+	const renderErrorOnSubmit = (error: LoginErrors) => {
+		const { type, i18n } = LOGIN_SUBMIT_ERRORS[error];
+		return <Callout type={type}>{i18n}</Callout>;
+	};
+
+	if (errors.username?.type === 'invalid-email') {
+		return <EmailConfirmationForm onBackToLogin={() => clearErrors('username')} email={getValues('username')} />;
 	}
 
 	return (
 		<Form
+			tabIndex={-1}
+			ref={loginFormRef}
 			aria-labelledby={formLabelId}
-			onSubmit={handleSubmit(async (data) => {
-				if (loginMutation.isLoading) {
-					return;
-				}
-
-				loginMutation.mutate(data);
-			})}
+			aria-describedby='welcomeTitle'
+			onSubmit={handleSubmit(async (data) => loginMutation.mutate(data))}
 		>
 			<Form.Header>
 				<Form.Title id={formLabelId}>{t('registration.component.login')}</Form.Title>
@@ -99,50 +122,47 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute
 					<Form.Container>
 						<FieldGroup disabled={loginMutation.isLoading}>
 							<Field>
-								<Field.Label htmlFor='username'>{t('registration.component.form.emailOrUsername')}</Field.Label>
+								<Field.Label required htmlFor={usernameId}>
+									{t('registration.component.form.emailOrUsername')}
+								</Field.Label>
 								<Field.Row>
 									<TextInput
 										{...register('username', {
-											required: true,
-											onChange: () => {
-												clearErrors(['username', 'password']);
-											},
+											required: t('registration.component.form.requiredField'),
 										})}
 										placeholder={usernameOrEmailPlaceholder || t('registration.component.form.emailPlaceholder')}
-										error={
-											errors.username?.message ||
-											(errors.username?.type === 'required' ? t('registration.component.form.requiredField') : undefined)
-										}
+										error={errors.username?.message}
 										aria-invalid={errors.username ? 'true' : 'false'}
-										id='username'
+										aria-describedby={`${usernameId}-error`}
+										id={usernameId}
 									/>
 								</Field.Row>
-								{errors.username && errors.username.type === 'required' && (
-									<Field.Error>{t('registration.component.form.requiredField')}</Field.Error>
+								{errors.username && (
+									<Field.Error aria-live='assertive' id={`${usernameId}-error`}>
+										{errors.username.message}
+									</Field.Error>
 								)}
 							</Field>
-
 							<Field>
-								<Field.Label htmlFor='password'>{t('registration.component.form.password')}</Field.Label>
+								<Field.Label required htmlFor={passwordId}>
+									{t('registration.component.form.password')}
+								</Field.Label>
 								<Field.Row>
 									<PasswordInput
 										{...register('password', {
-											required: true,
-											onChange: () => {
-												clearErrors(['username', 'password']);
-											},
+											required: t('registration.component.form.requiredField'),
 										})}
 										placeholder={passwordPlaceholder}
-										error={
-											errors.password?.message ||
-											(errors.password?.type === 'required' ? t('registration.component.form.requiredField') : undefined)
-										}
+										error={errors.password?.message}
 										aria-invalid={errors.password ? 'true' : 'false'}
-										id='password'
+										aria-describedby={`${passwordId}-error`}
+										id={passwordId}
 									/>
 								</Field.Row>
-								{errors.password && errors.password.type === 'required' && (
-									<Field.Error>{t('registration.component.form.requiredField')}</Field.Error>
+								{errors.password && (
+									<Field.Error aria-live='assertive' id={`${passwordId}-error`}>
+										{errors.password.message}
+									</Field.Error>
 								)}
 								{isResetPasswordAllowed && (
 									<Field.Row justifyContent='end'>
@@ -159,31 +179,7 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute
 								)}
 							</Field>
 						</FieldGroup>
-						<FieldGroup disabled={loginMutation.isLoading}>
-							{errorOnSubmit === 'error-user-is-not-activated' && (
-								<Callout type='warning'>{t('registration.page.registration.waitActivationWarning')}</Callout>
-							)}
-
-							{errorOnSubmit === 'error-app-user-is-not-allowed-to-login' && (
-								<Callout type='danger'>{t('registration.page.login.errors.AppUserNotAllowedToLogin')}</Callout>
-							)}
-
-							{errorOnSubmit === 'user-not-found' && (
-								<Callout type='danger'>{t('registration.page.login.errors.wrongCredentials')}</Callout>
-							)}
-
-							{errorOnSubmit === 'error-login-blocked-for-ip' && (
-								<Callout type='danger'>{t('registration.page.login.errors.loginBlockedForIp')}</Callout>
-							)}
-
-							{errorOnSubmit === 'error-login-blocked-for-user' && (
-								<Callout type='danger'>{t('registration.page.login.errors.loginBlockedForUser')}</Callout>
-							)}
-
-							{errorOnSubmit === 'error-license-user-limit-reached' && (
-								<Callout type='warning'>{t('registration.page.login.errors.licenseUserLimitReached')}</Callout>
-							)}
-						</FieldGroup>
+						{errorOnSubmit && <FieldGroup disabled={loginMutation.isLoading}>{renderErrorOnSubmit(errorOnSubmit)}</FieldGroup>}
 					</Form.Container>
 					<Form.Footer>
 						<ButtonGroup stretch>

diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx
@@ -5,7 +5,7 @@ import { Form, ActionLink } from '@rocket.chat/layout';
 import { CustomFieldsForm, PasswordVerifier, useValidatePassword } from '@rocket.chat/ui-client';
 import { useAccountsCustomFields, useSetting, useToastMessageDispatch } from '@rocket.chat/ui-contexts';
 import type { ReactElement } from 'react';
-import { useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { useForm } from 'react-hook-form';
 import { Trans, useTranslation } from 'react-i18next';
 
@@ -63,6 +63,14 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo
 	const { password } = watch();
 	const passwordIsValid = useValidatePassword(password);
 
+	const registerFormRef = useRef<HTMLElement>(null);
+
+	useEffect(() => {
+		if (registerFormRef.current) {
+			registerFormRef.current.focus();
+		}
+	}, []);
+
 	const handleRegister = async ({ password, passwordConfirmation: _, ...formData }: LoginRegisterPayload) => {
 		registerUser.mutate(
 			{ pass: password, ...formData },
@@ -102,7 +110,13 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo
 	}
 
 	return (
-		<Form aria-labelledby={formLabelId} onSubmit={handleSubmit(handleRegister)}>
+		<Form
+			tabIndex={-1}
+			ref={registerFormRef}
+			aria-labelledby={formLabelId}
+			aria-describedby='welcomeTitle'
+			onSubmit={handleSubmit(handleRegister)}
+		>
 			<Form.Header>
 				<Form.Title id={formLabelId}>{t('registration.component.form.createAnAccount')}</Form.Title>
 			</Form.Header>

diff --git a/packages/web-ui-registration/src/RegisterTemplate.tsx b/packages/web-ui-registration/src/RegisterTemplate.tsx
@@ -1,16 +1,18 @@
 import { useSetting } from '@rocket.chat/ui-contexts';
-import type { ReactElement } from 'react';
+import type { AllHTMLAttributes } from 'react';
 
 import HorizontalTemplate from './template/HorizontalTemplate';
 import VerticalTemplate from './template/VerticalTemplate';
 
-const RegisterTemplate = ({ children }: { children: ReactElement }): ReactElement => {
+const RegisterTemplate = ({ children, ...props }: AllHTMLAttributes<HTMLElement>) => {
 	const template = useSetting<'vertical-template' | 'horizontal-template'>('Layout_Login_Template');
 
-	if (template === 'vertical-template') {
-		return <VerticalTemplate>{children}</VerticalTemplate>;
-	}
-	return <HorizontalTemplate>{children}</HorizontalTemplate>;
+	return (
+		<main>
+			{template === 'vertical-template' && <VerticalTemplate {...props} children={children} />}
+			{template === 'horizontal-template' && <HorizontalTemplate {...props} children={children} />}
+		</main>
+	);
 };
 
 export default RegisterTemplate;
diff --git a/packages/web-ui-registration/src/components/RegisterTitle.tsx b/packages/web-ui-registration/src/components/RegisterTitle.tsx
@@ -9,5 +9,10 @@ export const RegisterTitle = (): ReactElement | null => {
 	if (hideTitle) {
 		return null;
 	}
-	return <Trans i18nKey='registration.component.welcome'>Welcome to {siteName} workspace</Trans>;
+
+	return (
+		<span id='welcomeTitle'>
+			<Trans i18nKey='registration.component.welcome'>Welcome to {siteName} workspace</Trans>
+		</span>
+	);
 };
diff --git a/packages/web-ui-registration/src/template/HorizontalTemplate.tsx b/packages/web-ui-registration/src/template/HorizontalTemplate.tsx
@@ -5,7 +5,6 @@ import {
 	HorizontalWizardLayoutContent,
 	HorizontalWizardLayoutTitle,
 	HorizontalWizardLayoutFooter,
-	HorizontalWizardLayoutDescription,
 } from '@rocket.chat/layout';
 import { useSetting, useAssetWithDarkModePath } from '@rocket.chat/ui-contexts';
 import type { ReactElement, ReactNode } from 'react';
@@ -29,9 +28,7 @@ const HorizontalTemplate = ({ children }: { children: ReactNode }): ReactElement
 				<HorizontalWizardLayoutTitle>
 					<RegisterTitle />
 				</HorizontalWizardLayoutTitle>
-				<HorizontalWizardLayoutDescription>
-					<LoginPoweredBy />
-				</HorizontalWizardLayoutDescription>
+				<LoginPoweredBy />
 			</HorizontalWizardLayoutAside>
 			<HorizontalWizardLayoutContent>
 				{children}