Skip to content

Commit 1f69f04

Browse files
committed
Fix display component a11y and form coverage
Add role forwarding for labeled display components to satisfy axe-core, and expand tests for keyboard handling, native form participation, and slotted-content edge cases across the component suite. Also update the changelog and TODO status for the completed hardening pass.
1 parent 7740abd commit 1f69f04

22 files changed

Lines changed: 707 additions & 9 deletions

CHANGELOG.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,48 @@ reflects package releases published to npm.
3232
applicable in later additions, is implemented as native Lit element state
3333
rather than ported scripts).
3434

35+
### Fixed
36+
37+
- `sp-badge`, `sp-card`, `sp-icon-box`, `sp-testimonial`, `sp-avatar`,
38+
`sp-pricing-card`, and `sp-tag` now render `role="group"` on their native
39+
element whenever an `aria-label` or `aria-labelledby` is forwarded. These
40+
components wrap a roleless `<div>`/`<span>`, and ARIA forbids `aria-label`/
41+
`aria-labelledby` on an element with no role — axe-core's
42+
`aria-prohibited-attr` rule flagged this as a violation. The role is
43+
omitted when no label is forwarded, so unlabeled instances are unaffected.
44+
Found while extending `tests/accessibility.test.ts` coverage (Phase 5 P1).
45+
3546
### Changed
3647

3748
- Bumped `@phcdevworks/spectre-tokens` to `^3.2.0`, closing dependency drift
3849
against the current published `project-design` version. The upstream
3950
release was additive only — no source changes required here.
4051

52+
### Testing
53+
54+
- Phase 5 P1 hardening pass (no other source changes required):
55+
- Keyboard interaction audit for `sp-select`, `sp-radio`, and `sp-checkbox`
56+
confirmed no custom `keydown` handling exists anywhere in the package —
57+
native browser keyboard semantics (space/enter activation, native radio
58+
group navigation) pass through untouched. Added tests asserting native
59+
keydown events are never intercepted or `preventDefault()`-ed, and that
60+
native input/change events correctly update host properties.
61+
- Form-association audit for `sp-input`, `sp-textarea`, `sp-select`,
62+
`sp-checkbox`, and `sp-radio` confirmed native form participation
63+
(`FormData`, `checkValidity()`, ancestor `form.checkValidity()`) works
64+
correctly without `ElementInternals`/`formAssociated`, since each
65+
component renders a real native form control as a light-DOM descendant.
66+
Added end-to-end tests submitting through an ancestor `<form>`.
67+
- Extended `tests/accessibility.test.ts` with axe-core scenarios covering
68+
populated, empty, and slot-projection states for `sp-badge`, `sp-card`,
69+
`sp-icon-box`, `sp-rating`, and `sp-testimonial` — this surfaced the
70+
`role="group"` fix above.
71+
- Audited `sp-card` and `sp-testimonial` for slotted-content edge cases
72+
(whitespace-only text, empty slotted elements, nested interactive
73+
elements, long-text overflow). Confirmed `hasMeaningfulContent()` and
74+
`SpectreProjectableElement` already handle all four correctly; added
75+
regression tests locking in the behavior.
76+
4177
## [1.5.0] - 2026-06-11
4278

4379
**Release Title:** Phase 5 Display Surface Expansion

TODO.md

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,34 @@ Each new component requires source, tests, exports, docs, `AGENTS.md` component
170170
inventory update, and `CHANGELOG.md [Unreleased]` entry, plus explicit approval
171171
per `AGENTS.md` before work begins.
172172

173-
### P1: Deeper Hardening and Coverage
173+
### P1: Deeper Hardening and Coverage — Completed
174174

175-
- [ ] Audit keyboard interaction coverage for `sp-select`, `sp-radio`, and
175+
- [x] Audit keyboard interaction coverage for `sp-select`, `sp-radio`, and
176176
`sp-checkbox` (arrow-key navigation, space/enter activation, focus order).
177+
No custom `keydown` handling exists anywhere in the package — native
178+
browser keyboard semantics pass through untouched. Added tests proving
179+
native keydown events are never intercepted.
177180

178-
- [ ] Audit form-association behavior (`formAssociated`, `ElementInternals`,
181+
- [x] Audit form-association behavior (`formAssociated`, `ElementInternals`,
179182
validity state reporting) across `sp-input`, `sp-textarea`, `sp-select`,
180-
`sp-checkbox`, and `sp-radio`.
183+
`sp-checkbox`, and `sp-radio`. Native form participation already works
184+
via light-DOM native controls — no `ElementInternals` shim needed.
185+
Added end-to-end `FormData`/`checkValidity()` tests submitting through
186+
an ancestor `<form>`.
181187

182-
- [ ] Extend `tests/accessibility.test.ts` with axe-core scenarios for the five
188+
- [x] Extend `tests/accessibility.test.ts` with axe-core scenarios for the five
183189
Phase 3 display components (`sp-badge`, `sp-card`, `sp-icon-box`,
184190
`sp-rating`, `sp-testimonial`) covering populated, empty, and
185-
slot-projection states.
191+
slot-projection states. Surfaced a real `aria-prohibited-attr` violation
192+
(forwarded `aria-label`/`aria-labelledby` on a roleless `<div>`/`<span>`)
193+
across `sp-badge`, `sp-card`, `sp-icon-box`, `sp-testimonial`,
194+
`sp-avatar`, `sp-pricing-card`, and `sp-tag`; fixed by rendering
195+
`role="group"` whenever a label is forwarded.
186196

187-
- [ ] Audit `sp-card` and `sp-testimonial` for slotted-content edge cases
197+
- [x] Audit `sp-card` and `sp-testimonial` for slotted-content edge cases
188198
(empty slots, nested interactive elements, long-text overflow).
199+
Confirmed `hasMeaningfulContent()` and `SpectreProjectableElement`
200+
already handle all four correctly; added regression tests.
189201

190202
### P2: Tooling and Contributor DX
191203

@@ -263,8 +275,9 @@ begins, per `AGENTS.md`.
263275
5. Phase 4 P1 — done.
264276
6. Phase 5 P0 — done. Added sp-alert, sp-avatar, sp-spinner, sp-tag, and
265277
sp-pricing-card.
266-
7. **Phase 5 P1 — active.** Hardening and coverage.
267-
8. Phase 5 P2 — tooling/DX; requires explicit approval.
278+
7. Phase 5 P1 — done. Hardening and coverage audits; fixed an
279+
`aria-prohibited-attr` violation found across seven display components.
280+
8. **Phase 5 P2 — next up.** Tooling/DX; requires explicit approval.
268281

269282
---
270283

src/components/avatar/sp-avatar.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ export class SpectreAvatarElement
128128
return (this.disabled ?? false) || (this.loading ?? false)
129129
}
130130

131+
private get hasForwardedLabel(): boolean {
132+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy)
133+
}
134+
131135
override render() {
132136
return html`<div
133137
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -137,6 +141,7 @@ export class SpectreAvatarElement
137141
class="${this.avatarClasses}"
138142
data-sp-avatar-native
139143
id="${ifDefined(this.id || undefined)}"
144+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
140145
title="${ifDefined(this.title || undefined)}"
141146
>
142147
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/badge/sp-badge.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ export class SpectreBadgeElement extends SpectreProjectableElement implements Sp
9999
return (this.disabled ?? false) || (this.loading ?? false);
100100
}
101101

102+
private get hasForwardedLabel(): boolean {
103+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy);
104+
}
105+
102106
override render() {
103107
return html`<span
104108
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -108,6 +112,7 @@ export class SpectreBadgeElement extends SpectreProjectableElement implements Sp
108112
class="${this.badgeClasses}"
109113
data-sp-badge-native
110114
id="${ifDefined(this.id || undefined)}"
115+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
111116
title="${ifDefined(this.title || undefined)}"
112117
>
113118
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/card/sp-card.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ export class SpectreCardElement extends SpectreProjectableElement implements Spe
101101
return (this.disabled ?? false) || (this.loading ?? false);
102102
}
103103

104+
private get hasForwardedLabel(): boolean {
105+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy);
106+
}
107+
104108
override render() {
105109
return html`<div
106110
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -110,6 +114,7 @@ export class SpectreCardElement extends SpectreProjectableElement implements Spe
110114
class="${this.cardClasses}"
111115
data-sp-card-native
112116
id="${ifDefined(this.id || undefined)}"
117+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
113118
title="${ifDefined(this.title || undefined)}"
114119
>
115120
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/icon-box/sp-icon-box.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ export class SpectreIconBoxElement extends SpectreProjectableElement implements
113113
return (this.disabled ?? false) || (this.loading ?? false);
114114
}
115115

116+
private get hasForwardedLabel(): boolean {
117+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy);
118+
}
119+
116120
override render() {
117121
return html`<div
118122
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -122,6 +126,7 @@ export class SpectreIconBoxElement extends SpectreProjectableElement implements
122126
class="${this.iconBoxClasses}"
123127
data-sp-icon-box-native
124128
id="${ifDefined(this.id || undefined)}"
129+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
125130
title="${ifDefined(this.title || undefined)}"
126131
>
127132
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/pricing-card/sp-pricing-card.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ export class SpectrePricingCardElement
9898
return (this.disabled ?? false) || (this.loading ?? false)
9999
}
100100

