Skip to content

Commit

Permalink
fix(forms): Handle error display when no focus and blur events is cal…
Browse files Browse the repository at this point in the history
…led (#2737)

Handle error display when no focus and blur events is called
  • Loading branch information
henit authored Oct 10, 2023
1 parent 25ffbf8 commit 63f6e87
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Field.String
value="abc"
schema={{ type: 'string', minLength: 6 }}
/>
)
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(
<Field.String
value="abc"
schema={{ type: 'string', minLength: 6 }}
/>
)
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(
<Field.String
value="abc"
schema={{ type: 'string', minLength: 6 }}
/>
)
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()
Expand Down Expand Up @@ -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(<Field.String value="a" required />)
Expand All @@ -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(<Field.String value="abc" minLength={5} />)
const input = document.querySelector('input')
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
28 changes: 22 additions & 6 deletions packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ interface ReturnAdditional<Value> {
value: Value
error: Error | FormError | undefined
setHasFocus: (hasFocus: boolean, valueOverride?: unknown) => void
handleFocus: FieldProps<unknown>['onFocus']
handleBlur: FieldProps<unknown>['onBlur']
handleFocus: () => void
handleBlur: () => void
handleChange: FieldProps<unknown>['onChange']
}

Expand All @@ -43,6 +43,7 @@ export default function useDataValue<
errorMessages,
validateInitially,
validateUnchanged,
continuousValidation,
toInput = (value) => value,
fromInput = (value) => value,
} = props
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -316,6 +331,7 @@ export default function useDataValue<
elementPath,
iterateElementIndex,
value,
continuousValidation,
onChange,
validateValue,
dataContextHandlePathChange,
Expand Down

0 comments on commit 63f6e87

Please sign in to comment.