Skip to content

Commit

Permalink
Improve form validation
Browse files Browse the repository at this point in the history
* Use `checkValidity()`
* Show error messages in helper text with `onInvalid` callback
* Use `number` input type and override default browser styling
* Replace event target value modifiers with validation constraints
* Remove `onMouseOver` callbacks

Signed-off-by: Marvin A. Ruder <[email protected]>
  • Loading branch information
marvinruder committed May 18, 2024
1 parent c49b15f commit 6aafbcc
Show file tree
Hide file tree
Showing 21 changed files with 539 additions and 245 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const CountryAutocomplete = (props: CountryAutocompleteProps): JSX.Element => {
multiple={false}
value={props.value}
onChange={props.onChange}
onInvalid={props.onInvalid}
filterOptions={(options) => {
const currentInputValue = countryInputValue.trim().toUpperCase();
// Filter the country names by the input value.
Expand All @@ -36,7 +37,16 @@ const CountryAutocomplete = (props: CountryAutocompleteProps): JSX.Element => {
}}
disableClearable
selectOnFocus
renderInput={(params) => <TextField {...params} label="Country" error={props.error} />}
renderInput={(params) => (
<TextField
{...params}
label="Country"
error={props.error}
helperText={props.helperText}
inputRef={props.inputRef}
required={props.required}
/>
)}
/>
);
};
Expand All @@ -50,10 +60,26 @@ interface CountryAutocompleteProps {
* The change handler of the autocomplete.
*/
onChange: (event: React.SyntheticEvent<Element, Event>, value: Country) => void;
/**
* The invalid handler of the Autocomplete component.
*/
onInvalid?: (event: React.SyntheticEvent<Element, Event>) => void;
/**
* Whether the input value is invalid.
*/
error: boolean;
/**
* The helper text of the input element.
*/
helperText: string;
/**
* The ref of the input element.
*/
inputRef?: React.RefObject<HTMLInputElement>;
/**
* Whether the field is required.
*/
required?: boolean;
}

export default CountryAutocomplete;
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const CurrencyAutocomplete = (props: CurrencyAutocompleteProps): JSX.Element =>
multiple={false}
value={props.value}
onChange={props.onChange}
onInvalid={props.onInvalid}
filterOptions={(options) => {
const currentInputValue = currencyInputValue.trim().toUpperCase();
// Filter the currency names by the input value.
Expand All @@ -42,7 +43,16 @@ const CurrencyAutocomplete = (props: CurrencyAutocompleteProps): JSX.Element =>
}}
disableClearable
selectOnFocus
renderInput={(params) => <TextField {...params} label="Currency" error={props.error} />}
renderInput={(params) => (
<TextField
{...params}
label="Currency"
error={props.error}
helperText={props.helperText}
inputRef={props.inputRef}
required={props.required}
/>
)}
/>
);
};
Expand All @@ -56,10 +66,26 @@ interface CurrencyAutocompleteProps {
* The change handler of the autocomplete.
*/
onChange: (event: React.SyntheticEvent<Element, Event>, value: Currency) => void;
/**
* The invalid handler of the Autocomplete component.
*/
onInvalid?: (event: React.SyntheticEvent<Element, Event>) => void;
/**
* Whether the input value is invalid.
*/
error: boolean;
/**
* The helper text of the input element.
*/
helperText: string;
/**
* The ref of the input element.
*/
inputRef?: React.RefObject<HTMLInputElement>;
/**
* Whether the field is required.
*/
required?: boolean;
}

export default CurrencyAutocomplete;
45 changes: 32 additions & 13 deletions packages/frontend/src/components/dialogs/portfolio/AddPortfolio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import LoadingButton from "@mui/lab/LoadingButton";
import { DialogTitle, Typography, DialogContent, Grid, TextField, DialogActions, Button } from "@mui/material";
import type { Currency } from "@rating-tracker/commons";
import { isCurrency, portfoliosAPIPath } from "@rating-tracker/commons";
import { useState } from "react";
import { useRef, useState } from "react";

