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

08 – Built in React APIs #10

Merged
merged 4 commits into from
Jun 14, 2022
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
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,8 @@
"lint-staged": "^12.4.1",
"prettier": "^2.6.2",
"typescript": "4.6.4"
},
"resolutions": {
"@typescript-eslint/eslint-plugin": "5.27.1"
}
}
115 changes: 87 additions & 28 deletions src/features/login/pages/LoginPage/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { NextPage } from 'next'
import type { FormEvent } from 'react'
import { useState } from 'react'
import { useState, useCallback } from 'react'

import { Button } from '~/features/ui/components/Button'
import { Container } from '~/features/ui/components/Container'
import { Input } from '~/features/ui/components/Input'
import { LayoutExternal } from '~/features/ui/components/LayoutExternal'
Expand All @@ -15,50 +14,110 @@ import {
ErrorMessage,
} from './styled'

const validators = {
email: (value: string) => {
if (typeof value !== 'string') return 'Invalid e-mail value type'
if (!value) return 'E-mail is required'
if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/u.test(value)) return 'Invalid e-mail'
},

password: (value: string) => {
if (typeof value !== 'string') return 'Invalid password value type'
if (!value) return 'Password is required'
},
}

Comment on lines +17 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here you may be confused about a couple of things:

1. Why an object?
We could, in theory, make each of these functions as top-level variables. But, having them declared as properties of an object gives us a couple of benefits:

  1. Simplified naming, as "email" is too generic, and "emailValidator" is not as simple anymore.
  2. validators becomes a map, which allow use to:
    1. Dynamically access a validator (e.g.: validators[inputName](inputValue))
    2. Iterate over validators

2. Why outside the component?
Generally speaking, anything that isn't dependent on props or state should live outside of the render cycle. This ensures, for instance, that these values never change, and thus memoization just becomes easier without the need of useMemo or useCallback all around. Besides that, you get better testability and overall simplification of the context where these assets are used.

3. Why the IFs and the undefined return
It's a common and generally nice pattern for value validators to return either a string explaining the failure, or undefined representing no issue is found. This way, any check on the validation result becomes as simple as this:

if (validator.email(value)) { /* handle error */ }

Choose a reason for hiding this comment

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

This is pretty clever! Damn, I wish I'd come up with this :D... With a combination of if statements it's super readable.

export const LoginPage: NextPage = () => {
const [error, setError] = useState('')
const [email, setEmail] = useState('')
const [password, setPassword] = useState('')

const [emailError, setEmailError] = useState<string | null>(null)
const [passwordError, setPasswordError] = useState<string | null>(null)
const [submitError, setSubmitError] = useState<string | null>(null)

const [isSubmitting, setIsSubmitting] = useState(false)

/**
* Login handler.
*/
const login = useCallback(
(e: FormEvent<HTMLFormElement>) => {
e.preventDefault()

const errors = {
email: validators.email(email),
password: validators.password(password),
}

if (errors.email) {
setEmailError(errors.email)
}

if (errors.password) {
setPasswordError(errors.password)
}

// Only submit in case of no errors.
if (!errors.email && !errors.password) {
setIsSubmitting(true)

const onSubmit = (event: FormEvent<HTMLFormElement>) => {
event.preventDefault()
setTimeout(() => {
// Mocking to represent login submit outcome.
const shouldFail = Math.random() < 0.5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a temporary "hack" for us to easily try our form with multiple possible outcomes: some times it will randomly fail, some times it will succeed. You can easily click a couple of times on submit to see both UI states :)


alert('TODO')
}
if (shouldFail) {
setSubmitError('Something went terribly wrong!')
} else {
alert('Success!')
}

setIsSubmitting(false)
}, 1000)
}
},
[email, password]
)

return (
<LayoutExternal>
<Container>
<FormWrapper>
<Title>Sign in to Eventio.</Title>
{error ? (
<ErrorMessage>{error}</ErrorMessage>

{submitError ? (
<ErrorMessage>{submitError}</ErrorMessage>
) : (
<Description>Enter your details below.</Description>
)}
<form onSubmit={onSubmit}>
<Input label="Email" type="email" name="email" error={error} />

<form onSubmit={login}>
<Input
label="Email"
type="email"
name="email"
value={email}
error={emailError}
onChange={(e) => {
setEmailError(null)
setEmail(e.target.value)
}}
Comment on lines +100 to +103
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we are validating only on submit (which sucks, by the way, but we'll fix in the future), the best possible UX we could figure here is to ensure we clean any validation errors when the user modifies the input.

Choose a reason for hiding this comment

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

I can imagine some kind of "debounce(500ms)" type of utility would be the best UX (users get informed even before they tab out), but I wonder why did you decide not to use onBlur property, isn't that a tiny bit better than validating on submit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct. Only then you’ll also have to validate on submit anyway. And suddenly you are repeating logic all around, doing very hard to read and imperative code.

You can check #11 on a more complete form state handling logic :)

/>

<Input
label="Password"
type="password"
name="password"
error={error}
value={password}
error={passwordError}
onChange={(e) => {
setPasswordError(null)
setPassword(e.target.value)
}}
/>
<p>
<SubmitButton>Sign In</SubmitButton>
</p>

{/*
Created just to showcase CSS animations.
To be removed. Please do not use style attribute.
*/}
<p style={{ marginTop: '1rem' }}>
<Button
type="button"
size="small"
accent="destructive"
onClick={() => setError(Date.now().toString())}
>
Trigger Error
</Button>
<SubmitButton disabled={isSubmitting}>
{isSubmitting ? 'Submitting' : 'Sign In'}
</SubmitButton>
</p>
</form>
</FormWrapper>
Expand Down
13 changes: 7 additions & 6 deletions src/features/ui/components/Input/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import {
InputWrapper,
Label,
LabelText,
ErrorMessage,
PasswordToggle,
StyledInput,
} from './styled'

type Props = InputHTMLAttributes<HTMLInputElement> & {
label: string
error?: string
error?: string | null
}

export const Input: FC<Props> = ({ label, name, type, error, ...rest }) => {
Expand All @@ -21,18 +22,16 @@ export const Input: FC<Props> = ({ label, name, type, error, ...rest }) => {

return (
<InputWrapper>
{/*
By changing the value of key prop, we're making the component
remount, which also triggers an attached animation again.
*/}
<Label hasError={Boolean(error)} key={error}>
<Label hasError={Boolean(error)}>
<StyledInput
placeholder={label}
name={name}
type={inputType}
{...rest}
/>

<LabelText>{label}</LabelText>

{type === 'password' && (
<PasswordToggle
isActive={isPasswordShown}
Expand All @@ -42,6 +41,8 @@ export const Input: FC<Props> = ({ label, name, type, error, ...rest }) => {
<EyeIcon />
</PasswordToggle>
)}

{error ? <ErrorMessage>{error}</ErrorMessage> : null}
</Label>
</InputWrapper>
)
Expand Down
4 changes: 4 additions & 0 deletions src/features/ui/components/Input/styled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export const Label = styled.label<{ hasError?: boolean }>`
`}
`

export const ErrorMessage = styled.span`
color: ${colors.accent.destructive};
`

export const PasswordToggle = styled.button.attrs({ type: 'button' })<{
isActive: boolean
}>`
Expand Down
88 changes: 61 additions & 27 deletions yarn.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.