-
Notifications
You must be signed in to change notification settings - Fork 21
Users/aubreyquinn/swatch picker #147
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7131930
added new lint rule for SwatchPicker component
aubreyquinn 21bc5e6
updated readme file
aubreyquinn 40861fc
Merge remote-tracking branch 'origin/main' into users/aubreyquinn/swa…
aubreyquinn f95bb3d
fixed prettier conflict
aubreyquinn 4f771ca
applying changes from code review
aubreyquinn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc. (`@microsoft/fluentui-jsx-a11y/swatchpicker-needs-labelling`) | ||
|
|
||
| 💼 This rule is enabled in the ✅ `recommended` config. | ||
|
|
||
| <!-- end auto-generated rule header --> | ||
|
|
||
| All interactive elements must have an accessible name. | ||
|
|
||
| SwatchPicker without a label or accessible labeling lack an accessible name for assistive technology users. | ||
|
|
||
| <https://www.w3.org/WAI/standards-guidelines/act/rules/e086e5/> | ||
|
|
||
| ## Ways to fix | ||
|
|
||
| - Add an aria-label or aria-labelledby attribute to the SwatchPicker tag. You can also use the Field component. | ||
|
|
||
| ## Rule Details | ||
|
|
||
| This rule aims to make SwatchPickers accessible. | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```jsx | ||
| <SwatchPicker /> | ||
| <Radio></Radio> | ||
| ``` | ||
|
|
||
| ```jsx | ||
| <Label>This is a switch.</Label> | ||
| <SwatchPicker | ||
| onChange={onChange} | ||
| /> | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```jsx | ||
| <Label id="my-label-1">This is a Radio.</Label> | ||
| <SwatchPicker | ||
| delectedValue="00B053" | ||
| onSelectionChange={onSel} | ||
| aria-labelledby="my-label-1" | ||
| /> | ||
| ``` | ||
|
|
||
| ```jsx | ||
| <SwatchPicker aria-label="anything" selectedValue="00B053" onSelectionChange={onSel}> | ||
| <ColorSwatch color="#FF1921" value="FF1921" aria-label="red" /> | ||
| <ColorSwatch color="#00B053" value="00B053" aria-label="green" /> | ||
| </SwatchPicker> | ||
| ``` | ||
|
|
||
| ```jsx | ||
| <Field label="Pick a colour"> | ||
| <SwatchPicker> | ||
| <ColorSwatch color="#FF1921" value="FF1921" aria-label="red" /> | ||
| <ColorSwatch color="#00B053" value="00B053" aria-label="green" /> | ||
| </SwatchPicker> | ||
| </Field> | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import { ESLintUtils } from "@typescript-eslint/utils"; | ||
| import { makeLabeledControlRule } from "../util/ruleFactory"; | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| // Rule Definition | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| export default ESLintUtils.RuleCreator.withoutDocs( | ||
| makeLabeledControlRule( | ||
| { | ||
| component: "SwatchPicker", | ||
| labelProps: ["aria-label"], | ||
| allowFieldParent: true, | ||
| allowFor: false, | ||
| allowLabelledBy: true, | ||
| allowWrappingLabel: false | ||
| }, | ||
| "noUnlabeledSwatchPicker", | ||
| "Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc.." | ||
| ) | ||
| ); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import { TSESLint, TSESTree } from "@typescript-eslint/utils"; | ||
| import { hasNonEmptyProp } from "./hasNonEmptyProp"; | ||
| import { hasAssociatedLabelViaAriaLabelledBy, isInsideLabelTag, hasAssociatedLabelViaHtmlFor } from "./labelUtils"; | ||
| import { hasFieldParent } from "./hasFieldParent"; | ||
| import { elementType } from "jsx-ast-utils"; | ||
| import { JSXOpeningElement } from "estree-jsx"; | ||
|
|
||
| export type LabeledControlConfig = { | ||
| component: string | RegExp; | ||
| labelProps: string[]; // e.g. ["label", "aria-label"] | ||
| allowFieldParent?: boolean; // e.g. <Field label=...><RadioGroup/></Field> | ||
| allowFor?: boolean; // htmlFor | ||
| allowLabelledBy?: boolean; // aria-labelledby | ||
| allowWrappingLabel?: boolean; // <label>...</label> | ||
| }; | ||
|
|
||
| /** | ||
| * Returns `true` if the JSX opening element is considered **accessibly labelled** | ||
| * per the rule configuration. This function centralizes all supported labelling | ||
| * strategies so the rule stays small and testable. | ||
| * | ||
| * The supported strategies (gated by `config` flags) are: | ||
| * 1) A parent `<Field>`-like wrapper that provides the label context (`allowFieldParent`). | ||
| * 2) A non-empty inline prop such as `aria-label` or `title` (`labelProps`). | ||
| * 3) Being wrapped by a `<label>` element (`allowWrappingLabel`). | ||
| * 4) Associated `<label for="...">` / `htmlFor` relation (`allowFor`). | ||
| * 5) `aria-labelledby` association to an element with textual content (`allowLabelledBy`). | ||
| * | ||
| * Note: This does not validate contrast or UX; it only checks the existence of | ||
| * an accessible **name** via common HTML/ARIA labelling patterns. | ||
| * | ||
| * @param node - The JSX opening element we’re inspecting (e.g., `<Input ...>` opening node). | ||
| * @param context - ESLint rule context or tree-walker context used by helper functions to | ||
| * resolve scope/ancestors and collect referenced nodes. | ||
| * @param config - Rule configuration describing which components/props/associations count as labelled. | ||
| * Expected shape: | ||
| * - `component: string | RegExp` — component tag name or regex to match. | ||
| * - `labelProps: string[]` — prop names that, when non-empty, count as labels (e.g., `["aria-label","title"]`). | ||
| * - `allowFieldParent?: boolean` — if true, a recognized parent “Field” wrapper satisfies labelling. | ||
| * - `allowWrappingLabel?: boolean` — if true, being inside a `<label>` satisfies labelling. | ||
| * - `allowFor?: boolean` — if true, `<label htmlFor>` association is considered. | ||
| * - `allowLabelledBy?: boolean` — if true, `aria-labelledby` association is considered. | ||
| * @returns `true` if any configured labelling strategy succeeds; otherwise `false`. | ||
| */ | ||
| export function hasAccessibleLabel(node: TSESTree.JSXOpeningElement, context: any, config: LabeledControlConfig): boolean { | ||
| if (config.allowFieldParent && hasFieldParent(context)) return true; | ||
| if (config.labelProps.some(p => hasNonEmptyProp(node.attributes, p))) return true; | ||
| if (config.allowWrappingLabel && isInsideLabelTag(context)) return true; | ||
| if (config.allowFor && hasAssociatedLabelViaHtmlFor(node, context)) return true; | ||
| if (config.allowLabelledBy && hasAssociatedLabelViaAriaLabelledBy(node, context)) return true; | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Factory for a minimal, strongly-configurable ESLint rule that enforces | ||
| * accessible labelling on a specific JSX element/component. | ||
| * | ||
| * The rule: | ||
| * • Matches opening elements by `config.component` (exact name or RegExp). | ||
| * • Uses `hasAccessibleLabel` to decide whether the element is labelled. | ||
| * • Reports with `messageId` if no labelling strategy succeeds. | ||
| * | ||
| * Example: | ||
| * ```ts | ||
| * export default makeLabeledControlRule( | ||
| * { | ||
| * component: /^(?:input|textarea|Select|ComboBox)$/i, | ||
| * labelProps: ["aria-label", "aria-labelledby", "title"], | ||
| * allowFieldParent: true, | ||
| * allowWrappingLabel: true, | ||
| * allowFor: true, | ||
| * allowLabelledBy: true, | ||
| * }, | ||
| * "missingLabel", | ||
| * "Provide an accessible label (e.g., via <label>, htmlFor, aria-label, or aria-labelledby)." | ||
| * ); | ||
| * ``` | ||
| * | ||
| * @param config - See `hasAccessibleLabel` for the configuration fields and semantics. | ||
| * @param messageId - The message key used in `meta.messages` (e.g., "missingLabel"). | ||
| * @param description - Human-readable rule description and the text displayed for `messageId`. | ||
| * @returns An ESLint `RuleModule` that reports when the configured component lacks an accessible label. | ||
| */ | ||
| export function makeLabeledControlRule( | ||
| config: LabeledControlConfig, | ||
| messageId: string, | ||
| description: string | ||
| ): TSESLint.RuleModule<string, []> { | ||
| return { | ||
| meta: { | ||
| type: "problem" as const, | ||
| messages: { [messageId]: description }, | ||
|
||
| docs: { | ||
| description, | ||
| recommended: "strict" as const, // not `true` | ||
| url: "https://www.w3.org/TR/html-aria/" | ||
| }, | ||
| schema: [] | ||
| }, | ||
| defaultOptions: [] as const, | ||
|
|
||
| create(context: TSESLint.RuleContext<string, []>) { | ||
| return { | ||
| JSXOpeningElement(node: TSESTree.JSXOpeningElement) { | ||
| // elementType expects an ESTree JSX node — cast is fine | ||
| const name = elementType(node as unknown as JSXOpeningElement); | ||
| const matches = typeof config.component === "string" ? name === config.component : config.component.test(name); | ||
|
|
||
| if (!matches) return; | ||
|
|
||
| if (!hasAccessibleLabel(node, context, config)) { | ||
| context.report({ node, messageId }); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| }; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: for readability, consider moving
messageIdanddescriptionto required properties in theLabeledControlConfiginstead of just passing as params here