Skip to content

Commit

Permalink
fix(FieldBlock): enhance fieldset/legend detection
Browse files Browse the repository at this point in the history
  • Loading branch information
tujoworker committed Nov 15, 2023
1 parent d7ffcf8 commit 5a62449
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,35 @@ function MyComponent(props: ComponentAllProps) {
}
```

### Form element support with `pickFormElementProps`
### "Form element" components

Form elements, like input, checkbox, slider etc. do support some form element properties. In order to support them, you can use `pickFormElementProps`, so only valid properties will effected the component.
Form elements, like input, checkbox, slider etc. should include some extra functionality in order to be used in various situations.

Basically, components we would place inside a HTML `<form>` element.

**Label vs fieldset/legend**

They should be declared as a form element:

```tsx
FormComponent._formElement = true
```

This helps e.g. to detect automated determination of label vs fieldset/legend.

**Spacing**

And they should be declared to support spacing props as well:

```tsx
FormComponent._supportsSpacingProps = true
```

This is needed in order to fully support [Flex](/uilib/layout/flex/) layouts.

#### Usage of `pickFormElementProps`

In order to support form element props, such as `vertical` or `labelDirection`, you can use `pickFormElementProps`, so only valid properties will effected the component.

```tsx
import { Context } from '../../shared'
Expand All @@ -161,14 +187,14 @@ const defaultProps = {
myParam: 'value',
}

function MyComponent(props: Types) {
function FormComponent(props: Types) {
const context = React.useContext(Context)

const { myParam, skeleton, ...rest } = extendPropsWithContext(
props,
defaultProps,
pickFormElementProps(context?.formElement)
context.MyComponent,
context.FormComponent,
)

// Use myParam and spread the ...rest
Expand Down
40 changes: 27 additions & 13 deletions packages/dnb-eufemia/src/extensions/forms/FieldBlock/FieldBlock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import classnames from 'classnames'
import { Space, FormLabel, FormStatus } from '../../../components'
import { FormError, ComponentProps, FieldProps } from '../types'
import FieldBlockContext from './FieldBlockContext'
import { findElementInChildren } from '../../../shared/component-helper'
import {
findElementInChildren,
warn,
} from '../../../shared/component-helper'

export type Props = Pick<
FieldProps,
Expand Down Expand Up @@ -128,17 +131,28 @@ function FieldBlock(props: Props) {
)

// A child component with a label was found, use fieldset/legend instead of div/label
const enableFieldset = useMemo(
() =>
label &&
(asFieldset ||
(!nestedFieldBlockContext &&
findElementInChildren(
children,
(child: React.ReactElement) => child.props.label
))),
[]
)
const enableFieldset = useMemo(() => {
let result = asFieldset

if (label && !result && !nestedFieldBlockContext) {
findElementInChildren(children, (child: React.ReactElement) => {
if (
typeof child?.props?.label !== 'undefined' ||
child?.type?.['_formElement'] === true
) {
return (result = true)
}
})
}

return Boolean(result)
}, [children])

if (forId && enableFieldset) {
warn(
`You may not use forId="${forId}" as there where given several form elements as children.`
)
}

const state = error || warning || info
const stateStatus = error
Expand All @@ -153,7 +167,7 @@ function FieldBlock(props: Props) {
return (
<FormLabel
element={enableFieldset ? 'legend' : 'label'}
for_id={enableFieldset ? undefined : forId}
forId={enableFieldset ? undefined : forId}
space={{ top: 0, bottom: 'x-small' }}
size={size}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,29 @@ describe('FieldBlock', () => {
)
})

it('should warn when "forId" and several form elements where given', () => {
const warn = jest.spyOn(console, 'log')

render(
<FieldBlock forId="invalid" label="A Label">
<MockComponent label="Label" id="foo" />
<MockComponent label="Label" id="bar" />
</FieldBlock>
)

expect(warn).toHaveBeenCalledTimes(1)
expect(warn).toHaveBeenCalledWith(
expect.anything(),
expect.stringContaining('forId="invalid"')
)

expect(document.querySelectorAll('fieldset')).toHaveLength(1)
expect(document.querySelectorAll('legend')).toHaveLength(1)
expect(document.querySelectorAll('label')).toHaveLength(2)

warn.mockRestore()
})

it('should render a "label"', () => {
render(<FieldBlock label="A Label">content</FieldBlock>)

Expand Down Expand Up @@ -182,6 +205,69 @@ describe('FieldBlock', () => {
expect(labelElements[4]).toBe(undefined)
})

it('should use fieldset/legend elements when nested component has a label property', () => {
const { rerender } = render(
<FieldBlock label="Legend">
<MockComponent label="Label" />
<MockComponent label="Label" />
</FieldBlock>
)

expect(document.querySelectorAll('fieldset')).toHaveLength(1)
expect(document.querySelectorAll('legend')).toHaveLength(1)
expect(document.querySelector('legend')).not.toHaveAttribute('for')
expect(document.querySelectorAll('label')).toHaveLength(2)

rerender(
<FieldBlock label="Legend">
<MockComponent label="Label" />
<MockComponent />
</FieldBlock>
)

expect(document.querySelectorAll('fieldset')).toHaveLength(1)
expect(document.querySelectorAll('legend')).toHaveLength(1)
expect(document.querySelectorAll('label')).toHaveLength(1)

rerender(
<FieldBlock label="Legend">
<MockComponent />
</FieldBlock>
)

expect(document.querySelector('fieldset')).not.toBeInTheDocument()
expect(document.querySelector('legend')).not.toBeInTheDocument()
expect(document.querySelectorAll('label')).toHaveLength(1)
})

it('should use fieldset/legend when _formElement is given', () => {
MockComponent._formElement = true

const { rerender } = render(
<FieldBlock label="Legend">
<MockComponent />
<MockComponent />
</FieldBlock>
)

expect(document.querySelectorAll('fieldset')).toHaveLength(1)
expect(document.querySelectorAll('legend')).toHaveLength(1)
expect(document.querySelector('legend')).not.toHaveAttribute('for')

delete MockComponent._formElement

rerender(
<FieldBlock label="Legend">
<MockComponent />
<MockComponent />
</FieldBlock>
)

expect(document.querySelector('fieldset')).not.toBeInTheDocument()
expect(document.querySelector('legend')).not.toBeInTheDocument()
expect(document.querySelectorAll('label')).toHaveLength(1)
})

it('should use fieldset/legend when "asFieldset" is given', () => {
render(
<FieldBlock label="Legend" asFieldset>
Expand Down Expand Up @@ -300,3 +386,13 @@ describe('FieldBlock', () => {
expect(element.classList).toContain('custom-class')
})
})

function MockComponent({ label = null, id = null }) {
return (
<>
{label && <label htmlFor={id}>{label}</label>}
<input id={id} />
</>
)
}
MockComponent._formElement = null

0 comments on commit 5a62449

Please sign in to comment.