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

chore(useDataValue): refactor and cleanup #3185

Merged
merged 1 commit into from
Jan 8, 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 @@ -173,25 +173,12 @@ The transformers are hooks to transform the value on different stages.

They should return a transformed value: `(value) => value`

- `toInput` transforms the value before it gets returned by the hook:
- `toInput` transforms the value before it gets returned as the `value`:

```ts
const { value } = useDataValue(props)
```
- `fromInput` transforms the value given by `handleChange` before it is used in the further process flow. Use it to destruct the value form the original event object.

- `fromInput` transforms the value given by `handleChange` before it is used in the further process flow.

```ts
handleChange(value)
```

- `toEvent` transforms the internal value before it gets returned by even callbacks such as `onChange`, `onFocus` and `onBlur`.
- `toEvent` transforms the internal value before it gets returned by even callbacks such as `onChange`, `onFocus` and `onBlur`. The second parameter returns the event type: `onChange`, `onFocus`, `onBlur` or `onBlurValidator`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `toEvent` transforms the internal value before it gets returned by even callbacks such as `onChange`, `onFocus` and `onBlur`. The second parameter returns the event type: `onChange`, `onFocus`, `onBlur` or `onBlurValidator`.
- `toEvent` transforms the internal value before it gets returned by event callbacks such as `onChange`, `onFocus` and `onBlur`. The second parameter returns the event type: `onChange`, `onFocus`, `onBlur` or `onBlurValidator`.


- `fromExternal` transforms the given props `value` before any other step gets entered.

#### Additional features

| Property | Type | Description |
| ------------ | --------- | -------------------------------------------------------------------------------------------------------------------------- |
| `capitalize` | `boolean` | _(optional)_ When set to `true`, it will capitalize the first letter of every word, transforming the rest to lowercase. |
| `trim` | `boolean` | _(optional)_ When `true`, it will trim leading and trailing whitespaces on blur, triggering onChange if the value changes. |
- `transformValue` transforms the value given by `handleChange` after `fromInput` and before `updateValue` and `toEvent`. The second parameter returns the current value
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import FieldBlock from '../../FieldBlock'
import { useDataValue } from '../../hooks'
import { FieldProps, FieldHelpProps } from '../../types'
import { pickSpacingProps } from '../../../../components/flex/utils'
import { toCapitalized } from '../../../../shared/component-helper'

interface ErrorMessages {
required?: string
Expand Down Expand Up @@ -79,12 +80,40 @@ function StringComponent(props: Props) {
},
[props.emptyValue]
)
const toEvent = useCallback(
(value: string, type: string) => {
if (props.trim && type === 'onBlur') {
const spaces = '[\\s ]'
if (new RegExp(`^${spaces}|${spaces}$`).test(value)) {
value = value.replace(
new RegExp(`^${spaces}+|${spaces}+$`, 'g'),
''
)
handleChange(value)
}
}
return value
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.trim]
)
const transformValue = useCallback(
(value: string) => {
if (props.capitalize) {
value = toCapitalized(String(value || ''))
}
return value
},
[props.capitalize]
)

const preparedProps: Props = {
...props,
errorMessages,
schema,
fromInput,
toEvent,
transformValue,
width: props.width ?? 'large',
}

Expand Down Expand Up @@ -123,6 +152,11 @@ function StringComponent(props: Props) {
handleChange,
} = useDataValue(preparedProps)

const transformInstantly = useCallback(
(value: string) => (props.capitalize ? toCapitalized(value) : value),
[props.capitalize]
)

