Skip to content

Commit

Permalink
Use native fieldset instead of div by default for <Fieldset />
Browse files Browse the repository at this point in the history
…component (#3237)

* improve TypeScript types for `Fieldset` component

* use `fieldset` instead of `div` by default

* only apply `role="group"` when not using a native `fieldset`

* apply `disabled` attribute

This is necessary if we want to make use of the default fieldset tag
(which also disables native form elements)

* adjust tests reflecting new changes

* conditionally apply props based on rendered element

* add `useResolvedTag` hook

This allows us to compute the `tag` name of a component. We can use a
shortcut based on the `props.as` and/or the `DEFAULT_XXX_TAG` of a
component. If this is not known/passed, then we compute it based on the
`ref` instead which requires an actual re-render.

* use `useResolvedTag` hook

* reflect change in `Field` related test

* update changelog

* inline variable
  • Loading branch information
RobinMalfait authored May 24, 2024
1 parent 8c3499c commit f740050
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 12 deletions.
4 changes: 4 additions & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Mark `SwitchGroup` as deprecated, prefer `Field` instead ([#3232](https://github.com/tailwindlabs/headlessui/pull/3232))

### Changed

- Use native `fieldset` instead of `div` by default for `<Fieldset />` component ([#3237](https://github.com/tailwindlabs/headlessui/pull/3237))

## [2.0.3] - 2024-05-07

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Rendering', () => {
let fieldset = container.firstChild
let field = fieldset?.firstChild

expect(fieldset).toHaveAttribute('aria-disabled', 'true')
expect(fieldset).toHaveAttribute('disabled')
expect(field).toHaveAttribute('aria-disabled', 'true')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,24 @@ describe('Rendering', () => {

let fieldset = container.firstChild

expect(fieldset).toBeInstanceOf(HTMLFieldSetElement)
expect(fieldset).not.toHaveAttribute('role', 'group')
})

it('should render a `Fieldset` using a custom component', async () => {
let { container } = render(
<Fieldset as="span">
<input />
</Fieldset>
)

let fieldset = container.firstChild

expect(fieldset).toBeInstanceOf(HTMLSpanElement)
expect(fieldset).toHaveAttribute('role', 'group')
})

it('should add an `aria-disabled` attribute when disabling the `Fieldset`', async () => {
it('should forward the `disabled` attribute when disabling the `Fieldset`', async () => {
let { container } = render(
<Fieldset disabled>
<input />
Expand All @@ -34,10 +48,33 @@ describe('Rendering', () => {

let fieldset = container.firstChild

expect(fieldset).toHaveAttribute('role', 'group')
expect(fieldset).toHaveAttribute('disabled')
})

it('should add an `aria-disabled` attribute when disabling the `Fieldset` when using another element via the `as` prop', async () => {
let { container } = render(
<Fieldset as="span" disabled>
<input />
</Fieldset>
)

let fieldset = container.firstChild

expect(fieldset).toHaveAttribute('aria-disabled', 'true')
})

it('should make nested inputs disabled when the fieldset is disabled', async () => {
let { container } = render(
<Fieldset disabled>
<input />
</Fieldset>
)

let fieldset = container.firstChild

expect(fieldset?.firstChild).toBeDisabled()
})

it('should link a `Fieldset` to a nested `Legend`', async () => {
let { container } = render(
<Fieldset>
Expand Down
29 changes: 20 additions & 9 deletions packages/@headlessui-react/src/components/fieldset/fieldset.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
'use client'

import React, { useMemo, type ElementType, type Ref } from 'react'
import { useResolvedTag } from '../../hooks/use-resolved-tag'
import { useSyncRefs } from '../../hooks/use-sync-refs'
import { DisabledProvider, useDisabled } from '../../internal/disabled'
import type { Props } from '../../types'
import { forwardRefWithAs, render, type HasDisplayName } from '../../utils/render'
import { useLabels } from '../label/label'

let DEFAULT_FIELDSET_TAG = 'div' as const
let DEFAULT_FIELDSET_TAG = 'fieldset' as const

type FieldsetRenderPropArg = {}
type FieldsetPropsWeControl = 'aria-controls'
type FieldsetPropsWeControl = 'aria-labelledby' | 'aria-disabled' | 'role'

export type FieldsetProps<TTag extends ElementType = typeof DEFAULT_FIELDSET_TAG> = Props<
TTag,
Expand All @@ -27,17 +29,26 @@ function FieldsetFn<TTag extends ElementType = typeof DEFAULT_FIELDSET_TAG>(
let providedDisabled = useDisabled()
let { disabled = providedDisabled || false, ...theirProps } = props

let [tag, resolveTag] = useResolvedTag(props.as ?? DEFAULT_FIELDSET_TAG)
let fieldsetRef = useSyncRefs(ref, resolveTag)

let [labelledBy, LabelProvider] = useLabels()

let slot = useMemo(() => ({ disabled }) satisfies FieldsetRenderPropArg, [disabled])

let ourProps = {
ref,
role: 'group',

'aria-labelledby': labelledBy,
'aria-disabled': disabled || undefined,
}
let ourProps =
tag === 'fieldset'
? {
ref: fieldsetRef,
'aria-labelledby': labelledBy,
disabled: disabled || undefined,
}
: {
ref: fieldsetRef,
role: 'group',
'aria-labelledby': labelledBy,
'aria-disabled': disabled || undefined,
}

return (
<DisabledProvider value={disabled}>
Expand Down
33 changes: 33 additions & 0 deletions packages/@headlessui-react/src/hooks/use-resolved-tag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { useCallback, useState } from 'react'

/**
* Resolve the actual rendered tag of a DOM node. If the `tag` provided is
* already a string we can use that as-is. This will happen when the `as` prop is
* not used or when it's used with a string value.
*
* If an actual component is used, then we need to do some more work because
* then we actually need to render the component to know what the tag name is.
*/
export function useResolvedTag<T extends React.ElementType>(tag: T) {
let tagName = typeof tag === 'string' ? tag : undefined
let [resolvedTag, setResolvedTag] = useState<string | undefined>(tagName)

return [
// The resolved tag name
tagName ?? resolvedTag,

// This callback should be passed to the `ref` of a component
useCallback(
(ref: any) => {
// Tag name is already known and it's a string, no need to re-render
if (tagName) return

if (ref instanceof HTMLElement) {
// Tag name is not known yet, render the component to find out
setResolvedTag(ref.tagName.toLowerCase())
}
},
[tagName]
),
] as const
}

0 comments on commit f740050

Please sign in to comment.