Skip to content

fix(widget): add accessible names to toggle buttons#47

Open
mhanelia wants to merge 1 commit into
multivmlabs:mainfrom
mhanelia:fix/widget-toggle-button-accessible-names
Open

fix(widget): add accessible names to toggle buttons#47
mhanelia wants to merge 1 commit into
multivmlabs:mainfrom
mhanelia:fix/widget-toggle-button-accessible-names

Conversation

@mhanelia
Copy link
Copy Markdown

@mhanelia mhanelia commented May 8, 2026

Summary

Fixes accessibility issue where widget toggle buttons had no accessible name in some layouts, causing screen readers to announce only “button”.

What changed

  • Added aria-label to .aeo-human-btn and .aeo-ai-btn in the widget toggle.
  • Labels are derived from configured button text (humanLabel / aiLabel) with mode suffix.

Why

In icon-only mode, visible text can be hidden, so buttons may lose an accessible name for assistive technologies.
This ensures both toggle buttons are always announced with meaningful names.
Captura de tela 2026-05-08 014648

Validation

  • npm run build
  • npm run lint
  • npx vitest run src/widget/core.test.ts
  • npm run test -- --run ⚠️ 1 unrelated pre-existing failure in src/plugins/angular.test.ts (should scan Angular routes from source)

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 8, 2026

@mhanelia is attempting to deploy a commit to the Cytonic Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 8, 2026

Greptile Summary

This PR adds aria-label attributes to the Human and AI toggle buttons in the widget to ensure screen readers announce a meaningful name in icon-only mode, where the visible <span> text may be hidden.

  • aria-label is constructed from humanLabel/aiLabel config values with a mode suffix and interpolated directly into innerHTML without HTML-escaping, introducing an attribute injection risk when those strings contain \".
  • The active toggle state is communicated only via the aeo-active CSS class, which remains invisible to assistive technologies — aria-pressed is not set or updated alongside the class changes.

Confidence Score: 3/5

The change solves a real accessibility gap but introduces an attribute injection path in the same edit; merge after applying escHtml() to the interpolated label values.

The new aria-label values are built with template literals inside innerHTML and the label strings are never escaped. A double-quote in humanLabel or aiLabel collapses the attribute boundary, leaving event-handler injection (onclick, etc.) as a concrete outcome. The class already has escHtml() that encodes double-quotes, so the fix is a one-line change per attribute, but the bug ships as-is unless it is applied before merge.

src/widget/core.ts — the createToggle method needs escHtml() applied to humanLabel and aiLabel before they are embedded in the aria-label attribute values.

Security Review

  • Attribute injection via unescaped aria-label in innerHTML (src/widget/core.ts, createToggle): humanLabel and aiLabel are interpolated directly into an HTML attribute value inside an innerHTML string. A double-quote in either value closes the attribute boundary, allowing injection of arbitrary HTML attributes (e.g., onclick event handlers). The class already provides escHtml() which encodes "&quot; and should be applied to both values.

Important Files Changed

Filename Overview
src/widget/core.ts Adds aria-label to both toggle buttons using unescaped template interpolation inside innerHTML; a double-quote in humanLabel/aiLabel breaks the attribute boundary and allows attribute injection — escHtml() (already on the class) should be applied. Active state is still not announced to screen readers (no aria-pressed).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[createToggle called] --> B[Build innerHTML string]
    B --> C[Interpolate humanLabel and aiLabel without escaping]
    C --> D{Label contains a double-quote?}
    D -- No --> E[aria-label rendered correctly]
    D -- Yes --> F[Attribute boundary broken - attribute injection possible]
    E --> G[Button: aria-label = Human mode]
    E --> H[Button: aria-label = AI mode]
    G & H --> I[Screen reader announces label correctly]
    G & H --> J[aeo-active class tracks state but aria-pressed not set]
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/widget/core.ts:91-99
The new `aria-label` values are interpolated directly into an `innerHTML` string without HTML-escaping. If `humanLabel` or `aiLabel` contains a double-quote character (e.g., `'Say "Hello"'`), the quote closes the attribute boundary early — anything that follows can be parsed as additional HTML attributes, including event handlers like `onclick`. The class already exposes `escHtml()` which encodes `"` to `&quot;`, and it should be applied here to prevent this attribute injection.

```suggestion
          aria-label="${this.escHtml(this.config.widget?.humanLabel || 'Human')} mode"
        >
          ${icons.human}
          <span>${this.config.widget?.humanLabel || 'Human'}</span>
        </button>
        <button
          class="aeo-toggle-btn aeo-ai-btn"
          data-mode="ai"
          aria-label="${this.escHtml(this.config.widget?.aiLabel || 'AI')} mode"
```

### Issue 2 of 2
src/widget/core.ts:88-103
**Missing `aria-pressed` on toggle buttons**

