a11y | A13 drop composer toolbar role - #74
Conversation
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>
📝 WalkthroughWalkthroughAdded a Wave 4 accessibility remediation plan covering findings, execution sequencing, verification criteria, and delivery scope. Changed the composer actions container from ARIA 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (3)
a11y-audits/8-5-26/wave-4-plan.mdpublic/index.htmltests/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) |
There was a problem hiding this comment.
📐 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.
| ### 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) | |
There was a problem hiding this comment.
📐 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 | |
There was a problem hiding this comment.
📐 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.mdRepository: 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)
PYRepository: 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` | |
There was a problem hiding this comment.
📐 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' . || trueRepository: 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))
PYRepository: 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. |
There was a problem hiding this comment.
🎯 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.
Summary
Closes #67 (A13). Composer action buttons were marked
role="toolbar"without roving tabindex / arrow-key behavior. This swaps that forrole="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
role="group"over implementing toolbar keyboard patterns for three attach/send controls — less code, honest ARIA.#sidebarResizerregion finding (A23), and app/DS issue links.Test plan
npm test -- --run tests/dom/render.test.js