feat(a11y): ElementInternals accessibility layer + WCAG 2.2 AA remediation across all components#1632
Open
plasticmind wants to merge 9 commits into
Open
feat(a11y): ElementInternals accessibility layer + WCAG 2.2 AA remediation across all components#1632plasticmind wants to merge 9 commits into
plasticmind wants to merge 9 commits into
Conversation
…to ElementInternals AOM Introduce packages/internals — a private, tree-shake-safe shared module of Lit mixins (FormControlMixin/NysFormControlElement, ReflectsAriaMixin/ NysReflectsAriaElement) centralizing ElementInternals form association, validity, default ARIA semantics, and id generation, plus a-la-carte associateControl/ associateHost/supportsElementRefs helpers (ARIA element-reference reflection with ID-based fallback). Migrate all form controls (button, checkbox+group, combobox, datepicker, fileinput, radiobutton+group, select, textarea, textinput, toggle) to extend NysFormControlElement: remove duplicated _internals/formAssociated/id-gen boilerplate; route value/validity through shared helpers; associate the native control accessible name to the visible nys-label via aria-labelledby instead of a synthetic aria-label (WCAG 1.3.1/2.5.3); self-register internal nys-label and nys-errormessage. nys-button drops the redundant role="button"; nys-datepicker associates its error message. Wiring: tsconfig.build.json references, build-order.js, CEM exclude for internals, sideEffects:true on form-control packages. Regenerate CEM + React wrappers. Full suite: 886 tests passing across 6 browsers; coverage 91.5% statements / 80% branches. Adds docs/accessibility-remediation-PRD.md.
…d NysElement; fix WCAG gaps Add a minimal id-only base (NysElement / IdentifiedMixin) layered below ReflectsAriaMixin, and define all concrete base classes (NysElement, NysReflectsAriaElement, NysFormControlElement) as pure /*@__PURE__*/ const assignments so unused mixin chains tree-shake out of every consuming bundle (previously the whole internals module was inlined into each component, bloating bundles and tanking coverage). Migrate presentational components to the shared base: id-only components (alert, accordion(+item), avatar, backtotop, badge, breadcrumbs, dropdownmenu(+item), globalfooter, globalheader, icon, modal, pagination, skipnav, stepper(+step), tab(+group,+panel), table, tooltip, unavfooter, unavheader, video, radiobutton-item) extend NysElement; nys-divider (host role=separator) extends NysReflectsAriaElement. Each drops its local id counter for the shared id gen. WCAG fixes from the per-component audit: breadcrumbs current item now has aria-current="page"; pagination wrapped in a named nav landmark with aria-current on the current page; dropdownmenu menu container gets an accessible name; accordion uses a heading wrapper (role=heading/aria-level) with a labelled region; avatar gives interactive avatars an accessible name and marks the decorative icon aria-hidden; and other per-component name/role/keyboard fixes. Wiring: @nysds/internals devDep, sideEffects:true, and tsconfig references on migrated packages. Regenerate CEM + React wrappers. Full suite: 1000 tests passing across 6 browsers; coverage 91.99% statements / 87.34% functions / 80.44% branches.
…validation guide Update the plop generator templates so new components are accessible by default: component.template.hbs extends NysFormControlElement (form-related) or NysElement (presentational) from @nysds/internals instead of LitElement, dropping the local id counter and the hand-rolled ElementInternals boilerplate in favor of the shared helpers (setFormValue/setValidityFromState/clearValidity) and documenting the aria-labelledby/aria-errormessage association pattern. package.template.hbs adds the @nysds/internals devDependency and sideEffects:true; tsconfig.template.hbs adds the project reference to ../internals. Verified by scaffolding throwaway form + presentational components and typechecking them against the internals types. Also correct the MCP form-validation guide: NYSDS form components associate errors via aria-errormessage (not aria-describedby) and names via aria-labelledby.
…remediation # Conflicts: # custom-elements.json # packages/nys-backtotop/src/nys-backtotop.ts # packages/nys-datepicker/src/nys-datepicker.ts # packages/nys-dropdownmenu/src/nys-dropdownmenu.ts # packages/nys-globalfooter/src/nys-globalfooter.ts # packages/nys-toggle/package.json # packages/react/NysDropdownMenu.d.ts # packages/react/nysds-jsx.d.ts
…essage
The merge with develop brought in datepicker tests asserting the pre-remediation
aria-describedby pattern and single-dash error id (${id}-error). Update them to
the remediated aria-errormessage mechanism and the ${id}--error convention used
across all migrated form controls.
Contributor
|
currently failing to build any packages: |
The form controls' self-registration imports (import "@nysds/nys-label" / "@nysds/nys-errormessage") are the first source-level sibling imports in the repo; prior cross-component imports lived only in excluded .stories.ts. Under `tsc -b tsconfig.build.json` (clean CI), TypeScript needs an explicit project reference to build those packages first, otherwise their .d.ts are absent and resolution fails (TS2307). Local builds masked this via stale dist artifacts. Add the missing references to the 9 form controls that self-register, and wire the plop templates to scaffold them (+ the deps) for new form-related components so the regression can't recur.
…nto fix/accessibility-remediation
clean:dist removed dist/ but left the per-package tsconfig.tsbuildinfo files (they sit next to tsconfig.json, not inside dist/). With dist gone but buildinfo retained, `tsc -b` considers a project up-to-date and skips emitting — which is now catastrophic because @nysds/internals is a shared referenced project at the root of the graph: a stale internals buildinfo means its .d.ts is never emitted and every consumer fails to resolve "@nysds/internals". Delete the buildinfo alongside dist (preserving mcp-server) so clean:dist + build is reliable.
Contributor
Author
|
@esteinborn Good catch. I'm going to assume you got that when running locally. Turns out there's a To see this in action, pull the latest and run |
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
2 participants
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
Integrates ElementInternals as the accessibility-semantics layer across all ~39
nys-*components, backed by a new shared@nysds/internalsworkspace module, and remediates a set of concrete WCAG 2.2 AA gaps surfaced by a full component audit. No breaking changes to public component API.The audit reframed the original premise: form-association (
formAssociated+attachInternalsforsetFormValue/setValidity) already existed on the form controls. The real gap was that ElementInternals was not used as the accessibility layer — default roles and ARIA state were hand-wired per component (or missing), there was no shared infrastructure, and a few real WCAG gaps existed (e.g.nys-textinputbuilding a synthetic duplicatedaria-labelinstead of associating its visible<nys-label>).What's in here
New shared layer —
packages/internals(@nysds/internals)A private, unpublished workspace module (consumed as a
devDependency, inlined at build) providing a layered, tree-shakeable Lit mixin architecture:NysElement/IdentifiedMixin— auto id generation only (presentational components that don't reflect a host role)NysReflectsAriaElement/ReflectsAriaMixin— adds hostrole+ ARIA-state reflection viaElementInternals(currentlynys-divider→role="separator")NysFormControlElement/FormControlMixin— adds form association, validity helpers (setFormValue,setValidityFromState,clearValidity), and hostaria-invalidPlus à-la-carte ARIA-association helpers (
associateControl/associateHostwith element-reference support + IDREF fallback) andgenerateId(preserves the legacy id format). 25 unit tests across 6 browsers.Component migrations
NysFormControlElement: removed per-component_internals/formAssociated/id-counter boilerplate; route value/validity through the shared helpers.NysElement(id-only);nys-divider→NysReflectsAriaElement.The hybrid accessibility approach (intentional, per component)
aria-invalid, and host roles where the host is the semantic element (nys-divider).aria-labelledby→ the visible<nys-label>, replacing syntheticaria-label) and for inner/landmark roles (alert live region, breadcrumbs/paginationnav, tabs). This is deliberate: ElementInternals reflects on the host's a11y node, not the inner<input>the AT interacts with underdelegatesFocus.WCAG fixes landed
nys-textinputvisible-label association (1.3.1 / 2.5.3);nys-datepickererror text associated viaaria-errormessage+aria-invalid;nys-breadcrumbs/nys-paginationaria-current+navlandmark;nys-dropdownmenumenu accessible name;nys-accordionheading/region;nys-avatarname + decorative icon;nys-backtotopreduced-motion handling; and others.Reliability + tooling
<nys-label>/<nys-errormessage>(with honestsideEffects) so the accessible-name association is reliable.sideEffects+ tsconfig reference.aria-errormessage/aria-labelledby, notaria-describedby).tsconfig.build.json, and CEM config wired for the new package (excluded from the manifest — it declares no elements).Notes for reviewers
@propertydeclarations stay on the leaf components; the CEM diff confirms public attributes/props/events are unchanged.export const X = /*@__PURE__*/ Mixin(LitElement)(notclass X extends Mixin(...)) so unused mixins tree-shake out of each bundle.Verification
develop(merge resolved: kept thearia-errormessageremediation over the olderaria-describedbypattern, combinednys-backtotopSSR-guard + reduced-motion, aligned versions to 1.19.1, dedupedsideEffectskeys, regenerated the manifest + React wrappers).The full PRD lives in-repo at
docs/accessibility-remediation-PRD.md.Out of scope (tracked separately)
nys-label/nys-errormessagestandalone-vs-internal architecture refactor.packages/reorganization intocomponents/foundations/frameworks/tools.