🎨 Palette: Add proper ARIA pressed attributes to filter buttons#538
🎨 Palette: Add proper ARIA pressed attributes to filter buttons#538EffortlessSteven wants to merge 2 commits into
Conversation
💡 What: Added dynamic `aria-pressed` states to the AC Coverage filter buttons in both UI templates (`crates/app-http/src/platform/ui.rs` and `crates/http-platform/src/ui.rs`). 🎯 Why: To properly communicate to screen reader users which filter (All, Passing, Failing, Unknown) is currently active. 📸 Before/After: Visual appearance is unchanged, but DOM reflects correct ARIA states. ♿ Accessibility: Improved screen reader support by communicating active button states programmatically.
|
👋 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. |
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughTwo UI modules ( Coverage Filter ARIA State
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves accessibility in the coverage view by adding and updating the aria-pressed attributes on the filter buttons. The feedback recommends fixing inconsistent indentation in the modified JavaScript blocks and adding defensive null checks when retrieving the active button element to prevent potential runtime TypeError exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Update active button and aria-pressed attributes | ||
| document.querySelectorAll('.filter-btn').forEach(btn => { | ||
| btn.classList.remove('active'); | ||
| btn.setAttribute('aria-pressed', 'false'); | ||
| }); | ||
| document.getElementById('filter-' + status).classList.add('active'); | ||
| const activeBtn = document.getElementById('filter-' + status); | ||
| activeBtn.classList.add('active'); | ||
| activeBtn.setAttribute('aria-pressed', 'true'); |
There was a problem hiding this comment.
The indentation on the comment at line 691 is inconsistent with the rest of the block. Additionally, to prevent potential runtime TypeError exceptions if the element with ID 'filter-' + status is not found in the DOM, we should add a defensive null check before accessing classList and calling setAttribute.
| // Update active button and aria-pressed attributes | |
| document.querySelectorAll('.filter-btn').forEach(btn => { | |
| btn.classList.remove('active'); | |
| btn.setAttribute('aria-pressed', 'false'); | |
| }); | |
| document.getElementById('filter-' + status).classList.add('active'); | |
| const activeBtn = document.getElementById('filter-' + status); | |
| activeBtn.classList.add('active'); | |
| activeBtn.setAttribute('aria-pressed', 'true'); | |
| // Update active button and aria-pressed attributes | |
| document.querySelectorAll('.filter-btn').forEach(btn => { | |
| btn.classList.remove('active'); | |
| btn.setAttribute('aria-pressed', 'false'); | |
| }); | |
| const activeBtn = document.getElementById('filter-' + status); | |
| if (activeBtn) { | |
| activeBtn.classList.add('active'); | |
| activeBtn.setAttribute('aria-pressed', 'true'); | |
| } |
| // Update active button and aria-pressed attributes | ||
| document.querySelectorAll('.filter-btn').forEach(btn => { | ||
| btn.classList.remove('active'); | ||
| btn.setAttribute('aria-pressed', 'false'); | ||
| }); | ||
| const activeBtn = document.getElementById('filter-' + status); | ||
| activeBtn.classList.add('active'); | ||
| activeBtn.setAttribute('aria-pressed', 'true'); |
There was a problem hiding this comment.
The indentation in this block is inconsistent with the rest of the function (which uses 8 spaces for statements). Additionally, we should add a defensive null check for activeBtn to prevent potential runtime TypeError exceptions if the element is not found in the DOM.
| // Update active button and aria-pressed attributes | |
| document.querySelectorAll('.filter-btn').forEach(btn => { | |
| btn.classList.remove('active'); | |
| btn.setAttribute('aria-pressed', 'false'); | |
| }); | |
| const activeBtn = document.getElementById('filter-' + status); | |
| activeBtn.classList.add('active'); | |
| activeBtn.setAttribute('aria-pressed', 'true'); | |
| // Update active button and aria-pressed attributes | |
| document.querySelectorAll('.filter-btn').forEach(btn => { | |
| btn.classList.remove('active'); | |
| btn.setAttribute('aria-pressed', 'false'); | |
| }); | |
| const activeBtn = document.getElementById('filter-' + status); | |
| if (activeBtn) { | |
| activeBtn.classList.add('active'); | |
| activeBtn.setAttribute('aria-pressed', 'true'); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/app-http/src/platform/ui.rs`:
- Around line 691-698: The coverage filter button/aria-pressed DOM update logic
is duplicated in app-http and should live only in the http-platform UI source.
Remove the platform-specific markup/script behavior from app-http and source it
from crates/http-platform/src/ui.rs instead, keeping app-http limited to route
aggregation; use the existing coverage filter UI code paths around the filter
button update logic to locate and delete the duplicate implementation.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 9c91a4d7-904f-40f3-8bd5-48af728f7016
📒 Files selected for processing (2)
crates/app-http/src/platform/ui.rscrates/http-platform/src/ui.rs
| // Update active button and aria-pressed attributes | ||
| document.querySelectorAll('.filter-btn').forEach(btn => { | ||
| btn.classList.remove('active'); | ||
| btn.setAttribute('aria-pressed', 'false'); | ||
| }); | ||
| document.getElementById('filter-' + status).classList.add('active'); | ||
| const activeBtn = document.getElementById('filter-' + status); | ||
| activeBtn.classList.add('active'); | ||
| activeBtn.setAttribute('aria-pressed', 'true'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep the coverage filter UI logic in http-platform only.
This PR adds another copy of platform-specific markup/script behavior in app-http, so future accessibility fixes will keep drifting across the two crates. Please source this coverage UI from crates/http-platform/src/ui.rs and keep app-http limited to route aggregation. As per coding guidelines, "Do not add specific endpoint logic to app-http; put endpoint logic in the appropriate http-* crate instead".
Also applies to: 806-809
🤖 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 `@crates/app-http/src/platform/ui.rs` around lines 691 - 698, The coverage
filter button/aria-pressed DOM update logic is duplicated in app-http and should
live only in the http-platform UI source. Remove the platform-specific
markup/script behavior from app-http and source it from
crates/http-platform/src/ui.rs instead, keeping app-http limited to route
aggregation; use the existing coverage filter UI code paths around the filter
button update logic to locate and delete the duplicate implementation.
Source: Coding guidelines
Test Results283 tests 245 ✅ 10m 52s ⏱️ Results for commit 466a3a9. ♻️ This comment has been updated with latest results. |
🚨 Severity: High 💡 Vulnerability: Several vulnerabilities were detected by `cargo deny`: - RUSTSEC-2026-0190: Unsoundness in `Error::downcast_mut()` in `anyhow` crate. - RUSTSEC-2026-0066, RUSTSEC-2026-0112, RUSTSEC-2026-0113, RUSTSEC-2026-0145: Insufficient validation and desynchronization in `astral-tokio-tar`. - RUSTSEC-2026-0049, RUSTSEC-2026-0098, RUSTSEC-2026-0099, RUSTSEC-2026-0104: Certificate constraint matching issues and reachable panics in `rustls-webpki`. 🎯 Impact: These vulnerabilities could lead to undefined behavior, parser differentials, malicious directory permission modifications, or bypassed revocation checks. 🔧 Fix: Updated vulnerable dependencies (`anyhow`, `astral-tokio-tar`, `rustls-webpki`). Also bumped `testcontainers` to version `0.27.3` in `crates/adapters-db-sqlx/Cargo.toml` as it was blocking the update to a patched version of `astral-tokio-tar`. ✅ Verification: Ran `cargo deny check` to ensure all advisories are resolved, and `cargo xtask selftest` to verify tests and build continue to pass successfully.
1 participant
aria-pressedstates to the AC Coverage filter buttons in both UI templates (crates/app-http/src/platform/ui.rsandcrates/http-platform/src/ui.rs).PR created automatically by Jules for task 2635188942588217288 started by @EffortlessSteven