Bugfix 2FA input UX

frontend/src/auth/SignInPage.js

@@ -125,7 +125,7 @@ export default function SignInPage() {
             </Box>
 
             <Box w={{ md: '100%', lg: '33%' }}>
-              <EmailField name="email" mb={4} />
+              <EmailField inputProps={{ autoFocus: true }} name="email" mb={4} />
 
               <PasswordField name="password" mb={2} />
 

frontend/src/auth/TwoFactorAuthenticatePage.js

@@ -1,10 +1,10 @@
 import React from 'react'
 import { t, Trans } from '@lingui/macro'
 import { useLingui } from '@lingui/react'
-import { Box, Button, Heading, Stack, useToast } from '@chakra-ui/react'
+import { Box, Button, Heading, Stack, Text, useToast } from '@chakra-ui/react'
 import { useHistory, useLocation, useParams } from 'react-router-dom'
 import { useMutation } from '@apollo/client'
-import { Formik } from 'formik'
+import { ErrorMessage, Formik } from 'formik'
 
 import { LoadingMessage } from '../components/LoadingMessage'
 import { AuthenticateField } from '../components/fields/AuthenticateField'
@@ -111,9 +111,11 @@ export default function TwoFactorAuthenticatePage() {
               <Trans>Two Factor Authentication</Trans>
             </Heading>
 
-            <AuthenticateField sendMethod={sendMethod} mb="4" />
-
             <Stack align="center">
+              <AuthenticateField sendMethod={sendMethod} />
+              <Text color="red">
+                <ErrorMessage name="twoFactorCode" />
+              </Text>
               <Button variant="primary" isLoading={isSubmitting} type="submit">
                 <Trans>Submit</Trans>
               </Button>

frontend/src/auth/__tests__/TwoFactorAuthenticatePage.test.js

@@ -28,15 +28,10 @@ describe('<TwoFactorAuthenticatePage />', () => {
   it('renders correctly', async () => {
     const { getByText } = render(
       <MockedProvider>
-        <UserVarProvider
-          userVar={makeVar({ jwt: null, tfaSendMethod: null, userName: null })}
-        >
+        <UserVarProvider userVar={makeVar({ jwt: null, tfaSendMethod: null, userName: null })}>
           <ChakraProvider theme={theme}>
             <I18nProvider i18n={i18n}>
-              <MemoryRouter
-                initialEntries={['/authenticate/phone/authenticate-token-test']}
-                initialIndex={0}
-              >
+              <MemoryRouter initialEntries={['/authenticate/phone/authenticate-token-test']} initialIndex={0}>
                 <Route path="/authenticate/:sendMethod/:authenticateToken">
                   <TwoFactorAuthenticatePage />
                 </Route>
@@ -47,9 +42,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
       </MockedProvider>,
     )
 
-    await waitFor(() =>
-      expect(getByText(/Two Factor Authentication/)).toBeInTheDocument(),
-    )
+    await waitFor(() => expect(getByText(/Two Factor Authentication/)).toBeInTheDocument())
   })
 
   describe('given no input', () => {
@@ -67,12 +60,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
               >
                 <ChakraProvider theme={theme}>
                   <I18nProvider i18n={i18n}>
-                    <MemoryRouter
-                      initialEntries={[
-                        '/authenticate/phone/authenticate-token-test',
-                      ]}
-                      initialIndex={0}
-                    >
+                    <MemoryRouter initialEntries={['/authenticate/phone/authenticate-token-test']} initialIndex={0}>
                       <Route path="/authenticate/:sendMethod/:authenticateToken">
                         <TwoFactorAuthenticatePage />
                       </Route>
@@ -86,9 +74,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
           fireEvent.click(submitButton)
 
           await waitFor(() => {
-            expect(
-              getByText(/Code field must not be empty/i),
-            ).toBeInTheDocument()
+            expect(getByText(/Code field must not be empty/i)).toBeInTheDocument()
           })
         })
       })
@@ -126,7 +112,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         initialIndex: 0,
       })
 
