Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(FieldBlock): enhance fieldset/legend detection #2902

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -2095,5 +2095,5 @@ class AutocompleteInstance extends React.PureComponent {
}

Autocomplete.HorizontalItem = DrawerList.HorizontalItem

Autocomplete._formElement = true
Autocomplete._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/button/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,5 @@ Content.defaultProps = {
isIconOnly: null,
}

Button._formElement = true
Button._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/checkbox/Checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,5 @@ CheckIcon.defaultProps = {
size: 'default',
}

CheckIcon._formElement = true
CheckIcon._supportsSpacingProps = true
Original file line number Diff line number Diff line change
Expand Up @@ -740,4 +740,5 @@ export default class DatePicker extends React.PureComponent {
}
}

DatePicker._formElement = true
DatePicker._supportsSpacingProps = true
2 changes: 1 addition & 1 deletion packages/dnb-eufemia/src/components/dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -698,5 +698,5 @@ class DropdownInstance extends React.PureComponent {
}

Dropdown.HorizontalItem = DrawerList.HorizontalItem

Dropdown._formElement = true
Dropdown._supportsSpacingProps = true
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,5 @@ export default function FormLabel(localProps: FormLabelAllProps) {
return <Element {...params}>{text || children}</Element>
}

FormLabel._formElement = true
FormLabel._supportsSpacingProps = true
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ InputMasked.defaultProps = {
on_submit_blur: null,
}

InputMasked._formElement = true
InputMasked._supportsSpacingProps = true

export default InputMasked
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,6 @@ function MultiInputMaskInput<T extends string>({
}

export default MultiInputMask

MultiInputMask._formElement = true
MultiInputMask._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/input/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -777,4 +777,5 @@ InputIcon.propTypes = {
]).isRequired,
}

Input._formElement = true
Input._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/input/InputPassword.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,5 @@ export default class InputPassword extends React.PureComponent {
}
}

InputPassword._formElement = true
InputPassword._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/radio/Radio.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,4 +461,5 @@ export default class Radio extends React.PureComponent {
}
}

Radio._formElement = true
Radio._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/radio/RadioGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,4 +294,5 @@ export default class RadioGroup extends React.PureComponent {
}
}

RadioGroup._formElement = true
RadioGroup._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/slider/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function Slider(localProps: SliderAllProps) {
)
}

Slider._formElement = true
Slider._supportsSpacingProps = true

export default Slider
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/switch/Switch.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,4 +363,5 @@ export default class Switch extends React.PureComponent {
}
}

Switch._formElement = true
Switch._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/tag/Tag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ const Tag = (localProps: TagProps & SpacingProps) => {

Tag.Group = TagGroup

Tag._formElement = true
Tag._supportsSpacingProps = true

export default Tag
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/textarea/Textarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,4 +543,5 @@ export default class Textarea extends React.PureComponent {
}
}

Textarea._formElement = true
Textarea._supportsSpacingProps = true
Original file line number Diff line number Diff line change
Expand Up @@ -501,4 +501,5 @@ export default class ToggleButton extends React.PureComponent {
}
}

ToggleButton._formElement = true
ToggleButton._supportsSpacingProps = true
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,5 @@ export default class ToggleButtonGroup extends React.PureComponent {
}
}

ToggleButtonGroup._formElement = true
ToggleButtonGroup._supportsSpacingProps = true
1 change: 1 addition & 0 deletions packages/dnb-eufemia/src/components/upload/Upload.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ const Upload = (localProps: UploadAllProps) => {

Upload.useUpload = useUpload

Upload._formElement = true
Upload._supportsSpacingProps = true

export default Upload
61 changes: 48 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,13 @@ 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 = useEnableFieldset({
label,
forId,
asFieldset,
children,
nestedFieldBlockContext,
})

const state = error || warning || info
const stateStatus = error
Expand All @@ -153,7 +152,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 Expand Up @@ -232,5 +231,41 @@ function FieldBlock(props: Props) {
)
}

function useEnableFieldset({
label,
forId,
asFieldset,
children,
nestedFieldBlockContext,
}) {
return useMemo(() => {
let result = asFieldset

if (label && !result && !nestedFieldBlockContext) {
let count = 0

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

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

return Boolean(result)
}, [children])
}

FieldBlock._supportsSpacingProps = true
export default FieldBlock
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,30 @@ describe('FieldBlock', () => {
)
})

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

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

expect(console.log).toHaveBeenCalledTimes(1)
expect(console.log).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)

console.log = orig
})

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

Expand Down Expand Up @@ -182,6 +206,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" forId="unique">
<MockComponent label="Label" />
<MockComponent id="unique" />
</FieldBlock>
)

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

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 +387,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
Loading