Skip to content

Commit

Permalink
[#271] improves form error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
GentlemanHal committed Oct 26, 2020
1 parent d89239f commit fbe3c0f
Show file tree
Hide file tree
Showing 23 changed files with 188 additions and 125 deletions.
6 changes: 3 additions & 3 deletions src/client/backup/local/Export.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import {toExportableConfigurationJson} from '../../configuration/Configuration'
export function Export(): ReactElement {
const configuration = useSelector(toExportableConfigurationJson)

const [messages, setMessages] = useState<ReadonlyArray<string>>([])
const [messages, setMessages] = useState('')
const [messageType, setMessageType] = useState(MessagesType.INFO)

const copySuccess = useCallback(() => {
setMessageType(MessagesType.INFO)
setMessages(['Successfully copied to clipboard'])
setMessages('Successfully copied to clipboard')
}, [])
const copyError = useCallback(() => {
setMessageType(MessagesType.ERROR)
setMessages(['Unfortunately your browser doesn\'t support automatically copying to clipboard, please manually copy'])
setMessages('Unfortunately your browser doesn\'t support automatically copying to clipboard, please manually copy')
}, [])

useClipboard('#copy-to-clipboard', copySuccess, copyError)
Expand Down
39 changes: 23 additions & 16 deletions src/client/backup/remote/AddBackup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ import {PrimaryButton} from '../../common/forms/Button'
import {iPlus} from '../../common/fonts/Icons'
import {DropDown} from '../../common/forms/DropDown'
import {useDispatch} from 'react-redux'
import {Messages, MessagesType} from '../../common/Messages'
import {errorMessage, isBlank} from '../../common/Utils'
import {DEFAULT_GITHUB_URL, DEFAULT_GITLAB_URL, RemoteLocationOptions} from './RemoteLocationOptions'
import {addBackupGitHubLab, addBackupCustomServer} from './RemoteLocationActionCreators'
import {addBackupCustomServer, addBackupGitHubLab} from './RemoteLocationActionCreators'
import {Input} from '../../common/forms/Input'
import {Password} from '../../common/forms/Password'
import {send} from '../../gateways/Gateway'
import {encrypt, EncryptResponse} from '../../gateways/SecurityGateway'
import styles from './add-backup.scss'
import {isHttp} from '../../domain/Url'
import {RemoteLocationLogo} from './RemoteLocationLogo'
import {firstError, FormErrors} from '../../common/forms/Validation'
import {ErrorMessages} from '../../common/Messages'

type Fields = 'url' | 'accessToken'

export function AddBackup(): ReactElement {
const dispatch = useDispatch()
Expand All @@ -25,28 +28,29 @@ export function AddBackup(): ReactElement {
const [id, setId] = useState('')
const [description, setDescription] = useState('Nevergreen configuration backup')
const [accessToken, setAccessToken] = useState('')
const [errors, setErrors] = useState<ReadonlyArray<string>>([])
const [validationErrors, setValidationErrors] = useState<Readonly<FormErrors<Fields>>>([])
const [submissionError, setSubmissionError] = useState('')
const [adding, setAdding] = useState(false)

const doAddBackup = async (evt: FormEvent) => {
evt.preventDefault()

const validationErrors = []
const validationErrors: FormErrors<Fields> = []

if (isBlank(url)) {
validationErrors.push('Please enter the URL')
validationErrors.push({field: 'url', message: 'Please enter the URL'})
} else if (!isHttp(url)) {
validationErrors.push('Only http and https URLs are supported')
validationErrors.push({field: 'url', message: 'Only http and https URLs are supported'})
}

if (where === RemoteLocationOptions.GitLab || where === RemoteLocationOptions.GitHub) {
if (isBlank(accessToken)) {
validationErrors.push('Please enter an access token')
validationErrors.push({field: 'accessToken', message: 'Please enter an access token'})
}
}

if (!isEmpty(validationErrors)) {
setErrors(validationErrors)
setValidationErrors(validationErrors)
return
}

Expand All @@ -57,7 +61,8 @@ export function AddBackup(): ReactElement {
setAdding(false)
dispatch(addBackupGitHubLab(where, id, url, description, encryptedAccessToken))
} catch (e) {
setErrors([errorMessage(e)])
setAdding(false)
setSubmissionError(errorMessage(e))
}
} else {
dispatch(addBackupCustomServer(url))
Expand All @@ -72,7 +77,7 @@ export function AddBackup(): ReactElement {
} else if (updatedWhere === RemoteLocationOptions.GitLab) {
setUrl(DEFAULT_GITLAB_URL)
}
setErrors([])
setValidationErrors([])
setWhere(updatedWhere)
}

Expand All @@ -97,29 +102,31 @@ export function AddBackup(): ReactElement {
</div>
<Input value={url}
onChange={({target}) => {
setErrors([])
setValidationErrors([])
setUrl(target.value)
}}
autoComplete='url'
error={firstError('url', validationErrors)}
disabled={adding}>
<span className={styles.label}>URL</span>
</Input>
<Input className={cn(styles.id, {[styles.gitHubLabOnly]: isCustomServer})}
value={id}
onChange={({target}) => {
setErrors([])
setValidationErrors([])
setId(target.value)
}}
disabled={adding || isCustomServer}
aria-hidden={isCustomServer}>
<span className={styles.label}>ID</span>
</Input>
<Password className={cn(styles.accessToken, {[styles.gitHubLabOnly]: isCustomServer})}
<Password className={cn({[styles.gitHubLabOnly]: isCustomServer})}
value={accessToken}
onChange={({target}) => {
setErrors([])
setValidationErrors([])
setAccessToken(target.value)
}}
error={firstError('accessToken', validationErrors)}
disabled={adding || isCustomServer}
aria-hidden={isCustomServer}
data-locator='access-token'>
Expand All @@ -128,21 +135,21 @@ export function AddBackup(): ReactElement {
<Input className={cn({[styles.gitHubLabOnly]: isCustomServer})}
value={description}
onChange={({target}) => {
setErrors([])
setValidationErrors([])
setDescription(target.value)
}}
maxLength={256}
disabled={adding || isCustomServer}
aria-hidden={isCustomServer}>
<span className={styles.label}>Description</span>
</Input>
<Messages type={MessagesType.ERROR} messages={errors}/>
<PrimaryButton className={styles.add}
icon={iPlus}
type='submit'
disabled={adding}>
Add location
</PrimaryButton>
<ErrorMessages messages={submissionError}/>
</form>
)
}
6 changes: 3 additions & 3 deletions src/client/backup/remote/RemoteLocation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import {
fetchConfigurationNew,
ImportResponse
} from '../../gateways/BackupGateway'
import {toExportableConfigurationJson, toConfiguration} from '../../configuration/Configuration'
import {toConfiguration, toExportableConfigurationJson} from '../../configuration/Configuration'
import {errorMessage, isBlank, isNotBlank} from '../../common/Utils'
import {Messages, MessagesType} from '../../common/Messages'
import {ErrorMessages} from '../../common/Messages'
import {isRight} from 'fp-ts/Either'
import {configurationImported} from '../BackupActionCreators'

Expand Down Expand Up @@ -107,7 +107,7 @@ export function RemoteLocation({location}: RemoteLocationProps): ReactElement {
disabled={!loaded}>
Import now
</SecondaryButton>
<Messages type={MessagesType.ERROR} messages={errors}/>
<ErrorMessages messages={errors}/>
</div>
</li>
)
Expand Down
6 changes: 1 addition & 5 deletions src/client/backup/remote/add-backup.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@
@include fixed-width(38em);
}

.accessToken {
@include fixed-width(38em);
}

.add {
margin: $margin-top * 2 0 0 0;
margin: $margin-top 0 0 0;
}
30 changes: 22 additions & 8 deletions src/client/common/Messages.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {ReactElement} from 'react'
import cn from 'classnames'
import {isEmpty} from 'lodash'
import {isEmpty, isString} from 'lodash'
import styles from './messages.scss'

export enum MessagesType {
Expand All @@ -9,13 +9,14 @@ export enum MessagesType {
ERROR = 'error'
}

export interface MessagesProps {
export type MessagesProps = {
readonly id?: string;
readonly type: MessagesType;
readonly messages: ReadonlyArray<string>;
readonly messages: ReadonlyArray<string> | string;
readonly className?: string;
}

export function Messages({messages, type, className}: MessagesProps): ReactElement | null {
export function Messages({messages, type, className, id}: MessagesProps): ReactElement | null {
if (isEmpty(messages)) {
return null
}
Expand All @@ -26,12 +27,25 @@ export function Messages({messages, type, className}: MessagesProps): ReactEleme
return (
<ul className={classes}
data-locator={`${type}-messages`}
aria-live={isError ? 'assertive' : 'polite'}>
aria-live={isError ? 'assertive' : 'polite'}
id={id}>
{
messages.map((msg) => {
return <li key={msg} className={styles.message}>{msg}</li>
})
isString(messages)
? <li className={styles.message}>{messages}</li>
: messages.map((msg) => <li key={msg} className={styles.message}>{msg}</li>)
}
</ul>
)
}

export function ErrorMessages(props: Omit<MessagesProps, 'type'>): ReactElement | null {
return <Messages type={MessagesType.ERROR} {...props}/>
}

export function WarningMessages(props: Omit<MessagesProps, 'type'>): ReactElement | null {
return <Messages type={MessagesType.WARNING} {...props}/>
}

export function InfoMessages(props: Omit<MessagesProps, 'type'>): ReactElement | null {
return <Messages type={MessagesType.INFO} {...props}/>
}
10 changes: 5 additions & 5 deletions src/client/common/forms/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ type CheckboxProps = {
readonly children: ReactNode;
readonly onToggle: (checked: boolean) => void;
readonly className?: string;
} & DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>
} & Omit<DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>, 'type'>

export function Checkbox({children, onToggle, className, ...inputProps}: CheckboxProps): ReactElement {
export function Checkbox({children, onToggle, className, id, ...inputProps}: CheckboxProps): ReactElement {
const classes = classNames(styles.container, className)
const id = uniqueId('c')
const actualId = id ?? uniqueId('c')

return (
<div className={classes}>
<div className={styles.checkbox}>
<input id={id}
<input id={actualId}
className={styles.input}
type='checkbox'
onChange={(evt) => onToggle(evt.target.checked)}
{...inputProps}/>
<label htmlFor={id} className={styles.children}>{children}</label>
<label htmlFor={actualId} className={styles.children}>{children}</label>
</div>
</div>
)
Expand Down
14 changes: 7 additions & 7 deletions src/client/common/forms/DropDown.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {DetailedHTMLProps, ReactElement, ReactNode, SelectHTMLAttributes} from 'react'
import classNames from 'classnames'
import cn from 'classnames'
import {uniqueId} from 'lodash'
import styles from './drop-down.scss'
import formStyles from './forms.scss'
Expand All @@ -14,21 +14,21 @@ type DropDownProps = {
readonly disabled?: boolean;
} & DetailedHTMLProps<SelectHTMLAttributes<HTMLSelectElement>, HTMLSelectElement>

export function DropDown({className, children, options, disabled, ...inputProps}: DropDownProps): ReactElement {
const labelClasses = classNames(formStyles.inputContainer, className)
const id = uniqueId('i')
export function DropDown({className, children, options, disabled, id, ...inputProps}: DropDownProps): ReactElement {
const labelClasses = cn(formStyles.inputContainer, className)
const actualId = id ?? uniqueId('i')

return (
<label className={labelClasses} htmlFor={id}>
<label className={labelClasses} htmlFor={actualId}>
<span className={formStyles.inputLabel}>{children}</span>
<select className={styles.input} {...inputProps} id={id}>
<select className={styles.input} {...inputProps} id={actualId}>
{
options.map((op) => {
return <option key={op.value} value={op.value}>{op.display}</option>
})
}
</select>
<span className={classNames(styles.arrow, {[styles.disabled]: disabled})} aria-hidden/>
<span className={cn(styles.arrow, {[styles.disabled]: disabled})} aria-hidden/>
</label>
)
}
38 changes: 26 additions & 12 deletions src/client/common/forms/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import styles from './input.scss'
import formStyles from './forms.scss'
import {InputButton} from './Button'
import {iLock} from '../fonts/Icons'
import {ErrorMessages} from '../Messages'
import {isBlank, isNotBlank} from '../Utils'

export type InputProps = {
readonly children: ReactNode;
Expand All @@ -22,16 +24,18 @@ export type InputProps = {
readonly readOnly?: boolean;
readonly focus?: boolean;
readonly button?: ReactElement;
readonly error?: string;
} & DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>

export function Input({children, onEnter, className, readOnly, focus, button, ...inputProps}: InputProps): ReactElement {
export function Input({children, onEnter, className, readOnly, focus, button, error = '', id, ...inputProps}: InputProps): ReactElement {
const inputNode = useRef<HTMLInputElement>(null)

useEffect(() => {
if (focus && inputNode.current) {
const shouldFocus = isNotBlank(error) || focus
if (shouldFocus && inputNode.current) {
inputNode.current.focus()
}
}, [focus])
}, [focus, error])

const moveCaretToEnd = (evt: FocusEvent<HTMLInputElement>) => {
const val = evt.target.value
Expand All @@ -45,39 +49,49 @@ export function Input({children, onEnter, className, readOnly, focus, button, ..
}
}

const actualId = id ?? uniqueId('i')
const errorId = uniqueId('e')

const labelClasses = classNames(formStyles.inputContainer, className)
const wrapperClasses = classNames(styles.wrapper, {
[styles.error]: !isBlank(error)
})
const inputClasses = classNames(styles.input, {
[styles.hasButton]: button
[styles.hasButton]: button || readOnly,
[styles.hasError]: !isBlank(error)
})

const id = uniqueId('i')

return (
<label className={labelClasses} htmlFor={id}>
<label className={labelClasses} htmlFor={actualId}>
<span className={formStyles.inputLabel}>{children}</span>
<span className={styles.wrapper}>
<span className={wrapperClasses}>
<input className={inputClasses}
onKeyPress={(evt) => onKeyPress(evt)}
spellCheck={false}
autoComplete='off'
readOnly={readOnly}
type='text'
id={id}
id={actualId}
{...inputProps}
ref={inputNode}
onFocus={moveCaretToEnd}/>
onFocus={moveCaretToEnd}
aria-describedby={errorId}/>
{
readOnly && (
<InputButton disabled
icon={iLock}>
icon={iLock}
aria-hidden>
read only
</InputButton>
)
}
{
!readOnly && button
}
</span>
<ErrorMessages id={errorId}
className={styles.errorMessage}
messages={error}/>
</span>
</label>
)
}
Loading

0 comments on commit fbe3c0f

Please sign in to comment.