Skip to content
Merged
Show file tree
Hide file tree
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
8 changes: 5 additions & 3 deletions .claude/skills/code-review-react/skill.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ Stick to the checklist below for every applicable file and mode. Apply only the
## Checklist
See [.agents/review-checklists/common.md](../../../.agents/review-checklists/common.md) for reviewer priority and standard review format, and [.agents/review-checklists/react/code-quality.md](../../../.agents/review-checklists/react/code-quality.md), [.agents/review-checklists/react/performance.md](../../../.agents/review-checklists/react/performance.md), [.agents/review-checklists/react/business-logic.md](../../../.agents/review-checklists/react/business-logic.md) for the living checklist split by category—treat it as the canonical set of rules to follow.

Additionally, check for WCAG 2.2 Level AA accessibility violations using [wcag-22-checklist.md](../wcag-compliance/wcag-22-checklist.md). Use category **Accessibility** for these findings. Prioritize urgent WCAG criteria (missing alt text, keyboard traps, no focus indicators, missing form labels, broken ARIA) alongside Correctness-level issues.

Use the rule's `Urgency` to place findings in the "urgent issues" vs "suggestions" sections.

## Review Process
Expand Down Expand Up @@ -66,7 +68,7 @@ unchanged code only if they directly interact with or are affected by the change
1. Open the relevant component/module. Gather all lines.
2. For each applicable checklist rule, evaluate the code against the rule text, confidence threshold, and exceptions/false positives before deciding to flag it.
3. For each confirmed violation, capture evidence (exact snippet and/or file/line), record the rule's primary category, and note confidence briefly.
4. Compose the review section per the template below. Group findings by **Urgency** section first (urgent issues, then suggestions). Within each section, order findings by the checklist primary category priority: **Correctness**, then **Maintainability**, then **Style**.
4. Compose the review section per the template below. Group findings by **Urgency** section first (urgent issues, then suggestions). Within each section, order findings by the checklist primary category priority: **Correctness**, then **Accessibility**, then **Maintainability**, then **Style**.

## Required output
When invoked, the response must exactly follow one of the two templates:
Expand All @@ -79,7 +81,7 @@ Found <N> urgent issues that need to be fixed:
## 1 <brief description of bug>
FilePath: <path> line <line>
Evidence: <relevant code snippet or pointer>
Category: <Correctness | Maintainability | Style>
Category: <Correctness | Accessibility | Maintainability | Style>
Confidence: <high | medium | low> - <brief justification>
Exceptions checked: <none apply | brief exception note>

Expand All @@ -95,7 +97,7 @@ Found <M> suggestions for improvement:
## 1 <brief description of suggestion>
FilePath: <path> line <line>
Evidence: <relevant code snippet or pointer>
Category: <Correctness | Maintainability | Style>
Category: <Correctness | Accessibility | Maintainability | Style>
Confidence: <high | medium | low> - <brief justification>
Exceptions checked: <none apply | brief exception note>

Expand Down
123 changes: 123 additions & 0 deletions .claude/skills/wcag-compliance/skill.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
---
name: wcag-compliance
description: "Review frontend code for WCAG 2.2 Level AA accessibility compliance. Checks semantic HTML, ARIA usage, keyboard navigation, color contrast, focus management, and more. Supports --full flag for complete file/directory review; defaults to staged changes when a path is provided."
---

# WCAG 2.2 Accessibility Compliance Review

## Intent
Use this skill whenever the user asks to check accessibility or WCAG compliance of frontend code (`.tsx`, `.ts`, `.js`, `.jsx`, `.scss`, `.css`, `.jsp`, `.jspf` files). Support three review modes:

1. **Pending-change review** -- bare invocation with no arguments; inspects staged/working-tree
files slated for commit across all repos.
2. **Path review (default)** -- a file or directory path is provided (no flag); reviews only the
git staged/working-tree changes for that path.
3. **Full review** -- a file or directory path is provided with `--full`; reviews the entire file
contents (or all matching files in a directory) regardless of git status.

Apply only the WCAG 2.2 Level AA checklist below. Focus on issues that can be detected through static code review.

## Checklist
See [wcag-22-checklist.md](wcag-22-checklist.md) for the full set of criteria organized by principle.

Each rule has an **Urgency** level:
- **urgent** -- Violations that block users from accessing content or functionality.
- **suggestion** -- Improvements that enhance the experience but don't fully block access.

## Review Process

**Argument parsing:**

Parse the invocation arguments to extract:
- An optional flag: `--full`
- An optional path: a file path or directory path

**File discovery:**

- **No path, no flag (bare invocation):** Pending-change review. Discover nested repos by
running `find server/modules -maxdepth 2 -name ".git" -type d` from the workspace root,
then for each discovered repo (and the top-level root) run `git diff --cached --name-only`
and `git diff --name-only`. Aggregate all results, filtering to applicable extensions
(`.tsx`, `.ts`, `.js`, `.jsx`, `.scss`, `.css`, `.jsp`, `.jspf`).

- **Path provided (no `--full` flag -- default):** Determine the git root via
`git -C <path> rev-parse --show-toplevel`.
- *File path:* Run `git diff --cached -- <file>` and `git diff -- <file>`. If there are
no changes, report "No staged or working-tree changes found for `<file>`." and stop.
If there are changes, proceed to review.
- *Directory path:* Run `git diff --cached --name-only -- <dir>` and
`git diff --name-only -- <dir>`. Filter to applicable extensions. If no changed files
are found, report "No staged or working-tree changes found under `<dir>`." and stop.

- **Path + `--full`:** Review the entire contents regardless of git status.
- *File path:* Use the file directly -- no git discovery needed.
- *Directory path:* Find all files matching applicable extensions under the directory
(using glob/find). Review every matching file.

- **`--full` with no path:** Ask the user to provide a file or directory path.

**Review scope when reviewing staged changes (default mode):**

When reviewing staged changes, read the full file for context but **focus the review on changed
lines and their immediate surroundings**. Use the diff output to identify which sections
changed, then apply the checklist rules primarily to those sections. Still flag issues in
unchanged code only if they directly interact with or are affected by the changes.

**Multi-file grouping:** When reviewing multiple files, group all findings together by urgency section (urgent issues first, then suggestions), not per-file. Include the file path in each finding's `FilePath` field.

1. Open the relevant file(s). Gather all lines.
2. For each applicable checklist rule, evaluate the code against the rule text.
3. For each confirmed violation, capture evidence (exact snippet and/or file/line), record the WCAG criterion, and note confidence.
4. Compose the review per the template below.

## Required output
When invoked, the response must exactly follow one of the two templates:

### Template A (any findings)
```
# Accessibility review (WCAG 2.2 AA)
Found <N> urgent issues that need to be fixed:

## 1 <brief description>
FilePath: <path> line <line>
WCAG Criterion: <number and name, e.g. "1.1.1 Non-text Content">
Evidence: <relevant code snippet or pointer>
Confidence: <high | medium | low> - <brief justification>


### Suggested fix
<brief description of suggested fix, with code example if helpful>

---
... (repeat for each urgent issue) ...

Found <M> suggestions for improvement:

## 1 <brief description>
FilePath: <path> line <line>
WCAG Criterion: <number and name>
Evidence: <relevant code snippet or pointer>
Confidence: <high | medium | low> - <brief justification>


### Suggested fix
<brief description of suggested fix>

---

... (repeat for each suggestion) ...
```

If there are no urgent issues, omit that section. If there are no suggestions, omit that section.

Always list every urgent issue -- never cap or truncate them. If there are more than 10 suggestions, summarize as "10+ suggestions" and output the first 10.

Don't compress the blank lines between sections; keep them as-is for readability.

If you use Template A and at least one issue requires code changes, append a brief follow-up question after the structured output asking whether the user wants you to apply the suggested fix(es).

### Template B (no issues)
```
# Accessibility review (WCAG 2.2 AA)
No issues found.
```
203 changes: 203 additions & 0 deletions .claude/skills/wcag-compliance/wcag-22-checklist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
# WCAG 2.2 Level AA -- Static Code Review Checklist

This checklist covers WCAG 2.2 Level AA success criteria that can be detected through code review. Criteria that require runtime testing (e.g., timing, audio descriptions) are excluded.

---

## Principle 1: Perceivable

### 1.1.1 Non-text Content (Level A)
**Urgency: urgent**
- `<img>` elements must have an `alt` attribute. Decorative images should use `alt=""` or `role="presentation"`.
- Icon-only buttons/links must have an accessible name (`aria-label`, `aria-labelledby`, or visually hidden text).
- `<svg>` elements used as content must have `role="img"` and an accessible name (`aria-label` or `<title>`).
- Font icon elements (e.g., `<i className="fa fa-..."`) used as interactive controls must have an accessible name.
- **Exception:** Purely decorative elements that don't convey information.

### 1.3.1 Info and Relationships (Level A)
**Urgency: urgent**
- Use semantic HTML elements (`<nav>`, `<main>`, `<header>`, `<footer>`, `<section>`, `<article>`) instead of generic `<div>` for landmarks.
- Headings (`<h1>`-`<h6>`) must follow a logical hierarchy -- no skipped levels.
- Form inputs must have associated `<label>` elements (via `htmlFor`/`id`) or `aria-label`/`aria-labelledby`.
- Data tables must use `<th>` with `scope` attributes. Do not use tables for layout.
- Related form controls should be grouped with `<fieldset>` and `<legend>`.
- Lists of items should use `<ul>`, `<ol>`, or `<dl>` -- not styled `<div>` sequences.

### 1.3.2 Meaningful Sequence (Level A)
**Urgency: suggestion**
- DOM order should match the visual reading order. Avoid CSS that reorders content (`order`, `flex-direction: row-reverse`) in ways that break reading sequence.

### 1.3.4 Orientation (Level AA)
**Urgency: suggestion**
- Do not restrict display to a single orientation via CSS or JS unless essential.

### 1.3.5 Identify Input Purpose (Level AA)
**Urgency: suggestion**
- Form fields for personal data (name, email, phone, address, etc.) should have appropriate `autoComplete` attributes.

### 1.4.1 Use of Color (Level A)
**Urgency: urgent**
- Color must not be the only visual means of conveying information (e.g., error states, required fields, status). Provide text, icons, or patterns as well.

### 1.4.3 Contrast (Minimum) (Level AA)
**Urgency: urgent**
- Text color against its background must meet 4.5:1 ratio for normal text, 3:1 for large text (18pt+ or 14pt+ bold).
- Check hardcoded color values in CSS/SCSS and inline styles. Flag combinations that are visually likely to fail (e.g., light gray on white).
- **Note:** Exact contrast can only be verified with a tool; flag suspicious values for manual check.

### 1.4.4 Resize Text (Level AA)
**Urgency: suggestion**
- Text sizes should use relative units (`rem`, `em`, `%`) rather than fixed `px` for body text.
- Layouts should not break at 200% zoom. Avoid fixed-height containers with `overflow: hidden` on text content.

### 1.4.11 Non-text Contrast (Level AA)
**Urgency: suggestion**
- UI components (form controls, buttons) and meaningful graphics must have at least 3:1 contrast against adjacent colors.

### 1.4.13 Content on Hover or Focus (Level AA)
**Urgency: suggestion**
- Tooltips/popovers shown on hover must also be dismissible (Escape key), hoverable (user can move pointer to the tooltip), and persistent (don't disappear while the user is interacting).

---

## Principle 2: Operable

### 2.1.1 Keyboard (Level A)
**Urgency: urgent**
- All interactive elements must be keyboard accessible. `onClick` on non-interactive elements (`<div>`, `<span>`) requires `onKeyDown`/`onKeyUp`, `tabIndex="0"`, and `role`.
- Prefer semantic elements (`<button>`, `<a>`, `<input>`) over ARIA-enhanced `<div>`s.
- Drag-and-drop must have a keyboard alternative.
- Custom widgets must handle expected key interactions (e.g., arrow keys for menus/tabs, Space/Enter for buttons).

### 2.1.2 No Keyboard Trap (Level A)
**Urgency: urgent**
- Modal dialogs must trap focus *within* the modal but allow dismissal via Escape.
- Focus must not get stuck in any component. Verify that custom focus management doesn't prevent tabbing away.

### 2.4.1 Bypass Blocks (Level A)
**Urgency: suggestion**
- Pages with repeated navigation should provide a "skip to main content" link or use landmark regions.

### 2.4.2 Page Titled (Level A)
**Urgency: suggestion**
- Each page/view should set a descriptive `<title>` or use `document.title`.

### 2.4.3 Focus Order (Level A)
**Urgency: urgent**
- `tabIndex` values greater than 0 disrupt natural focus order -- flag any `tabIndex` > 0.
- Tab order must follow a logical sequence through the page.

### 2.4.4 Link Purpose (Level A)
**Urgency: suggestion**
- Link text must describe the destination. Flag "click here", "here", "read more" without context.
- If link text is generic, `aria-label` or `aria-describedby` should provide context.

### 2.4.6 Headings and Labels (Level AA)
**Urgency: suggestion**
- Headings and labels must be descriptive. Flag empty headings or labels.

### 2.4.7 Focus Visible (Level AA)
**Urgency: urgent**
- Do not remove focus indicators. Flag `outline: none`, `outline: 0`, or `:focus { outline: none }` without a replacement focus style.
- Custom focus styles must be clearly visible.

### 2.4.11 Focus Not Obscured (Minimum) (Level AA) -- NEW in 2.2
**Urgency: urgent**
- When a component receives focus, it must not be entirely hidden by other content (e.g., sticky headers, footers, overlays).
- Check for `position: fixed`/`sticky` elements that could cover focused items. Ensure `scroll-padding` or `scroll-margin` accounts for sticky elements.

### 2.4.13 Focus Appearance (Level AAA, but recommended)
**Urgency: suggestion**
- Custom focus indicators should have at least a 2px solid outline with 3:1 contrast against adjacent colors.

### 2.5.7 Dragging Movements (Level AA) -- NEW in 2.2
**Urgency: urgent**
- Any functionality that uses dragging must provide a single-pointer alternative (e.g., up/down buttons to reorder, click-to-select then click-to-place).
- **Exception:** Dragging is essential to the function (rare).

### 2.5.8 Target Size (Minimum) (Level AA) -- NEW in 2.2
**Urgency: suggestion**
- Interactive targets should be at least 24x24 CSS pixels, or have sufficient spacing so the target + spacing meets 24px.
- **Exceptions:** Inline text links, targets where the size is determined by the user agent (e.g., default checkboxes), essential presentation.

---

## Principle 3: Understandable

### 3.1.1 Language of Page (Level A)
**Urgency: suggestion**
- HTML element should have a `lang` attribute (`<html lang="en">`).

### 3.1.2 Language of Parts (Level AA)
**Urgency: suggestion**
- Content in a different language than the page should use `lang` attribute on its container.

### 3.2.1 On Focus (Level A)
**Urgency: urgent**
- Focus must not trigger a change of context (e.g., page navigation, form submission, opening a new window).

### 3.2.2 On Input (Level A)
**Urgency: urgent**
- Changing a form control value must not automatically trigger a change of context unless the user is informed beforehand.
- Flag `onChange` handlers that submit forms or navigate without user confirmation.

### 3.3.1 Error Identification (Level A)
**Urgency: urgent**
- Form errors must be described in text, not just color. Error messages must identify which field has the error.
- Error messages must be programmatically associated with their inputs (`aria-describedby`, `aria-errormessage`, or `aria-invalid`).

### 3.3.2 Labels or Instructions (Level A)
**Urgency: urgent**
- Form inputs must have visible labels. Placeholder text alone is not sufficient as a label.
- Required fields must be indicated in a way that doesn't rely solely on color.

### 3.3.3 Error Suggestion (Level AA)
**Urgency: suggestion**
- When an input error is detected, provide a suggestion for correction if possible.

### 3.3.7 Redundant Entry (Level A) -- NEW in 2.2
**Urgency: suggestion**
- Do not require users to re-enter information they have already provided in the same process/session.
- **Exceptions:** Re-entering for security purposes, or when previously entered info is no longer valid.

### 3.3.8 Accessible Authentication (Minimum) (Level AA) -- NEW in 2.2
**Urgency: urgent**
- Authentication must not require cognitive function tests (e.g., CAPTCHA, puzzle) unless an alternative is provided (e.g., object recognition, personal content).
- Allow pasting into password fields. Do not block password managers.

---

## Principle 4: Robust

### 4.1.2 Name, Role, Value (Level A)
**Urgency: urgent**
- Custom components must expose correct ARIA roles, states, and properties.
- `aria-expanded`, `aria-selected`, `aria-checked`, `aria-pressed` must accurately reflect component state.
- `aria-hidden="true"` must not be set on focusable or interactive elements.
- `role` values must be valid WAI-ARIA roles.
- IDs referenced by `aria-labelledby`, `aria-describedby`, `aria-controls`, etc. must exist in the DOM.

### 4.1.3 Status Messages (Level AA)
**Urgency: suggestion**
- Status messages (success, error, loading, progress) that don't receive focus must use `role="status"`, `role="alert"`, or `aria-live` regions so screen readers announce them.
- Use `aria-live="polite"` for non-urgent updates, `aria-live="assertive"` for critical alerts.

---

## LabKey-Specific Patterns

These patterns are common in the LabKey codebase and deserve extra attention:

### React Components (`@labkey/components`, `@labkey/premium`)
- Verify `Alert` components use appropriate ARIA roles.
- Check `Modal`/`ModalDialog` components trap focus and are dismissible via Escape.
- Ensure `Grid`/`QueryGrid` table components use proper `<table>` semantics with headers.
- Check custom dropdown/select components for keyboard navigation (arrow keys, Escape, Enter).

### JSP Pages
- Verify `<labkey:form>` and `<labkey:input>` render with proper label associations.
- Check `<labkey:link>` and `<labkey:button>` produce accessible HTML.

### ExtJS Components
- ExtJS components often lack accessibility. Flag custom ExtJS widgets that have no ARIA markup.
- Verify ExtJS modals/windows are keyboard dismissible.
Loading