diff --git a/packages/css/documentation/coding-conventions.md b/packages/css/documentation/coding-conventions.md index 944fd6a1c7..6190d39cf0 100644 --- a/packages/css/documentation/coding-conventions.md +++ b/packages/css/documentation/coding-conventions.md @@ -41,3 +41,9 @@ Still, we define its thickness and offset for the initial state. We use Sass mixins to extract common patterns, especially if they need more than 1 declaration. We collect reset styles in mixins as well. Both the name of the mixins and their documentation help explain the approach. + +## Form validation styling + +We style both the native invalid state (`:invalid`) as the invalid state you can set manually, using `aria-invalid`. +We’re currently not sure how our users will implement forms, which is why both options are supported. +[Native form validation isn’t without its issues](https://adrianroselli.com/2019/02/avoid-default-field-validation.html) though, so we might only support manual validation in the future. diff --git a/packages/css/src/components/search-field/search-field.scss b/packages/css/src/components/search-field/search-field.scss index 8757fc4985..b22c3aae37 100644 --- a/packages/css/src/components/search-field/search-field.scss +++ b/packages/css/src/components/search-field/search-field.scss @@ -46,6 +46,16 @@ } } +.ams-search-field__input:invalid, +.ams-search-field__input[aria-invalid="true"] { + box-shadow: var(--ams-search-field-input-invalid-box-shadow); + + &:hover { + // TODO: this should be the (currently non-existent) dark red hover color + box-shadow: var(--ams-search-field-input-invalid-hover-box-shadow); + } +} + .ams-search-field__input::placeholder { color: var(--ams-search-field-input-placeholder-color); opacity: 100%; // This resets the lower opacity set by Firefox diff --git a/packages/css/src/components/select/select.scss b/packages/css/src/components/select/select.scss index f8de4f618b..70e12f99e5 100644 --- a/packages/css/src/components/select/select.scss +++ b/packages/css/src/components/select/select.scss @@ -37,12 +37,9 @@ @include reset; } +.ams-select:invalid, .ams-select[aria-invalid="true"] { box-shadow: var(--ams-select-invalid-box-shadow); - - &:hover { - box-shadow: var(--ams-select-invalid-hover-box-shadow); - } } .ams-select:disabled { @@ -55,6 +52,11 @@ } } +.ams-select:invalid:hover, +.ams-select[aria-invalid="true"]:hover { + box-shadow: var(--ams-select-invalid-hover-box-shadow); +} + .ams-select__option:disabled { color: var(--ams-select-option-disabled-color); } diff --git a/packages/react/src/Checkbox/Checkbox.test.tsx b/packages/react/src/Checkbox/Checkbox.test.tsx index d6afa3f442..10446f0176 100644 --- a/packages/react/src/Checkbox/Checkbox.test.tsx +++ b/packages/react/src/Checkbox/Checkbox.test.tsx @@ -43,97 +43,97 @@ describe('Checkbox', () => { expect(wrapper).toHaveClass('ams-checkbox') }) - // checked state + describe('Checked state', () => { + it('is not checked by default', () => { + render() - it('is not checked by default', () => { - render() + const input = screen.getByRole('checkbox') - const input = screen.getByRole('checkbox') + expect(input).not.toBeChecked() + }) - expect(input).not.toBeChecked() - }) + it('can have a checked state', () => { + const handleChange = () => {} + render() - it('can have a checked state', () => { - const handleChange = () => {} - render() + const input = screen.getByRole('checkbox') - const input = screen.getByRole('checkbox') - - expect(input).toBeChecked() + expect(input).toBeChecked() + }) }) - // indeterminate state - - it('is not indeterminate by default', () => { - render() + describe('Indeterminate state', () => { + it('is not indeterminate by default', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).not.toBePartiallyChecked() - }) + expect(input).not.toBePartiallyChecked() + }) - it('can have an indeterminate state', () => { - render() + it('can have an indeterminate state', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).toBePartiallyChecked() + expect(input).toBePartiallyChecked() + }) }) - // invalid state + describe('Invalid state', () => { + it('is not invalid by default', () => { + render() - it('is not invalid by default', () => { - render() - - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).not.toBeInvalid() - }) + expect(input).not.toBeInvalid() + }) - it('can have an invalid state', () => { - render() + it('can have an invalid state', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).toHaveAttribute('aria-invalid') - expect(input).toBeInvalid() - }) + expect(input).toHaveAttribute('aria-invalid') + expect(input).toBeInvalid() + }) - it('omits non-essential invalid attributes when not invalid', () => { - render() + it('omits non-essential invalid attributes when not invalid', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).not.toHaveAttribute('aria-invalid') + expect(input).not.toHaveAttribute('aria-invalid') + }) }) - // disabled state - - it('is not disabled by default', () => { - render() + describe('Disabled state', () => { + it('is not disabled by default', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).not.toBeDisabled() - }) + expect(input).not.toBeDisabled() + }) - it('can have a disabled state', () => { - render() + it('can have a disabled state', () => { + render() - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).toBeDisabled() + expect(input).toBeDisabled() + }) }) - // disabled invalid state + describe('Disabled invalid state', () => { + it('can have a disabled invalid state', () => { + render() - it('can have a disabled invalid state', () => { - render() - - const input = screen.getByRole('checkbox') + const input = screen.getByRole('checkbox') - expect(input).toBeDisabled() - expect(input).toBeInvalid() + expect(input).toBeDisabled() + expect(input).toBeInvalid() + }) }) it('can trigger a change event', () => { diff --git a/packages/react/src/Checkbox/Checkbox.tsx b/packages/react/src/Checkbox/Checkbox.tsx index 52872b8246..b553a4eaef 100644 --- a/packages/react/src/Checkbox/Checkbox.tsx +++ b/packages/react/src/Checkbox/Checkbox.tsx @@ -12,7 +12,7 @@ export type CheckboxProps = { invalid?: boolean /** Allows being neither checked nor unchecked. */ indeterminate?: boolean -} & PropsWithChildren> +} & PropsWithChildren, 'aria-invalid' | 'type'>> export const Checkbox = forwardRef( ( @@ -38,11 +38,11 @@ export const Checkbox = forwardRef(