refactor(ui): tokenise context-warning banner + a11y fixes - #81
Merged
Conversation
Three fixes guided by the new frontend-ui-engineering skill, all on the context-usage warning surface plus one neighbour: 1. #context-warning was a <div onclick> styled with hardcoded #e67e22 and a click-anywhere-to-dismiss handler. Replaced with a proper sticky banner using --warning / --text-warning tokens (matching the existing-but-unused .banner.warn rule), aria-live='polite' for announce-on-show, and a real <button> with aria-label='Dismiss context warning' that handles keyboard close. 2. updateTokenSummary() in dodo-chat.js coloured the token meter via hardcoded #e74c3c / #e67e22. Switched to --text-danger / --text-warning. Theme-aware now. 3. session-item's role='button' only handled Enter, not Space — an ARIA-spec violation for any element with role=button. Added Space handler with preventDefault() (otherwise the page scrolls) plus aria-label so screen readers announce the session title. The banner's show/hide functions extracted to showContextWarning / hideContextWarning so the three call sites (token-summary update, deleteSession, selectSession) stop writing style.display directly and the dismiss button's markup isn't blown away on every update. Behaviour preserved: banner appears at >80% context, hides otherwise, and resets on session change/delete. Dismissal is until the next update tick — same as before, just no longer a click-anywhere foot-gun. typecheck + lint + 777 tests green.
aria-label on an element hides inner text from AT (only the label is
announced). The session-item div contains both the title and a meta line
with status + timestamp; with aria-label='Open session ${title}' screen
readers would lose access to status and timestamp.
Removing aria-label lets accessible-name computation fall through to the
title + meta children, matching what sighted users see. role='button' +
the surrounding context still convey that it's actionable.
beep-boop-ruskin-agent-🤖
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First-pass refactor guided by the newly-adapted
frontend-ui-engineeringskill (from addyosmani/agent-skills, tuned for Dodo's vanilla-HTML + Kumo-token stack).What
Three fixes on and around the context-usage warning surface:
#context-warningbanner — was a<div onclick>with hardcoded#e67e22and click-anywhere-to-dismiss. Replaced with a proper sticky banner using--warning/--text-warningtokens (matching the existing-but-unused.banner.warnrule),aria-live="polite", and a real<button>witharia-label="Dismiss context warning".dodo-chat.js—#e74c3c/#e67e22→var(--text-danger)/var(--text-warning). Theme-aware now.session-itemrole="button"— only handled Enter, not Space. ARIA-spec violation. Added Space handler withpreventDefault()plusaria-labelso screen readers announce the session title.Why this scope
These three are concrete, named violations of the skill's red-flag list:
<div onclick>instead of<button>aria-liveon a status bannerBounded to one logical surface so it's reviewable.
Verification
npm run typecheck✓npm run lint✓npm test✓ (777/777)Not in scope (deferred)
The skill audit also flagged:
<div role="button">items indodo-files.js/dodo-kanban.js(large, touches drag-and-drop selection state), and the 1021-line monolithicindex.html(worth splitting, but a separate change). Happy to do either as follow-ups.Skill
Lives at
~/agent-hq/.opencode/skill/frontend-ui-engineering/SKILL.md(committed here) — vanilla-HTML/Kumo-token flavour, positioned to complementhtml/web-accessibility/design-uxrather than overlap.