fix(FieldBlock): label can be clicked after focusing input (#3190)

Fixes #2979 The label was an internal component and was re-initialized on each re-render of FieldBlock. Fixed by removing the internal component and instead just generating the props. --------- Co-authored-by: Tobias Høegh <[email protected]>
dnbexperience · Jan 9, 2024 · a759d8f · a759d8f
1 parent 16ed821
commit a759d8f
Show file tree

Hide file tree

Showing 9 changed files with 244 additions and 31 deletions.
diff --git a/packages/dnb-design-system-portal/src/docs/uilib/layout/flex/container/info.mdx b/packages/dnb-design-system-portal/src/docs/uilib/layout/flex/container/info.mdx
@@ -28,6 +28,27 @@ You may else wrap your custom component in a `Flex.Item` – this way, you still
 
 Technically, `Flex.Container` checks if a nested component has a property called `_supportsSpacingProps`. So if you have a component that supports the [spacing properties](/uilib/layout/space/), you can add this property `ComponentName._supportsSpacingProps = true`.
 
+If the component is a wrapper component, and you want its children to support spacing, you can add this property `ComponentName._supportsSpacingProps = 'children'`.
+
+But for simplicity, you rather can use the HOC `Flex.withChildren`:
+
+```tsx
+const MyComponent = Flex.withChildren(() => {
+  return (
+    <div>
+      <ShouldGetSpacing />
+      <ShouldGetSpacing />
+    </div>
+  )
+})
+
+render(
+  <Flex.Container>
+    <MyComponent />
+  </Flex.Container>,
+)
+```
+
 ### Horizontal and Vertical aliases
 
 For shortening the usage of `direction="..."`, you can use:

diff --git a/packages/dnb-eufemia/src/components/flex/Container.tsx b/packages/dnb-eufemia/src/components/flex/Container.tsx
@@ -5,6 +5,7 @@ import { Hr } from '../../elements'
 import useMedia from '../../shared/useMedia'
 import {
   getSpaceValue,
+  getSpacingPropsChildren,
   isHeadingElement,
   renderWithSpacing,
 } from './utils'
@@ -94,7 +95,10 @@ function FlexContainer(props: Props) {
     ...rest
   } = props
 
-  const childrenArray = React.Children.toArray(children)
+  const firstChild = React.Children.toArray(children)?.[0]
+  const childrenArray = React.Children.toArray(
+    getSpacingPropsChildren(firstChild) || children
+  )
   const hasHeading = childrenArray.some((child, i) => {
     const previousChild = childrenArray?.[i - 1]
     return (

diff --git a/packages/dnb-eufemia/src/components/flex/__tests__/Container.test.tsx b/packages/dnb-eufemia/src/components/flex/__tests__/Container.test.tsx
@@ -312,21 +312,8 @@ describe('Flex.Container', () => {
     expect(children[2].className).toContain('dnb-space__right--zero')
   })
 
-  it('should set element', () => {
-    render(<Flex.Container element="section">content</Flex.Container>)
-
-    const element = document.querySelector('.dnb-flex-container')
-
-    expect(element.tagName).toBe('SECTION')
-  })
-
   it('should not add a wrapper when _supportsSpacingProps is given', () => {
-    const { rerender } = render(
-      <Flex.Vertical>
-        <Flex.Item>content</Flex.Item>
-        <Flex.Item>content</Flex.Item>
-      </Flex.Vertical>
-    )
+    const { rerender } = render(<></>)
 
     const TestComponent = (props: SpaceProps) => {
       const cn = createSpacingClasses(props)
@@ -383,6 +370,130 @@ describe('Flex.Container', () => {
     }
   })
 
+  it('should transform children of children if _supportsSpacingProps is given', () => {
+    const { rerender } = render(<></>)
+
+    const Wrapper = ({ children }) => {
+      return <div className="wrapper">{children}</div>
+    }
+
+    const TestComponent = (props: SpaceProps) => {
+      const cn = createSpacingClasses(props)
+      cn.push('test-item')
+      return <div className={cn.join(' ')}>content</div>
+    }
+
+    {
+      rerender(
+        <Flex.Vertical>
+          <Wrapper>
+            <TestComponent />
+            <TestComponent top="large" />
+          </Wrapper>
+        </Flex.Vertical>
+      )
+
+      const elements = document.querySelectorAll(
+        '.dnb-flex-container > div'
+      )
+      expect(elements[0].className).toBe(
+        'dnb-space dnb-space__top--zero dnb-space__bottom--zero'
+      )
+      expect((elements[0].firstChild as HTMLElement).className).toBe(
+        'wrapper'
+      )
+    }
+
+    {
+      Wrapper._supportsSpacingProps = 'children'
+
+      rerender(
+        <Flex.Vertical>
+          <Wrapper>
+            <TestComponent />
+            <TestComponent top="large" />
+          </Wrapper>
+        </Flex.Vertical>
+      )
+
+      const elements = document.querySelectorAll(
+        '.dnb-flex-container > div'
+      )
+      expect((elements[0].firstChild as HTMLElement).className).toBe(
+        'test-item'
+      )
+      expect(elements[0].className).toBe(
+        'dnb-space dnb-space__top--zero dnb-space__bottom--zero'
+      )
+      expect(elements[1].className).toBe(
+        'dnb-space dnb-space__top--large dnb-space__bottom--zero'
+      )
+    }
+
+    {
+      TestComponent._supportsSpacingProps = true
+
+      rerender(
+        <Flex.Vertical>
+          <Wrapper>
+            <TestComponent />
+            <TestComponent top="x-large" />
+          </Wrapper>
+        </Flex.Vertical>
+      )
+
+      const elements = document.querySelectorAll(
+        '.dnb-flex-container > div'
+      )
+      expect(elements[0].className).toBe(
+        'dnb-space__top--zero dnb-space__bottom--zero test-item'
+      )
+      expect(elements[1].className).toBe(
+        'dnb-space__top--x-large dnb-space__bottom--zero test-item'
+      )
+      expect((elements[0].firstChild as HTMLElement).className).toBeFalsy()
+      expect((elements[1].firstChild as HTMLElement).className).toBeFalsy()
+    }
+
+    {
+      TestComponent._supportsSpacingProps = true
+
+      const Wrapper = Flex.withChildren(function MyWrapper({ children }) {
+        return <div className="wrapper">{children}</div>
+      })
+
+      rerender(
+        // render(
+        <Flex.Vertical>
+          <Wrapper>
+            <TestComponent />
+            <TestComponent top="x-large" />
+          </Wrapper>
+        </Flex.Vertical>
+      )
+
+      const elements = document.querySelectorAll(
+        '.dnb-flex-container > div'
+      )
+      expect(elements[0].className).toBe(
+        'dnb-space__top--zero dnb-space__bottom--zero test-item'
+      )
+      expect(elements[1].className).toBe(
+        'dnb-space__top--x-large dnb-space__bottom--zero test-item'
+      )
+      expect((elements[0].firstChild as HTMLElement).className).toBeFalsy()
+      expect((elements[1].firstChild as HTMLElement).className).toBeFalsy()
+    }
+  })
+
+  it('should set custom element', () => {
+    render(<Flex.Container element="section">content</Flex.Container>)
+
+    const element = document.querySelector('.dnb-flex-container')
+
+    expect(element.tagName).toBe('SECTION')
+  })
+
   it('gets valid ref element', () => {
     let ref: React.RefObject<HTMLInputElement>
 

diff --git a/packages/dnb-eufemia/src/components/flex/export.ts b/packages/dnb-eufemia/src/components/flex/export.ts
@@ -3,3 +3,4 @@ export { default as Item } from './Item'
 export { default as Stack } from './Stack'
 export { default as Horizontal } from './Horizontal'
 export { default as Vertical } from './Vertical'
+export { default as withChildren } from './withChildren'
diff --git a/packages/dnb-eufemia/src/components/flex/utils.tsx b/packages/dnb-eufemia/src/components/flex/utils.tsx
@@ -63,6 +63,15 @@ export const isSpacePropsComponent = (
   )
 }
 
+export const getSpacingPropsChildren = (child: React.ReactNode) => {
+  if (
+    React.isValidElement(child) &&
+    child?.type?.['_supportsSpacingProps'] === 'children'
+  ) {
+    return child?.props?.children
+  }
+}
+
 export const renderWithSpacing = (
   element: React.ReactNode,
   props: SpacingProps & { key?: string; className?: string }

diff --git a/packages/dnb-eufemia/src/components/flex/withChildren.tsx b/packages/dnb-eufemia/src/components/flex/withChildren.tsx
@@ -0,0 +1,14 @@
+import React from 'react'
+
+type WithChildrenProps = {
+  children?: React.ReactNode
+}
+
+function withChildren<T>(
+  Component: React.ComponentType<T>
+): React.ComponentType<T & WithChildrenProps> {
+  Component['_supportsSpacingProps'] = 'children'
+  return Component
+}
+
+export default withChildren
diff --git a/packages/dnb-eufemia/src/extensions/forms/FieldBlock/FieldBlock.tsx b/packages/dnb-eufemia/src/extensions/forms/FieldBlock/FieldBlock.tsx
@@ -4,6 +4,7 @@ import { Space, FormLabel, FormStatus } from '../../../components'
 import { FormError, ComponentProps, FieldProps } from '../types'
 import FieldBlockContext from './FieldBlockContext'
 import { findElementInChildren } from '../../../shared/component-helper'
+import type { FormLabelAllProps } from '../../../components/FormLabel'
 
 export type Props = Pick<
   FieldProps,
@@ -146,18 +147,12 @@ function FieldBlock(props: Props) {
     ? 'info'
     : null
 
-  const Label = ({ children }) => {
-    return (
-      <FormLabel
-        element={enableFieldset ? 'legend' : 'label'}
-        forId={enableFieldset ? undefined : forId}
-        space={{ top: 0, bottom: 'x-small' }}
-        size={size}
-        disabled={disabled}
-      >
-        {children}
-      </FormLabel>
-    )
+  const labelProps: FormLabelAllProps = {
+    element: enableFieldset ? 'legend' : 'label',
+    forId: enableFieldset ? undefined : forId,
+    space: { top: 0, bottom: 'x-small' },
+    size,
+    disabled,
   }
 
   return (
@@ -176,14 +171,14 @@ function FieldBlock(props: Props) {
           {labelDescription || labelSecondary ? (
             <div className="dnb-forms-field-block__label">
               {label || labelDescription ? (
-                <Label>
+                <FormLabel {...labelProps}>
                   {label}
                   {labelDescription && (
                     <span className="dnb-forms-field-block__label-description">
                       {labelDescription}
                     </span>
                   )}
-                </Label>
+                </FormLabel>
               ) : (
                 <>&nbsp;</>
               )}
@@ -194,7 +189,7 @@ function FieldBlock(props: Props) {
               )}
             </div>
           ) : (
-            label && <Label>{label}</Label>
+            label && <FormLabel {...labelProps}>{label}</FormLabel>
           )}
 
           <div

diff --git a/packages/dnb-eufemia/src/extensions/forms/FieldBlock/__tests__/FieldBlock.test.tsx b/packages/dnb-eufemia/src/extensions/forms/FieldBlock/__tests__/FieldBlock.test.tsx
@@ -1,8 +1,10 @@
 import React from 'react'
-import { render } from '@testing-library/react'
+import { render, waitFor } from '@testing-library/react'
 import { Input } from '../../../../components'
 import FieldBlock from '../FieldBlock'
 import { FormError } from '../../types'
+import userEvent from '@testing-library/user-event'
+import { useDataValue } from '../../hooks'
 
 describe('FieldBlock', () => {
   it('should forward HTML attributes', () => {
@@ -144,6 +146,34 @@ describe('FieldBlock', () => {
     expect(labelElement.textContent).toBe('A Secondary Label')
   })
 
+  it('click on label should set focus on input after value change', async () => {
+    const MockComponent = () => {
+      const fromInput = React.useCallback(({ value }) => value, [])
+      const { value, handleChange } = useDataValue({
+        value: '',
+        fromInput,
+      })
+
+      return (
+        <FieldBlock label="Label" forId="unique">
+          <Input id="unique" value={value} on_change={handleChange} />
+        </FieldBlock>
+      )
+    }
+
+    render(<MockComponent />)
+
+    const label = document.querySelector('label')
+    const input = document.querySelector('input')
+
+    await userEvent.type(input, 'foo')
+    await userEvent.click(label)
+
+    await waitFor(() => {
+      expect(input).toHaveFocus()
+    })
+  })
+
   it('should not use fieldset/legend elements when no label is given', () => {
     render(
       <FieldBlock>

diff --git a/packages/dnb-eufemia/src/extensions/forms/FieldBlock/stories/FieldBlock.stories.tsx b/packages/dnb-eufemia/src/extensions/forms/FieldBlock/stories/FieldBlock.stories.tsx
@@ -0,0 +1,28 @@
+import React, { useCallback } from 'react'
+import FieldBlock from '../FieldBlock'
+import Input from '../../../../components/Input'
+import { useDataValue } from '../../hooks'
+
+export default {
+  title: 'Eufemia/Extensions/Forms/FieldBlock',
+}
+
+export function FieldBlockLabel() {
+  const fromInput = useCallback(({ value }) => value, [])
+  const { value, handleChange, handleFocus, handleBlur } = useDataValue({
+    value: 'foo',
+    fromInput,
+  })
+
+  return (
+    <FieldBlock label="Label" forId="unique">
+      <Input
+        id="unique"
+        value={value}
+        on_change={handleChange}
+        on_focus={handleFocus}
+        on_blur={handleBlur}
+      />
+    </FieldBlock>
+  )
+}