✅ test: shared render helper, axe backfill and role-based queries - #677
Merged
Conversation
futjesus
force-pushed
the
chore/update-dependencies
branch
2 times, most recently
from
July 27, 2026 15:53
32035db to
0ce5b08
Compare
- Add tests/utils/renderWithProviders with '@tests' alias; refactor the two VirtualizedTable test files that duplicated QueryClientProvider setup - Backfill jest-axe checks in Button, MultiSelectDropdown, RadioCardGroup, Sidebar, Tabs, TagSelect and VirtualizedTable Actions tests - Fix the violations axe surfaced instead of skipping tests: - MultiSelectDropdown/TagSelect combobox now gets aria-labelledby from its label; hidden sync inputs marked aria-hidden - VirtualizedTable Actions buttons lose the role=presentation that voided their semantics - Sidebar NavigationOption drops the invalid role=option (li outside a listbox); with role=button it now renders a real button inside the li, giving keyboard and screen-reader support - Input password toggle is a real button with a Show/Hide password label - PhoneNumberInput country search input gains an accessible name - Rewrite queries to follow the Testing Library philosophy (getByRole with accessible name) and document the rule in CLAUDE.md
futjesus
force-pushed
the
test/infra-and-axe
branch
from
July 27, 2026 16:07
382c6e2 to
824a02b
Compare
futjesus
added a commit
that referenced
this pull request
Jul 27, 2026
The NavigationOption hover/active colors lived in Sidebar.css keyed on `li[role='option']`. #677 dropped that invalid role from the `<li>`, so the rules stopped matching and every consumer theming through the `--konstruct-sidebar-hover-bg` / `--konstruct-sidebar-active-bg` custom properties silently fell back to the library default. Rather than re-point the selector, move the styling into the cva variants: a descriptor selector like `.konstruct-sidebar li:hover` (0,2,2) outranks a single utility class (0,1,0), so a consumer could never win through `className` even when the selector did match. - Drop the dead `li[role='option']` rules from Sidebar.css; the variants already cover the layout, padding and anchor styling they mirrored - Add the one rule that was not mirrored (`shrink-0`) to the variants - Prefix the active classes with `data-[active=true]:` so tailwind-merge detects the conflict against a consumer override and drops the default - Remove the now-unused `text-black` and duplicated `font-normal` - Cover both directions with tests: an override wins, no override keeps the default BREAKING CHANGE: `--konstruct-sidebar-hover-bg`, `--konstruct-sidebar-hover-color`, `--konstruct-sidebar-active-bg` and `--konstruct-sidebar-active-color` no longer do anything. Pass the colors to `NavigationOption` via `className` instead: `hover:bg-metal-800 data-[active=true]:bg-metal-800`.
futjesus
added a commit
that referenced
this pull request
Jul 27, 2026
* 🐛 fix: let consumers retheme Sidebar options through className The NavigationOption hover/active colors lived in Sidebar.css keyed on `li[role='option']`. #677 dropped that invalid role from the `<li>`, so the rules stopped matching and every consumer theming through the `--konstruct-sidebar-hover-bg` / `--konstruct-sidebar-active-bg` custom properties silently fell back to the library default. Rather than re-point the selector, move the styling into the cva variants: a descriptor selector like `.konstruct-sidebar li:hover` (0,2,2) outranks a single utility class (0,1,0), so a consumer could never win through `className` even when the selector did match. - Drop the dead `li[role='option']` rules from Sidebar.css; the variants already cover the layout, padding and anchor styling they mirrored - Add the one rule that was not mirrored (`shrink-0`) to the variants - Prefix the active classes with `data-[active=true]:` so tailwind-merge detects the conflict against a consumer override and drops the default - Remove the now-unused `text-black` and duplicated `font-normal` - Cover both directions with tests: an override wins, no override keeps the default BREAKING CHANGE: `--konstruct-sidebar-hover-bg`, `--konstruct-sidebar-hover-color`, `--konstruct-sidebar-active-bg` and `--konstruct-sidebar-active-color` no longer do anything. Pass the colors to `NavigationOption` via `className` instead: `hover:bg-metal-800 data-[active=true]:bg-metal-800`. * 📝 docs: tighten the no-comments rule to cover change rationale and CSS
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Final PR of the audit series (stacked on #676). Test infrastructure, axe coverage for the remaining test files, and — per the new project rule — every query rewritten to the Testing Library philosophy, fixing the component semantics that blocked role-based queries instead of skipping tests.
Changes
Infra
tests/utils/renderWithProviders(QueryClient + optional theme wrapper) with an@testsalias; the two VirtualizedTable test files that duplicatedQueryClientProvidersetup now use it. Lives outsidelib/so it never enters the bundle or coverage.axe backfill (7 files) + the real violations it surfaced, fixed
role="combobox"div had no accessible name (alabel forcan't name a div) — now wired viaaria-labelledby; hidden form-sync inputs markedaria-hidden.role="presentation"on focusable buttons voided their semantics — removed.role="option"(li outside a listbox, axearia-required-parent). Withrole="button"it now renders a real<button>inside the li — keyboard operable and screen-reader announced, no skipped test.<svg>— now a real button with aShow/Hide passwordaccessible name.Testing Library philosophy as a project rule
All queries in the touched tests use
getByRole+ accessible name; the rule (role-first priority, no testid/querySelector, missing semantics = fix the component) is now documented in CLAUDE.md's Testing section.Follow-up (documented, not in this PR)
The legacy
data-label/querySelectorqueries in the Filter dropdown and Breadcrumb tests need the BadgeMultiSelect options to gain real option/checkbox semantics before they can be role-queried — that's a component redesign of the same shape as NavigationOption's, worth its own PR.Testing
Full suite green: 47 files, 558 tests, 0 skipped, lint, types, prettier, coverage thresholds.