Skip to content

Commit

Permalink
feat!: Change Accordion section boolean prop to sectionAs enum (#…
Browse files Browse the repository at this point in the history
…1244)

Co-authored-by: Vincent Smedinga <[email protected]>
  • Loading branch information
alimpens and VincentSmedinga authored May 31, 2024
1 parent 0c8d0a4 commit fef3fb1
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 31 deletions.
8 changes: 4 additions & 4 deletions packages/react/src/Accordion/Accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import { useKeyboardFocus } from '../common/useKeyboardFocus'
export type AccordionProps = {
/** The hierarchical level of the Accordion Section heading(s) within the document. */
headingLevel: HeadingLevel
/** Whether to use a ‘section’ element for each Accordion Section. */
section?: boolean
/** The HTML element to use for each Accordion Section. */
sectionAs?: 'div' | 'section'
} & PropsWithChildren<HTMLAttributes<HTMLDivElement>>

const AccordionRoot = forwardRef(
({ children, className, headingLevel, section = true }: AccordionProps, ref: ForwardedRef<HTMLDivElement>) => {
({ children, className, headingLevel, sectionAs = 'section' }: AccordionProps, ref: ForwardedRef<HTMLDivElement>) => {
const innerRef = useRef<HTMLDivElement>(null)

// use a passed ref if it's there, otherwise use innerRef
Expand All @@ -28,7 +28,7 @@ const AccordionRoot = forwardRef(
const { keyDown } = useKeyboardFocus(innerRef, { rotating: true })

return (
<AccordionContext.Provider value={{ headingLevel: headingLevel, section: section }}>
<AccordionContext.Provider value={{ headingLevel: headingLevel, sectionAs: sectionAs }}>
<div className={clsx('ams-accordion', className)} onKeyDown={keyDown} ref={innerRef}>
{children}
</div>
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/Accordion/AccordionContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import { HeadingLevel } from '../Heading/Heading'

export type AccordionContextValue = {
headingLevel: HeadingLevel
section?: boolean
sectionAs?: 'div' | 'section'
}

const defaultValues: AccordionContextValue = {
headingLevel: 1,
section: true,
sectionAs: 'section',
}

const AccordionContext = createContext(defaultValues)
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/Accordion/AccordionSection.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ describe('Accordion section', () => {
expect(sectionQuery).toBeInTheDocument()
})

it('does not render a section HTML tag when the section prop is false', () => {
it('does not render a section HTML tag when the sectionAs is "div"', () => {
render(
<Accordion headingLevel={1} section={false}>
<Accordion headingLevel={1} sectionAs="div">
<Accordion.Section label={testLabel}>{testContent}</Accordion.Section>
</Accordion>,
)
Expand Down
27 changes: 9 additions & 18 deletions packages/react/src/Accordion/AccordionSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ export const AccordionSection = forwardRef(
{ label, expanded = false, children, className, ...otherProps }: AccordionSectionProps,
ref: ForwardedRef<HTMLDivElement>,
) => {
const { headingLevel, section } = useContext(AccordionContext)
const { headingLevel, sectionAs } = useContext(AccordionContext)
const [isExpanded, setIsExpanded] = useState(expanded)

const HeadingX = getHeadingElement(headingLevel)
const Tag = sectionAs || 'section'
const id = useId()
const buttonId = `button-${id}`
const panelId = `panel-${id}`
Expand All @@ -46,23 +47,13 @@ export const AccordionSection = forwardRef(
{label}
</button>
</HeadingX>
{section ? (
<section
id={panelId}
aria-labelledby={buttonId}
className={clsx('ams-accordion__panel', { 'ams-accordion__panel--expanded': isExpanded })}
>
{children}
</section>
) : (
<div
id={panelId}
aria-labelledby={buttonId}
className={clsx('ams-accordion__panel', { 'ams-accordion__panel--expanded': isExpanded })}
>
{children}
</div>
)}
<Tag
aria-labelledby={buttonId}
className={clsx('ams-accordion__panel', { 'ams-accordion__panel--expanded': isExpanded })}
id={panelId}
>
{children}
</Tag>
</div>
)
},
Expand Down
18 changes: 14 additions & 4 deletions storybook/src/components/Accordion/Accordion.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,24 @@ If you want the contents of an accordion section to appear initially, pass the `

By default, an accordion section uses the HTML tag `section` to render the content.
Having many accordion sections on your page (more than 6) can lead to too many landmark regions, confusing screen reader users.
In that case, use `section={false}`.
Please note: Storybook does not correctly display this property in the code block.
In that case, use `sectionAs="div"`.

<Canvas of={AccordionStories.TooManyLandmarks} />

## Technical explanation
## Technical considerations

Currently (28-6-2023), the HTML element `details` element has some accessibility problems.
### The `Accordion` parent component

The parent component is used for several reasons:

- It sets the spacing between Accordion Sections.
- It allows you to set the same heading level for all Accordion Sections.
- It allows you to set the same HTML element for all Accordion Sections.
- It adds the extra keyboard navigation described in the guidelines.

### The HTML `details` element

Currently (28-6-2023), the `details` element has some accessibility problems.
[For example, a heading in a summary is not adequately understood by screen readers](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary#summaries_as_headings) and
NVDA does not read Chrome and Edge’s collapsed and expanded status correctly.
That’s why we chose not to use this native element.
Expand Down
2 changes: 1 addition & 1 deletion storybook/src/components/Accordion/Accordion.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const ExpandedByDefault: Story = {

export const TooManyLandmarks: Story = {
args: {
section: false,
sectionAs: 'div',
children: [
<Accordion.Section key={1} label="Eerste sectie">
<Paragraph>{paragraph1}</Paragraph>
Expand Down

0 comments on commit fef3fb1

Please sign in to comment.