The buttons act as a two-state toggle (one active, one inactive), but the active state is communicated only through the CSS class `aeo-active`, which is invisible to assistive technologies. Screen readers will not announce which mode is currently selected. Adding `aria-pressed="true/false"` (or wrapping with `role="group"` and using `aria-checked` inside a `role="radio"` pattern) and keeping it in sync in `updateToggleState()` would make the current selection perceivable without sight.

Reviews (1): Last reviewed commit: "fix(widget): add accessible names to tog..." | Re-trigger Greptile

Comment thread src/widget/core.ts
Comment on lines +91 to +99
aria-label="${this.config.widget?.humanLabel || 'Human'} mode"
>
${icons.human}
<span>${this.config.widget?.humanLabel || 'Human'}</span>
</button>
<button class="aeo-toggle-btn aeo-ai-btn" data-mode="ai">
<button
class="aeo-toggle-btn aeo-ai-btn"
data-mode="ai"
aria-label="${this.config.widget?.aiLabel || 'AI'} mode"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security The new aria-label values are interpolated directly into an innerHTML string without HTML-escaping. If humanLabel or aiLabel contains a double-quote character (e.g., 'Say "Hello"'), the quote closes the attribute boundary early — anything that follows can be parsed as additional HTML attributes, including event handlers like onclick. The class already exposes escHtml() which encodes " to &quot;, and it should be applied here to prevent this attribute injection.

Suggested change
aria-label="${this.config.widget?.humanLabel || 'Human'} mode"
>
${icons.human}
<span>${this.config.widget?.humanLabel || 'Human'}</span>
</button>
<button class="aeo-toggle-btn aeo-ai-btn" data-mode="ai">
<button
class="aeo-toggle-btn aeo-ai-btn"
data-mode="ai"
aria-label="${this.config.widget?.aiLabel || 'AI'} mode"
aria-label="${this.escHtml(this.config.widget?.humanLabel || 'Human')} mode"
>
${icons.human}
<span>${this.config.widget?.humanLabel || 'Human'}</span>
</button>
<button
class="aeo-toggle-btn aeo-ai-btn"
data-mode="ai"
aria-label="${this.escHtml(this.config.widget?.aiLabel || 'AI')} mode"
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/widget/core.ts
Line: 91-99

Comment:
The new `aria-label` values are interpolated directly into an `innerHTML` string without HTML-escaping. If `humanLabel` or `aiLabel` contains a double-quote character (e.g., `'Say "Hello"'`), the quote closes the attribute boundary early — anything that follows can be parsed as additional HTML attributes, including event handlers like `onclick`. The class already exposes `escHtml()` which encodes `"` to `&quot;`, and it should be applied here to prevent this attribute injection.

```suggestion
          aria-label="${this.escHtml(this.config.widget?.humanLabel || 'Human')} mode"
        >
          ${icons.human}
          <span>${this.config.widget?.humanLabel || 'Human'}</span>
        </button>
        <button
          class="aeo-toggle-btn aeo-ai-btn"
          data-mode="ai"
          aria-label="${this.escHtml(this.config.widget?.aiLabel || 'AI')} mode"
```

How can I resolve this? If you propose a fix, please make it concise.

Comment thread src/widget/core.ts
Comment on lines +88 to 103
<button
class="aeo-toggle-btn aeo-human-btn aeo-active"
data-mode="human"
aria-label="${this.config.widget?.humanLabel || 'Human'} mode"
>
${icons.human}
<span>${this.config.widget?.humanLabel || 'Human'}</span>
</button>
<button class="aeo-toggle-btn aeo-ai-btn" data-mode="ai">
<button
class="aeo-toggle-btn aeo-ai-btn"
data-mode="ai"
aria-label="${this.config.widget?.aiLabel || 'AI'} mode"
>
${icons.ai}
<span>${this.config.widget?.aiLabel || 'AI'}</span>
</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing aria-pressed on toggle buttons

The buttons act as a two-state toggle (one active, one inactive), but the active state is communicated only through the CSS class aeo-active, which is invisible to assistive technologies. Screen readers will not announce which mode is currently selected. Adding aria-pressed="true/false" (or wrapping with role="group" and using aria-checked inside a role="radio" pattern) and keeping it in sync in updateToggleState() would make the current selection perceivable without sight.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/widget/core.ts
Line: 88-103

Comment:
**Missing `aria-pressed` on toggle buttons**

The buttons act as a two-state toggle (one active, one inactive), but the active state is communicated only through the CSS class `aeo-active`, which is invisible to assistive technologies. Screen readers will not announce which mode is currently selected. Adding `aria-pressed="true/false"` (or wrapping with `role="group"` and using `aria-checked` inside a `role="radio"` pattern) and keeping it in sync in `updateToggleState()` would make the current selection perceivable without sight.

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant