a11y | Bump design-system for D2–D5 and A7/A8 - #60
Conversation
Pull in dropdown, slider, and contrast token fixes; rewire Temperature labeling onto the D5 handle; drop composer-hint opacity; shrink axe baseline. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe change updates accessibility semantics for the temperature slider and composer hint. Portal dropdowns now receive deterministic menu IDs. Accessibility audit baselines record resolved findings and a new date. Vitest excludes the design-system directory, which uses a separate test runner. The design-system submodule reference also advances. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/dom/render.test.js (1)
557-563: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the complete slider accessibility contract.
The test verifies the role move, but it does not assert that the handle has no
aria-labelor that the wrapper has noaria-labelledbyandaria-describedby. Add these assertions so a later design-system change cannot restore the old semantics without failing this test.Suggested assertions
expect(wrapper?.getAttribute('role')).toBeNull(); + expect(wrapper?.getAttribute('aria-labelledby')).toBeNull(); + expect(wrapper?.getAttribute('aria-describedby')).toBeNull(); + expect(slider?.getAttribute('aria-label')).toBeNull();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/dom/render.test.js` around lines 557 - 563, Extend the slider accessibility assertions in the test around the temperature slider handles: verify the slider handle has no aria-label, and the presentational wrapper has no aria-labelledby or aria-describedby. Keep the existing role assertions and label/description checks unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/dom/render.test.js`:
- Around line 557-563: Extend the slider accessibility assertions in the test
around the temperature slider handles: verify the slider handle has no
aria-label, and the presentational wrapper has no aria-labelledby or
aria-describedby. Keep the existing role assertions and label/description checks
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 183cb804-4081-4bb0-adb6-4a44be6dc310
📒 Files selected for processing (7)
a11y-audits/tools/baseline.jsondesign-systempublic/app.csspublic/app.jspublic/portal-dropdown.jstests/dom/render.test.jsvitest.config.js
Summary
Bumps
design-systemtolearn_bespoke-design-systemmainatd21d15dso ChatCPT picks up the remaining Wave 2/3 DS a11y fixes (D2–D5, A7, A8) plus the number-input wheel helper.Also adapts the app to the new DS contracts and ratchets the shrink-only axe baseline (settings scans are now clean; empty-state only keeps
region).Changes
8f6c1da→d21d15d: dropdown focus/listbox/aria-selected, slider role on handles, contrast tokens, optionalinput.jsaria-labelledby/aria-describedbyon the handle (role="slider"), not the wrapperportal-dropdown-menu-*id (base D3 always sets a listbox id)opacity: 0.8soText-Body-Lighterkeeps 4.5:1color-contrastandnested-interactiveleftovers from settings scansdesign-system/**(Playwright suite lives there)Test plan
npm test(206 passed)aria-selected: 5/5regionlandmark gap (moderate; out of critical/serious scope)Made with Cursor