A11y/usage bar progressbar - #237
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds ARIA progressbar semantics and coverage to ChangesUsageBar accessibility
Contributor guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop-tauri/src/surfaces/tray/UsageBar.test.tsx (1)
43-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that the visible percentage stays raw.
The overshoot test checks only the clamped ARIA value and label. For
usedPercent: 140, the visible percentage should remain140%whilearia-valuenowremains100.Suggested assertion
expect(track).toHaveAttribute("aria-label", "Session usage: 100%"); + expect(screen.getByText("140%")).toBeTruthy();🤖 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 `@apps/desktop-tauri/src/surfaces/tray/UsageBar.test.tsx` around lines 43 - 50, Extend the overshoot test for UsageBar with an assertion that the visible percentage remains the raw 140%, while preserving the existing aria-valuenow and aria-label assertions at 100%.
🤖 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.
Nitpick comments:
In `@apps/desktop-tauri/src/surfaces/tray/UsageBar.test.tsx`:
- Around line 43-50: Extend the overshoot test for UsageBar with an assertion
that the visible percentage remains the raw 140%, while preserving the existing
aria-valuenow and aria-label assertions at 100%.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: acb621a8-2c7f-4263-b0a5-addde9e83f62
📒 Files selected for processing (3)
README.mdapps/desktop-tauri/src/surfaces/tray/UsageBar.test.tsxapps/desktop-tauri/src/surfaces/tray/UsageBar.tsx
Signed-off-by: cromewar <c.akachukwu.c@gmail.com>
7ec4f66 to
34da593
Compare
slegarraga
left a comment
slegarraga
left a comment
There was a problem hiding this comment.
The ARIA progressbar semantics are correct: aria-valuenow stays within 0-100, the labels are meaningful with and without label, and the tests cover the clamp. One coordination note: this PR also carries README contributing changes that overlap with #236, so it may be worth keeping the README change in one PR. Also consider asserting the visible raw 140% in the overshoot test, since that is the deliberate behavior. Not blocking.
|
Thanks for flagging this — let's keep |
aria-valuenow correctly stays clamped to aria-valuemax per the ARIA spec, but the label was built from the same clamped value. At 140% usage, sighted users saw "140%" in the visible text while a screen reader announced only "100%" — silently hiding the over-limit state from exactly the audience that needs it most.
|
Sorry for the delay getting back to you — this turned out to be a one-line fix so we just pushed it to your branch directly rather than bouncing it back for another round. Thanks for catching the ARIA semantics gap, merging now. |
3 participants
Adds role="progressbar" with aria-valuenow/aria-valuemin/aria-valuemax and a meaningful aria-label to the usage-bar__track div, per #76.
aria-valuenow and the label use the clamped pct (not rawPct), so the announced value never exceeds aria-valuemax={100}. Visible text still shows raw overshoot; flagging in case you'd rather these match.
Added UsageBar.test.tsx covering role, aria-valuenow, label with/without the label prop, and the clamp-to-100 case.
No visual change; only ARIA attributes added
Summary by CodeRabbit
Accessibility
Documentation