Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0c203b0

Browse files
authored
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 stylesas 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 wereactually *correct overall*, but something with our MUI+Tailwind setupcreated conflicting styles, and we accidentally introduced an underlinestyle that shouldn't be there- Removed the Aria label because I've realized in the past year thatAria is really easy to misuse, and it's best just to do things with thebase HTML features as much as possible. There's a risk that the old codehad compliance issues with certain types of screen readers (even thoughit worked fine when I did manual testing back in 2023). These changeshopefully remove those risks completely
1 parent1ffc5a0 commit0c203b0

File tree

3 files changed

+50
-39
lines changed

3 files changed

+50
-39
lines changed

‎site/src/components/Abbr/Abbr.stories.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ const meta: Meta<typeof Abbr> = {
66
component:Abbr,
77
decorators:[
88
(Story)=>(
9-
<>
9+
<divclassName="max-w-prose text-base">
1010
<p>Try the following text out in a screen reader!</p>
1111
<Story/>
12-
</>
12+
</div>
1313
),
1414
],
1515
};
@@ -25,9 +25,9 @@ export const InlinedShorthand: Story = {
2525
},
2626
decorators:[
2727
(Story)=>(
28-
<pclassName="max-w-2xl">
28+
<p>
2929
The physical pain of getting bonked on the head with a cartoon mallet
30-
lasts precisely 593{" "}
30+
lasts precisely 593
3131
<spanclassName="underline decoration-dotted">
3232
<Story/>
3333
</span>

‎site/src/components/Abbr/Abbr.test.tsx‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,14 @@
11
import{render,screen}from"@testing-library/react";
2-
import{Abbr,typePronunciation}from"./Abbr";
2+
import{Abbr}from"./Abbr";
33

44
typeAbbreviationData={
55
abbreviation:string;
66
title:string;
77
expectedLabel:string;
88
};
99

10-
typeAssertionInput=AbbreviationData&{
11-
pronunciation:Pronunciation;
12-
};
13-
14-
functionassertAccessibleLabel({
15-
abbreviation,
16-
title,
17-
expectedLabel,
18-
pronunciation,
19-
}:AssertionInput){
20-
const{ unmount}=render(
21-
<Abbrtitle={title}pronunciation={pronunciation}>
22-
{abbreviation}
23-
</Abbr>,
24-
);
25-
26-
screen.getByLabelText(expectedLabel,{selector:"abbr"});
27-
unmount();
28-
}
29-
3010
describe(Abbr.name,()=>{
31-
it("Has an aria-label that equals the titleifthe abbreviation is shorthand",()=>{
11+
it("Omits abbreviation from screen-reader outputifit is shorthand",()=>{
3212
constsampleShorthands:AbbreviationData[]=[
3313
{
3414
abbreviation:"ms",
@@ -43,11 +23,22 @@ describe(Abbr.name, () => {
4323
];
4424

4525
for(constshorthandofsampleShorthands){
46-
assertAccessibleLabel({ ...shorthand,pronunciation:"shorthand"});
26+
const{ unmount}=render(
27+
<Abbrtitle={shorthand.title}pronunciation="shorthand">
28+
{shorthand.abbreviation}
29+
</Abbr>,
30+
);
31+
32+
// The <abbr> element doesn't have any ARIA role semantics baked in,
33+
// so we have to get a little bit more creative with making sure the
34+
// expected content is on screen in an accessible way
35+
constelement=screen.getByTitle(shorthand.title);
36+
expect(element).toHaveTextContent(shorthand.expectedLabel);
37+
unmount();
4738
}
4839
});
4940

50-
it("Has an aria label with title and 'flattened' pronunciation if abbreviation is acronym",()=>{
41+
it("Adds title and 'flattened' pronunciation if abbreviation is acronym",()=>{
5142
constsampleAcronyms:AbbreviationData[]=[
5243
{
5344
abbreviation:"NASA",
@@ -67,11 +58,19 @@ describe(Abbr.name, () => {
6758
];
6859

6960
for(constacronymofsampleAcronyms){
70-
assertAccessibleLabel({ ...acronym,pronunciation:"acronym"});
61+
const{ unmount}=render(
62+
<Abbrtitle={acronym.title}pronunciation="acronym">
63+
{acronym.abbreviation}
64+
</Abbr>,
65+
);
66+
67+
constelement=screen.getByTitle(acronym.title);
68+
expect(element).toHaveTextContent(acronym.expectedLabel);
69+
unmount();
7170
}
7271
});
7372

74-
it("Has an aria label with title and initialized pronunciation if abbreviation is initialism",()=>{
73+
it("Adds title and initialized pronunciation if abbreviation is initialism",()=>{
7574
constsampleInitialisms:AbbreviationData[]=[
7675
{
7776
abbreviation:"FBI",
@@ -91,7 +90,15 @@ describe(Abbr.name, () => {
9190
];
9291

9392
for(constinitialismofsampleInitialisms){
94-
assertAccessibleLabel({ ...initialism,pronunciation:"initialism"});
93+
const{ unmount}=render(
94+
<Abbrtitle={initialism.title}pronunciation="initialism">
95+
{initialism.abbreviation}
96+
</Abbr>,
97+
);
98+
99+
constelement=screen.getByTitle(initialism.title);
100+
expect(element).toHaveTextContent(initialism.expectedLabel);
101+
unmount();
95102
}
96103
});
97104
});

‎site/src/components/Abbr/Abbr.tsx‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
importtype{FC,HTMLAttributes}from"react";
22
import{cn}from"utils/cn";
33

4-
exporttypePronunciation="shorthand"|"acronym"|"initialism";
4+
typePronunciation="shorthand"|"acronym"|"initialism";
55

66
typeAbbrProps=HTMLAttributes<HTMLElement>&{
77
children:string;
88
title:string;
99
pronunciation?:Pronunciation;
10+
className?:string;
1011
};
1112

1213
/**
@@ -22,23 +23,26 @@ export const Abbr: FC<AbbrProps> = ({
2223
children,
2324
title,
2425
pronunciation="shorthand",
26+
className,
2527
...delegatedProps
2628
})=>{
2729
return(
2830
<abbr
29-
// Title attributes usually aren't natively available to screen readers;
30-
// always have to supplement with aria-label
31+
// Adding title to make things a little easier for sighted users,
32+
// but titles aren't always exposed via screen readers. Still have
33+
// to inject the actual text content inside the abbr itself
3134
title={title}
32-
aria-label={getAccessibleLabel(children,title,pronunciation)}
3335
className={cn(
34-
"decoration-inherit",
35-
children===children.toUpperCase()
36-
?"tracking-wide"
37-
:"tracking-normal",
36+
"no-underline tracking-normal",
37+
children===children.toUpperCase()&&"tracking-wide",
38+
className,
3839
)}
3940
{...delegatedProps}
4041
>
4142
<spanaria-hidden>{children}</span>
43+
<spanclassName="sr-only">
44+
{getAccessibleLabel(children,title,pronunciation)}
45+
</span>
4246
</abbr>
4347
);
4448
};

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp