Skip to content

Semantic HTML improvements #343

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

Open
wants to merge 8 commits into
base: feature/react-18-useid
Choose a base branch
from
Open
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
24 changes: 14 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
@@ -83,6 +83,9 @@ import 'react-accessible-accordion/dist/fancy-example.css';
We recommend that you copy them into your own app and modify them to suit your
needs, particularly if you're using your own `className`s.
The accordion trigger is built using native button and heading elements which
have default browser styling, these can be overridden in your stylesheet.
## Component API
### Accordion
@@ -137,9 +140,9 @@ Class(es) to apply to the 'heading' element.
#### aria-level : `number` [*optional*, default: `3`]
Semantics to apply to the 'heading' element. A value of `1` would make your
heading element hierarchically equivalent to an `<h1>` tag, and likewise a value
of `6` would make it equivalent to an `<h6>` tag.
Will determine which 'heading' element is used in the markup. A value of `1`
would make your element an `<h1>` tag, and likewise a value of `6` would make it
an `<h6>` tag.
### AccordionItemButton
@@ -185,7 +188,8 @@ you, including:
- Applying appropriate aria attributes (`aria-expanded`, `aria-controls`,
`aria-disabled`, `aria-hidden` and `aria-labelledby`).
- Applying appropriate `role` attributes (`button`, `heading`, `region`).
- Applying appropriate `role` attributes (`region`).
- Using semantic HTML elements (`h1` - `h6`, `button`).
- Applying appropriate `tabindex` attributes.
- Applying keyboard interactivity ('space', 'end', 'tab', 'up', 'down', 'home'
and 'end' keys).
@@ -196,10 +200,10 @@ spec-compliant:
- Only ever use
[phrasing content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Phrasing_content)
inside of your `AccordionItemHeading` component. If in doubt, use text only.
- Always provide an `aria-level` prop to your `AccordionItemHeading`
component, _especially_ if you are nesting accordions. This attribute is a
signal used by assistive technologies (eg. screenreaders) to determine which
heading level (ie. `h1`-`h6`) to treat your heading as.
- Remember to provide an `aria-level` prop to your `AccordionItemHeading`
component, when you are nesting accordions. The levels are used by assistive
technologies (eg. screenreaders) to infer structure, by default each heading
uses `h3` .
If you have any questions about your implementation, then please don't be afraid
to get in touch via our
@@ -225,8 +229,8 @@ description, as written above. By "accordion-like", we mean components which
have collapsible items but require bespoke interactive mechanisms in order to
expand, collapse and 'disable' them. This includes (but is not limited to)
multi-step forms, like those seen in many cart/checkout flows, which we believe
require (other) complex markup in order to be considered 'accessible'.
This also includes disclosure widgets.
require (other) complex markup in order to be considered 'accessible'. This also
includes disclosure widgets.
### How do I disable an item?
5 changes: 4 additions & 1 deletion demo/src/main.css
Original file line number Diff line number Diff line change
@@ -134,11 +134,14 @@ footer {
margin-top: 10px;
}
.code__button {
font-weight: bold;
cursor: pointer;
display: inline-block;
padding: 10px 0;
color: var(--colorPageLinks);
background-color: transparent;
font: inherit;
font-weight: bold;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI guess there's a question of how styled this should be. Are our current buttons bold? https://react-accessible-accordion.springload.co.nz/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the Show code button css, yep its bold, I just moved the rule down to be with the other text ones

border: none;
text-decoration: underline solid var(--colorGreenShadow);
}
.code__button:hover {
40 changes: 7 additions & 33 deletions integration/wai-aria.spec.js
Original file line number Diff line number Diff line change
@@ -237,42 +237,16 @@ describe('WAI ARIA Spec', () => {
});

describe('WAI-ARIA Roles, States, and Properties', () => {
it(`The title of each accordion header is contained in an element with
role button.`, async () => {
// TODO: Use 'title' elements inside the headings.

const { browser, page, buttonsHandles } = await setup();
expect(buttonsHandles).toHaveLength(3);
for (const buttonHandle of buttonsHandles) {
expect(
await page.evaluate(
(button) => button.getAttribute('role'),
buttonHandle,
),
).toBe('button');
}
});

it(`Each accordion header button is wrapped in an element with role
heading that has a value set for aria-level that is appropriate for
the information architecture of the page.`, async () => {
const { browser, page, buttonsHandles } = await setup();
expect(buttonsHandles).toHaveLength(3);
for (const buttonHandle of buttonsHandles) {
expect(
await page.evaluate(
(button) => button.parentElement.getAttribute('role'),
buttonHandle,
),
).toBe('heading');

it(`The title of each accordion header is contained in a heading tag`, async () => {
const { browser, page, headingsHandles } = await setup();
expect(headingsHandles).toHaveLength(3);
for (const headingsHandle of headingsHandles) {
expect(
await page.evaluate(
(button) =>
button.parentElement.getAttribute('aria-level'),
buttonHandle,
(heading) => heading.tagName,
headingsHandle,
),
).toBeTruthy();
).toContain('H3');
}
});

8 changes: 0 additions & 8 deletions src/components/AccordionContext.tsx
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
import * as React from 'react';
import AccordionStore, {
InjectedButtonAttributes,
InjectedHeadingAttributes,
InjectedPanelAttributes,
} from '../helpers/AccordionStore';
import { ID } from './ItemContext';
@@ -28,7 +27,6 @@ export interface AccordionContext {
uuid: ID,
dangerouslySetExpanded?: boolean,
): InjectedPanelAttributes;
getHeadingAttributes(uuid: ID): InjectedHeadingAttributes;
getButtonAttributes(
uuid: ID,
dangerouslySetExpanded?: boolean,
@@ -78,11 +76,6 @@ export class Provider extends React.PureComponent<
return this.state.getPanelAttributes(key, dangerouslySetExpanded);
};

getHeadingAttributes = (): InjectedHeadingAttributes => {
// uuid: UUID
return this.state.getHeadingAttributes();
};

getButtonAttributes = (
key: ID,
dangerouslySetExpanded?: boolean,
@@ -102,7 +95,6 @@ export class Provider extends React.PureComponent<
isItemDisabled: this.isItemDisabled,
isItemExpanded: this.isItemExpanded,
getPanelAttributes: this.getPanelAttributes,
getHeadingAttributes: this.getHeadingAttributes,
getButtonAttributes: this.getButtonAttributes,
}}
>
16 changes: 8 additions & 8 deletions src/components/AccordionItemButton.tsx
Original file line number Diff line number Diff line change
@@ -7,12 +7,12 @@ import {
focusPreviousSiblingOf,
} from '../helpers/focus';
import keycodes from '../helpers/keycodes';
import { DivAttributes } from '../helpers/types';
import { assertValidHtmlId } from '../helpers/id';
import { ButtonAttributes } from '../helpers/types';

import { Consumer as ItemConsumer, ItemContext } from './ItemContext';

type Props = DivAttributes & {
type Props = ButtonAttributes & {
toggleExpanded(): void;
};

@@ -21,7 +21,9 @@ const AccordionItemButton = ({
className = 'accordion__button',
...rest
}: Props) => {
const handleKeyPress = (evt: React.KeyboardEvent<HTMLDivElement>): void => {
const handleKeyPress = (
evt: React.KeyboardEvent<HTMLButtonElement>,
): void => {
const keyCode = evt.key;

if (
@@ -74,11 +76,9 @@ const AccordionItemButton = ({
}

return (
<div
<button
className={className}
{...rest}
role="button"
tabIndex={0}
onClick={toggleExpanded}
onKeyDown={handleKeyPress}
data-accordion-component="AccordionItemButton"
@@ -87,8 +87,8 @@ const AccordionItemButton = ({
};

type WrapperProps = Pick<
DivAttributes,
Exclude<keyof DivAttributes, keyof InjectedButtonAttributes>
ButtonAttributes,
Exclude<keyof ButtonAttributes, keyof InjectedButtonAttributes>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to rename all these 👍

>;

const AccordionItemButtonWrapper: React.SFC<WrapperProps> = (
33 changes: 33 additions & 0 deletions src/components/AccordionItemHeading.spec.tsx
Original file line number Diff line number Diff line change
@@ -59,6 +59,39 @@ describe('AccordionItem', () => {
});
});

describe('aria-level prop', () => {
it('is h3 by default', () => {
const { getByTestId } = render(
<Accordion>
<AccordionItem>
<AccordionItemHeading data-testid={UUIDS.FOO}>
<AccordionItemButton />
</AccordionItemHeading>
</AccordionItem>
</Accordion>,
);

expect(getByTestId(UUIDS.FOO).tagName).toEqual('H3');
});

it('can be overridden', () => {
const { getByTestId } = render(
<Accordion>
<AccordionItem>
<AccordionItemHeading
data-testid={UUIDS.FOO}
aria-level={4}
>
<AccordionItemButton />
</AccordionItemHeading>
</AccordionItem>
</Accordion>,
);

expect(getByTestId(UUIDS.FOO).tagName).toEqual('H4');
});
});

describe('children prop', () => {
it('is respected', () => {
const { getByText } = render(
80 changes: 38 additions & 42 deletions src/components/AccordionItemHeading.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
import * as React from 'react';
import { InjectedHeadingAttributes } from '../helpers/AccordionStore';
import DisplayName from '../helpers/DisplayName';
import { DivAttributes } from '../helpers/types';
import { assertValidHtmlId } from '../helpers/id';
import { HeadingAttributes } from '../helpers/types';

import { Consumer as ItemConsumer, ItemContext } from './ItemContext';

type Props = DivAttributes;

const defaultProps = {
className: 'accordion__heading',
'aria-level': 3,
};
interface AccordionItemHeadingProps extends HeadingAttributes {
className?: string;
'aria-level'?: number;
}

export const SPEC_ERROR = `AccordionItemButton may contain only one child element, which must be an instance of AccordionItemButton.
@@ -21,12 +16,30 @@ From the WAI-ARIA spec (https://www.w3.org/TR/wai-aria-practices-1.1/#accordion)
`;

export class AccordionItemHeading extends React.PureComponent<Props> {
static defaultProps: typeof defaultProps = defaultProps;
const Heading = React.forwardRef<HTMLHeadingElement, AccordionItemHeadingProps>(
(
{
'aria-level': ariaLevel = 3,
className = 'accordion__heading',
...props
}: AccordionItemHeadingProps,
ref,
) => {
const headingTag = `h${ariaLevel}`;
return React.createElement(headingTag, {
className,
...props,
ref,
'data-accordion-component': 'AccordionItemHeading',
});
},
);
Heading.displayName = 'Heading';

ref: HTMLDivElement | undefined;
export class AccordionItemHeading extends React.PureComponent<AccordionItemHeadingProps> {
ref: HTMLHeadingElement | undefined;

static VALIDATE(ref: HTMLDivElement | undefined): void | never {
static VALIDATE(ref: HTMLHeadingElement | undefined): void | never {
if (ref === undefined) {
throw new Error('ref is undefined');
}
@@ -43,7 +56,7 @@ export class AccordionItemHeading extends React.PureComponent<Props> {
}
}

setRef = (ref: HTMLDivElement): void => {
setRef = (ref: HTMLHeadingElement): void => {
this.ref = ref;
};

@@ -56,36 +69,19 @@ export class AccordionItemHeading extends React.PureComponent<Props> {
}

render(): JSX.Element {
return (
<div
data-accordion-component="AccordionItemHeading"
{...this.props}
ref={this.setRef}
/>
);
return <Heading ref={this.setRef} {...this.props} />;
}
}

type WrapperProps = Pick<
DivAttributes,
Exclude<keyof DivAttributes, keyof InjectedHeadingAttributes>
>;

const AccordionItemHeadingWrapper: React.SFC<DivAttributes> = (
props: WrapperProps,
): JSX.Element => (
<ItemConsumer>
{(itemContext: ItemContext): JSX.Element => {
const { headingAttributes } = itemContext;

if (props.id) {
assertValidHtmlId(props.id);
}

return <AccordionItemHeading {...props} {...headingAttributes} />;
}}
</ItemConsumer>
);
const AccordionItemHeadingWrapper: React.FC<AccordionItemHeadingProps> = (
props: AccordionItemHeadingProps,
): JSX.Element => {
if (props.id) {
assertValidHtmlId(props.id);
}

return <AccordionItemHeading {...props} />;
};

AccordionItemHeadingWrapper.displayName = DisplayName.AccordionItemHeading;

4 changes: 0 additions & 4 deletions src/components/ItemContext.tsx
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
import * as React from 'react';
import {
InjectedButtonAttributes,
InjectedHeadingAttributes,
InjectedPanelAttributes,
} from '../helpers/AccordionStore';
import {
@@ -30,7 +29,6 @@ export type ItemContext = {
expanded: boolean;
disabled: boolean;
panelAttributes: InjectedPanelAttributes;
headingAttributes: InjectedHeadingAttributes;
buttonAttributes: InjectedButtonAttributes;
toggleExpanded(): void;
};
@@ -57,7 +55,6 @@ const Provider = ({
uuid,
dangerouslySetExpanded,
);
const headingAttributes = accordionContext.getHeadingAttributes(uuid);
const buttonAttributes = accordionContext.getButtonAttributes(
uuid,
dangerouslySetExpanded,
@@ -71,7 +68,6 @@ const Provider = ({
disabled,
toggleExpanded: toggleExpanded,
panelAttributes,
headingAttributes,
buttonAttributes,
}}
>
7 changes: 6 additions & 1 deletion src/css/fancy-example.css
Original file line number Diff line number Diff line change
@@ -11,7 +11,10 @@
.accordion__item + .accordion__item {
border-top: 1px solid rgba(0, 0, 0, 0.1);
}

.accordion__heading {
margin: 0;
font: inherit;
}
.accordion__button {
background-color: #f4f4f4;
color: #444;
@@ -20,6 +23,8 @@
width: 100%;
text-align: left;
border: none;
font: inherit;
margin: 0;
}

.accordion__button:hover {
15 changes: 0 additions & 15 deletions src/helpers/AccordionStore.ts
Original file line number Diff line number Diff line change
@@ -8,17 +8,11 @@ export interface InjectedPanelAttributes {
hidden: boolean | undefined;
}

export interface InjectedHeadingAttributes {
role: string;
}

export interface InjectedButtonAttributes {
id: string;
'aria-controls': string;
'aria-expanded': boolean;
'aria-disabled': boolean;
role: string;
tabIndex: number;
}

export default class AccordionStore {
@@ -96,13 +90,6 @@ export default class AccordionStore {
};
};

public readonly getHeadingAttributes = (): // uuid: UUID,
InjectedHeadingAttributes => {
return {
role: 'heading',
};
};

public readonly getButtonAttributes = (
uuid: ID,
dangerouslySetExpanded?: boolean,
@@ -115,8 +102,6 @@ export default class AccordionStore {
'aria-disabled': disabled,
'aria-expanded': expanded,
'aria-controls': this.getPanelId(uuid),
role: 'button',
tabIndex: 0,
};
};

2 changes: 2 additions & 0 deletions src/helpers/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as React from 'react';

export type DivAttributes = React.HTMLAttributes<HTMLDivElement>;
export type ButtonAttributes = React.HTMLAttributes<HTMLButtonElement>;
export type HeadingAttributes = React.HTMLAttributes<HTMLHeadingElement>;