Skip to content

Commit 4f771ca

Browse files
committed
applying changes from code review
1 parent f95bb3d commit 4f771ca

File tree

3 files changed

+57
-64
lines changed

3 files changed

+57
-64
lines changed

lib/rules/swatchpicker-needs-labelling.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,14 @@ import { makeLabeledControlRule } from "../util/ruleFactory";
99
//------------------------------------------------------------------------------
1010

1111
export default ESLintUtils.RuleCreator.withoutDocs(
12-
makeLabeledControlRule(
13-
{
14-
component: "SwatchPicker",
15-
labelProps: ["aria-label"],
16-
allowFieldParent: true,
17-
allowFor: false,
18-
allowLabelledBy: true,
19-
allowWrappingLabel: false
20-
},
21-
"noUnlabeledSwatchPicker",
22-
"Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc.."
23-
)
12+
makeLabeledControlRule({
13+
component: "SwatchPicker",
14+
labelProps: ["aria-label"],
15+
allowFieldParent: true,
16+
allowFor: false,
17+
allowLabelledBy: true,
18+
allowWrappingLabel: false,
19+
messageId: "noUnlabeledSwatchPicker",
20+
description: "Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc.."
21+
})
2422
);

lib/util/ruleFactory.ts

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ import { JSXOpeningElement } from "estree-jsx";
1111
export type LabeledControlConfig = {
1212
component: string | RegExp;
1313
labelProps: string[]; // e.g. ["label", "aria-label"]
14-
allowFieldParent?: boolean; // e.g. <Field label=...><RadioGroup/></Field>
15-
allowFor?: boolean; // htmlFor
16-
allowLabelledBy?: boolean; // aria-labelledby
17-
allowWrappingLabel?: boolean; // <label>...</label>
14+
allowFieldParent: boolean; // e.g. <Field label=...><RadioGroup/></Field>
15+
allowFor: boolean; // htmlFor
16+
allowLabelledBy: boolean; // aria-labelledby
17+
allowWrappingLabel: boolean; // <label>...</label>
18+
messageId: string;
19+
description: string;
1820
};
1921

2022
/**
@@ -80,27 +82,21 @@ export function hasAccessibleLabel(node: TSESTree.JSXOpeningElement, context: an
8082
* ```
8183
*
8284
* @param config - See `hasAccessibleLabel` for the configuration fields and semantics.
83-
* @param messageId - The message key used in `meta.messages` (e.g., "missingLabel").
84-
* @param description - Human-readable rule description and the text displayed for `messageId`.
8585
* @returns An ESLint `RuleModule` that reports when the configured component lacks an accessible label.
8686
*/
87-
export function makeLabeledControlRule(
88-
config: LabeledControlConfig,
89-
messageId: string,
90-
description: string
91-
): TSESLint.RuleModule<string, []> {
87+
export function makeLabeledControlRule(config: LabeledControlConfig): TSESLint.RuleModule<string, []> {
9288
return {
9389
meta: {
94-
type: "problem" as const,
95-
messages: { [messageId]: description },
90+
type: "problem",
91+
messages: { [config.messageId]: config.description },
9692
docs: {
97-
description,
98-
recommended: "strict" as const, // not `true`
93+
description: config.description,
94+
recommended: "strict",
9995
url: "https://www.w3.org/TR/html-aria/"
10096
},
10197
schema: []
10298
},
103-
defaultOptions: [] as const,
99+
defaultOptions: [],
104100

105101
create(context: TSESLint.RuleContext<string, []>) {
106102
return {
@@ -112,7 +108,7 @@ export function makeLabeledControlRule(
112108
if (!matches) return;
113109

114110
if (!hasAccessibleLabel(node, context, config)) {
115-
context.report({ node, messageId });
111+
context.report({ node, messageId: config.messageId });
116112
}
117113
}
118114
};

tests/lib/rules/utils/ruleFactory.test.ts

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ describe("hasAccessibleLabel (unit)", () => {
6161
allowFieldParent: true,
6262
allowFor: true,
6363
allowLabelledBy: true,
64-
allowWrappingLabel: true
64+
allowWrappingLabel: true,
65+
messageId: "errorMsg",
66+
description: "anything"
6567
};
6668

6769
test("returns false when no heuristics pass", () => {
@@ -119,25 +121,22 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
119121
allowFieldParent: true,
120122
allowFor: true,
121123
allowLabelledBy: true,
122-
allowWrappingLabel: true
124+
allowWrappingLabel: true,
125+
messageId: "noUnlabeledRadioGroup",
126+
description: "Accessibility: RadioGroup must have a programmatic and visual label."
123127
};
124128

125-
const messageId = "noUnlabeledRadioGroup";
126-
127-
const description = "Accessibility: RadioGroup must have a programmatic and visual label.";
128-
129129
// 1) No heuristics -> report
130130
describe("reports when component matches and no label heuristics pass", () => {
131131
beforeEach(() => resetAllMocksToFalse());
132-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
132+
const rule = makeLabeledControlRule(baseCfg);
133133

134134
ruleTester.run("no-unlabeled-radio-group", rule as unknown as Rule.RuleModule, {
135135
valid: [],
136136
invalid: [
137137
{
138-
filename: "example.tsx",
139138
code: `<RadioGroup />`,
140-
errors: [{ messageId }]
139+
errors: [{ messageId: baseCfg.messageId }]
141140
}
142141
]
143142
});
@@ -153,13 +152,10 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
153152
);
154153
});
155154

156-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
155+
const rule = makeLabeledControlRule(baseCfg);
157156

158157
ruleTester.run("no-unlabeled-radio-group (label prop)", rule as unknown as Rule.RuleModule, {
159-
valid: [
160-
{ filename: "example.tsx", code: `<RadioGroup label="Account type" />` },
161-
{ filename: "example.tsx", code: `<RadioGroup aria-label={"Account type"} />` }
162-
],
158+
valid: [{ code: `<RadioGroup label="Account type" />` }, { code: `<RadioGroup aria-label={"Account type"} />` }],
163159
invalid: []
164160
});
165161
});
@@ -175,33 +171,38 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
175171
);
176172
});
177173

178-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
174+
const rule = makeLabeledControlRule(baseCfg);
179175

180176
ruleTester.run("no-unlabeled-radio-group (aria-labelledby)", rule as unknown as Rule.RuleModule, {
181-
valid: [{ filename: "example.tsx", code: `<RadioGroup aria-labelledby="groupLabelId" />` }],
182-
invalid: [{ filename: "example.tsx", code: `<RadioGroup />`, errors: [{ messageId }] }]
177+
valid: [{ code: `<RadioGroup aria-labelledby="groupLabelId" />` }],
178+
invalid: [{ code: `<RadioGroup />`, errors: [{ messageId: baseCfg.messageId }] }]
183179
});
184180
});
185181

186182
// 4) non-matching component -> ignored
187183
describe("does not report for non-matching component names", () => {
188184
beforeEach(() => resetAllMocksToFalse());
189-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
185+
const rule = makeLabeledControlRule(baseCfg);
190186

191187
ruleTester.run("no-unlabeled-radio-group (non-matching)", rule as unknown as Rule.RuleModule, {
192-
valid: [{ filename: "example.tsx", code: `<Dropdown />` }],
188+
valid: [{ code: `<Dropdown />` }],
193189
invalid: []
194190
});
195191
});
196192

197193
// 5) RegExp component match
198194
describe("supports RegExp component config (/Group$/)", () => {
199195
beforeEach(() => resetAllMocksToFalse());
200-
const rule = makeLabeledControlRule({ ...baseCfg, component: /Group$/ }, messageId, description);
196+
const rule = makeLabeledControlRule({ ...baseCfg, component: /Group$/ });
201197

202198
ruleTester.run("no-unlabeled-*Group (regex)", rule as unknown as Rule.RuleModule, {
203199
valid: [],
204-
invalid: [{ filename: "example.tsx", code: `<CheckboxGroup />`, errors: [{ messageId }] }]
200+
invalid: [
201+
{
202+
code: `<CheckboxGroup />`,
203+
errors: [{ messageId: baseCfg.messageId }]
204+
}
205+
]
205206
});
206207
});
207208

@@ -212,19 +213,18 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
212213
(hasFieldParent as jest.Mock).mockReturnValue(true);
213214
});
214215

215-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
216+
const rule = makeLabeledControlRule(baseCfg);
216217

217218
ruleTester.run("no-unlabeled-radio-group (Field parent)", rule as unknown as Rule.RuleModule, {
218219
valid: [
219220
{
220-
filename: "example.tsx",
221221
code: `
222-
<>
223-
<Field label="Account type">
224-
<RadioGroup />
225-
</Field>
226-
</>
227-
`
222+
<>
223+
<Field label="Account type">
224+
<RadioGroup />
225+
</Field>
226+
</>
227+
`
228228
}
229229
],
230230
invalid: []
@@ -238,17 +238,16 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
238238
(isInsideLabelTag as jest.Mock).mockReturnValue(true);
239239
});
240240

241-
const rule = makeLabeledControlRule(baseCfg, messageId, description);
241+
const rule = makeLabeledControlRule(baseCfg);
242242

243243
ruleTester.run("no-unlabeled-radio-group (wrapping <label>)", rule as unknown as Rule.RuleModule, {
244244
valid: [
245245
{
246-
filename: "example.tsx",
247246
code: `
248-
<label>
249-
<RadioGroup />
250-
</label>
251-
`
247+
<label>
248+
<RadioGroup />
249+
</label>
250+
`
252251
}
253252
],
254253
invalid: []

0 commit comments

Comments
 (0)