fix: correct markup for Abbr component (#19317)

Fixes some accidental styling issues introduced in #19242

## Changes made
- Updated styles
- Added support for `className` prop so that we can override the styles
as needed
- Removed the aria-label in favor of injecting the main text directly

## Notes
- This feels like a case where the changes in the previous PR were
actually *correct overall*, but something with our MUI+Tailwind setup
created conflicting styles, and we accidentally introduced an underline
style that shouldn't be there
- Removed the Aria label because I've realized in the past year that
Aria is really easy to misuse, and it's best just to do things with the
base HTML features as much as possible. There's a risk that the old code
had compliance issues with certain types of screen readers (even though
it worked fine when I did manual testing back in 2023). These changes
hopefully remove those risks completely
This commit is contained in:
Michael Smith
2025-08-14 00:20:00 -04:00
committed by GitHub
parent 1ffc5a0e97
commit 0c203b0cf8
3 changed files with 50 additions and 39 deletions
+4 -4
View File
@@ -6,10 +6,10 @@ const meta: Meta<typeof Abbr> = {
component: Abbr,
decorators: [
(Story) => (
<>
<div className="max-w-prose text-base">
<p>Try the following text out in a screen reader!</p>
<Story />
</>
</div>
),
],
};
@@ -25,9 +25,9 @@ export const InlinedShorthand: Story = {
},
decorators: [
(Story) => (
<p className="max-w-2xl">
<p>
The physical pain of getting bonked on the head with a cartoon mallet
lasts precisely 593{" "}
lasts precisely 593
<span className="underline decoration-dotted">
<Story />
</span>
+34 -27
View File
@@ -1,5 +1,5 @@
import { render, screen } from "@testing-library/react";
import { Abbr, type Pronunciation } from "./Abbr";
import { Abbr } from "./Abbr";
type AbbreviationData = {
abbreviation: string;
@@ -7,28 +7,8 @@ type AbbreviationData = {
expectedLabel: string;
};
type AssertionInput = AbbreviationData & {
pronunciation: Pronunciation;
};
function assertAccessibleLabel({
abbreviation,
title,
expectedLabel,
pronunciation,
}: AssertionInput) {
const { unmount } = render(
<Abbr title={title} pronunciation={pronunciation}>
{abbreviation}
</Abbr>,
);
screen.getByLabelText(expectedLabel, { selector: "abbr" });
unmount();
}
describe(Abbr.name, () => {
it("Has an aria-label that equals the title if the abbreviation is shorthand", () => {
it("Omits abbreviation from screen-reader output if it is shorthand", () => {
const sampleShorthands: AbbreviationData[] = [
{
abbreviation: "ms",
@@ -43,11 +23,22 @@ describe(Abbr.name, () => {
];
for (const shorthand of sampleShorthands) {
assertAccessibleLabel({ ...shorthand, pronunciation: "shorthand" });
const { unmount } = render(
<Abbr title={shorthand.title} pronunciation="shorthand">
{shorthand.abbreviation}
</Abbr>,
);
// The <abbr> element doesn't have any ARIA role semantics baked in,
// so we have to get a little bit more creative with making sure the
// expected content is on screen in an accessible way
const element = screen.getByTitle(shorthand.title);
expect(element).toHaveTextContent(shorthand.expectedLabel);
unmount();
}
});
it("Has an aria label with title and 'flattened' pronunciation if abbreviation is acronym", () => {
it("Adds title and 'flattened' pronunciation if abbreviation is acronym", () => {
const sampleAcronyms: AbbreviationData[] = [
{
abbreviation: "NASA",
@@ -67,11 +58,19 @@ describe(Abbr.name, () => {
];
for (const acronym of sampleAcronyms) {
assertAccessibleLabel({ ...acronym, pronunciation: "acronym" });
const { unmount } = render(
<Abbr title={acronym.title} pronunciation="acronym">
{acronym.abbreviation}
</Abbr>,
);
const element = screen.getByTitle(acronym.title);
expect(element).toHaveTextContent(acronym.expectedLabel);
unmount();
}
});
it("Has an aria label with title and initialized pronunciation if abbreviation is initialism", () => {
it("Adds title and initialized pronunciation if abbreviation is initialism", () => {
const sampleInitialisms: AbbreviationData[] = [
{
abbreviation: "FBI",
@@ -91,7 +90,15 @@ describe(Abbr.name, () => {
];
for (const initialism of sampleInitialisms) {
assertAccessibleLabel({ ...initialism, pronunciation: "initialism" });
const { unmount } = render(
<Abbr title={initialism.title} pronunciation="initialism">
{initialism.abbreviation}
</Abbr>,
);
const element = screen.getByTitle(initialism.title);
expect(element).toHaveTextContent(initialism.expectedLabel);
unmount();
}
});
});
+12 -8
View File
@@ -1,12 +1,13 @@
import type { FC, HTMLAttributes } from "react";
import { cn } from "utils/cn";
export type Pronunciation = "shorthand" | "acronym" | "initialism";
type Pronunciation = "shorthand" | "acronym" | "initialism";
type AbbrProps = HTMLAttributes<HTMLElement> & {
children: string;
title: string;
pronunciation?: Pronunciation;
className?: string;
};
/**
@@ -22,23 +23,26 @@ export const Abbr: FC<AbbrProps> = ({
children,
title,
pronunciation = "shorthand",
className,
...delegatedProps
}) => {
return (
<abbr
// Title attributes usually aren't natively available to screen readers;
// always have to supplement with aria-label
// Adding title to make things a little easier for sighted users,
// but titles aren't always exposed via screen readers. Still have
// to inject the actual text content inside the abbr itself
title={title}
aria-label={getAccessibleLabel(children, title, pronunciation)}
className={cn(
"decoration-inherit",
children === children.toUpperCase()
? "tracking-wide"
: "tracking-normal",
"no-underline tracking-normal",
children === children.toUpperCase() && "tracking-wide",
className,
)}
{...delegatedProps}
>
<span aria-hidden>{children}</span>
<span className="sr-only">
{getAccessibleLabel(children, title, pronunciation)}
</span>
</abbr>
);
};