import { useNotificationContextUpdater } from "../../../contexts/NotificationContext";
import api from "../../../utils/api";
Expand All @@ -18,19 +18,21 @@ export const AddPortfolio = (props: AddPortfolioProps): JSX.Element => {
const [requestInProgress, setRequestInProgress] = useState<boolean>(false);
const [name, setName] = useState<string>("");
const [currency, setCurrency] = useState<Currency>();
const [nameError, setNameError] = useState<boolean>(false); // Error in the name text field.
const [currencyError, setCurrencyError] = useState<boolean>(false); // Error in the currency input field.
const [nameError, setNameError] = useState<string>(""); // Error message for the name text field.
const [currencyError, setCurrencyError] = useState<string>(""); // Error message for the currency input field.
const { setErrorNotificationOrClearSession } = useNotificationContextUpdater();

const nameInputRef = useRef<HTMLInputElement>(null);
const currencyInputRef = useRef<HTMLInputElement>(null);

/**
* Checks for errors in the input fields.
* @returns Whether the input fields are valid.
*/
const validate = (): boolean => {
// The following fields are required.
setNameError(!name);
setCurrencyError(!currency);
return !!name && !!currency;
const isNameValid = nameInputRef.current?.checkValidity();
const isCurrencyValid = currencyInputRef.current?.checkValidity();
return isNameValid && isCurrencyValid;
};

/**
Expand All @@ -55,8 +57,16 @@ export const AddPortfolio = (props: AddPortfolioProps): JSX.Element => {
<Grid container spacing={1} mt={0} maxWidth={600} alignItems="center">
<Grid item xs={12}>
<TextField
onChange={(event) => (setName(event.target.value), setNameError(false))}
error={nameError}
onChange={(event) => {
setName(event.target.value);
// If in error state, check whether error is resolved. If so, clear the error.
if (nameError && event.target.checkValidity()) setNameError("");
}}
onInvalid={(event) => setNameError((event.target as HTMLInputElement).validationMessage)}
error={!!nameError}
helperText={nameError}
inputRef={nameInputRef}
required
label="Portfolio name"
value={name}
placeholder="e.g. Monthly Savings"
Expand All @@ -66,8 +76,18 @@ export const AddPortfolio = (props: AddPortfolioProps): JSX.Element => {
<Grid item xs={12}>
<CurrencyAutocomplete
value={currency ?? null}
onChange={(_, value) => isCurrency(value) && (setCurrency(value), setCurrencyError(false))}
error={currencyError}
onChange={(_, value) => {
if (isCurrency(value)) {
setCurrency(value);
// If in error state, check whether error is resolved. If so, clear the error.
if (currencyError && currencyInputRef.current?.checkValidity()) setCurrencyError("");
}
}}
onInvalid={(event) => setCurrencyError((event.target as HTMLInputElement).validationMessage)}
error={!!currencyError}
helperText={currencyError}
inputRef={currencyInputRef}
required
/>
</Grid>
</Grid>
Expand All @@ -80,8 +100,7 @@ export const AddPortfolio = (props: AddPortfolioProps): JSX.Element => {
loading={requestInProgress}
variant="contained"
onClick={putPortfolio}
onMouseOver={validate} // Validate input fields on hover
disabled={nameError || currencyError}
disabled={!!nameError || !!currencyError}
startIcon={<AddBoxIcon />}
>
Create Portfolio
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from "@mui/material";
import type { Stock, PortfolioSummary, Currency } from "@rating-tracker/commons";
import { stocksAPIPath, portfoliosAPIPath, currencyMinorUnits } from "@rating-tracker/commons";
import { Fragment, useEffect, useState } from "react";
import { Fragment, useEffect, useRef, useState } from "react";

import { useNotificationContextUpdater } from "../../../contexts/NotificationContext";
import api from "../../../utils/api";
Expand All @@ -36,23 +36,32 @@ export const AddStockToPortfolio = (props: AddStockToPortfolioProps): JSX.Elemen
const [portfolioSummaries, setPortfolioSummaries] = useState<PortfolioSummary[]>([]);
const [portfolioSummariesFinal, setPortfolioSummariesFinal] = useState<boolean>(false);
const [amountInput, setAmountInput] = useState<string>("");
const [amountError, setAmountError] = useState<boolean>(false);
const [amountError, setAmountError] = useState<string>(""); // Error message for the amount text field.
const [addPortfolioOpen, setAddPortfolioOpen] = useState<boolean>(false);
const [hoverCurrency, setHoverCurrency] = useState<Currency | "…">("…");
const { setErrorNotificationOrClearSession } = useNotificationContextUpdater();

const amountInputRef = useRef<HTMLInputElement>(null);

const theme = useTheme();

useEffect(() => getPortfolios(), []);

/**
* Checks for errors in the input fields.
* @param id The ID of the portfolio.
* @returns Whether the input fields are valid.
*/
const validate = (): boolean => {
// The following fields are required.
setAmountError(!amountInput || Number.isNaN(+amountInput) || +amountInput <= 0);
return !!amountInput && !Number.isNaN(+amountInput) && +amountInput > 0;
const validate = (id: number): boolean => {
// For currency minor unit validation, we need to set the minimum value of the input field after the touchend event
const currency = portfolioSummaries.find((portfolio) => portfolio.id === id)?.currency;
if (!currency) return false;
if (!amountInputRef?.current) return false;
amountInputRef.current.min = Math.pow(10, -1 * currencyMinorUnits[currency]).toString();
amountInputRef.current.step = Math.pow(10, -1 * currencyMinorUnits[currency]).toString();

const isAmountValid = amountInputRef.current.checkValidity();
return isAmountValid;
};

/**
Expand All @@ -78,7 +87,7 @@ export const AddStockToPortfolio = (props: AddStockToPortfolioProps): JSX.Elemen
* @param id The ID of the portfolio.
*/
const addStockToPortfolio = (id: number) => {
if (!validate()) return;
if (!validate(id)) return;
api
.put(`${portfoliosAPIPath}/${id}${stocksAPIPath}/${props.stock.ticker}`, {
params: { amount: +amountInput },
Expand All @@ -105,14 +114,21 @@ export const AddStockToPortfolio = (props: AddStockToPortfolioProps): JSX.Elemen
}}
inputProps={{
inputMode: "decimal",
pattern: "\\d+(\\.\\d+)?",
step: Math.pow(10, -1 * currencyMinorUnits[hoverCurrency]) || undefined,
type: "number",
// Amount must be divisible by the currency's minor unit
step: Math.pow(10, -1 * currencyMinorUnits[hoverCurrency]),
min: Math.pow(10, -1 * currencyMinorUnits[hoverCurrency]), // Amount must be positive
}}
onChange={(event) => {
setAmountInput(event.target.value.replaceAll(/[^0-9.]/g, ""));
setAmountError(false);
setAmountInput(event.target.value);
// If in error state, check whether error is resolved. If so, clear the error.
if (amountError && event.target.checkValidity()) setAmountError("");
}}
error={amountError}
onInvalid={(event) => setAmountError((event.target as HTMLInputElement).validationMessage)}
error={!!amountError}
helperText={amountError}
inputRef={amountInputRef}
required
label="Amount"
value={amountInput}
autoFocus
Expand All @@ -136,8 +152,7 @@ export const AddStockToPortfolio = (props: AddStockToPortfolioProps): JSX.Elemen
>
<ListItemButton
onClick={() => addStockToPortfolio(portfolioSummary.id)}
onMouseOver={validate}
disabled={portfoliosAlreadyContainingStock.includes(portfolioSummary.id) || amountError}
disabled={portfoliosAlreadyContainingStock.includes(portfolioSummary.id) || !!amountError}
>
<ListItemText
inset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import LoadingButton from "@mui/lab/LoadingButton";
import { DialogTitle, Typography, DialogContent, Grid, TextField, DialogActions, Button } from "@mui/material";
import type { Currency, PortfolioSummary } from "@rating-tracker/commons";
import { isCurrency, portfoliosAPIPath } from "@rating-tracker/commons";
import { useState } from "react";
import { useRef, useState } from "react";

import { useNotificationContextUpdater } from "../../../contexts/NotificationContext";
import api from "../../../utils/api";
Expand All @@ -18,19 +18,21 @@ export const EditPortfolio = (props: EditPortfolioProps): JSX.Element => {
const [requestInProgress, setRequestInProgress] = useState<boolean>(false);
const [name, setName] = useState<string>(props.portfolio?.name);
const [currency, setCurrency] = useState<Currency>(props.portfolio?.currency);
const [nameError, setNameError] = useState<boolean>(false); // Error in the name text field.
const [currencyError, setCurrencyError] = useState<boolean>(false); // Error in the currency input field.
const [nameError, setNameError] = useState<string>(""); // Error message for the name text field.
const [currencyError, setCurrencyError] = useState<string>(""); // Error message for the currency input field.
const { setErrorNotificationOrClearSession } = useNotificationContextUpdater();

const nameInputRef = useRef<HTMLInputElement>(null);
const currencyInputRef = useRef<HTMLInputElement>(null);

/**
* Checks for errors in the input fields.
* @returns Whether the input fields are valid.
*/
const validate = (): boolean => {
// The following fields are required.
setNameError(!name);
setCurrencyError(!currency);
return !!name && !!currency;
const isNameValid = nameInputRef.current?.checkValidity();
const isCurrencyValid = currencyInputRef.current?.checkValidity();
return isNameValid && isCurrencyValid;
};

/**
Expand Down Expand Up @@ -63,9 +65,14 @@ export const EditPortfolio = (props: EditPortfolioProps): JSX.Element => {
<TextField
onChange={(event) => {
setName(event.target.value);
setNameError(false);
// If in error state, check whether error is resolved. If so, clear the error.
if (nameError && event.target.checkValidity()) setNameError("");
}}
error={nameError}
onInvalid={(event) => setNameError((event.target as HTMLInputElement).validationMessage)}
error={!!nameError}
helperText={nameError}
inputRef={nameInputRef}
required
label="Portfolio name"
value={name}
placeholder="e.g. Monthly Savings"
Expand All @@ -75,8 +82,18 @@ export const EditPortfolio = (props: EditPortfolioProps): JSX.Element => {
<Grid item xs={12}>
<CurrencyAutocomplete
value={currency ?? null}
onChange={(_, value) => isCurrency(value) && (setCurrency(value), setCurrencyError(false))}
error={currencyError}
onChange={(_, value) => {
if (isCurrency(value)) {
setCurrency(value);
// If in error state, check whether error is resolved. If so, clear the error.
if (currencyError && currencyInputRef.current?.checkValidity()) setCurrencyError("");
}
}}
onInvalid={(event) => setCurrencyError((event.target as HTMLInputElement).validationMessage)}
error={!!currencyError}
helperText={currencyError}
inputRef={currencyInputRef}
required
/>
</Grid>
</Grid>
Expand All @@ -89,8 +106,7 @@ export const EditPortfolio = (props: EditPortfolioProps): JSX.Element => {
loading={requestInProgress}
variant="contained"
onClick={updatePortfolio}
onMouseOver={validate} // Validate input fields on hover
disabled={nameError || currencyError}
disabled={!!nameError || !!currencyError}
startIcon={<PublishedWithChangesIcon />}
>
Update Portfolio
Expand Down
Loading

0 comments on commit 6aafbcc

Please sign in to comment.