Skip to content

a11y | A13 drop composer toolbar role - #74

Merged
BrianGenisio merged 1 commit into
mainfrom
fix/a11y-a13-drop-toolbar-role
Aug 12, 2026
Merged

a11y | A13 drop composer toolbar role#74
BrianGenisio merged 1 commit into
mainfrom
fix/a11y-a13-drop-toolbar-role

Conversation

@BrianGenisio

@BrianGenisio BrianGenisio commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #67 (A13). Composer action buttons were marked role="toolbar" without roving tabindex / arrow-key behavior. This swaps that for role="group" so the grouping label stays but keyboard semantics match ordinary Tab stops.

Also lands a11y-audits/8-5-26/wave-4-plan.md — the Wave 4 moderate/minor sequencing plan and GitHub issue map after critical/serious work.

Changes

  • Prefer role="group" over implementing toolbar keyboard patterns for three attach/send controls — less code, honest ARIA.
  • DOM regression asserts the composer chrome is a group, not a toolbar.
  • Wave 4 plan records verify-first closures (A19–A22), residual #sidebarResizer region finding (A23), and app/DS issue links.

Test plan

  • npm test -- --run tests/dom/render.test.js
  • Tab through composer attach/send controls — each remains its own stop
  • VoiceOver/rotor: composer actions announce as a group, not a toolbar
  • Confirm axe baseline does not grow

Drop role=toolbar so Tab-stop buttons are not advertised as needing
arrow-key navigation. Add the Wave 4 moderate/minor remediation plan.

Co-authored-by: Cursor <cursoragent@cursor.com>
@BrianGenisio
BrianGenisio merged commit 323e1f7 into main Aug 12, 2026
1 of 2 checks passed
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Added a Wave 4 accessibility remediation plan covering findings, execution sequencing, verification criteria, and delivery scope. Changed the composer actions container from ARIA toolbar to group while preserving its label and button behavior. Added DOM tests that verify the group role, accessible label, and send button semantics.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The Wave 4 plan covers A19–A23 and broader app and design-system work beyond linked issue #67. Move the Wave 4 plan to a separate PR or link the additional issues that define its scope.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the A13 change that removes the composer toolbar role.
Description check ✅ Passed The description explains the role change, regression test, accessibility rationale, and related plan.
Linked Issues check ✅ Passed The PR replaces the unsupported toolbar role with group and verifies the labeled group and button semantics for issue #67.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@a11y-audits/8-5-26/wave-4-plan.md`:
- Line 246: Recompute the “Net new delivery after verify” summary using the
finding and PR totals from the application and design-system tables, accounting
for optional A22 and submodule bumps. Replace the ~10–12 estimates with
consistent totals, or explicitly state the inclusion rules that produce them.
- Line 8: Change the “Issue map” heading in the Wave 4 plan from level-three to
level-two so it directly follows the top-level heading with consistent Markdown
hierarchy.
- Line 12: Keep A22’s status consistent across the document: either change the
combined A19–A22 status entry to pending mitigation, or remove the later A22
decision-dependent and optional work in the sections around the partial-easing,
product-decision, and conditional follow-up references once that decision is
complete.
- Line 57: Update the A23 entry to scope the axe finding and bundling guidance
consistently to `#sidebarResizer`, removing `#chatStatus` and .chat-main references
from this issue.
- Line 15: Update the A13 entry in the accessibility plan to reflect that the
issue is resolved, and remove A13 from all remaining-work sections. If the
document represents a pre-implementation baseline, explicitly label the A13
status accordingly.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2bd09db6-f36e-4234-8b08-66f63d79cddf

📥 Commits

Reviewing files that changed from the base of the PR and between 7f17f90 and be983e7.

📒 Files selected for processing (3)
  • a11y-audits/8-5-26/wave-4-plan.md
  • public/index.html
  • tests/dom/render.test.js

**Model:** one GitHub issue per finding, one PR per issue — except where a shared root cause clearly warrants bundling (same bar as A1/A2/A11)
**Milestone:** `WCAG 2.2 AA — Moderate & Minor` (app + DS)

### Issue map (filed Wave 4a)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the Markdown heading hierarchy.

### Issue map follows the top-level # heading without an intervening ## heading. Change it to ## Issue map or add the missing section level so heading navigation remains consistent.

Proposed fix
-### Issue map (filed Wave 4a)
+## Issue map (filed Wave 4a)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### Issue map (filed Wave 4a)
## Issue map (filed Wave 4a)
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)

[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@a11y-audits/8-5-26/wave-4-plan.md` at line 8, Change the “Issue map” heading
in the Wave 4 plan from level-three to level-two so it directly follows the
top-level heading with consistent Markdown hierarchy.

Source: Linters/SAST tools


| ID | Issue | State |
|---|---|---|
| A19–A22 | [#61](https://github.com/CodeSignal/learn_cosmo-chat/issues/61)–[#64](https://github.com/CodeSignal/learn_cosmo-chat/issues/64) | Closed (verified / mitigated) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep A22's status consistent.

Line 12 marks A19–A22 as closed. Line 36 describes A22 as partially eased, Line 99 requires a product decision, and Lines 165 and 173 retain optional or conditional A22 work. Mark A22 as pending mitigation, or remove the later work after the decision is complete.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@a11y-audits/8-5-26/wave-4-plan.md` at line 12, Keep A22’s status consistent
across the document: either change the combined A19–A22 status entry to pending
mitigation, or remove the later A22 decision-dependent and optional work in the
sections around the partial-easing, product-decision, and conditional follow-up
references once that decision is complete.

| A19–A22 | [#61](https://github.com/CodeSignal/learn_cosmo-chat/issues/61)–[#64](https://github.com/CodeSignal/learn_cosmo-chat/issues/64) | Closed (verified / mitigated) |
| A10 | [#65](https://github.com/CodeSignal/learn_cosmo-chat/issues/65) | Open |
| A12 | [#66](https://github.com/CodeSignal/learn_cosmo-chat/issues/66) | Open |
| A13 | [#67](https://github.com/CodeSignal/learn_cosmo-chat/issues/67) | Open |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(a11y-audits|.*composer.*|.*chat.*|.*wave-4.*)' | head -200
printf '%s\n' '--- plan excerpt ---'
sed -n '1,130p' a11y-audits/8-5-26/wave-4-plan.md
printf '%s\n' '--- later plan excerpt ---'
sed -n '220,250p' a11y-audits/8-5-26/wave-4-plan.md
printf '%s\n' '--- A13 and toolbar references ---'
rg -n -i -C 3 'A13|role="toolbar"|role=.group.|composer container|composer' .

Repository: CodeSignal/learn_cosmo-chat

Length of output: 50384


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- changed files ---'
git status --short
git diff --stat
printf '%s\n' '--- relevant diff ---'
git diff -- a11y-audits/8-5-26/wave-4-plan.md public/index.html tests/dom/render.test.js
printf '%s\n' '--- current A13 references in the plan ---'
rg -n -C 2 '\bA13\b|role="toolbar"|role="group"' a11y-audits/8-5-26/wave-4-plan.md

Repository: CodeSignal/learn_cosmo-chat

Length of output: 3014


🏁 Script executed:

#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re

html = Path("public/index.html").read_text()
test = Path("tests/dom/render.test.js").read_text()
plan = Path("a11y-audits/8-5-26/wave-4-plan.md").read_text()

toolbar = re.search(
    r'<div\b(?=[^>]*class="[^"]*\bcomposer__toolbar\b[^"]*)[^>]*>',
    html,
)
assert toolbar, "composer toolbar element not found"
attrs = toolbar.group(0)
print("composer toolbar:", attrs)
print("role is group:", 'role="group"' in attrs)
print("role is toolbar:", 'role="toolbar"' in attrs)
print("A13 test asserts group:", 'expect(group?.getAttribute(\'role\')).toBe(\'group\')' in test)
print("A13 remains in open issue map:", bool(re.search(r'\| A13 \| .* \| Open \|', plan)))
print("A13 remains in remaining-work table:", "| A13 |" in plan)
PY

Repository: CodeSignal/learn_cosmo-chat

Length of output: 426


Update A13 status and remaining-work entries.

public/index.html uses role="group" for .composer__toolbar, and tests/dom/render.test.js covers this state. Mark A13 as resolved in the plan and remove it from the remaining-work sections. If this is a pre-implementation baseline, label it as such.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@a11y-audits/8-5-26/wave-4-plan.md` at line 15, Update the A13 entry in the
accessibility plan to reflect that the issue is resolved, and remove A13 from
all remaining-work sections. If the document represents a pre-implementation
baseline, explicitly label the A13 status accordingly.

| A17 | Moderate | Focus not restored when edit cancelled | S | `startEditingMessage` / `exitEditMode` |
| A18 | Moderate | Delete control only reachable on hover | S | `public/app.css` (`(hover: none)` or always-visible below breakpoint) |
| A22 | Minor | Skip link (if kept) | S | `public/index.html` + CSS |
| A23* | Moderate | Empty-state axe `region: 1` | XS–S | Landmark placement of `#chatStatus` / `.chat-main` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file="$(git ls-files | awk '$0=="a11y-audits/8-5-26/wave-4-plan.md"{print; exit}')"
if [ -z "$file" ]; then
  echo "file not found"
  exit 0
fi
printf '%s\n' "== relevant lines =="
sed -n '1,130p' "$file"
printf '%s\n' "== delivery section =="
sed -n '220,247p' "$file"
printf '%s\n' "== all A23 references =="
rg -n -C 3 'A23|chatStatus|chat-main|sidebarResizer' "$file"
printf '%s\n' "== repository references to these identifiers =="
rg -n -C 2 'sidebarResizer|chatStatus|chat-main|A23' --glob '!a11y-audits/8-5-26/wave-4-plan.md' . || true

Repository: CodeSignal/learn_cosmo-chat

Length of output: 26378


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
from html.parser import HTMLParser
from pathlib import Path
import json

html_path = Path("public/index.html")
report_path = Path("a11y-audits/8-5-26/evidence/report.json")

class Tree:
    def __init__(self, tag=None, attrs=None, parent=None):
        self.tag = tag
        self.attrs = dict(attrs or [])
        self.parent = parent
        self.children = []

class Parser(HTMLParser):
    def __init__(self):
        super().__init__()
        self.root = Tree("ROOT")
        self.stack = [self.root]
    def handle_starttag(self, tag, attrs):
        node = Tree(tag, attrs, self.stack[-1])
        self.stack[-1].children.append(node)
        if tag not in {"meta", "link", "input", "img", "br", "hr"}:
            self.stack.append(node)
    def handle_startendtag(self, tag, attrs):
        self.handle_starttag(tag, attrs)
        if self.stack[-1].tag == tag:
            self.stack.pop()
    def handle_endtag(self, tag):
        for i in range(len(self.stack) - 1, 0, -1):
            if self.stack[i].tag == tag:
                del self.stack[i:]
                break

def walk(node):
    yield node
    for child in node.children:
        yield from walk(child)

p = Parser()
p.feed(html_path.read_text())
nodes = {n.attrs["id"]: n for n in walk(p.root) if "id" in n.attrs}
for ident in ("sidebar", "sidebarResizer", "chatStatus"):
    node = nodes.get(ident)
    if not node:
        print(f"{ident}: missing")
        continue
    ancestors = []
    cur = node.parent
    while cur and cur.tag != "ROOT":
        ancestors.append(
            f"{cur.tag}#{cur.attrs['id']}" if "id" in cur.attrs else cur.tag
        )
        cur = cur.parent
    print(f"{ident}: tag={node.tag}, role={node.attrs.get('role')}, ancestors={' > '.join(ancestors)}")

data = json.loads(report_path.read_text())
targets = []
def collect(value):
    if isinstance(value, dict):
        if value.get("target") == ["`#sidebarResizer`"] or value.get("target") == "`#sidebarResizer`":
            targets.append(value)
        for child in value.values():
            collect(child)
    elif isinstance(value, list):
        for child in value:
            collect(child)
collect(data)
print(f"report records targeting `#sidebarResizer`: {len(targets)}")
for item in targets[:3]:
    print(json.dumps({k: item[k] for k in ("target", "summary") if k in item}, sort_keys=True))
PY

Repository: CodeSignal/learn_cosmo-chat

Length of output: 760


Align A23 with #sidebarResizer. The axe finding targets #sidebarResizer; #chatStatus is inside .chat-main and is not the reported target. Update the A23 scope and bundling guidance to use #sidebarResizer consistently.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@a11y-audits/8-5-26/wave-4-plan.md` at line 57, Update the A23 entry to scope
the axe finding and bundling guidance consistently to `#sidebarResizer`, removing
`#chatStatus` and .chat-main references from this issue.

| 4e — DS hygiene/tokens | D8(+D10+D11), D12 + bumps | DS → app | Fully vs app 4b |
| 4f — Optional | A22 skip link | app | Yes |

**Net new delivery after verify:** ~10–12 findings → ~10–12 issues → ~10–12 PRs including bumps, with A14+A15 as the one deliberate app bundle and an optional modal hygiene bundle in the DS.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Recompute the delivery totals.

The plan lists 9 application findings after excluding optional A22, plus 5 design-system findings. That is 14 findings, or 15 if A22 remains open. Lines 174–176 produce 11–17 PRs when app PRs, design-system PRs, and submodule bumps are combined. The ~10–12 summary is not consistent with those tables. Update the totals or state the inclusion rules.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@a11y-audits/8-5-26/wave-4-plan.md` at line 246, Recompute the “Net new
delivery after verify” summary using the finding and PR totals from the
application and design-system tables, accounting for optional A22 and submodule
bumps. Replace the ~10–12 estimates with consistent totals, or explicitly state
the inclusion rules that produce them.

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.

[a11y][A13] role=toolbar without arrow-key navigation

1 participant