diff --git a/packages/dnb-eufemia/src/extensions/forms/Field/String/__tests__/String.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Field/String/__tests__/String.test.tsx index fb88a5aad8f..a2cac368325 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Field/String/__tests__/String.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Field/String/__tests__/String.test.tsx @@ -186,51 +186,25 @@ describe('Field.String', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) - it('should show error message if changing the value to an invalid one', async () => { + it('should show error only after changing when the value is still invalid', async () => { render( ) - const input = document.querySelector('input') - await userEvent.type(input, 'd') - act(() => { - input.blur() - }) - expect(screen.getByRole('alert')).toBeInTheDocument() - }) - - it('should hide then error once you start typing, even if the current value is invalid', async () => { - render( - - ) - const input = document.querySelector('input') - await userEvent.type(input, 'd') - act(() => { - input.blur() - }) - expect(screen.getByRole('alert')).toBeInTheDocument() - await userEvent.type(input, 'e') + // Do not show error initially when validateInitially is not enabled, to avoid initial error messages all over empty forms expect(screen.queryByRole('alert')).not.toBeInTheDocument() - }) - - it('does not show an error for valid values', async () => { - render( - - ) const input = document.querySelector('input') + // Errors should be hidden while typing (field is in focus) await userEvent.type(input, 'd') + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + // Error should be visible after blurring the field act(() => { input.blur() }) expect(screen.getByRole('alert')).toBeInTheDocument() + // But remain gone when it becomes valid before blurring await userEvent.type(input, 'ef') act(() => { input.blur() @@ -272,6 +246,10 @@ describe('Field.String', () => { }) }) + // Assumption: When error messages should be shown or hidden is controlled by the same logic as + // the json schema tests above, so it should be enough to test that each validation prop + // lead to error correctly based on the given value. + describe('validation based on required-prop', () => { it('should show error for empty value', async () => { render() @@ -295,10 +273,6 @@ describe('Field.String', () => { }) describe('validation based on minLength-prop', () => { - // Assumption: When error messages should be shown or hidden is controlled by the same logic as - // the json schema tests above, so it should be enough to test that each validation prop - // lead to error correctly based on the given value. - it('should show error for invalid value', async () => { render() const input = document.querySelector('input') diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useDataValue.test.ts b/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useDataValue.test.ts new file mode 100644 index 00000000000..e478a05386c --- /dev/null +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useDataValue.test.ts @@ -0,0 +1,73 @@ +import { act, renderHook } from '@testing-library/react' +import useDataValue from '../useDataValue' + +describe('useDataValue', () => { + it('should call external onChange based change callbacks', () => { + const onChange = jest.fn() + const { result } = renderHook(() => useDataValue({ onChange })) + + const { handleChange } = result.current + + act(() => { + handleChange('new-value') + }) + expect(onChange).toHaveBeenCalledTimes(1) + expect(onChange).toHaveBeenNthCalledWith(1, 'new-value') + }) + + describe('using focus callbacks', () => { + it('should return the error only when the value is invalid AND it is not in focus', () => { + const { result } = renderHook(() => + useDataValue({ + value: 'foo', + emptyValue: '', + required: true, + }) + ) + + const { handleFocus, handleBlur, handleChange } = result.current + + act(() => { + handleFocus() + handleChange('') + }) + expect(result.current.error).toBeUndefined() + + act(() => { + handleBlur() + }) + expect(result.current.error).toBeInstanceOf(Error) + + act(() => { + handleFocus() + handleChange('a') + handleBlur() + }) + expect(result.current.error).toBeUndefined() + }) + }) + + describe('without using focus callbacks', () => { + it('should return the error as long as the value is invalud', () => { + const { result } = renderHook(() => + useDataValue({ + value: 'foo', + emptyValue: '', + required: true, + }) + ) + + const { handleChange } = result.current + + act(() => { + handleChange('') + }) + expect(result.current.error).toBeInstanceOf(Error) + + act(() => { + handleChange('abc') + }) + expect(result.current.error).toBeUndefined() + }) + }) +}) diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts b/packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts index fbb25303e4d..843b3fdb63a 100644 --- a/packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts @@ -19,8 +19,8 @@ interface ReturnAdditional { value: Value error: Error | FormError | undefined setHasFocus: (hasFocus: boolean, valueOverride?: unknown) => void - handleFocus: FieldProps['onFocus'] - handleBlur: FieldProps['onBlur'] + handleFocus: () => void + handleBlur: () => void handleChange: FieldProps['onChange'] } @@ -43,6 +43,7 @@ export default function useDataValue< errorMessages, validateInitially, validateUnchanged, + continuousValidation, toInput = (value) => value, fromInput = (value) => value, } = props @@ -125,6 +126,7 @@ export default function useDataValue< // and to handle errors in Eufemia on components that does not take updated callback functions into account. const [value, setValue] = useState(externalValue) const changedRef = useRef(false) + const hasFocusRef = useRef(false) useEffect(() => { // When receiving the initial value, or receiving an updated value by props, update the internal value @@ -238,9 +240,11 @@ export default function useDataValue< (hasFocus: boolean, valueOverride?: unknown) => { if (hasFocus) { // Field was put in focus (like when clicking in a text field or opening a dropdown menu) + hasFocusRef.current = true onFocus?.(valueOverride ?? value) } else { // Field was removed from focus (like when tabbing out of a text field or closing a dropdown menu) + hasFocusRef.current = false onBlur?.(valueOverride ?? value) if (!changedRef.current && !validateUnchanged) { @@ -292,13 +296,24 @@ export default function useDataValue< } setValue(newValue) changedRef.current = true - // When changing the value, hide errors to avoid annoying the user before they are finished filling in that value - setShowError(false) - setShowFieldBlockError?.(path ?? id, false) + + if ( + continuousValidation || + (continuousValidation !== false && !hasFocusRef.current) + ) { + // When there is a change to the value without there having been any focus callback beforehand, it is likely + // to believe that the blur callback will not be called either, which would trigger the display of the error. + // The error is therefore displayed immediately (unless instructed not to with continuousValidation set to false). + setShowError(true) + setShowFieldBlockError?.(path ?? id, true) + } else { + // When changing the value, hide errors to avoid annoying the user before they are finished filling in that value + setShowError(false) + setShowFieldBlockError?.(path ?? id, false) + } // Always validate the value immediately when it is changed validateValue(newValue) - // Tell any parent data context about the error, so they can take it into consideration when a submit button is clicked for instance onChange?.(newValue) if (path) { dataContextHandlePathChange?.(path, newValue) @@ -316,6 +331,7 @@ export default function useDataValue< elementPath, iterateElementIndex, value, + continuousValidation, onChange, validateValue, dataContextHandlePathChange,