fix(a11y): remove role=option from slick carousel slides (nested-interactive WCAG 4.1.2) - #13031
Closed
mekarpeles wants to merge 2 commits into
Closed
fix(a11y): remove role=option from slick carousel slides (nested-interactive WCAG 4.1.2)#13031mekarpeles wants to merge 2 commits into
mekarpeles wants to merge 2 commits into
Conversation
…1.4.3) .login-links__secondary renders on the --light-beige app-drawer background. --primary-blue (hsl 202 96% 37%) against --light-beige (hsl 48 29% 93%) yields 4.27:1 — below the 4.5:1 AA threshold. --link-blue (hsl 202 96% 28%) gives 6.51:1 against --light-beige and 7.41:1 against white (hover state). Fixes the 11 login-links__secondary violations tracked in #13009.
mekarpeles
added a commit
that referenced
this pull request
Jun 24, 2026
Regression guard for PR #13031: home page carousel slides must not have role=option (nested-interactive WCAG 4.1.2). Test currently passes against local env with fix applied.
mekarpeles
force-pushed
the
a11y/slick-nested-interactive
branch
from
June 24, 2026 20:48
333e468 to
cf47228
Compare
…ractive WCAG 4.1.2) Slick adds role="option" + tabindex="-1" to each slide when accessibility mode is on. When slides contain links the combination creates nested-interactive violations (9 nodes on /: tutorial + category carousels). Fix: after init, remove role="option" from all slides. A MutationObserver covers dynamically added slides (the loadMore path). Slick's prev/next keyboard navigation is unaffected. Fixes #13009 (WCAG 4.1.2 nested-interactive on home page).
mekarpeles
force-pushed
the
a11y/slick-nested-interactive
branch
from
June 24, 2026 20:49
cf47228 to
fbdb19a
Compare
lokesh
requested changes
Jul 3, 2026
lokesh
left a comment
Collaborator
There was a problem hiding this comment.
Blocking
static/css/components/header-bar.css:187-188— This login-links color change (--primary-blue→--link-blue) isn't part of the carousel a11y fix. Let's pull it out so this PR stays focused on the one thing. Happy to see it land on its own.
Non-blocking
Carousel.js:95-102— The observer filters onroleand the handler callsremoveAttr('role'), so each removal re-fires the observer. It terminates right away (nothing left to remove), so no loop, but it's an extra pass every time. Minor, fine to leave.
Nits
Carousel.js— Worth a one-line note in the PR body that this is interim: whenol-carousellands it should own the presentational roles from the start so we don't reintroduce the nested-interactive pattern.
Tested locally before merging? If yes and CI is green, this looks ready.
Collaborator
|
Closing as we are building out our own carousel component. Additional work on Slick carousel should be limited. Please reopen if you think this is worth investing in. |
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.
Summary
role="option"+tabindex="-1"to each slide (whenaccessibility: true, the default).role="option"makes the slide an interactive element, and interactive elements cannot contain other interactive elements per WCAG 4.1.2.Fix: After slick initializes, remove
role="option"from all.slick-slideelements. AMutationObserverhandles dynamically added slides (theloadMorepath). Slick's prev/next keyboard navigation and arrow key support are unaffected — those are driven by theslick-next/slick-prevbuttons, not therole="listbox"pattern.Test plan
/— inspect carousel slides: norole="option"attribute.12885/playwright-e2e-testsworktree (see PR test(e2e): add Playwright smoke test suite for core OL pages #12998)./— nonested-interactiveviolations.Part of WCAG 2.1 AA a11y remediation — tracked in #13009