101+
private get hasForwardedLabel(): boolean {
102+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy)
103+
}
104+
101105
override render() {
102106
return html`<div
103107
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -107,6 +111,7 @@ export class SpectrePricingCardElement
107111
class="${this.pricingCardClasses}"
108112
data-sp-pricing-card-native
109113
id="${ifDefined(this.id || undefined)}"
114+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
110115
title="${ifDefined(this.title || undefined)}"
111116
>
112117
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/tag/sp-tag.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ export class SpectreTagElement
135135
return (this.disabled ?? false) || (this.loading ?? false)
136136
}
137137

138+
private get hasForwardedLabel(): boolean {
139+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy)
140+
}
141+
138142
override render() {
139143
return html`<span
140144
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -144,6 +148,7 @@ export class SpectreTagElement
144148
class="${this.tagClasses}"
145149
data-sp-tag-native
146150
id="${ifDefined(this.id || undefined)}"
151+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
147152
title="${ifDefined(this.title || undefined)}"
148153
>
149154
${this.hasProjectedContent ? this.projectedContent : nothing}

src/components/testimonial/sp-testimonial.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ export class SpectreTestimonialElement
100100
return (this.disabled ?? false) || (this.loading ?? false);
101101
}
102102

103+
private get hasForwardedLabel(): boolean {
104+
return Boolean(this.forwardedAriaLabel || this.forwardedAriaLabelledBy);
105+
}
106+
103107
override render() {
104108
return html`<div
105109
aria-busy="${this.loading ? 'true' : 'false'}"
@@ -109,6 +113,7 @@ export class SpectreTestimonialElement
109113
class="${this.testimonialClasses}"
110114
data-sp-testimonial-native
111115
id="${ifDefined(this.id || undefined)}"
116+
role="${ifDefined(this.hasForwardedLabel ? 'group' : undefined)}"
112117
title="${ifDefined(this.title || undefined)}"
113118
>
114119
${this.hasProjectedContent ? this.projectedContent : nothing}

tests/accessibility.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,31 @@ import { afterEach, beforeAll, describe, expect, it } from 'vitest'
22
import axe from 'axe-core'
33

44
import {
5+
defineSpectreBadge,
56
defineSpectreButton,
7+
defineSpectreCard,
68
defineSpectreCheckbox,
79
defineSpectreFieldset,
10+
defineSpectreIconBox,
811
defineSpectreInput,
912
defineSpectreLabel,
1013
defineSpectreRadio,
14+
defineSpectreRating,
1115
defineSpectreSelect,
16+
defineSpectreTestimonial,
1217
defineSpectreTextarea,
18+
SpectreBadgeElement,
1319
SpectreButtonElement,
20+
SpectreCardElement,
1421
SpectreCheckboxElement,
1522
SpectreFieldsetElement,
23+
SpectreIconBoxElement,
1624
SpectreInputElement,
1725
SpectreLabelElement,
1826
SpectreRadioElement,
27+
SpectreRatingElement,
1928
SpectreSelectElement,
29+
SpectreTestimonialElement,
2030
SpectreTextareaElement,
2131
} from '../src'
2232

@@ -38,13 +48,18 @@ async function audit(el: HTMLElement): Promise<axe.Result[]> {
3848

3949
describe('accessibility audit', () => {
4050
beforeAll(() => {
51+
defineSpectreBadge()
4152
defineSpectreButton()
53+
defineSpectreCard()
4254
defineSpectreCheckbox()
4355
defineSpectreFieldset()
56+
defineSpectreIconBox()
4457
defineSpectreInput()
4558
defineSpectreLabel()
4659
defineSpectreRadio()
60+
defineSpectreRating()
4761
defineSpectreSelect()
62+
defineSpectreTestimonial()
4863
defineSpectreTextarea()
4964
})
5065

@@ -175,4 +190,104 @@ describe('accessibility audit', () => {
175190
const violations = await audit(el)
176191
expect(violations).toEqual([])
177192
})
193+
194+
it('sp-badge has no violations with populated text content', async () => {
195+
const el = document.createElement('sp-badge') as SpectreBadgeElement
196+
el.textContent = 'New'
197+
const violations = await audit(el)
198+
expect(violations).toEqual([])
199+
})
200+
201+
it('sp-badge has no violations when empty with an aria-label', async () => {
202+
const el = document.createElement('sp-badge') as SpectreBadgeElement
203+
el.setAttribute('aria-label', 'Unread notifications')
204+
const violations = await audit(el)
205+
expect(violations).toEqual([])
206+
})
207+
208+
it('sp-badge has no violations with slotted markup', async () => {
209+
const el = document.createElement('sp-badge') as SpectreBadgeElement
210+
el.innerHTML = '<svg aria-hidden="true"></svg><span>3 new</span>'
211+
const violations = await audit(el)
212+
expect(violations).toEqual([])
213+
})
214+
215+
it('sp-card has no violations with populated heading content', async () => {
216+
const el = document.createElement('sp-card') as SpectreCardElement
217+
el.innerHTML = '<h2>Card title</h2><p>Card body</p>'
218+
const violations = await audit(el)
219+
expect(violations).toEqual([])
220+
})
221+
222+
it('sp-card has no violations when empty with an aria-label', async () => {
223+
const el = document.createElement('sp-card') as SpectreCardElement
224+
el.setAttribute('aria-label', 'Empty placeholder card')
225+
const violations = await audit(el)
226+
expect(violations).toEqual([])
227+
})
228+
229+
it('sp-card interactive state has no violations with slotted content', async () => {
230+
const el = document.createElement('sp-card') as SpectreCardElement
231+
el.interactive = true
232+
el.innerHTML = '<h2>Plan</h2><a href="/plan">View details</a>'
233+
const violations = await audit(el)
234+
expect(violations).toEqual([])
235+
})
236+
237+
it('sp-icon-box has no violations with a projected icon and aria-label', async () => {
238+
const el = document.createElement('sp-icon-box') as SpectreIconBoxElement
239+
el.setAttribute('aria-label', 'Security feature')
240+
el.innerHTML = '<svg aria-hidden="true"></svg>'
241+
const violations = await audit(el)
242+
expect(violations).toEqual([])
243+
})
244+
245+
it('sp-icon-box has no violations when empty with an aria-label', async () => {
246+
const el = document.createElement('sp-icon-box') as SpectreIconBoxElement
247+
el.setAttribute('aria-label', 'Placeholder icon')
248+
const violations = await audit(el)
249+
expect(violations).toEqual([])
250+
})
251+
252+
it('sp-rating has no violations with the default self-generated label', async () => {
253+
const el = document.createElement('sp-rating') as SpectreRatingElement
254+
const violations = await audit(el)
255+
expect(violations).toEqual([])
256+
})
257+
258+
it('sp-rating has no violations with an explicit aria-label override', async () => {
259+
const el = document.createElement('sp-rating') as SpectreRatingElement
260+
el.setAttribute('aria-label', 'Average customer rating')
261+
const violations = await audit(el)
262+
expect(violations).toEqual([])
263+
})
264+
265+
it('sp-rating has no violations with a visible text label', async () => {
266+
const el = document.createElement('sp-rating') as SpectreRatingElement
267+
el.label = '4.5 out of 5 stars'
268+
const violations = await audit(el)
269+
expect(violations).toEqual([])
270+
})
271+
272+
it('sp-testimonial has no violations with populated quote content', async () => {
273+
const el = document.createElement('sp-testimonial') as SpectreTestimonialElement
274+
el.innerHTML = '<blockquote>Great product.</blockquote><cite>Jane Doe</cite>'
275+
const violations = await audit(el)
276+
expect(violations).toEqual([])
277+
})
278+
279+
it('sp-testimonial has no violations when empty with an aria-label', async () => {
280+
const el = document.createElement('sp-testimonial') as SpectreTestimonialElement
281+
el.setAttribute('aria-label', 'Empty testimonial placeholder')
282+
const violations = await audit(el)
283+
expect(violations).toEqual([])
284+
})
285+
286+
it('sp-testimonial has no violations with slotted nested interactive content', async () => {
287+
const el = document.createElement('sp-testimonial') as SpectreTestimonialElement
288+
el.innerHTML =
289+
'<blockquote>Great product.</blockquote><a href="/reviews/1">Read full review</a>'
290+
const violations = await audit(el)
291+
expect(violations).toEqual([])
292+
})
178293
})

0 commit comments

Comments
 (0)