const characterCounterElement = characterCounter
? props.maxLength
? `${value?.length ?? '0'}/${props.maxLength}`
Expand All @@ -147,7 +181,7 @@ function StringComponent(props: Props) {
stretch: width !== undefined,
inner_ref: innerRef,
status: error || hasError ? 'error' : undefined,
value: value?.toString() ?? '',
value: transformInstantly(value?.toString() ?? ''),
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ describe('Field.String', () => {

const input = document.querySelector('input')

expect(input).toHaveValue('First Word')

await userEvent.type(input, ' second')
expect(input).toHaveValue('First Word Second')

Expand All @@ -112,19 +114,19 @@ describe('Field.String', () => {
render(
<Field.String
trim
value=" first"
value=" first"
onChange={onChange}
onBlur={onBlur}
/>
)

const input = document.querySelector('input')

expect(input).toHaveValue(' first')
expect(input).toHaveValue(' first')

await userEvent.type(input, ' second ')

expect(onChange).toHaveBeenLastCalledWith(' first second ')
expect(onChange).toHaveBeenLastCalledWith(' first second ')

fireEvent.blur(input)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ describe('useDataValue', () => {
})

expect(toEvent).toHaveBeenCalledTimes(3)
expect(toEvent).toHaveBeenLastCalledWith(2)
expect(toEvent).toHaveBeenLastCalledWith(2, 'onBlur')

expect(onChange).toHaveBeenCalledTimes(1)
expect(onChange).toHaveBeenLastCalledWith(3)
Expand All @@ -485,7 +485,7 @@ describe('useDataValue', () => {
})

expect(toEvent).toHaveBeenCalledTimes(6)
expect(toEvent).toHaveBeenLastCalledWith(4)
expect(toEvent).toHaveBeenLastCalledWith(4, 'onBlur')

expect(onChange).toHaveBeenCalledTimes(2)
expect(onChange).toHaveBeenLastCalledWith(5)
Expand Down Expand Up @@ -546,54 +546,38 @@ describe('useDataValue', () => {

expect(onChange).toHaveBeenCalledTimes(1)
})
})

describe('manipulate string value', () => {
it('should capitalize value', () => {
const onBlur = jest.fn()
const onChange = jest.fn()
it('should call "transformValue"', () => {
const transformValue = jest.fn((v) => v + 1)

const { result } = renderHook(() =>
useDataValue({
value: 'foo',
capitalize: true,
onBlur,
onChange,
value: 1,
transformValue,
})
)

const { handleBlur, handleChange } = result.current
const { handleFocus, handleBlur, handleChange } = result.current

expect(transformValue).toHaveBeenCalledTimes(0)

act(() => {
handleFocus()
handleChange(2)
handleBlur()
handleChange('bar')
})

expect(onBlur).toHaveBeenLastCalledWith('Foo')
expect(onChange).toHaveBeenLastCalledWith('Bar')
})

it('should trim value', () => {
const onBlur = jest.fn()
const onChange = jest.fn()

const { result } = renderHook(() =>
useDataValue({
value: ' foo',
trim: true,
onBlur,
onChange,
})
)

const { handleBlur } = result.current
expect(transformValue).toHaveBeenCalledTimes(1)
expect(transformValue).toHaveBeenLastCalledWith(2, 1)

act(() => {
handleFocus()
handleChange(4)
handleBlur()
})

expect(onBlur).toHaveBeenLastCalledWith('foo')
expect(onChange).toHaveBeenLastCalledWith('foo')
expect(transformValue).toHaveBeenCalledTimes(2)
expect(transformValue).toHaveBeenLastCalledWith(4, 3)
})
})

Expand Down
61 changes: 25 additions & 36 deletions packages/dnb-eufemia/src/extensions/forms/hooks/useDataValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import { FormError, FieldProps, AdditionalEventArgs } from '../types'
import { Context, ContextState } from '../DataContext'
import FieldBlockContext from '../FieldBlock/FieldBlockContext'
import IterateElementContext from '../Iterate/IterateElementContext'
import {
makeUniqueId,
toCapitalized,
} from '../../../shared/component-helper'
import { makeUniqueId } from '../../../shared/component-helper'
import useMountEffect from './useMountEffect'
import useUpdateEffect from './useUpdateEffect'
import useProcessManager from './useProcessManager'
Expand Down Expand Up @@ -59,6 +56,7 @@ export default function useDataValue<
toInput = (value: Value) => value,
fromInput = (value: Value) => value,
toEvent = (value: Value) => value,
transformValue = (value: Value) => value,
fromExternal = (value: Value) => value,
validateRequired = (value: Value, { emptyValue, required }) => {
const res =
Expand All @@ -85,6 +83,7 @@ export default function useDataValue<
fromInput,
toEvent,
fromExternal,
transformValue,
validateRequired,
})

Expand Down Expand Up @@ -130,14 +129,8 @@ export default function useDataValue<

const externalValue = useMemo(() => {
if (props.value !== undefined) {
let value = transformers.current.fromExternal(props.value)

if (props.capitalize) {
value = toCapitalized(String(value || '')) as Value
}

// Value-prop sent directly to the field has highest priority, overriding any surrounding source
return value
return transformers.current.fromExternal(props.value)
}

if (inIterate && itemPath) {
Expand Down Expand Up @@ -411,15 +404,19 @@ export default function useDataValue<
if (hasFocus) {
// Field was put in focus (like when clicking in a text field or opening a dropdown menu)
hasFocusRef.current = true
onFocus?.(
transformers.current.toEvent(valueOverride ?? valueRef.current)
const value = transformers.current.toEvent(
valueOverride ?? valueRef.current,
'onFocus'
)
onFocus?.(value)
} else {
// Field was removed from focus (like when tabbing out of a text field or closing a dropdown menu)
hasFocusRef.current = false
onBlur?.(
transformers.current.toEvent(valueOverride ?? valueRef.current)
const value = transformers.current.toEvent(
valueOverride ?? valueRef.current,
'onBlur'
)
onBlur?.(value)

if (!changedRef.current && !validateUnchanged) {
// Avoid showing errors when blurring without having changed the value, so tabbing through several
Expand All @@ -431,13 +428,11 @@ export default function useDataValue<
// expensive validation calling external services etc.
if (typeof onBlurValidator === 'function') {
// Since the validator can return either a synchronous result or an asynchronous
Promise.resolve(
onBlurValidator(
transformers.current.toEvent(
valueOverride ?? valueRef.current
)
)
).then(persistErrorState)
const value = transformers.current.toEvent(
valueOverride ?? valueRef.current,
'onBlurValidator'
)
Promise.resolve(onBlurValidator(value)).then(persistErrorState)
}

// Since the user left the field, show error (if any)
Expand Down Expand Up @@ -484,23 +479,25 @@ export default function useDataValue<
argFromInput: Value,
additionalArgs: AdditionalEventArgs = undefined
) => {
const currentValue = valueRef.current
let newValue = transformers.current.fromInput(argFromInput)

if (newValue === valueRef.current) {
if (newValue === currentValue) {
// Avoid triggering a change if the value was not actually changed. This may be caused by rendering components
// calling onChange even if the actual value did not change.
return
}

if (props.capitalize) {
newValue = toCapitalized(String(newValue || '')) as Value
}
newValue = transformers.current.transformValue(
newValue,
currentValue
)

updateValue(newValue)

changedRef.current = true

const value = transformers.current.toEvent(newValue)
const value = transformers.current.toEvent(newValue, 'onChange')
onChange?.apply(
this,
typeof additionalArgs !== 'undefined'
Expand All @@ -516,7 +513,6 @@ export default function useDataValue<
}
},
[
props.capitalize,
updateValue,
onChange,
itemPath,
Expand All @@ -527,14 +523,7 @@ export default function useDataValue<

const handleFocus = useCallback(() => setHasFocus(true), [setHasFocus])

const handleBlur = useCallback(() => {
if (props.trim && /^\s|\s$/.test(String(valueRef.current))) {
const value = String(valueRef.current).trim()
handleChange(value as Value)
}

setHasFocus(false)
}, [props.trim, setHasFocus, handleChange])
const handleBlur = useCallback(() => setHasFocus(false), [setHasFocus])

useMountEffect(() => {
dataContext?.handleMountField(identifier)
Expand Down
14 changes: 9 additions & 5 deletions packages/dnb-eufemia/src/extensions/forms/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,16 @@ export interface FieldProps<
continuousValidation?: boolean
errorMessages?: ErrorMessages
// Derivatives
toInput?: (external: Value | undefined) => any
fromInput?: (...args: any[]) => Value | undefined
toEvent?: (internal: Value | undefined) => any
fromExternal?: (...args: any[]) => Value | undefined
toInput?: (external: Value | unknown) => Value | unknown
fromInput?: (external: Value | unknown) => Value
toEvent?: (
internal: Value,
type: 'onChange' | 'onFocus' | 'onBlur' | 'onBlurValidator'
) => Value
fromExternal?: (external: Value) => Value
transformValue?: (value: Value, currentValue?: Value) => Value
validateRequired?: (
internal: Value | undefined,
internal: Value,
{
emptyValue,
required,
Expand Down
Loading