From 89ea339f067f22e3ebc8c4d8164f31cb5f990eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20H=C3=B8egh?= Date: Wed, 25 Sep 2024 07:24:02 +0200 Subject: [PATCH] Fix propper Wizard and defaultValue support --- .../extensions/forms/Iterate/Array/Array.tsx | 14 +- .../Iterate/Array/__tests__/Array.test.tsx | 133 ++++++++++++++---- .../forms/Iterate/stories/Iterate.stories.tsx | 41 +++++- .../__tests__/WizardContainer.test.tsx | 105 +++++++++++++- .../extensions/forms/hooks/useFieldProps.ts | 62 +++++--- .../dnb-eufemia/src/extensions/forms/types.ts | 1 + 6 files changed, 294 insertions(+), 62 deletions(-) diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx index c906ce14d50..bee8172fcd7 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx @@ -24,7 +24,6 @@ import IterateItemContext, { import SummaryListContext from '../../Value/SummaryList/SummaryListContext' import ValueBlockContext from '../../ValueBlock/ValueBlockContext' import FieldBoundaryProvider from '../../DataContext/FieldBoundary/FieldBoundaryProvider' -import DataContext from '../../DataContext/Context' import useDataValue from '../../hooks/useDataValue' import { useArrayLimit, useSwitchContainerMode } from '../hooks' import { getMessage } from '../../FieldBlock' @@ -75,6 +74,7 @@ function ArrayComponent(props: Props) { } return { + isIterateArray: true, required: false, ...props, value: newValue, @@ -82,7 +82,7 @@ function ArrayComponent(props: Props) { } } - return { required: false, ...props } + return { isIterateArray: true, required: false, ...props } }, [getValueByPath, props]) const { @@ -90,7 +90,6 @@ function ArrayComponent(props: Props) { value: arrayValue, limit, error, - defaultValue, withoutFlex, emptyValue, placeholder, @@ -129,15 +128,6 @@ function ArrayComponent(props: Props) { const omitFlex = withoutFlex ?? (summaryListContext || valueBlockContext) - // To support React.StrictMode, we inject the defaultValue into the data context this way. - // The routine inside useFieldProps where updateDataValueDataContext is called, does not support React.StrictMode - const { handlePathChange } = useContext(DataContext) || {} - useMountEffect(() => { - if (defaultValue) { - handlePathChange?.(path, defaultValue) - } - }) - useEffect(() => { // Update inside the useEffect, to support React.StrictMode valueCountRef.current = arrayValue || [] diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx index 73d8afc2f66..63e8db3b773 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx @@ -562,31 +562,110 @@ describe('Iterate.Array', () => { expect(onChangeIterate).toHaveBeenLastCalledWith(['foo']) }) - it('should handle "defaultValue" with React.StrictMode', () => { - const onSubmit = jest.fn() + describe('defaultValue', () => { + it('should handle "defaultValue" (empty string) in React.StrictMode', () => { + const onSubmit = jest.fn() - render( - - - - - - - - ) + render( + + + + + + + + ) - const form = document.querySelector('form') - const input = document.querySelector('input') + const form = document.querySelector('form') + const input = document.querySelector('input') - expect(input).toHaveValue('') + expect(input).toHaveValue('') - fireEvent.submit(form) + fireEvent.submit(form) - expect(onSubmit).toHaveBeenCalledTimes(1) - expect(onSubmit).toHaveBeenLastCalledWith( - { myList: [''] }, - expect.anything() - ) + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenLastCalledWith( + { myList: [''] }, + expect.anything() + ) + }) + + it('should handle "defaultValue" (with value) in React.StrictMode', () => { + const onSubmit = jest.fn() + + render( + + + + + + + + ) + + const form = document.querySelector('form') + const input = document.querySelector('input') + + expect(input).toHaveValue('foo') + + fireEvent.submit(form) + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenLastCalledWith( + { myList: ['foo'] }, + expect.anything() + ) + }) + + it('should handle "defaultValue" (with null) in React.StrictMode', () => { + const onSubmit = jest.fn() + + render( + + + + + + + + ) + + const form = document.querySelector('form') + const input = document.querySelector('input') + + expect(input).toHaveValue('foo') + + fireEvent.submit(form) + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenLastCalledWith( + { myList: ['foo'] }, + expect.anything() + ) + }) + + it('should set empty array in the data context', () => { + const onSubmit = jest.fn() + + render( + + + + content + + + + ) + + const form = document.querySelector('form') + fireEvent.submit(form) + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenLastCalledWith( + { myList: [] }, + expect.anything() + ) + }) }) describe('with primitive elements', () => { @@ -1294,7 +1373,7 @@ describe('Iterate.Array', () => { @@ -1311,26 +1390,20 @@ describe('Iterate.Array', () => { ) const form = document.querySelector('form') - const [first, second, third, forth] = Array.from( + const [first, second, third] = Array.from( document.querySelectorAll('input') ) expect(first).toHaveValue('default value 1') expect(second).toHaveValue('default value 2') - expect(third).toHaveValue('default value 3') - expect(forth).toHaveValue('default value 4') + expect(third).toHaveValue('something') fireEvent.submit(form) expect(onSubmit).toHaveBeenCalledTimes(1) expect(onSubmit).toHaveBeenLastCalledWith( { - myList: [ - 'default value 1', - 'default value 2', - 'default value 3', - 'default value 4', - ], + myList: ['default value 1', 'default value 2', 'something'], }, expect.anything() ) diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/stories/Iterate.stories.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/stories/Iterate.stories.tsx index 141db4d1067..85179810776 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/stories/Iterate.stories.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/stories/Iterate.stories.tsx @@ -1,5 +1,5 @@ import React, { useCallback } from 'react' -import { Field, Form, Iterate, Value } from '../..' +import { Field, Form, Iterate, Tools, Value, Wizard } from '../..' import { Card, Flex, Section } from '../../../../components' export default { @@ -256,3 +256,42 @@ export const WithArrayValidator = () => { ) } + +export function InWizard() { + return ( + + + + + + + + + + + + + + + + + + + + + + + + + + + ) +} diff --git a/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx index 5eefdaf0be5..1cc6df7b644 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx @@ -2,7 +2,7 @@ import React from 'react' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { wait } from '../../../../../core/jest/jestSetup' -import { Field, Form, OnSubmit, Wizard } from '../../..' +import { Field, Form, Iterate, OnSubmit, Wizard } from '../../..' import nbNO from '../../../constants/locales/nb-NO' const nb = nbNO['nb-NO'] @@ -1964,4 +1964,107 @@ describe('Wizard.Container', () => { expect(iframe.parentElement).toBeNull() }) }) + + describe('defaultValue', () => { + it('should set defaultValue of a Field.* only once between step changes', async () => { + const onChange = jest.fn() + const onStepChange = jest.fn() + + render( + + + + + + + + + + + + + ) + + expect(document.querySelector('input')).toHaveValue('123') + + await userEvent.type(document.querySelector('input'), '4') + + expect(document.querySelector('input')).toHaveValue('1234') + + await userEvent.click(nextButton()) + + expect(onStepChange).toHaveBeenLastCalledWith( + 1, + 'next', + expect.anything() + ) + + await userEvent.click(previousButton()) + + expect(onStepChange).toHaveBeenLastCalledWith( + 0, + 'previous', + expect.anything() + ) + + expect(document.querySelector('input')).toHaveValue('1234') + }) + + it('should set defaultValue of Iterate.Array only once between step changes', async () => { + const onChange = jest.fn() + const onStepChange = jest.fn() + + render( + + + + + + + + + + + + + + + + + + ) + + expect(document.querySelectorAll('input')).toHaveLength(1) + expect(document.querySelector('input')).toHaveValue('123') + + await userEvent.type(document.querySelector('input'), '4') + + expect(document.querySelector('input')).toHaveValue('1234') + + const pushButton = document.querySelector( + '.dnb-forms-iterate-push-button' + ) + await userEvent.click(pushButton) + + expect(document.querySelectorAll('input')).toHaveLength(2) + + await userEvent.click(nextButton()) + + expect(onStepChange).toHaveBeenLastCalledWith( + 1, + 'next', + expect.anything() + ) + + await userEvent.click(previousButton()) + + expect(document.querySelectorAll('input')).toHaveLength(2) + expect(onStepChange).toHaveBeenLastCalledWith( + 0, + 'previous', + expect.anything() + ) + expect(document.querySelector('input')).toHaveValue('1234') + }) + }) }) diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts index 436c16cefcb..a440e69df4a 100644 --- a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts @@ -1566,18 +1566,23 @@ export default function useFieldProps( // Form.Visibility is an example of a logic, where a field value/defaultValue can be used to set the set state of a path, // where again other fields depend on it. const tmpValueRef = useRef>({}) - const setData = useCallback(() => { + const setContextData = useCallback(() => { if (hasPath || hasItemPath) { let value = valueProp + // const data = dataContext.data + const data = + hasItemPath || props.isIterateArray + ? dataContext.internalDataRef?.current + : dataContext.data + // First, look for existing data in the context - const hasValue = - pointer.has(dataContext.data, identifier) || identifier === '/' + const hasValue = pointer.has(data, identifier) || identifier === '/' const existingValue = identifier === '/' - ? dataContext.data + ? data : hasValue - ? pointer.get(dataContext.data, identifier) + ? pointer.get(data, identifier) : undefined // If no data where found in the dataContext, look for shared data @@ -1607,11 +1612,7 @@ export default function useFieldProps( } if (hasItemPath) { - if (hasDefaultValue) { - // When an itemPath is given, we rather want to use the defaultValue, - // even if the data context exists. Because the defaultValue should now be set in the data context. - valueRef.current = value - } else if ( + if ( typeof value === 'undefined' && typeof existingValue !== 'undefined' ) { @@ -1622,9 +1623,25 @@ export default function useFieldProps( } } + if (props.isIterateArray) { + // When an array is given (iterate), we don't want to overwrite the existing array + if (hasDefaultValue && hasValue) { + return // stop here, we don't want to overwrite the existing array + } + + // React.StrictMode will come with "undefined" on the second render, + // because "defaultValueRef.current" was removed. + // But because we run "useMemo" on the first render when isIterateArray is true, + // we have still a valid value/array. + if (!Array.isArray(value)) { + return // stop here, never use a non-array value when in "isIterateArray" + } + } + if ( - (!hasValue && !hasItemPath) || - (value !== existingValue && + !hasValue || + (hasValue && + value !== existingValue && // Prevents an infinite loop by skipping the update if the value hasn't changed valueRef.current !== existingValue) ) { @@ -1648,7 +1665,8 @@ export default function useFieldProps( // When an itemPath is given, we don't want to rerender the context on every iteration because of performance reasons. // We know when the last item is reached, so we can prevent rerenders during the iteration. const preventUpdate = - hasItemPath && iterateIndex < iterateArrayValue?.length - 1 + props.isIterateArray || + (hasItemPath && iterateIndex < iterateArrayValue?.length - 1) // Update the data context when a pointer not exists, // but was given initially. @@ -1662,11 +1680,13 @@ export default function useFieldProps( }, [ dataContext.data, dataContext.id, + dataContext.internalDataRef, hasItemPath, hasPath, identifier, iterateArrayValue?.length, iterateIndex, + props.isIterateArray, updateDataValueDataContext, validateDataDataContext, valueProp, @@ -1698,18 +1718,24 @@ export default function useFieldProps( } }, [clearErrorState, defaultValue, hideError, isEmptyData]) + useMemo(() => { + if (props.isIterateArray && !isEmptyData()) { + setContextData() + } + }, [isEmptyData, props.isIterateArray, setContextData]) + useLayoutEffect(() => { - if (!isEmptyData()) { - setData() + if (!props.isIterateArray && !isEmptyData()) { + setContextData() } - }, [isEmptyData, setData]) + }, [isEmptyData, props.isIterateArray, setContextData]) useEffect(() => { if (isEmptyData()) { - setData() + setContextData() validateValue() } - }, [isEmptyData, setData, validateValue]) + }, [isEmptyData, setContextData, validateValue]) useEffect(() => { if (showAllErrors || showBoundaryErrors) { diff --git a/packages/dnb-eufemia/src/extensions/forms/types.ts b/packages/dnb-eufemia/src/extensions/forms/types.ts index 9e058eea074..0293855f0d5 100644 --- a/packages/dnb-eufemia/src/extensions/forms/types.ts +++ b/packages/dnb-eufemia/src/extensions/forms/types.ts @@ -283,6 +283,7 @@ export interface UseFieldProps< name?: string disabled?: boolean readOnly?: boolean + isIterateArray?: boolean autoComplete?: | HTMLInputElement['autocomplete'] | HTMLTextAreaElement['autocomplete']