Skip to content

Add ADR-001 component architecture decisions and upda implement#9

Merged
imagineux merged 4 commits intomainfrom
CU-86af6pb9j_Write-ADR-001-Component-Architecture-Decisions_Matthew-Van-Dusen
Feb 18, 2026
Merged

Add ADR-001 component architecture decisions and upda implement#9
imagineux merged 4 commits intomainfrom
CU-86af6pb9j_Write-ADR-001-Component-Architecture-Decisions_Matthew-Van-Dusen

Conversation

@imagineux
Copy link
Contributor

Summary

ADR document: New Architecture decision record covering (shadow DOM, CSS API, cascade layers, form participation, testing, slots, accessibility baseline) plus non-goals, consequences, and open questions.

Test infrastructure: Added flush() alias and content parameter to createElement() in test-utils. Added setup.ts for vitest-axe matcher registration. Added vitest-axe dependency. Updated vitest config with setup file.

Accessibility specs: Three new *.a11y.spec.ts files running axe-core audits under jsdom for button (7 tests), focus-ring (1 test), and
ripple (1 test).

Component CSS: All three component stylesheets updated to follow the class-scoped selector guardrails documented in the ADR (no bare tag selectors except host display).

Spec updates: Existing specs updated to use the shared harness's createElement with its new content parameter signature.

Changes

Notes

@Brandon-Anubis
Copy link

@qodo-code-review
Copy link

Review Summary by Qodo

Add ADR-001 component architecture and implement accessibility testing infrastructure

✨ Enhancement 🧪 Tests 📝 Documentation

Grey Divider

Walkthroughs

Description
• Add ADR-001 documenting component architecture decisions (shadow DOM, CSS API, cascade layers,
  form participation, testing, slots, accessibility)
• Implement accessibility test infrastructure with vitest-axe and jsdom environment support
• Add three new *.a11y.spec.ts files running axe-core audits for button, focus-ring, and ripple
  components
• Wrap all component CSS in @layer components to enforce cascade layer positioning per ADR
• Refactor test utilities with createElement() content parameter, flush() alias, and
  polyfillAnimate() helper
• Update existing component specs to use shared test harness and new waitForHydration() helper
Diagram
flowchart LR
  ADR["ADR-001<br/>Architecture Decisions"]
  CSS["Component CSS<br/>@layer components"]
  TestUtils["Test Utilities<br/>createElement, flush, polyfillAnimate"]
  A11yTests["Accessibility Tests<br/>vitest-axe + jsdom"]
  ComponentSpecs["Component Specs<br/>Refactored to use harness"]
  
  ADR --> CSS
  ADR --> TestUtils
  TestUtils --> A11yTests
  TestUtils --> ComponentSpecs
  A11yTests --> Button["button.a11y.spec.ts"]
  A11yTests --> FocusRing["focus-ring.a11y.spec.ts"]
  A11yTests --> Ripple["ripple.a11y.spec.ts"]
Loading

Grey Divider

File Changes

1. docs/adr/001-component-architecture.md 📝 Documentation +382/-0

New ADR documenting component architecture decisions

docs/adr/001-component-architecture.md


2. src/test-utils/setup.ts ⚙️ Configuration changes +9/-0

Setup file for vitest-axe matcher registration

src/test-utils/setup.ts


3. src/test-utils/component-helpers.ts ✨ Enhancement +8/-1

Add flush alias and content parameter to createElement

src/test-utils/component-helpers.ts


View more (12)
4. src/components/ankh-button/ankh-button.a11y.spec.ts 🧪 Tests +46/-0

New accessibility audit tests for button component

src/components/ankh-button/ankh-button.a11y.spec.ts


5. src/components/ankh-button/ankh-button.spec.ts 🧪 Tests +5/-8

Refactor to use shared test harness utilities

src/components/ankh-button/ankh-button.spec.ts


6. src/components/ankh-button/ankh-button.css ⚙️ Configuration changes +192/-190

Wrap styles in @layer components cascade layer

src/components/ankh-button/ankh-button.css


7. src/components/ankh-focus-ring/ankh-focus-ring.a11y.spec.ts 🧪 Tests +25/-0

New accessibility audit tests for focus-ring component

src/components/ankh-focus-ring/ankh-focus-ring.a11y.spec.ts


8. src/components/ankh-focus-ring/ankh-focus-ring.spec.ts 🧪 Tests +9/-11

Refactor to use shared test harness and waitForHydration

src/components/ankh-focus-ring/ankh-focus-ring.spec.ts


9. src/components/ankh-focus-ring/ankh-focus-ring.css ⚙️ Configuration changes +38/-36

Wrap styles in @layer components cascade layer

src/components/ankh-focus-ring/ankh-focus-ring.css


10. src/components/ankh-ripple/ankh-ripple.a11y.spec.ts 🧪 Tests +25/-0

New accessibility audit tests for ripple component

src/components/ankh-ripple/ankh-ripple.a11y.spec.ts


11. src/components/ankh-ripple/ankh-ripple.spec.ts 🧪 Tests +33/-42

Refactor to use shared test harness and polyfillAnimate

src/components/ankh-ripple/ankh-ripple.spec.ts


12. src/components/ankh-ripple/ankh-ripple.css ⚙️ Configuration changes +59/-57

Wrap styles in @layer components cascade layer

src/components/ankh-ripple/ankh-ripple.css


13. vitest.config.ts ⚙️ Configuration changes +1/-0

Add setup file for vitest-axe matcher registration

vitest.config.ts


14. package.json Dependencies +3/-1

Add jsdom and vitest-axe dependencies

package.json


