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

[v17] Adds a model-level validation capability to our validation library #49775

Merged
merged 3 commits into from
Dec 5, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ import React, { useState } from 'react';

import Box from 'design/Box';

import { Button } from 'design/Button';

import Validation from 'shared/components/Validation';

import { arrayOf, requiredField } from '../Validation/rules';

import { FieldMultiInput } from './FieldMultiInput';

export default {
Expand All @@ -30,7 +36,21 @@ export function Story() {
const [items, setItems] = useState([]);
return (
<Box width="500px">
<FieldMultiInput label="Some items" value={items} onChange={setItems} />
<Validation>
{({ validator }) => (
<>
<FieldMultiInput
label="Some items"
value={items}
onChange={setItems}
rule={arrayOf(requiredField('required'))}
/>
<Button mt={3} onClick={() => validator.validate()}>
Validate
</Button>
</>
)}
</Validation>
</Box>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,36 @@
import userEvent from '@testing-library/user-event';
import React, { useState } from 'react';

import { render, screen } from 'design/utils/testing';
import { act, render, screen } from 'design/utils/testing';

import Validation, { Validator } from 'shared/components/Validation';

import { arrayOf, requiredField } from '../Validation/rules';

import { FieldMultiInput, FieldMultiInputProps } from './FieldMultiInput';

const TestFieldMultiInput = ({
onChange,
refValidator,
...rest
}: Partial<FieldMultiInputProps>) => {
}: Partial<FieldMultiInputProps> & {
refValidator?: (v: Validator) => void;
}) => {
const [items, setItems] = useState<string[]>([]);
const handleChange = (it: string[]) => {
setItems(it);
onChange?.(it);
};
return <FieldMultiInput value={items} onChange={handleChange} {...rest} />;
return (
<Validation>
{({ validator }) => {
refValidator?.(validator);
return (
<FieldMultiInput value={items} onChange={handleChange} {...rest} />
);
}}
</Validation>
);
};

test('adding, editing, and removing items', async () => {
Expand Down Expand Up @@ -69,3 +85,35 @@ test('keyboard handling', async () => {
await user.keyboard('{Enter}bananas');
expect(onChange).toHaveBeenLastCalledWith(['apples', 'bananas', 'oranges']);
});

test('validation', async () => {
const user = userEvent.setup();
let validator: Validator;
render(
<TestFieldMultiInput
refValidator={v => {
validator = v;
}}
rule={arrayOf(requiredField('required'))}
/>
);

act(() => validator.validate());
expect(validator.state.valid).toBe(true);
expect(screen.getByRole('textbox')).toHaveAccessibleDescription('');

await user.click(screen.getByRole('button', { name: 'Add More' }));
await user.type(screen.getAllByRole('textbox')[1], 'foo');
act(() => validator.validate());
expect(validator.state.valid).toBe(false);
expect(screen.getAllByRole('textbox')[0]).toHaveAccessibleDescription(
'required'
);
expect(screen.getAllByRole('textbox')[1]).toHaveAccessibleDescription('');

await user.type(screen.getAllByRole('textbox')[0], 'foo');
act(() => validator.validate());
expect(validator.state.valid).toBe(true);
expect(screen.getAllByRole('textbox')[0]).toHaveAccessibleDescription('');
expect(screen.getAllByRole('textbox')[1]).toHaveAccessibleDescription('');
});
36 changes: 34 additions & 2 deletions web/packages/shared/components/FieldMultiInput/FieldMultiInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,35 @@ import { ButtonSecondary } from 'design/Button';
import ButtonIcon from 'design/ButtonIcon';
import Flex from 'design/Flex';
import * as Icon from 'design/Icon';
import Input from 'design/Input';
import { useRef } from 'react';
import styled, { useTheme } from 'styled-components';

import {
precomputed,
Rule,
ValidationResult,
} from 'shared/components/Validation/rules';
import { useRule } from 'shared/components/Validation';

import FieldInput from '../FieldInput';

type StringListValidationResult = ValidationResult & {
/**
* A list of validation results, one per list item. Note: results are
* optional just because `useRule` by default returns only
* `ValidationResult`. For the actual validation, it's not optional; if it's
* undefined, or there are fewer results in this list than the list items,
* the corresponding items will be treated as valid.
*/
results?: ValidationResult[];
};

export type FieldMultiInputProps = {
label?: string;
value: string[];
disabled?: boolean;
onChange?(val: string[]): void;
rule?: Rule<string[], StringListValidationResult>;
};

/**
Expand All @@ -45,7 +65,13 @@ export function FieldMultiInput({
value,
disabled,
onChange,
rule = defaultRule,
}: FieldMultiInputProps) {
// It's important to first validate, and then treat an empty array as a
// single-item list with an empty string, since this "synthetic" empty
// string is technically not a part of the model and should not be
// validated.
const validationResult: StringListValidationResult = useRule(rule(value));
if (value.length === 0) {
value = [''];
}
Expand Down Expand Up @@ -90,15 +116,19 @@ export function FieldMultiInput({
// procedure whose complexity would probably outweigh the benefits).
<Flex key={i} alignItems="center" gap={2}>
<Box flex="1">
<Input
<FieldInput
value={val}
rule={precomputed(
validationResult.results?.[i] ?? { valid: true }
)}
ref={toFocus.current === i ? setFocus : null}
onChange={e =>
onChange?.(
value.map((v, j) => (j === i ? e.target.value : v))
)
}
onKeyDown={e => handleKeyDown(i, e)}
mb={0}
/>
</Box>
<ButtonIcon
Expand All @@ -123,6 +153,8 @@ export function FieldMultiInput({
);
}

const defaultRule = () => () => ({ valid: true });

const Fieldset = styled.fieldset`
border: none;
margin: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ export function Default() {
return (
<Validation>
{({ validator }) => {
validator.validate();
// Prevent rendering loop.
if (!validator.state.validating) {
validator.validate();
}
return (
<Flex flexDirection="column">
<FieldSelect
Expand Down
105 changes: 0 additions & 105 deletions web/packages/shared/components/Validation/Validation.jsx

This file was deleted.

Loading
Loading