Skip to content

Expose slideout widgets as dialogs to assistive technology - #671

Open
sean-cstk wants to merge 2 commits into
developfrom
fix/slideout-dialog-semantics
Open

Expose slideout widgets as dialogs to assistive technology#671
sean-cstk wants to merge 2 commits into
developfrom
fix/slideout-dialog-semantics

Conversation

@sean-cstk

Copy link
Copy Markdown

Addresses #670 for the slideout layout. Opening as a draft for engineering to approve or redirect — the semantics choice below is a judgment call and I'd rather have it challenged than assumed.

Problem

Slideout templates render no dialog container, so the widget root carried no role and no accessible name. A screen reader landing on a slideout announces only the "Close" button — no headline, no message, no indication a widget opened.

Confirmed against a customer's live site (ZD 22915). Their developer ran:

document.querySelector('.pf-widget').closest('[role="dialog"]')  // -> null

Change

One block in createWidgetHtml, scoped to layout === 'slideout':

  • role="dialog" on the widget root
  • aria-labelledby / aria-describedby pointed at the headline and message elements
  • ids derived from the widget's own config.id, so multiple widgets on a page don't collide

The judgment call (please push back if you disagree)

No aria-modal, and no focus trap. A slideout doesn't block the page or contain focus. Setting aria-modal="true" without a real trap tells assistive-technology users they're confined to a dialog when the rest of the page is still live and reachable — worse than the current silence. So this marks the widget as a dialog and names it, without claiming modality.

If the team would rather slideouts behave as true modals (trap + aria-modal), that's a bigger behavioral change and I'd want that decided here rather than assumed.

Verified

jsdom harness rendering a real slideout config against the built bundle, before and after:

before after
role null dialog
aria-labelledby null clorox-wipes-win-headline
aria-describedby null clorox-wipes-win-message
aria-modal null null (intended)
closest('[role="dialog"]') null found
accessible name none "Wipe. Wipe. Win!"

Modal regression check: root role stays null, container keeps role="dialog" + aria-modal="true". Unchanged.

Scope

Slideout only. bar and inline have the same gap but likely want different semantics (region/complementary rather than dialog) — left for follow-up under #670.

Independent of #669 (the aria-labelledby spelling fix on modal/gate). Neither depends on the other.

Slideout templates render no dialog container, so the widget root carried no
role and no accessible name. A screen reader landing on a slideout announced
only the "Close" button - no headline, no message, no indication a widget had
opened. Confirmed against a customer's live site (ZD 22915): on a slideout,
document.querySelector('.pf-widget').closest('[role="dialog"]') returned null.

Set role="dialog" on the widget root for slideout layouts, and point
aria-labelledby / aria-describedby at the headline and message elements, using
ids derived from the widget's own config.id so multiple widgets on one page
don't collide.

Deliberately no aria-modal and no focus trap: a slideout does not block the
page or contain focus, and claiming otherwise would tell assistive technology
users they are trapped in a dialog when they are not.

Scoped to slideout only. Modal and gate behavior is unchanged. bar and inline
layouts have the same gap but likely want different semantics, so they are left
for follow-up.

Refs #670

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sean-cstk
sean-cstk marked this pull request as ready for review August 20, 2026 20:38
@cthorn-cs

Copy link
Copy Markdown

CI failure is a test-coupling issue, not a problem with the accessibility change — the slideout semantics are correct.

The three failing A/B specs count rendered widgets with a substring id selector, e.g. $('[id*="ab-widget1"]'), which assumes each widget contributes exactly one id-bearing element. This PR's aria wiring correctly adds two more per slideout (…-headline, …-message, the targets of aria-labelledby/aria-describedby), so the count triples — hence "Expected 3 to be 1" and "Expected 6 to be 2".

Fix is test-only — scope those selectors to the widget root class so the new child ids don't inflate the count:

-    var w = $('[id*="ab-widget1"]');     // line 31
+    var w = $('.pf-widget[id*="ab-widget1"]');
-    var w = $('[id*="ab-widget2"]');     // line 72
+    var w = $('.pf-widget[id*="ab-widget2"]');
-    var w = $('[id*="ab-widget10"]');    // line 236
+    var w = $('.pf-widget[id*="ab-widget10"]');

.pf-widget matches only the root (headline/message are .pf-widget-headline/.pf-widget-message), so the counts go back to 1 and 2. Lines 185–187 don't fail because those widgets are modals (fixed pf-widget-headline id, not config.id-derived), but they could be tightened the same way for consistency. The slideout change itself — including keeping aria-modal off — is unaffected.

The A/B acceptance specs counted rendered widgets with a bare substring id
selector, e.g. $('[id*="ab-widget1"]'), which assumed each widget contributes
exactly one id-bearing element. The slideout aria wiring adds two more per
widget (<id>-headline and <id>-message, the targets of aria-labelledby and
aria-describedby), so the counts inflated: "Expected 3 to be 1" and
"Expected 6 to be 2".

Scope the selectors to .pf-widget, which matches only the widget root
(headline/message carry pf-widget-headline / pf-widget-message, distinct
class tokens). Counts return to 1 and 2.

Also scoped the three modal assertions on lines 185-187. Those did not fail,
because the aria wiring is gated on layout === 'slideout' and modals keep
their fixed template ids, but scoping them makes all six assertions count
widgets rather than id-bearing elements.

Test-only change; no production code touched. Full suite: 278/278 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cthorn-cs cthorn-cs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Slideout dialog semantics look right, and keeping aria-modal off (no focus trap without real containment) is the correct call. CI's green after the A/B selector fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants