From e5b83d0a201e7360db434d9ebaf231f18df158db Mon Sep 17 00:00:00 2001 From: Joe Elstner Date: Sun, 20 Sep 2026 10:35:15 -0500 Subject: [PATCH] =?UTF-8?q?fix(concierge):=201.4.3=20=E2=80=94=20the=20inp?= =?UTF-8?q?ut=20says=20what=20the=20user=20can=20see,=20the=20band=20exist?= =?UTF-8?q?s=20before=20it=20speaks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two a11y defects on the concierge bar, both on a surface that is about to go public on every page of scrimmage.football. THE INPUT HAD A NAME NOBODY COULD SEE aria-label was the literal "Chat with concierge", hardcoded, with no prop to change it. The only visible label this control has is its placeholder, so WCAG 2.5.3 (Label in Name) requires the accessible name to contain it. On gridiron the placeholder reads "Ask how it works" — not one word in common with the name the machine reported. A speech-input user saying what they can see addressed nothing; a screen-reader user never heard the host's wording at all. The name now defaults to `placeholder`, and `inputAriaLabel` is there for hosts who want to say more. A host rendering no placeholder gets the old literal back rather than an unnamed input — no name is worse than a generic one. Every fleet consumer improves by default: marque, adellion and endsights all ship placeholders more specific than "Chat with concierge", and none of them passes the new prop. THE BAND WAS NOT A LIVE REGION, AND aria-live ALONE WOULD NOT HAVE FIXED IT role="note" is not a live region, so a disclaimer appearing mid-session was never announced. The `sseDisclaimer` path is exactly that case: it arrives on the done event of the first answer. Adding aria-live to a conditionally-mounted element does not fix this. A live region has to be in the document BEFORE its content — a region inserted together with its text is a new node, not a change to an observed one, and screen readers routinely say nothing. So the band is always mounted and only its CONTENT is conditional. Empty, it takes `position: absolute`, which is load-bearing twice: it keeps the element in the accessibility tree, where `display: none` would not, and it takes the element out of flex layout so the panel's `gap: 12px` does not reserve a slot. A merely zero-height child would still take its gap and push the thread down 12px. VERIFICATION 55/55 tests, typecheck clean. The new guards were mutation-tested rather than trusted: restoring the hardcoded aria-label fails 5, dropping aria-live fails 2, and re-wrapping the band in a conditional fails 4 — including the one that pins NODE IDENTITY across the text arriving, which is the property the whole fix rests on. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 2 +- src/concierge.test.tsx | 148 ++++++++++++++++++++++++++++++++++++++++- src/concierge.tsx | 106 ++++++++++++++++++++++------- 3 files changed, 231 insertions(+), 25 deletions(-) diff --git a/package.json b/package.json index 6051161..3a4e14c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@isimplifyme/ui", - "version": "1.4.2", + "version": "1.4.3", "description": "React/Next.js UI library for iSimplifyMe properties \u2014 design system, article layouts, SEO helpers, bot middleware, and the iSM Concierge widget.", "homepage": "https://isimplifyme.com", "type": "module", diff --git a/src/concierge.test.tsx b/src/concierge.test.tsx index 5a8d77d..bb7e051 100644 --- a/src/concierge.test.tsx +++ b/src/concierge.test.tsx @@ -105,7 +105,7 @@ describe('with a leading node', () => { // A second announced element here would read as a separate control. The // input's own label is the one the user is meant to hear. const { input, bar } = render({ leading: sprite }); - expect(input.getAttribute('aria-label')).toBe('Chat with concierge'); + expect(input.getAttribute('aria-label')).toBe('Ask a question...'); const named = [...bar.querySelectorAll('[aria-label]')]; expect(named.filter((el) => el !== input && !el.closest('[aria-hidden="true"]'))) .toHaveLength(1); // the send button, which was always there @@ -185,3 +185,149 @@ describe('the keyboard-shortcut chip', () => { expect(narrow).toBeLessThan(wide); }); }); + +/** + * The input's accessible name. + * + * It was the literal 'Chat with concierge' with no way for a host to change + * it, which fails WCAG 2.5.3 (Label in Name): the only visible label this + * control has is its placeholder, and the accessible name has to contain + * the visible one. On gridiron the placeholder reads "Ask how it works" and + * the machine called the control something with no word in common — so a + * speech-input user saying what they can see addressed nothing, and a + * screen-reader user never heard the host's wording at all. + */ +describe('the input carries the name the user can SEE', () => { + it('defaults the accessible name to the placeholder', () => { + const { input } = render({ placeholder: 'Ask how it works' }); + expect(input.getAttribute('aria-label')).toBe('Ask how it works'); + }); + + it('defaults it for a host that sets no placeholder either', () => { + const { input } = render(); + expect(input.getAttribute('aria-label')).toBe(input.placeholder); + }); + + it('falls back rather than leaving the input unnamed', () => { + // A host that renders no placeholder at all would otherwise get an input + // with an empty accessible name, which is worse than the generic literal + // this change replaced. + const { input } = render({ placeholder: '' }); + expect(input.getAttribute('aria-label')).toBe('Chat with concierge'); + }); + + it('lets a host say more, for the cases where the placeholder is terse', () => { + const { input } = render({ + placeholder: 'Ask how it works', + inputAriaLabel: 'Ask how it works — chat with the Scrimmage assistant', + }); + expect(input.getAttribute('aria-label')).toBe( + 'Ask how it works — chat with the Scrimmage assistant', + ); + }); + + it('keeps the override containing the visible text — 2.5.3 in one assert', () => { + // Not a style rule: an accessible name that does not CONTAIN the visible + // label is the failure mode this prop exists to let hosts avoid, so the + // shipped pairing is pinned rather than left to a reviewer's eye. + const placeholder = 'Ask how it works'; + const { input } = render({ + placeholder, + inputAriaLabel: `${placeholder} — chat with the Scrimmage assistant`, + }); + expect(input.getAttribute('aria-label')).toContain(placeholder); + }); +}); + +/** + * The disclaimer band as a live region. + * + * `role="note"` is not a live region, so a band that appeared mid-session — + * the `sseDisclaimer` path, which arrives on the done event of the first + * answer — was never announced. Adding `aria-live` alone would not have + * fixed it: the region has to be in the document BEFORE its content, or the + * screen reader sees a new node rather than a change to an observed one. + * So the band is always mounted and only its CONTENT is conditional. + */ +describe('the disclaimer band exists before it has anything to say', () => { + // jsdom implements no scrollIntoView, and the panel scrolls itself to the + // newest message on mount. Every test here opens the panel, so the stub is + // scoped to this block rather than added to the shared harness. + beforeEach(() => { + Element.prototype.scrollIntoView = function scrollIntoView() {}; + }); + + /** Sends one message, which is what mounts the panel the band lives in. */ + async function openPanel(props: Record = {}) { + const r = render(props); + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + 'value', + )?.set as (v: string) => void; + await act(async () => { + setValue.call(r.input, 'hello'); + r.input.dispatchEvent(new Event('input', { bubbles: true })); + }); + await act(async () => { + r.input.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }), + ); + }); + return { ...r, band: container.querySelector('[role="note"]') as HTMLElement }; + } + + it('mounts the region even with no disclaimer to show', async () => { + const { band } = await openPanel(); + expect(band).not.toBeNull(); + expect(band.getAttribute('aria-live')).toBe('polite'); + }); + + it('announces politely once there is text', async () => { + const { band } = await openPanel({ disclaimerOpener: 'Not a person.' }); + expect(band.getAttribute('aria-live')).toBe('polite'); + expect(band.textContent).toContain('Not a person.'); + expect(band.getAttribute('aria-label')).toBe('AI assistant disclaimer'); + }); + + it('takes no space in the panel while empty', async () => { + // `position: absolute` is the whole mechanism: an absolutely-positioned + // child is not a flex item, so the panel's `gap: 12px` skips it. A + // zero-HEIGHT child would still take its gap and push the thread down. + const { band } = await openPanel(); + expect(band.style.position).toBe('absolute'); + expect(band.textContent).toBe(''); + }); + + it('names nothing while empty — an unnamed empty note is quieter', async () => { + const { band } = await openPanel(); + expect(band.getAttribute('aria-label')).toBeNull(); + }); + + it('is the same node before and after the text arrives', async () => { + // The claim the whole fix rests on. If React swapped the node, the live + // region would be new at the moment its content appeared, which is the + // bug — so this pins identity, not just presence. + const r = render({}); + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + 'value', + )?.set as (v: string) => void; + await act(async () => { + setValue.call(r.input, 'hello'); + r.input.dispatchEvent(new Event('input', { bubbles: true })); + }); + await act(async () => { + r.input.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }), + ); + }); + const before = container.querySelector('[role="note"]'); + expect(before).not.toBeNull(); + await act(async () => { + root.render(); + }); + const after = container.querySelector('[role="note"]'); + expect(after).toBe(before); + expect(after?.textContent).toContain('Not a person.'); + }); +}); diff --git a/src/concierge.tsx b/src/concierge.tsx index 479cabf..7e6c882 100644 --- a/src/concierge.tsx +++ b/src/concierge.tsx @@ -73,6 +73,20 @@ export interface ConciergeWidgetProps { leadEndpoint?: string; /** Input placeholder. */ placeholder?: string; + /** Accessible name for the input. Defaults to `placeholder`, and that + * default is the point: the placeholder is the ONLY visible label this + * control has, so WCAG 2.5.3 (Label in Name) requires the accessible + * name to contain it. A hardcoded name that ignores the host's wording + * means a speech-input user who says what they can SEE — "ask how it + * works" — addresses a control the machine calls something else. + * + * Pass this only to say MORE than the placeholder does; whatever you + * pass should still contain the placeholder text. + * + * A host that deliberately renders NO placeholder gets the old literal + * back rather than an unnamed input — no name at all is worse than a + * generic one. */ + inputAriaLabel?: string; /** Send button fill color. */ accentColor?: string; /** Bar width. Default 900px matches the iSM reference. */ @@ -138,6 +152,27 @@ interface SseErrorEvent { } type SseEvent = SseTokenEvent | SseDoneEvent | SseEmergencyEvent | SseErrorEvent; +/** + * The disclaimer band's style while it has nothing to say. + * + * `position: absolute` is load-bearing twice over. It keeps the element in + * the accessibility tree — which `display: none` would not, and an + * unrendered live region announces nothing ever — and it takes the element + * out of flex layout, so the panel's `gap: 12px` does not reserve a slot + * for an empty band. The rest is the standard visually-hidden recipe. + */ +const EMPTY_LIVE_REGION_STYLE = { + position: 'absolute', + width: '1px', + height: '1px', + overflow: 'hidden', + clipPath: 'inset(50%)', + whiteSpace: 'nowrap', + border: 0, + padding: 0, + margin: '-1px', +} as const; + // ── Utilities ────────────────────────────────────────────────────────── function substituteTokens(template: string, values: Record): string { @@ -212,6 +247,7 @@ export default function ConciergeWidget({ endpoint = '/api/concierge', leadEndpoint = '/api/concierge-lead', placeholder = 'Ask a question...', + inputAriaLabel, accentColor = '#EB1C23', maxWidth = 900, theme = 'dark', @@ -680,6 +716,10 @@ export default function ConciergeWidget({ // clips mid-word. const showShortcut = !isFocused && !input && !isLoading && !isNarrow; + // The band's live region mounts with the panel and stays; this only says + // whether it currently has anything to announce. + const hasDisclaimer = Boolean(disclaimerOpener || sseDisclaimer); + // ── Theme tokens ── // Light-mode bar + panel opacities intentionally kept low (<=0.75) so // the backdrop-filter blur reads as actual frosted glass on white pages. @@ -897,26 +937,45 @@ export default function ConciergeWidget({ persona bubbles. Renders when host configures `disclaimerOpener` OR when the SSE done event surfaces a disclaimer from the persona JSON. Prop wins on conflict. - Hotlines are bolded inline. */} - {(disclaimerOpener || sseDisclaimer) && ( -
+ Hotlines are bolded inline. + + ⚡ ALWAYS MOUNTED, and that is the a11y fix, not the + `aria-live` beside it. A live region has to be in the document + BEFORE its content arrives — a region inserted together with + its text is a new node, not a change to an observed one, and + screen readers routinely say nothing. The `sseDisclaimer` path + is exactly that case: the band appears mid-session, on the + done event of the first answer. + + Empty, it is `position: absolute`, which is doing real work: + an absolutely-positioned child is NOT a flex item, so the + panel's `gap: 12px` skips it. A merely zero-sized child would + still take its gap and push the thread down 12px. */} +
+ {hasDisclaimer && ( + <> -
- )} + + )} +
{/* Message thread */} {messages.map((msg, i) => ( @@ -1384,7 +1444,7 @@ export default function ConciergeWidget({ } }} placeholder={placeholder} - aria-label="Chat with concierge" + aria-label={inputAriaLabel || placeholder || 'Chat with concierge'} style={{ flex: 1, border: 'none',