fix: hide decorative pds-icon from assistive tech + stop suppressing harness color-contrast#757
Merged
Merged
Conversation
b25cb3a to
4e2c91a
Compare
582a432 to
421ca40
Compare
✅ Deploy Preview for pine-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
pixelflips
reviewed
Jun 5, 2026
pixelflips
left a comment
Member
There was a problem hiding this comment.
Two non-blocking test-coverage notes inline.
| {errorMessage && ( | ||
| <p class="pds-input__error-message" id={messageId(componentId, 'error')}> | ||
| <pds-icon icon={danger} size="small" /> | ||
| <pds-icon aria-hidden="true" icon={danger} size="small" /> |
Member
There was a problem hiding this comment.
Non-blocking, for consistency: the same aria-hidden="true" change on the decorative icon had its .spec.tsx snapshot updated for pds-accordion, pds-select, and pds-sortable-item, but pds-input.spec.tsx wasn't updated to assert it. The existing error-message spec only checks expect(errorMessage).not.toBeNull(), so this attribute isn't pinned by a spec snapshot here. Worth adding for parity — the e2e axe test already covers the behavior end-to-end.
pixelflips
approved these changes
Jun 5, 2026
pixelflips
left a comment
Member
There was a problem hiding this comment.
Couple of non-blockers, otherwise LGTM! 👍🏼
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
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.
Description
Resolves two follow-ups surfaced by the accessibility rollout (#752–#754).
1.
role-img-alt— fix decorative icons (was suppressed)pds-icon(external,@pine-ds/icons) rendersrole="img"with no accessible name, so axe'srole-img-altfired wherever an icon is used decoratively. Rather than per-test suppressions, this marks the decorative usagesaria-hidden="true"at the call sites:pds-copytext(copy-button icon — button keeps its value<span>as its accessible name)pds-accordion(chevron) ·pds-select(dropdown chevron + error icon) ·pds-sortable-item(drag handle) ·pds-input(error icon)Each is decorative reinforcement; the control/text already carries the accessible name (verified no
button-nameregressions). Therole-img-altsuppressions in pds-input/accordion/select/sortable e2e tests are removed and now enforce.2.
color-contrast— unreliable in the e2e harness (was suppressed per-test)Moved
color-contrastintoDEFAULT_DISABLED_RULESinutils/test/axe.tswith a rationale comment: the bare Stencil E2E harness renders without the theme/global stylesheet and with fonts that may fail to load, so axe measures contrast against incidental colors (flaky run-to-run). Removed the per-test suppressions in pds-filters/pds-toast. Contrast should be verified in a themed context (Storybook/visual).Not addressed (deferred, by design)
The off-grid motion token follow-up (100/150ms) is a design-token decision and would conflict with
_motion.scssin #736 — left as documented disables.Stacked on #754. Note:
pds-copytext'srole-img-altsuppression lives in #753 (not in this branch); itspds-copytext.tsxicon is fixed here, so that suppression becomes a harmless no-op — drop it when #753 lands (happy to push a one-liner).New dependencies: none.
Fixes #(no-issue)
Type of change
How Has This Been Tested?
stencil test --e2efor input/accordion/select/sortable/filters/toast — all pass with suppressions removed (267 tests); confirms thearia-hiddenfix clearsrole-img-altwith no newbutton-nameviolations.stencil test --specfor the 5 changed components — all pass (2608 tests) after updating the expectedtoEqualHtmlmarkup to includearia-hidden.unit tests
e2e tests
accessibility tests
tested manually
other:
Test Configuration:
Checklist:
Note
Low Risk
Changes are limited to decorative ARIA on icons and test expectations, with no functional or API behavior changes to the controls themselves.
Overview
Decorative
pds-iconinstances are nowaria-hidden="true"so assistive tech ignores unnamedrole="img"chevrons and error glyphs while labels and error text stay the accessible name. This is applied onpds-accordion(chevron),pds-input/pds-select(error icons),pds-select(dropdown chevron), andpds-sortable-item(drag handle).Component e2e accessibility tests no longer disable axe’s
role-img-altfor those cases;pds-filtersandpds-toastlikewise stop passing localcolor-contrastoverrides (aligned with harness-level contrast handling described in the PR stack). Spec snapshots and apds-inputerror-icon assertion were updated, andpds-selectgained an e2e case with an error message.Reviewed by Cursor Bugbot for commit 058967e. Bugbot is set up for automated code reviews on this repo. Configure here.