15. test-results/junit.xml Miscellaneous +119/-0

Test results from accessibility and component test runs

test-results/junit.xml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 8, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. JUnit report committed 🐞 Bug ⛯ Reliability
Description
test-results/junit.xml looks like generated test output (timestamps/hostname), which will churn
  diffs, cause merge conflicts, and leak local machine metadata.
• The repo has no CI/scripts referencing this file, and .gitignore does not ignore
  test-results/, so it’s likely accidental and will keep reappearing in PRs.
Code

test-results/junit.xml[R1-4]

+<?xml version="1.0" encoding="UTF-8" ?>
+<testsuites name="vitest tests" tests="55" failures="0" errors="0" time="0.55952576">
+    <testsuite name="src/components/ankh-button/ankh-button.spec.ts" timestamp="2026-02-08T19:57:35.322Z" hostname="DESKTOP-K4SKV0E" tests="25" failures="0" errors="0" skipped="0" time="0.024706656">
+        <testcase classname="src/components/ankh-button/ankh-button.spec.ts" name="ankh-button &gt; default rendering &gt; renders with filled variant by default" time="0.007040871">
Evidence
The committed XML includes machine-specific metadata (hostname/timestamp), strongly indicating it is
a generated artifact. The repo’s .gitignore does not exclude test-results/, so contributors are
likely to re-commit regenerated versions unintentionally.

test-results/junit.xml[1-4]
.gitignore[1-10]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A generated JUnit report (`test-results/junit.xml`) is committed to the repository. This will cause noisy diffs/merge conflicts and leaks machine-specific metadata (e.g., hostname).
### Issue Context
There are no repo scripts/workflows referencing this report, and `.gitignore` does not ignore the `test-results/` directory.
### Fix Focus Areas
- test-results/junit.xml[1-119]
- .gitignore[1-50]
### Suggested change
1. Delete `test-results/junit.xml` from the repo.
2. Add an ignore rule such as:
 - `test-results/`
 (or `test-results/*.xml` if you want to keep other files there).
3. (Optional) If CI needs JUnit, update the CI workflow to generate the report during the run and upload it as an artifact rather than committing it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

✅ 2. ADR links escape repo 🐞 Bug ✓ Correctness
Description
• The new ADR contains relative links that traverse above the repository root
  (../../../tokens/..., ../../../themes/...), so they will be broken when rendered in this repo
  (e.g., on GitHub).
• This reduces doc usability for contributors unless a monorepo layout is guaranteed and documented.
Code

docs/adr/001-component-architecture.md[8]

+We're building a web component library on Stencil (≥ 4.12) that consumes `@ankh-studio/tokens` for structure and `@ankh-studio/themes` for color. The tokens ADR ([001-design-intentions](../../../tokens/docs/adr/001-design-intentions.md)) establishes cognitive contexts (Expressive, Focus, Intense). The themes ADR ([001-themes-architecture](../../../themes/docs/adr/001-themes-architecture.md)) establishes themes as a composable color layer with the cascade order `@layer reset, tokens, intention, palette, base`.
Evidence
From docs/adr/001-component-architecture.md, using ../../../ goes up three directories (docs/adr
→ docs → repo root → outside repo), so those links won’t resolve within this repository context.

docs/adr/001-component-architecture.md[6-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
ADR-001 includes relative markdown links that traverse outside this repository, making the links broken when the docs are rendered in this repo.
### Issue Context
The ADR currently uses `../../../tokens/...` and `../../../themes/...` from within `docs/adr/`, which will resolve above the repo root.
### Fix Focus Areas
- docs/adr/001-component-architecture.md[6-12]
### Suggested change
Choose one:
1) Update links to absolute URLs (preferred for standalone repo docs).
2) If this repo is intended to live in a monorepo with `tokens/` and `themes/` as siblings, add an explicit note in the ADR about that assumption and adjust the links to the correct relative location for the published docs site (if any).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

✅ 3. Axe matchers loaded globally 🐞 Bug ➹ Performance
Description
vitest.config.ts now loads src/test-utils/setup.ts for every test file via setupFiles, and
  that setup imports vitest-axe matchers.
• This is not wrong, but it couples *all* tests to the a11y tooling and adds some avoidable
  overhead; consider scoping matcher setup to only *.a11y.spec.ts if you want leaner unit test runs.
Code

vitest.config.ts[R6-10]

 test: {
   environment: 'happy-dom',
   include: ['src/**/*.spec.ts', 'src/**/*.test.ts'],
+    setupFiles: ['./src/test-utils/setup.ts'],
 },
Evidence
The Vitest config runs a setup file for the entire test suite, and that setup file unconditionally
imports/extends vitest-axe matchers. This means even non-a11y unit tests load the a11y matcher
module.

vitest.config.ts[4-10]
src/test-utils/setup.ts[1-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The vitest-axe matcher setup is loaded for every test via `setupFiles`, even though only `*.a11y.spec.ts` uses `axe()`/`toHaveNoViolations()`.
### Issue Context
This is mostly a suite organization/perf concern: it increases coupling and adds some overhead to all unit tests.
### Fix Focus Areas
- vitest.config.ts[6-10]
- src/test-utils/setup.ts[1-9]
### Suggested change
Pick one approach:
1) Remove `setupFiles` and instead create a small helper (e.g., `src/test-utils/a11y.ts`) that extends matchers, then import it only from `*.a11y.spec.ts`.
2) Use separate Vitest projects/configs: one for fast unit tests (happy-dom) and one for a11y (jsdom + vitest-axe setup).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@imagineux imagineux merged commit 28b33b5 into main Feb 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants