From 3677715ba0c1e517d2f281c5ff62bc32ea2dbad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20H=C3=B8egh?= Date: Sat, 31 Aug 2024 07:08:43 +0200 Subject: [PATCH] Revert support for value and defaultValue inside iterate --- .../forms/Iterate/Array/Examples.tsx | 16 +- .../extensions/forms/Iterate/Array/demos.mdx | 4 +- .../extensions/forms/DataContext/Context.ts | 1 - .../forms/DataContext/Provider/Provider.tsx | 1 - .../extensions/forms/Iterate/Array/Array.tsx | 12 ++ .../Iterate/Array/__tests__/Array.test.tsx | 41 ++++- .../PushButton/__tests__/PushButton.test.tsx | 10 +- .../Iterate/PushContainer/PushContainer.tsx | 2 +- .../__tests__/PushContainer.test.tsx | 156 +++++++++++++----- .../forms/Iterate/stories/Iterate.stories.tsx | 101 ++++++------ .../ChildrenWithAge/ChildrenWithAge.tsx | 2 - .../ChildrenWithAge.test.tsx.snap | 4 - .../extensions/forms/hooks/useFieldProps.ts | 38 +++-- 13 files changed, 254 insertions(+), 134 deletions(-) diff --git a/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/Examples.tsx b/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/Examples.tsx index fab4d17a8de..32734193cb2 100644 --- a/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/Examples.tsx +++ b/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/Examples.tsx @@ -365,19 +365,17 @@ export const WithVisibility = () => { ) } -export const WithInitialValue = () => { +export const InitialOpen = () => { return ( {() => { const MyEditItemForm = () => { return ( - - - + ) } @@ -403,7 +401,7 @@ export const WithInitialValue = () => { const MyForm = () => { return ( console.log('onSubmit', data)} + onSubmit={async (data) => console.log('onSubmit', data)} onSubmitRequest={() => console.log('onSubmitRequest')} > diff --git a/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/demos.mdx b/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/demos.mdx index 188e041b121..96a2ac3316d 100644 --- a/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/demos.mdx +++ b/packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Iterate/Array/demos.mdx @@ -59,8 +59,8 @@ With an optional `title` and [Iterate.Toolbar](/uilib/extensions/forms/Iterate/T -### With initial value and `containerMode="initial-open"` +### Initially open By using `containerMode="initial-open"` and providing an initial value of `null` to the array, the user needs to fill out the first item. - + diff --git a/packages/dnb-eufemia/src/extensions/forms/DataContext/Context.ts b/packages/dnb-eufemia/src/extensions/forms/DataContext/Context.ts index ff12786b1f2..551e61830bd 100644 --- a/packages/dnb-eufemia/src/extensions/forms/DataContext/Context.ts +++ b/packages/dnb-eufemia/src/extensions/forms/DataContext/Context.ts @@ -66,7 +66,6 @@ export interface ContextState { hasContext: boolean /** The dataset for the form / form wizard */ data: any - internalDataRef?: React.MutableRefObject /** Should the form validate data before submitting? */ errors?: Record /** Will set autoComplete="on" on each nested Field.String and Field.Number */ diff --git a/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/Provider.tsx b/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/Provider.tsx index 57e39acb969..2f55779b97f 100644 --- a/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/Provider.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/Provider.tsx @@ -1183,7 +1183,6 @@ export default function Provider( /** Additional */ id, data: internalDataRef.current, - internalDataRef, props, ...rest, }} 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 282c415f8c7..89d0827a589 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx @@ -13,6 +13,7 @@ import { useFieldProps } from '../../hooks' import { makeUniqueId } from '../../../../shared/component-helper' import { Flex } from '../../../../components' import { pickSpacingProps } from '../../../../components/flex/utils' +import useMountEffect from '../../../../shared/helpers/useMountEffect' import { BasicProps as FlexContainerProps, Props as FlexContainerAllProps, @@ -24,6 +25,7 @@ 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 { useSwitchContainerMode } from '../hooks' @@ -82,6 +84,7 @@ function ArrayComponent(props: Props) { const { path, value: arrayValue, + defaultValue, withoutFlex, emptyValue, placeholder, @@ -104,6 +107,15 @@ 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 50951fca5e1..c59b73ffbaa 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 @@ -416,7 +416,35 @@ describe('Iterate.Array', () => { expect(onChangeIterate).toHaveBeenLastCalledWith(['foo']) }) - it('should handle "defaultValue" and not overwrite "defaultValue" set by nested fields', () => { + it('should handle "defaultValue" with React.StrictMode', () => { + const onSubmit = jest.fn() + + render( + + + + + + + + ) + + const form = document.querySelector('form') + const input = document.querySelector('input') + + expect(input).toHaveValue('') + + fireEvent.submit(form) + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenLastCalledWith( + { myList: [''] }, + expect.anything() + ) + }) + + it('should warn when "defaultValue" is used inside iterate', () => { + const log = jest.spyOn(console, 'log').mockImplementation() const onSubmit = jest.fn() render( @@ -430,15 +458,22 @@ describe('Iterate.Array', () => { const form = document.querySelector('form') const input = document.querySelector('input') - expect(input).toHaveValue('default value') + expect(input).toHaveValue('') fireEvent.submit(form) expect(onSubmit).toHaveBeenCalledTimes(1) expect(onSubmit).toHaveBeenLastCalledWith( - { myList: ['default value'] }, + { myList: [''] }, expect.anything() ) + + expect(log).toHaveBeenCalledWith( + expect.any(String), + 'Using defaultValue="default value" prop inside iterate is not supported yet' + ) + + log.mockRestore() }) describe('with primitive elements', () => { diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/__tests__/PushButton.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/__tests__/PushButton.test.tsx index 2273e1de04a..0141dd16f37 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/__tests__/PushButton.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/__tests__/PushButton.test.tsx @@ -128,10 +128,11 @@ describe('PushButton', () => { it('should not overwrite initial data because of the same path as the Iterate.Array', async () => { const onSubmit = jest.fn() + render( - + @@ -139,16 +140,13 @@ describe('PushButton', () => { ) const form = document.querySelector('form') - const input = document.querySelector('input') const button = document.querySelector('.dnb-forms-iterate-push-button') - expect(input).toHaveValue('bar') - fireEvent.submit(form) expect(onSubmit).toHaveBeenCalledTimes(1) expect(onSubmit).toHaveBeenLastCalledWith( - { myList: ['bar'] }, + { myList: [null] }, expect.anything() ) @@ -157,7 +155,7 @@ describe('PushButton', () => { expect(onSubmit).toHaveBeenCalledTimes(2) expect(onSubmit).toHaveBeenLastCalledWith( - { myList: ['bar', 'push value'] }, + { myList: [null, 'push value'] }, expect.anything() ) }) diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/PushContainer.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/PushContainer.tsx index 046f03e1a43..c0bc40b529c 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/PushContainer.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/PushContainer.tsx @@ -36,7 +36,7 @@ export type Props = { /** * Prefilled data to add to the fields. */ - data?: Record + data?: unknown | Record /** * A custom toolbar to be shown below the container. diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/__tests__/PushContainer.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/__tests__/PushContainer.test.tsx index 923c35ad0a3..c99a290e92d 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/__tests__/PushContainer.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushContainer/__tests__/PushContainer.test.tsx @@ -138,7 +138,7 @@ describe('PushContainer', () => { expect(title).toHaveTextContent('New entry') }) - it('should render children with initial value', () => { + it('should render children with initial data value as an object', () => { render( { expect(input).toHaveValue('Tony') }) + it('should render children with initial data value as a string', async () => { + const onChange = jest.fn() + + render( + + + View Content + Edit Content + + + + + + + ) + + const blocks = Array.from( + document.querySelectorAll('.dnb-forms-section-block') + ) + const [, , thirdBlock] = blocks + + const input = thirdBlock.querySelector('input') + expect(input).toHaveValue('bar') + + await userEvent.click(thirdBlock.querySelector('button')) + + expect(onChange).toHaveBeenCalledTimes(1) + expect(onChange).toHaveBeenLastCalledWith( + ['foo', 'bar'], + expect.anything() + ) + + await userEvent.type(input, '{Backspace>3}baz') + expect(input).toHaveValue('baz') + + await userEvent.click(thirdBlock.querySelector('button')) + + expect(onChange).toHaveBeenCalledTimes(2) + expect(onChange).toHaveBeenLastCalledWith( + ['foo', 'bar', 'baz'], + expect.anything() + ) + }) + it('should render children and not the button', () => { render( @@ -354,41 +398,33 @@ describe('PushContainer', () => { Edit Content - } - showOpenButtonWhen={(list) => list.length > 0} - > - + + ) - await userEvent.click( - document.querySelector('button.dnb-forms-iterate-open-button') - ) - const blocks = Array.from( document.querySelectorAll('.dnb-forms-section-block') ) const [, , thirdBlock] = blocks const input = thirdBlock.querySelector('input') - expect(input).toHaveValue('bar') + expect(input).toHaveValue('') - await userEvent.type(input, ' addition') - expect(input).toHaveValue('bar addition') + await userEvent.type(input, 'bar') + expect(input).toHaveValue('bar') await userEvent.click(thirdBlock.querySelector('button')) expect(onChange).toHaveBeenCalledTimes(1) expect(onChange).toHaveBeenLastCalledWith( - ['foo', 'bar addition'], + ['foo', 'bar'], expect.anything() ) }) - it('should support default data in field', async () => { + it('should support initial data as a string', async () => { const onChange = jest.fn() render( @@ -398,20 +434,12 @@ describe('PushContainer', () => { Edit Content - } - showOpenButtonWhen={(list) => list.length > 0} - > - + + ) - await userEvent.click( - document.querySelector('button.dnb-forms-iterate-open-button') - ) - const blocks = Array.from( document.querySelectorAll('.dnb-forms-section-block') ) @@ -420,19 +448,66 @@ describe('PushContainer', () => { const input = thirdBlock.querySelector('input') expect(input).toHaveValue('bar') - await userEvent.type(input, ' addition') - expect(input).toHaveValue('bar addition') - await userEvent.click(thirdBlock.querySelector('button')) expect(onChange).toHaveBeenCalledTimes(1) expect(onChange).toHaveBeenLastCalledWith( - ['foo', 'bar addition'], + ['foo', 'bar'], + expect.anything() + ) + + await userEvent.type(input, '{Backspace>3}baz') + expect(input).toHaveValue('baz') + + await userEvent.click(thirdBlock.querySelector('button')) + + expect(onChange).toHaveBeenCalledTimes(2) + expect(onChange).toHaveBeenLastCalledWith( + ['foo', 'bar', 'baz'], expect.anything() ) }) - it('should push initial input value', async () => { + it('should warn when "value" prop is used', () => { + const log = jest.spyOn(console, 'log').mockImplementation() + + render( + + + + + + ) + + expect(log).toHaveBeenCalledWith( + expect.any(String), + 'Using value="bar" prop inside iterate is not supported yet' + ) + + log.mockRestore() + }) + + it('should warn when "defaultValue" prop is used', () => { + const log = jest.spyOn(console, 'log').mockImplementation() + + render( + + + + + + ) + + expect(log).toHaveBeenCalledWith( + expect.any(String), + 'Using defaultValue="bar" prop inside iterate is not supported yet' + ) + + log.mockRestore() + }) + + it('should push "null" to the array when "defaultValue" is given because it is not supported', async () => { + const log = jest.spyOn(console, 'log').mockImplementation() const onChange = jest.fn() render( @@ -442,20 +517,12 @@ describe('PushContainer', () => { Edit Content - } - showOpenButtonWhen={(list) => list.length > 0} - > + ) - await userEvent.click( - document.querySelector('button.dnb-forms-iterate-open-button') - ) - const blocks = Array.from( document.querySelectorAll('.dnb-forms-section-block') ) @@ -468,8 +535,15 @@ describe('PushContainer', () => { expect(onChange).toHaveBeenCalledTimes(1) expect(onChange).toHaveBeenLastCalledWith( - ['foo', 'bar'], + ['foo', null], expect.anything() ) + + expect(log).toHaveBeenCalledWith( + expect.any(String), + 'Using defaultValue="bar" prop inside iterate is not supported yet' + ) + + log.mockRestore() }) }) 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 3917a4be868..a896a116492 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,4 +1,4 @@ -import React from 'react' +import React, { useCallback } from 'react' import { Field, Form, Iterate, Value } from '../..' import { Card, Flex, Section } from '../../../../components' @@ -135,28 +135,26 @@ export const ViewAndEditContainer = () => { ) } -export const MyForm = () => { - const MyEditItemForm = () => { +export const InitialOpen = () => { + const MyEditItemForm = useCallback(() => { return ( - - - + ) - } + }, []) - const MyEditItem = (props) => { + const MyEditItem = useCallback(() => { return ( - + ) - } + }, [MyEditItemForm]) - const MyViewItem = () => { + const MyViewItem = useCallback(() => { return ( { /> ) - } + }, []) + + const [count, setCount] = React.useState(0) return ( - console.log('onSubmit', data)} - onSubmitRequest={() => console.log('onSubmitRequest')} - > - - Statsborgerskap - - - { - if (items?.length === 1) { - return 'initial-open' - } - }} - > - - - + + console.log('onSubmit', data)} + onSubmitRequest={() => console.log('onSubmitRequest')} + > + + Statsborgerskap - - + + { + if (items?.length === 1) { + return 'initial-open' + } + }} + > + + + - + + + + - - - + + + {count} + + + + ) } diff --git a/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/ChildrenWithAge.tsx b/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/ChildrenWithAge.tsx index 05db24eb0e3..2c57b4a2f3a 100644 --- a/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/ChildrenWithAge.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/ChildrenWithAge.tsx @@ -114,7 +114,6 @@ function EditContent({ enableAdditionalQuestions }: Props) { diff --git a/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/__tests__/__snapshots__/ChildrenWithAge.test.tsx.snap b/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/__tests__/__snapshots__/ChildrenWithAge.test.tsx.snap index a8952bb93b4..091abceb90e 100644 --- a/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/__tests__/__snapshots__/ChildrenWithAge.test.tsx.snap +++ b/packages/dnb-eufemia/src/extensions/forms/blocks/ChildrenWithAge/__tests__/__snapshots__/ChildrenWithAge.test.tsx.snap @@ -107,7 +107,6 @@ exports[`ChildrenWithAge should match snapshot 1`] = ` "width": "small", }, "hasDaycare": { - "defaultValue": false, "itemPath": "/hasDaycare", "label": "Er på SFO/AKS", "required": true, @@ -194,7 +193,6 @@ exports[`ChildrenWithAge should match snapshot 1`] = ` "valueType": "boolean", }, "jointResponsibility": { - "defaultValue": false, "itemPath": "/jointResponsibility", "label": "Delt omsorg", "required": true, @@ -385,7 +383,6 @@ exports[`ChildrenWithAge should match snapshot 1`] = ` "width": "small", }, "hasDaycare": { - "defaultValue": false, "itemPath": "/hasDaycare", "label": "Er på SFO/AKS", "required": true, @@ -472,7 +469,6 @@ exports[`ChildrenWithAge should match snapshot 1`] = ` "valueType": "boolean", }, "jointResponsibility": { - "defaultValue": false, "itemPath": "/jointResponsibility", "label": "Delt omsorg", "required": true, diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts index 040a99f95fb..9fe93559db5 100644 --- a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts @@ -22,7 +22,7 @@ import { import { Context as DataContext, ContextState } from '../DataContext' import { clearedData } from '../DataContext/Provider/Provider' import FieldPropsContext from '../Form/FieldProps/FieldPropsContext' -import { combineDescribedBy } from '../../../shared/component-helper' +import { combineDescribedBy, warn } from '../../../shared/component-helper' import useId from '../../../shared/helpers/useId' import useUpdateEffect from '../../../shared/helpers/useUpdateEffect' import useMountEffect from '../../../shared/helpers/useMountEffect' @@ -153,7 +153,6 @@ export default function useFieldProps( setFieldProps: setPropsDataContext, setHasVisibleError: setHasVisibleErrorDataContext, errors: dataContextErrors, - internalDataRef, showAllErrors, contextErrorMessages, } = dataContext || {} @@ -172,7 +171,6 @@ export default function useFieldProps( const { setFieldError, showBoundaryErrors } = fieldBoundaryContext || {} const hasPath = Boolean(pathProp) - const hasItemPath = Boolean(itemPath && (valueProp || defaultValue)) const { path, identifier, makeIteratePath } = usePath({ id, path: pathProp, @@ -1148,18 +1146,30 @@ export default function useFieldProps( ]) useEffect(() => { - if (hasPath || hasItemPath) { + if (itemPath && valueProp !== undefined) { + warn( + `Using value="${valueProp}" prop inside iterate is not supported yet` + ) + } + if (itemPath && defaultValue !== undefined) { + warn( + `Using defaultValue="${defaultValue}" prop inside iterate is not supported yet` + ) + } + }, [defaultValue, itemPath, valueProp]) + + useEffect(() => { + if (hasPath) { let value = valueProp // First, look for existing data in the context - const internalData = internalDataRef?.current const hasValue = - pointer.has(internalData, identifier) || identifier === '/' + pointer.has(dataContext.data, identifier) || identifier === '/' const existingValue = identifier === '/' ? dataContext.data : hasValue - ? pointer.get(internalData, identifier) + ? pointer.get(dataContext.data, identifier) : undefined // If no data where found in the dataContext, look for shared data @@ -1179,9 +1189,6 @@ export default function useFieldProps( } } - const hasDefaultValueAndValue = - typeof defaultValue !== 'undefined' && hasValue - if ( typeof defaultValueRef.current !== 'undefined' && typeof value === 'undefined' @@ -1191,11 +1198,10 @@ export default function useFieldProps( } if ( - !hasDefaultValueAndValue && - (!hasValue || - (value !== existingValue && - // Prevents an infinite loop by skipping the update if the value hasn't changed - valueRef.current !== existingValue)) + !hasValue || + (value !== existingValue && + // Prevents an infinite loop by skipping the update if the value hasn't changed + valueRef.current !== existingValue) ) { // Update the data context when a pointer not exists, // but was given initially. @@ -1207,10 +1213,8 @@ export default function useFieldProps( dataContext.data, dataContext.id, defaultValue, - hasItemPath, hasPath, identifier, - internalDataRef, updateDataValueDataContext, validateDataDataContext, valueProp,