-      const { container, getByRole, queryByText } = render(
+      const { getAllByRole, getByRole, queryByText } = render(
         <MockedProvider mocks={mocks} addTypename={false}>
           <UserVarProvider
             userVar={makeVar({
@@ -148,7 +134,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         </MockedProvider>,
       )
 
-      const twoFactorCode = container.querySelector('#twoFactorCode')
+      const twoFactorCode = getAllByRole('textbox', { name: 'Please enter your pin code' })[0]
       const form = getByRole('form')
 
       fireEvent.change(twoFactorCode, {
@@ -160,9 +146,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
       fireEvent.submit(form)
 
       await waitFor(() => {
-        expect(
-          queryByText(/Unable to sign in to your account, please try again./i),
-        )
+        expect(queryByText(/Unable to sign in to your account, please try again./i))
       })
     })
     it('client-side error', async () => {
@@ -202,7 +186,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         initialIndex: 0,
       })
 
-      const { container, getByRole, queryByText } = render(
+      const { getAllByRole, getByRole, queryByText } = render(
         <MockedProvider mocks={mocks} addTypename={false}>
           <UserVarProvider
             userVar={makeVar({
@@ -224,7 +208,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         </MockedProvider>,
       )
 
-      const twoFactorCode = container.querySelector('#twoFactorCode')
+      const twoFactorCode = getAllByRole('textbox', { name: 'Please enter your pin code' })[0]
       const form = getByRole('form')
 
       fireEvent.change(twoFactorCode, {
@@ -276,7 +260,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         initialIndex: 0,
       })
 
-      const { container, getByRole, queryByText } = render(
+      const { getAllByRole, getByRole, queryByText } = render(
         <MockedProvider mocks={mocks} addTypename={false}>
           <UserVarProvider
             userVar={makeVar({
@@ -298,7 +282,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         </MockedProvider>,
       )
 
-      const twoFactorCode = container.querySelector('#twoFactorCode')
+      const twoFactorCode = getAllByRole('textbox', { name: 'Please enter your pin code' })[0]
       const form = getByRole('form')
 
       fireEvent.change(twoFactorCode, {
@@ -358,7 +342,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         initialIndex: 0,
       })
 
-      const { container, getByRole } = render(
+      const { getAllByRole, getByRole } = render(
         <MockedProvider mocks={mocks} addTypename={false}>
           <UserVarProvider
             userVar={makeVar({
@@ -380,7 +364,7 @@ describe('<TwoFactorAuthenticatePage />', () => {
         </MockedProvider>,
       )
 
-      const twoFactorCode = container.querySelector('#twoFactorCode')
+      const twoFactorCode = getAllByRole('textbox', { name: 'Please enter your pin code' })[0]
       const form = getByRole('form')
 
       fireEvent.change(twoFactorCode, {

frontend/src/components/fields/AuthenticateField.js

@@ -2,17 +2,10 @@ import React from 'react'
 import { func, object, oneOfType, shape, string } from 'prop-types'
 import { t } from '@lingui/macro'
 
-import { FormField } from './FormField'
+import { Field } from 'formik'
+import { FormControl, FormLabel, HStack, PinInput, PinInputField } from '@chakra-ui/react'
 
-import { TwoFactorIcon } from '../../theme/Icons'
-
-function AuthenticateField({
-  name,
-  forwardedRef,
-  sendMethod,
-  inputProps,
-  ...props
-}) {
+function AuthenticateField({ name = 'twoFactorCode', sendMethod }) {
   const codeSendMessage =
     sendMethod.toLowerCase() === 'email'
       ? t`
@@ -26,22 +19,32 @@ function AuthenticateField({
       : ''
 
   return (
-    <FormField
-      name={name}
-      label={
-        codeSendMessage + ' ' + t`Please enter your two factor code below.`
-      }
-      leftElement={<TwoFactorIcon color="gray.300" size="1.25rem" />}
-      placeholder={t`Enter two factor code`}
-      ref={forwardedRef}
-      autoFocus
-      autoComplete="off"
-      inputMode="numeric"
-      w="auto"
-      align="center"
-      inputProps={inputProps}
-      {...props}
-    />
+    <Field name={name}>
+      {({ field, form }) => (
+        <FormControl id={name}>
+          <FormLabel htmlFor={name} fontWeight="bold" textAlign="center">
+            {codeSendMessage + ' ' + t`Please enter your two factor code below.`}
+          </FormLabel>
+          <HStack justify="center">
+            <PinInput
+              id={name}
+              otp
+              type="number"
+              autoFocus
+              name={name}
+              onChange={(val) => form.setFieldValue(field.name, val)}
+            >
+              <PinInputField borderColor="black" isRequired={true} />
+              <PinInputField borderColor="black" isRequired={true} />
+              <PinInputField borderColor="black" isRequired={true} />
+              <PinInputField borderColor="black" isRequired={true} />
+              <PinInputField borderColor="black" isRequired={true} />
+              <PinInputField borderColor="black" isRequired={true} />
+            </PinInput>
+          </HStack>
+        </FormControl>
+      )}
+    </Field>
   )
 }
 

frontend/src/user/EditableUserPhoneNumber.js

@@ -221,7 +221,7 @@ export function EditableUserPhoneNumber({ detailValue, ...props }) {
           }}
         >
           {({ handleSubmit, isSubmitting }) => (
-            <form id="form" onSubmit={handleSubmit}>
+            <form id="form" role="form" onSubmit={handleSubmit}>
               <ModalHeader>
                 <Trans>Verify</Trans>
               </ModalHeader>

frontend/src/user/__tests__/EditableUserPhoneNumber.test.js

@@ -116,9 +116,7 @@ describe('<EditableUserPhoneNumber />', () => {
           fireEvent.click(confirmButton)
 
           await waitFor(() => {
-            expect(
-              getByText(/Phone number field must not be empty/i),
-            ).toBeInTheDocument()
+            expect(getByText(/Phone number field must not be empty/i)).toBeInTheDocument()
           })
         })
       })
@@ -160,14 +158,13 @@ describe('<EditableUserPhoneNumber />', () => {
             {
               request: {
                 query: VERIFY_PHONE_NUMBER,
-                variables: { twoFactorCode: 1234 },
+                variables: { twoFactorCode: 123456 },
               },
               result: {
                 data: {
                   verifyPhoneNumber: {
                     result: {
-                      status:
-                        'You have successfully verified your phone number.',
+                      status: 'You have successfully verified your phone number.',
                       user: {
                         id: '1234asdf',
                         phoneNumber: '+19025555555',
@@ -182,7 +179,7 @@ describe('<EditableUserPhoneNumber />', () => {
               },
             },
           ]
-          const { queryByText, getByText, getByRole, findByRole } = render(
+          const { queryByText, getByText, getByRole, getAllByRole } = render(
             <Suspense fallback="test loading">
               <MockedProvider addTypename={false} mocks={mocks}>
                 <UserVarProvider
@@ -223,30 +220,26 @@ describe('<EditableUserPhoneNumber />', () => {
           userEvent.clear(displayNameInput)
           userEvent.type(displayNameInput, '19025555555')
 
-          // ensure verify phone number modal is not open
-          expect(
-            queryByText(/Please enter your two factor code below/i),
-          ).not.toBeInTheDocument()
-
           const confirmButton = getByRole('button', { name: 'Confirm' })
           fireEvent.click(confirmButton)
 
-          const twoFactorCodeInput = await findByRole('textbox', {
-            name: /Please enter your two factor code below/i,
+          await waitFor(() => {
+            expect(queryByText(/Please enter your two factor code below/i)).toBeInTheDocument()
           })
 
-          userEvent.type(twoFactorCodeInput, '1234')
+          const twoFactorCode = getAllByRole('textbox', { name: 'Please enter your pin code' })[0]
+          const form = getByRole('form')
 
-          const confirmVerifyPhoneNumberButton = getByRole('button', {
-            name: 'Confirm',
+          fireEvent.change(twoFactorCode, {
+            target: {
+              value: '123456',
+            },
           })
 
-          userEvent.click(confirmVerifyPhoneNumberButton)
+          fireEvent.submit(form)
 
           await waitFor(() =>
-            expect(
-              queryByText(/You have successfully updated your phone number\./),
-            ).toBeInTheDocument(),
+            expect(queryByText(/You have successfully updated your phone number\./)).toBeInTheDocument(),
           )
         })
       })