🎨 Palette: Accessibility improvements for interactive elements#60
🎨 Palette: Accessibility improvements for interactive elements#60tachyon-beep wants to merge 2 commits into
Conversation
Co-authored-by: tachyon-beep <544926+tachyon-beep@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9de4c253f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div class="flex items-center gap-3 text-xs text-secondary"> | ||
| <span id="refreshIndicator" class="opacity-0 text-accent">Refreshing...</span> | ||
| <span id="healthBadge" onclick="showHealthBreakdown()" class="cursor-pointer px-2 py-0.5 rounded text-xs font-bold" title="System Health Score — click for breakdown">--</span> | ||
| <button id="healthBadge" onclick="showHealthBreakdown()" class="cursor-pointer px-2 py-0.5 rounded text-xs font-bold" title="System Health Score — click for breakdown" style="min-height: unset">--</button> |
There was a problem hiding this comment.
When this badge is reachable by keyboard, its accessible name comes from its text content, which is just -- initially and later only the numeric score from computeHealthScore(). Screen reader users tabbing to it will hear something like “82 button” without the “system health” context or that it opens a breakdown; the title is not a reliable accessible name when the button has content. Add an aria-label here and update it alongside the score.
Useful? React with 👍 / 👎.
| const canReusePositions = | ||
| cyNodes.length > 0 && | ||
| cyNodes.every((n) => Object.prototype.hasOwnProperty.call(previousPositions, n.data.id)); | ||
| cyNodes.length > 0 && cyNodes.every((n) => Object.hasOwn(previousPositions, n.data.id)); |
There was a problem hiding this comment.
This dashboard JS is served directly as browser modules without transpilation, and replacing Object.prototype.hasOwnProperty.call(...) with Object.hasOwn(...) makes graph re-renders throw in browsers that do not implement the newer ES2022 helper. In those environments, any graph topology change reaches this branch and prevents the graph from rebuilding; keep the previous helper or add a compatibility wrapper.
Useful? React with 👍 / 👎.
| <span>Deps: <b id="footDeps" class="text-primary">0</b></span> | ||
| <canvas id="sparkline" width="100" height="20" title="14-day throughput trend" class="opacity-70"></canvas> | ||
| <span id="staleBadge" class="hidden text-xs bg-red-900/50 text-red-400 px-2 py-0.5 rounded border border-red-800 cursor-pointer" onclick="showStaleIssues()" title="WIP issues with no updates for >2 hours. Click to view."></span> | ||
| <button id="staleBadge" class="hidden text-xs bg-red-900/50 text-red-400 px-2 py-0.5 rounded border border-red-800 cursor-pointer" onclick="showStaleIssues()" title="WIP issues with no updates for >2 hours. Click to view." style="min-height: unset"></button> |
There was a problem hiding this comment.
The inline min-height: unset on the converted badge buttons overrides the coarse-pointer rule that makes buttons at least 44px tall, so on touch devices the stale badge remains a tiny text pill despite now being the primary tap target for opening stale issues. Remove the override for coarse pointers or replace it with styling that keeps the 44px hit area while preserving the visual pill size.
Useful? React with 👍 / 👎.
**What:** - Converted interactive `<span>` elements (`#healthBadge`, `#staleBadge`) to `<button>` elements to properly support keyboard navigation and screen readers. - Added missing `aria-label` attributes to form elements (inputs and selects) that lack explicit visual `<label>` associations. - Documented findings regarding clickable spans in `.Jules/palette.md` for future consistency. - Formatted JS files with Biome (`pnpm run format`). - Fixed failing test `test_topology_change_reuses_positions_only_when_all_nodes_have_positions` that was checking for old JS string. **Why:** To improve the accessibility of the dashboard, making it usable via keyboard and assistive technologies by providing correct semantic roles for interactive elements and accessible names for form inputs. Interactive elements built as generic containers (`span` or `div`) cannot be focused and lack semantic meaning unless appropriately extended with `role` and `tabindex` - using a native `button` is the simplest, most accessible approach. **Accessibility:** - Interactive badges are now focusable via `Tab` and activated via `Enter`/`Space`. - Screen readers will now announce the selects and inputs correctly due to proper `aria-label`s. Co-authored-by: tachyon-beep <544926+tachyon-beep@users.noreply.github.com>
1 participant
What:
<span>elements (#healthBadge,#staleBadge) to<button>elements to properly support keyboard navigation and screen readers.aria-labelattributes to form elements (inputs and selects) that lack explicit visual<label>associations..Jules/palette.mdfor future consistency.pnpm run format).Why:
To improve the accessibility of the dashboard, making it usable via keyboard and assistive technologies by providing correct semantic roles for interactive elements and accessible names for form inputs. Interactive elements built as generic containers (
spanordiv) cannot be focused and lack semantic meaning unless appropriately extended withroleandtabindex- using a nativebuttonis the simplest, most accessible approach.Accessibility:
Taband activated viaEnter/Space.aria-labels.PR created automatically by Jules for task 12290299917889712158 started by @tachyon-beep