Ensure auto-closing mechanism works consistently – remove useUnmountE…

…ffect because of the lack of possible changing deps
dnbexperience · Sep 1, 2024 · f0c4b9a · f0c4b9a
1 parent aa5c4f1
commit f0c4b9a
Show file tree

Hide file tree

Showing 9 changed files with 171 additions and 174 deletions.
diff --git a/...ages/dnb-eufemia/src/extensions/forms/DataContext/FieldBoundary/FieldBoundaryProvider.tsx b/...ages/dnb-eufemia/src/extensions/forms/DataContext/FieldBoundary/FieldBoundaryProvider.tsx
@@ -11,7 +11,7 @@ export default function FieldBoundaryProvider({ children }) {
 
   const errorsRef = useRef<Record<Path, boolean>>({})
   const showBoundaryErrorsRef = useRef<boolean>(false)
-  const hasError = Object.keys(errorsRef.current || {}).length > 0
+  const hasError = Object.keys(errorsRef.current).length > 0
   const hasSubmitError = showAllErrors && hasError
 
   const setFieldError = useCallback((path: Path, error: Error) => {

diff --git a/packages/dnb-eufemia/src/extensions/forms/FieldBlock/FieldBlock.tsx b/packages/dnb-eufemia/src/extensions/forms/FieldBlock/FieldBlock.tsx
@@ -30,7 +30,6 @@ import {
   warn,
 } from '../../../shared/component-helper'
 import useId from '../../../shared/helpers/useId'
-import useUnmountEffect from '../../../shared/helpers/useUnmountEffect'
 import {
   ComponentProps,
   FieldProps,
@@ -355,10 +354,13 @@ function FieldBlock(props: Props) {
     }
   }, [errorProp, blockId, showFieldError, nestedFieldBlockContext])
 
-  useUnmountEffect(() => () => {
-    mountedFieldsRef.current = {}
-    stateRecordRef.current = {}
-  })
+  useEffect(
+    () => () => {
+      mountedFieldsRef.current = {}
+      stateRecordRef.current = {}
+    },
+    []
+  )
 
   const mainClasses = classnames(
     'dnb-forms-field-block',

diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx
@@ -124,91 +124,90 @@ function ArrayComponent(props: Props) {
   const { getNextContainerMode } = useSwitchContainerMode()
 
   const elementData = useMemo(() => {
-    return ((valueWhileClosingRef.current || arrayValue) ?? []).map(
-      (value, index) => {
-        const id = idsRef.current[index] || makeUniqueId()
+    const list = valueWhileClosingRef.current || arrayValue
+    return (list ?? []).map((value, index) => {
+      const id = idsRef.current[index] || makeUniqueId()
 
-        const hasNewItems =
-          arrayValue?.length > valueCountRef.current?.length
+      const hasNewItems =
+        arrayValue?.length > valueCountRef.current?.length
 
-        if (!idsRef.current[index]) {
-          isNewRef.current[id] = hasNewItems
-          idsRef.current.push(id)
-        }
-
-        const isNew = isNewRef.current[id] || false
-        if (!modesRef.current[id]) {
-          modesRef.current[id] = isNew
-            ? getNextContainerMode() ?? 'edit'
-            : 'view'
-        }
-        const containerModeForAllItems =
-          typeof containerMode === 'function'
-            ? containerMode(arrayValue)
-            : containerMode
-        if (containerModeForAllItems) {
-          modesRef.current[id] = containerModeForAllItems
-        }
-
-        return {
-          id,
-          path,
-          value,
-          index,
-          arrayValue,
-          containerRef,
-          isNew,
-          containerMode: modesRef.current[id],
-          initialContainerMode: modesRef.current[id],
-          switchContainerMode: (mode: ContainerMode) => {
-            modesRef.current[id] = mode
-            delete isNewRef.current?.[id]
-            forceUpdate()
-          },
-          handleChange: (path: Path, value: unknown) => {
-            const newArrayValue = structuredClone(arrayValue)
-
-            // Make sure we have a new object reference,
-            // else two new objects will be the same
-            newArrayValue[index] = { ...newArrayValue[index] }
-
-            pointer.set(newArrayValue, path, value)
-            handleChange(newArrayValue)
-          },
-          handlePush: (element: unknown) => {
-            hadPushRef.current = true
-            handleChange([...(arrayValue || []), element])
-          },
-          handleRemove: ({ keepItems = false } = {}) => {
-            if (keepItems) {
-              // Add a backup as the array value while animating
-              valueWhileClosingRef.current = arrayValue
-            }
+      if (!idsRef.current[index]) {
+        isNewRef.current[id] = hasNewItems
+        idsRef.current.push(id)
+      }
 
-            const newArrayValue = structuredClone(arrayValue)
-            newArrayValue.splice(index, 1)
-            handleChange(newArrayValue)
-          },
-
-          // - Called after animation end
-          fulfillRemove: () => {
-            valueWhileClosingRef.current = null
-            delete modesRef.current?.[id]
-            delete isNewRef.current?.[id]
-            const findIndex = idsRef.current.indexOf(id)
-            idsRef.current.splice(findIndex, 1)
-            forceUpdate()
-          },
-
-          // - Called when cancel button press
-          restoreOriginalValue: (value: unknown) => {
-            const newArrayValue = structuredClone(arrayValue)
-            newArrayValue[index] = value
-            handleChange(newArrayValue)
-          },
-        } as IterateItemContextState
+      const isNew = isNewRef.current[id] || false
+      if (!modesRef.current[id]) {
+        modesRef.current[id] = isNew
+          ? getNextContainerMode() ?? 'edit'
+          : 'view'
       }
-    )
+      const containerModeForAllItems =
+        typeof containerMode === 'function'
+          ? containerMode(list)
+          : containerMode
+      if (containerModeForAllItems) {
+        modesRef.current[id] = containerModeForAllItems
+      }
+
+      return {
+        id,
+        path,
+        value,
+        index,
+        arrayValue,
+        containerRef,
+        isNew,
+        containerMode: modesRef.current[id],
+        initialContainerMode: modesRef.current[id],
+        switchContainerMode: (mode: ContainerMode) => {
+          modesRef.current[id] = mode
+          delete isNewRef.current?.[id]
+          forceUpdate()
+        },
+        handleChange: (path: Path, value: unknown) => {
+          const newArrayValue = structuredClone(arrayValue)
+
+          // Make sure we have a new object reference,
+          // else two new objects will be the same
+          newArrayValue[index] = { ...newArrayValue[index] }
+
+          pointer.set(newArrayValue, path, value)
+          handleChange(newArrayValue)
+        },
+        handlePush: (element: unknown) => {
+          hadPushRef.current = true
+          handleChange([...(arrayValue || []), element])
+        },
+        handleRemove: ({ keepItems = false } = {}) => {
+          if (keepItems) {
+            // Add a backup as the array value while animating
+            valueWhileClosingRef.current = arrayValue
+          }
+
+          const newArrayValue = structuredClone(arrayValue)
+          newArrayValue.splice(index, 1)
+          handleChange(newArrayValue)
+        },
+
+        // - Called after animation end
+        fulfillRemove: () => {
+          valueWhileClosingRef.current = null
+          delete modesRef.current?.[id]
+          delete isNewRef.current?.[id]
+          const findIndex = idsRef.current.indexOf(id)
+          idsRef.current.splice(findIndex, 1)
+          forceUpdate()
+        },
+
+        // - Called when cancel button press
+        restoreOriginalValue: (value: unknown) => {
+          const newArrayValue = structuredClone(arrayValue)
+          newArrayValue[index] = value
+          handleChange(newArrayValue)
+        },
+      } as IterateItemContextState
+    })
 
     // In order to update "valueWhileClosingRef" we need to have "salt" in the deps array
     // eslint-disable-next-line react-hooks/exhaustive-deps

diff --git a/...ufemia/src/extensions/forms/Iterate/EditContainer/__tests__/EditAndViewContainer.test.tsx b/...ufemia/src/extensions/forms/Iterate/EditContainer/__tests__/EditAndViewContainer.test.tsx
@@ -400,7 +400,9 @@ describe('EditContainer and ViewContainer', () => {
             <Iterate.Array
               path="/"
               containerMode={(items) => {
-                return items.length === 1 ? 'initial-open' : undefined
+                if (items?.length === 1) {
+                  return 'initial-open'
+                }
               }}
             >
               <Iterate.ViewContainer>View Content</Iterate.ViewContainer>
@@ -429,24 +431,32 @@ describe('EditContainer and ViewContainer', () => {
         }
 
         render(
-          <Form.Handler data={['foo']}>
+          <Form.Handler>
             <Iterate.Array
               path="/"
+              defaultValue={[null]}
               containerMode={(items) => {
-                return items.length === 1 ? 'initial-open' : undefined
+                if (items?.length === 1) {
+                  return 'initial-open'
+                }
               }}
             >
               <Iterate.ViewContainer>View Content</Iterate.ViewContainer>
-              <Iterate.EditContainer>Edit Content</Iterate.EditContainer>
+              <Iterate.EditContainer>
+                <Field.String itemPath="/" required />
+              </Iterate.EditContainer>
               <ContextConsumer />
             </Iterate.Array>
 
-            <Iterate.PushButton path="/" pushValue={'bar'} />
+            <Iterate.PushButton path="/" pushValue={null} />
           </Form.Handler>
         )
 
         expect(containerMode[0]).toBe('edit')
 
+        const input = document.querySelector('input')
+        await userEvent.type(input, 'foo')
+
         await userEvent.click(
           document.querySelector('button.dnb-forms-iterate-push-button')
         )
@@ -457,13 +467,14 @@ describe('EditContainer and ViewContainer', () => {
         expect(blocks).toHaveLength(4)
         const [, secondBlock] = blocks
 
-        expect(containerMode[0]).toBe('view')
         expect(containerMode[1]).toBe('edit')
 
         await userEvent.click(secondBlock.querySelector('button'))
 
-        expect(containerMode[0]).toBe('view')
-        expect(containerMode[1]).toBe('edit')
+        await waitFor(() => {
+          expect(containerMode[0]).toBe('view')
+          expect(containerMode[1]).toBe('edit')
+        })
       })
 
       it('should keep first item in "edit" mode when there is an error', async () => {
@@ -477,21 +488,23 @@ describe('EditContainer and ViewContainer', () => {
         }
 
         render(
-          <Form.Handler data={['']}>
+          <Form.Handler data={[null]}>
             <Iterate.Array
               path="/"
               containerMode={(items) => {
-                return items.length === 1 ? 'initial-open' : undefined
+                if (items?.length === 1) {
+                  return 'initial-open'
+                }
               }}
             >
               <Iterate.ViewContainer>View Content</Iterate.ViewContainer>
               <Iterate.EditContainer>
-                <Field.String required itemPath="/" />
+                <Field.String itemPath="/" required />
               </Iterate.EditContainer>
               <ContextConsumer />
             </Iterate.Array>
 
-            <Iterate.PushButton path="/" pushValue={'bar'} />
+            <Iterate.PushButton path="/" pushValue={null} />
           </Form.Handler>
         )
 

diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/PushButton.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/PushButton/PushButton.tsx
@@ -36,7 +36,7 @@ function PushButton(props: Props) {
     path,
   })
 
-  const handleClick = useCallback(() => {
+  const handleClick = useCallback(async () => {
     const newValue =
       typeof pushValue === 'function' ? pushValue(value) : pushValue
 
@@ -45,10 +45,12 @@ function PushButton(props: Props) {
       handlePush(newValue)
     } else {
       // If not inside an iterate, it could still manipulate a source data set through useFieldProps
-      handlePathChange?.(path, [...(value ?? []), newValue])
+      await handlePathChange?.(path, [...(value ?? []), newValue])
     }
 
-    setLastItemContainerMode('view')
+    setTimeout(() => {
+      setLastItemContainerMode('view')
+    }, 100) // UX improvement because of the "openDelay"
   }, [
     handlePathChange,
     handlePush,