🎨 Palette: Add ARIA progressbar roles to Progress component#959
🎨 Palette: Add ARIA progressbar roles to Progress component#959
Conversation
Added `role="progressbar"` along with `aria-valuemin`, `aria-valuemax`, and `aria-valuenow` attributes to the Progress UI component to make it accessible to screen readers, reflecting the loading state visually provided by the component. Also added tests to verify the attributes are correctly applied. Co-authored-by: anchapin <6326294+anchapin@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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds ARIA progressbar semantics to the Progress UI component and introduces tests to verify accessibility attributes. Class diagram for updated Progress component with ARIA attributesclassDiagram
class ProgressProps {
+number value
+number max
+string className
}
class Progress {
+number value
+number max
+string className
+string role
+number ariaValuemin
+number ariaValuemax
+number ariaValuenow
+render() ReactElement
}
ProgressProps <|.. Progress
Progress : role = progressbar
Progress : ariaValuemin = 0
Progress : ariaValuemax = max
Progress : ariaValuenow = value
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider handling cases where
valueisundefinedor outside the[0, max]range so thataria-valuenowis either omitted for indeterminate progress or clamped to a valid range to avoid confusing assistive technologies. - If the component supports custom min values conceptually, you may want to expose
aria-valueminvia props instead of hardcoding0to keep the semantic range aligned with the logical range.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider handling cases where `value` is `undefined` or outside the `[0, max]` range so that `aria-valuenow` is either omitted for indeterminate progress or clamped to a valid range to avoid confusing assistive technologies.
- If the component supports custom min values conceptually, you may want to expose `aria-valuemin` via props instead of hardcoding `0` to keep the semantic range aligned with the logical range.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR improves the accessibility semantics of the shared Progress UI component by adding WAI-ARIA progressbar attributes, and introduces unit tests to validate the rendered ARIA attributes.
Changes:
- Added
role="progressbar"plusaria-valuemin,aria-valuemax, andaria-valuenowto theProgresscomponent root element. - Added Vitest/Testing Library unit tests to assert the ARIA attributes for default and custom values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
frontend/src/components/ui/progress.tsx |
Adds progressbar role and ARIA value attributes to improve screen reader support. |
frontend/src/components/ui/progress.test.tsx |
Adds unit tests verifying the new ARIA attributes are present and correct. |
| role="progressbar" | ||
| aria-valuemin={0} | ||
| aria-valuemax={max} | ||
| aria-valuenow={value} | ||
| className={`relative w-full overflow-hidden rounded-full bg-gray-200 ${className}`} |
There was a problem hiding this comment.
aria-valuenow is set to the raw value, but the visual fill is clamped to 0–100% via percentage. If callers pass value < 0 or value > max, the UI will clamp while the ARIA attributes can become invalid/inconsistent (e.g., aria-valuenow > aria-valuemax). Consider clamping value into [0, max] (and using that clamped value for both percentage and aria-valuenow).
| const progressbar = screen.getByRole('progressbar'); | ||
| expect(progressbar).toHaveAttribute('aria-valuenow', '50'); | ||
| expect(progressbar).toHaveAttribute('aria-valuemax', '200'); | ||
| }); |
There was a problem hiding this comment.
The new tests assert the presence of ARIA attributes for default/custom cases, but they don't cover the out-of-range scenarios where the component currently clamps the visual fill. Adding tests for value > max and value < 0 would prevent regressions and ensure ARIA values stay consistent with the rendered progress.
| }); | |
| }); | |
| it('clamps aria-valuenow to max when value exceeds max', () => { | |
| render(<Progress value={150} max={100} />); | |
| const progressbar = screen.getByRole('progressbar'); | |
| expect(progressbar).toHaveAttribute('aria-valuenow', '100'); | |
| expect(progressbar).toHaveAttribute('aria-valuemax', '100'); | |
| }); | |
| it('clamps aria-valuenow to the minimum when value is below zero', () => { | |
| render(<Progress value={-10} max={100} />); | |
| const progressbar = screen.getByRole('progressbar'); | |
| expect(progressbar).toHaveAttribute('aria-valuenow', '0'); | |
| expect(progressbar).toHaveAttribute('aria-valuemin', '0'); | |
| }); |
Added ARIA attributes to the
ProgressUI component to improve accessibility for screen readers.💡 What: Added
role="progressbar",aria-valuemin,aria-valuemax, andaria-valuenowattributes to the maindivelement of theProgresscomponent. Also added unit tests to verify the attributes.🎯 Why: To make the component accessible, allowing screen readers to inform users of the loading state visually represented by the progress bar.
♿ Accessibility: The
Progresscomponent is now fully semantically correct and identifiable as a progress bar by assistive technologies.PR created automatically by Jules for task 565289702158579289 started by @anchapin
Summary by Sourcery
Improve accessibility of the Progress UI component by exposing ARIA progressbar attributes.
New Features:
Tests: