Skip to content
This repository was archived by the owner on Mar 7, 2026. It is now read-only.
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
49 changes: 29 additions & 20 deletions .cursor/rules/coding.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,35 @@ alwaysApply: true

# Recipe for implementing tasks
Always follow the following recipe for implementing tasks:
1. Only start implementing a task after the user has explicitly told so.
2. Before starting coding, create a task planning document (.md) in the docs/task-planning folder.
3. The task planning document contains a table with three columns: tasks description, DoD (Definition of Done) and Status. Status is open, working, checking, review, complete. Initially all tasks are "open".
1. Open = the task hasn't been worked on.
2. Working = the task is currently being implemented.
3. Checking = the DoD of the tasks are being checked.
4. Review = the user is reviewing the result.
5. complete = the user has approved the result and the task is complete.
4. After creating the task planning document, let the user review the doc.
5. After the user has approved the task planning document, implement each task one after the other.
6. After implementing a task, check the criteria for DoD and report the result to the user.
7. If DoD is not met, fix the problem until DoD is met.
8. Before and after each task, update each task in the table with the right status.
9. After finishing implementing, perform all necessary tests and check if the DoD for this task is met.
10. Report on test results and DoD criteria and ask user if a PR should be submitted.
11. If the user approves submitting a PR, and before PR is submitted, the task is marked with "PR" in docs/project_plan.md.
12. After PR ist submitted, report to the user and wait for manual instructions.
13. The user will then review and merge the PR or ask for updates.
14. Pull the repo again to check if the PR has been merged.
15. After task is completed (PR merged), the task is marked with a checkmark in docs/project_plan.md. This change does not warrant a separate PR, it will be included in the next PR. Just do git add for the file. Delete the feature branch locally and remotely.
1. Task planning:
1. Only start implementing a task after the user has explicitly told so.
2. Before starting coding, create a task planning document (.md) in the docs/task-planning folder.
3. The task planning document contains a table with three columns: tasks description, DoD (Definition of Done) and Status. Status is open, working, checking, review, complete. Initially all tasks are "open".
- Open = the task hasn't been worked on.
- Working = the task is currently being implemented.
- Checking = the DoD of the tasks are being checked.
- Review = the user is reviewing the result.
- complete = the user has approved the result and the task is complete.
4. After creating the task planning document, let the user review the doc.
3. Task implementation:
1. Do this only after the user has approved the task planning document.
2. Implement each task in the task-planning document one after the other.
3. After implementing a task, check the criteria for DoD and report the result to the user.
4. If DoD is not met, fix the problem until DoD is met.
5. Before and after each task, update each task in the table with the right status.
6. After finishing implementing, perform all necessary tests and check if the DoD for this task is met.
7. Report on test results and DoD criteria and ask user if a PR should be submitted.
8. If the user approves submitting a PR, and before PR is submitted, the task is marked with "PR" in docs/project_plan.md.
4. PR submission:
1. Do this only after the user approves submitting a PR.
2. Create a temporary markdown file with the PR description.
3. Submit the PR, using the temporary markdown file.
4. After PR ist submitted, report to the user and wait for manual instructions.
5. The user will then review and merge the PR or ask for updates.
5. Post-PR cleanup:
1. Do this after the user confirms that the PR has been successfully merged.
2. Checkout develop and do `git pull` to update the branch with the merged PR.
3. After task is completed (PR merged), the task is marked with a checkmark in docs/project_plan.md. This change does not warrant a separate PR, it will be included in the next PR. Just do git add for the file. Delete the feature branch locally and remotely.

# Instructions to handle error situations:
- CI Pipeline fails:
Expand Down
31 changes: 31 additions & 0 deletions .github/workflows/a11y-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Accessibility Tests

on:
push:
branches: [main, develop]
pull_request:
branches: [main, develop]

jobs:
a11y-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v3
with:
version: 8
- uses: actions/setup-node@v4
with:
node-version: 20
cache: 'pnpm'

- name: Install dependencies
run: pnpm install

- name: Install Playwright browsers
run: cd packages/ui-kit && npx playwright install --with-deps

- name: Run accessibility tests with Storybook
run: cd packages/ui-kit && pnpm test-storybook:ci
env:
CI: true
3 changes: 2 additions & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

pnpm lint-staged && pnpm lint && pnpm test && pnpm build
# Only run lint-staged and lint for faster commits
pnpm lint-staged && pnpm lint
6 changes: 6 additions & 0 deletions .husky/pre-push
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

# Run full test and build suite before pushing
echo "Running full test and build suite before pushing..."
pnpm lint && pnpm test && pnpm test:a11y && pnpm build
4 changes: 2 additions & 2 deletions docs/project_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ A pragmatic breakdown into **four one‑week sprints** plus a preparatory **Spri
| 2.1 | Add NumberInput, Select, Checkbox, RadioGroup components. | Unit & a11y tests pass; form story displays all. | ✓ |
| 2.2 | Integrate **React Hook Form + Zod**; create `FormGrid` + `FormGroup`. | Story "Form Example" submits & reports validation errors in Storybook interaction test. | ✓ |
| 2.3 | Implement Zustand session store skeleton with dark‑mode flag. | Vitest verifies default state + setter actions. | ✓ |
| 2.4 | ESLint rule enforcing named `useEffect` & cleanup. | Failing example in test repo triggers lint error; real code passes. | PR |
| 2.5 | Extend CI to run axe‑core on all stories. | Pipeline fails if any new a11y violations introduced. | |
| 2.4 | ESLint rule enforcing named `useEffect` & cleanup. | Failing example in test repo triggers lint error; real code passes. | |
| 2.5 | Extend CI to run axe‑core on all stories. | Pipeline fails if any new a11y violations introduced. | |

---

Expand Down
122 changes: 122 additions & 0 deletions docs/task-planning/task-2.5-a11y-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# Task 2.5 - Accessibility Fixes

## Overview

Based on the accessibility test results, we need to address several accessibility issues in our components to make the UI-Kit fully accessible. The goal is to ensure that all components pass the accessibility tests and the CI pipeline can run without errors.

## Issues Identified

From the test results, we have identified the following main accessibility issues:

1. **Checkbox Component**: All checkbox variants have `button-name` violations - Critical

- Issue: Buttons (checkbox role) do not have discernible text
- Components affected: All checkbox variants
- Status: FIXED ✅ - Added proper aria-label attributes and ID associations

2. **Select Component**: Select trigger has `button-name` violations - Critical

- Issue: Buttons (combobox role) do not have discernible text
- Components affected: All select variants
- Status: FIXED ✅ - Added proper aria-label attributes and ID associations

3. **RadioGroup Component**: Radio buttons have `button-name` violations - Critical

- Issue: Buttons (radio role) do not have discernible text
- Components affected: All radio group variants
- Status: FIXED ✅ - Added proper aria-label attributes and ID associations

4. **ThemeToggle Component**: React maximum update depth exceeded errors

- Issue: Infinite rendering loop in the ThemeToggle stories
- Components affected: ThemeToggle and ThemeProvider stories
- Status: FIXED ✅ - Created mock hook for testing and dependency injection

5. **Example Pages**: Color contrast issues - Serious
- Issue: Insufficient contrast between foreground and background colors
- Components affected: Example/Page stories
- Status: FIXED ✅ - Improved contrast for links, SVG elements, and tip component

## Tasks

| Task Description | DoD (Definition of Done) | Status |
| ------------------------------------------ | ------------------------------------------------------------------------------ | ----------- |
| Fix Checkbox button-name violations | Checkbox component passes accessibility tests with no button-name violations | Complete ✅ |
| Fix Select button-name violations | Select component passes accessibility tests with no button-name violations | Complete ✅ |
| Fix RadioGroup button-name violations | RadioGroup component passes accessibility tests with no button-name violations | Complete ✅ |
| Fix ThemeToggle infinite update loop | ThemeToggle stories render without errors | Complete ✅ |
| Fix ThemeProvider infinite update loop | ThemeProvider stories render without errors | Complete ✅ |
| Fix color contrast issues in example pages | Example pages pass contrast testing | Complete ✅ |
| Verify all fixes with test-storybook:ci | All tests pass with no accessibility violations | Complete ✅ |

## Implementation Plan

### 1. ✅ Fix Checkbox Component

The checkbox component needed proper accessible names. This was achieved by:

- Adding appropriate aria-label attributes for checkboxes without visible text labels
- Ensuring ID association between checkboxes and their labels using React.useId()
- Properly associating description and error text with the checkboxes

### 2. ✅ Fix Select Component

The Select component needed proper accessible names. This was achieved by:

- Adding appropriate aria-label attributes for the select trigger when no label is present
- Ensuring ID association between select triggers and their labels using React.useId()
- Properly connecting description and error messages using aria-describedby

### 3. ✅ Fix RadioGroup Component

The RadioGroup component needed proper accessible names. This was achieved by:

- Adding unique IDs for each radio button option using generatedId + option value
- Creating proper label associations using htmlFor/id pairs
- Adding aria-label to each RadioGroupItem for screen readers
- Properly connecting the group label, description and error messages using aria-labelledby and aria-describedby

### 4. ✅ Fix ThemeToggle Component

The ThemeToggle component's stories had infinite update loops that were addressed by:

- Creating a mock useTheme hook (useThemeMock) with stable state management
- Making the ThemeToggle component accept an optional useThemeHook prop for dependency injection
- Updating stories to use the mock hook instead of the real one
- Breaking circular dependencies between story rendering and theme state updates

### 5. ✅ Fix ThemeProvider Component

The ThemeProvider's stories also had infinite update loops, which we fixed by:

- Extending the ThemeProvider component to accept an optional useThemeHook prop
- Using the useThemeMock hook in stories to avoid infinite updates
- Ensuring both ThemeProvider and ThemeToggle components in the stories use the same mock hook instance
- Restructuring stories to use the render function instead of args.children

### 6. ✅ Fix Color Contrast Issues

We've fixed the color contrast issues in example pages by:

- Improving contrast of links by using a darker blue (#0a6bb8 instead of #1ea7fd)
- Adding a high-contrast class for strong text with darker blue (#085394) and higher font-weight (800)
- Enhancing the tip component's contrast with a darker green text (#2e7a1a) on light background
- Darkening the SVG icon color from #999 to #666 for better visibility
- All example pages now pass accessibility contrast testing

## Testing Methodology

We've created a script to test individual components for accessibility:

- `pnpm test:component "ComponentName"` - Tests a specific component for accessibility violations
- This script starts Storybook if not already running and runs axe-core tests on the specified stories

## Conclusion

We've successfully fixed all the identified accessibility issues:

1. Added proper aria-labels and ID associations to form controls
2. Fixed infinite update loops in ThemeToggle and ThemeProvider components
3. Improved color contrast in example pages for better readability

All components now pass their individual accessibility tests, making the UI-Kit more inclusive and accessible to all users.
68 changes: 68 additions & 0 deletions docs/task-planning/task-2.5-axe-core-ci.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Task 2.5 - Extend CI to run axe-core on all stories

## Overview

Task 2.5 involves extending the CI pipeline to run axe-core accessibility tests on all Storybook stories. The goal is to ensure that the pipeline fails if any new accessibility violations are introduced.

## Tasks

| Task Description | DoD (Definition of Done) | Status |
| ------------------------------------------------------------ | ----------------------------------------------------------- | -------- |
| Research how to run axe-core tests in CI | Documentation on chosen approach | Complete |
| Configure axe-core to test all stories in Storybook | axe-core tests run locally on all stories | Complete |
| Integrate axe-core tests into GitHub Actions workflow | CI workflow file updated with axe-core testing step | Complete |
| Ensure CI fails when accessibility violations are introduced | Test fails when a component with a11y violations is present | Complete |
| Document how to interpret and fix a11y issues | Documentation added for developers | Complete |

## Implementation Plan

1. Research methods for running axe-core tests in CI (Storybook addon vs standalone) ✅
2. Configure axe-core tests to run against all Storybook stories ✅
3. Update GitHub Actions workflow to include axe-core testing ✅
4. Create test cases to verify pipeline fails on a11y violations ✅
5. Add documentation about the a11y testing in CI ✅

## Implementation Notes

- We've implemented the axe-core accessibility testing in CI using the Storybook test runner
- A test component with intentional accessibility violations was created (`A11yTestButton` and `A11yViolatingForm`)
- We created a standalone test script (`scripts/test-a11y-violation.js`) to verify axe-core detection
- Manual verification confirms axe-core successfully detects accessibility violations:
```
┌─────────┬─────────────┬────────────┬──────────────────────────────────────────────────────────────────────────────┬───────┐
│ (index) │ id │ impact │ description │ nodes │
├─────────┼─────────────┼────────────┼──────────────────────────────────────────────────────────────────────────────┼───────┤
│ 0 │ 'image-alt' │ 'critical' │ 'Ensure <img> elements have alternative text or a role of none or presentation' │ 1 │
│ 1 │ 'region' │ 'moderate' │ 'Ensure all page content is contained by landmarks' │ 2 │
└─────────┴─────────────┴────────────┴──────────────────────────────────────────────────────────────────────────────┴───────┘
```
- The CI pipeline will fail when new accessibility violations are introduced, meeting the Definition of Done
- Documentation explaining how to interpret and fix a11y issues has been added to the `packages/ui-kit/docs/accessibility-testing.md` file

## Key Accessibility Improvements

1. Created proper patterns for extending shadcn components without modification:

- Composition pattern (`A11yButton`)
- Associate labels with form controls (`A11yFormExamples`)
- Use aria attributes correctly (`AccessibleComponentsExample`)

2. Added comprehensive documentation:
- `packages/ui-kit/docs/PROTECTED_FOLDERS.md` - Guidelines for not modifying third-party components
- `packages/ui-kit/src/components/ui/README.md` - Instructions for extending shadcn components

## Expected Outcome

After implementation, the CI pipeline will automatically test all Storybook stories for accessibility issues using axe-core. If any new accessibility violations are introduced, the pipeline will fail, alerting developers to fix the issues before merging.

## Final Status

Pull request [#13](https://github.com/etherisc/ui-kit/pull/13) has been created and is awaiting review and merge.

## Cleanup Notes

After the PR is merged:

1. Delete the `feature/axe-core-ci` branch locally and remotely
2. Delete the `test/a11y-violation-detection` branch locally and remotely (it was used for testing/development)
3. Verify a11y tests are running in CI on the develop branch
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"build": "pnpm -r build",
"dev": "pnpm --filter @org/ui-kit dev",
"test": "pnpm -r test",
"test:a11y": "cd packages/ui-kit && pnpm run test-storybook:ci",
"lint": "pnpm -r lint",
"format": "prettier --write \"**/*.{ts,tsx,md}\"",
"prepare": "husky install"
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-kit/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const preview: Preview = {
},
a11y: {
// axe-core configuration options
element: '#root',
element: '#storybook-root',
config: {},
disable: false,
},
Expand Down
Loading