feat(progresscircle): ensure s2 visual fidelity#6151
feat(progresscircle): ensure s2 visual fidelity#6151marissahuysentruyt wants to merge 10 commits intoswc-1668/poc-componentsfrom
Conversation
|
📚 Branch Preview Links🔍 First Generation Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
| .swc-ProgressCircle { | ||
| --swc-progress-circle-fill-border-color: Highlight; | ||
| --swc-progress-circle-track-color: Canvas; | ||
| --swc-progress-circle-track-border-color: Canvas; |
There was a problem hiding this comment.
@5t3ph do you think we could delete this Canvas definition for the track color? I think Canvas effectively "removes" the track, making it not visible. The additional dark and light queries also seem to be overriding Canvas anyways in AssistivLabs. Are you experiencing the same thing?
There was a problem hiding this comment.
I didn't resolve anything yet, but I wanted to make sure I could remove that initial assignment to Canvas! I was seeing the light/dark theme transparent values like you screenshotted as well! Canvas was being overwritten by light/dark no matter what theme I was in.
I'm going to remove that initial track-border-color and just rely on the light/dark definitions. 👍
There was a problem hiding this comment.
Following 👀 and will make sure that #6146 doesn't conflict, I removed that custom prop because it wasn't being used.
| export const StaticColors: Story = { | ||
| render: (args) => html` | ||
| ${ProgressCircle.STATIC_COLORS.map( | ||
| ${PROGRESS_CIRCLE_STATIC_COLORS_S2.map( |
There was a problem hiding this comment.
Currently, I am importing the S2 suffixed types, even though I know the code styles say not to suffix those. I didn't want to overstep the progress circle code styles.
| @media (prefers-reduced-motion: reduce) { | ||
| .swc-ProgressCircle--indeterminate .swc-ProgressCircle-fill { | ||
| animation: none; |
There was a problem hiding this comment.
I added the reduced-motion query since it was in the a11y migration guide. Do we have a philosophy on reduced-motion? Right now, if a user has reduced motion turned on, it just doesn't do the indeterminate animation. Is that what we are envisioning?
There was a problem hiding this comment.
@nikkimk just wanted to tag you here since I made some of the accessibility updates. I'd love to get your opinions on the reduced-motion and then the dashOffset concept so that even an empty/0% progress circle still shows a bit of the fill to meet contrast.
| // At progress=0, a dashoffset of 100 makes the fill fully invisible, which fails WCAG 1.4.11 | ||
| // non-text contrast (the track alone may not meet 3:1 against the background). | ||
| // Clamp to 98 to show a minimum 2-unit fill so the graphical element remains perceivable. | ||
| // aria-valuenow stays at 0 — this is a visual-only adjustment. | ||
| const dashOffset = this.indeterminate | ||
| ? 100 - this.progress | ||
| : this.progress === 0 | ||
| ? 98 | ||
| : 100 - this.progress; | ||
|
|
There was a problem hiding this comment.
In the a11y migration, I saw this:
When **`progress`** is **0**, show a **small** filled segment of the circle (or another treatment that keeps **graphical** ring details at **at least 3:1** with adjacent colors). A **fully empty** ring at **0%** often fails **non-text contrast** because the **track** alone is too weak against the background.
So, that's what this dashOffset is attempting to do. Do we have opinions on like, how much of the fill we'd like to continue to show when the value is 0? 98 is a completely magic number. ✨ ✨ ✨
The 0% fill is the first one in this set.
| [`swc-ProgressCircle--indeterminate`]: this.indeterminate, | ||
| [`swc-ProgressCircle--static${capitalize(this.staticColor)}`]: | ||
| typeof this.staticColor !== 'undefined', | ||
| [`swc-ProgressCircle--size${this.size?.toUpperCase()}`]: |
There was a problem hiding this comment.
We don't need the size classes on the internal wrapper div.
419e8bf to
5038447
Compare
f635672 to
181944f
Compare
| * #### Non-interactive element | ||
| * | ||
| * - Progress circles have no interactive behavior and are not focusable | ||
| * - Screen readers will announce the progress circle content as static text | ||
| * - No keyboard interaction is required or expected |
There was a problem hiding this comment.
@nikkimk is this the right language for progress circles? I'm most concerned with the screen reader bullet point (like is it "static text" or something else?)
a099390 to
06a2855
Compare
nikkimk
left a comment
There was a problem hiding this comment.
I have some requested changes based on these proposed design guidelines: https://www.figma.com/design/42VzvpW262EAUbYsadO4e8/Loading-animation-discovery
There was a problem hiding this comment.
The SVG is showing up in the a11y tree as an image with no alt text. Try this:
| <svg aria-hidden="true" fill="none" width="100%" height="100%" class="swc-outerCircle"> |
| args: { | ||
| indeterminate: true, | ||
| size: 'm', | ||
| label: 'Processing request', |
There was a problem hiding this comment.
New guidelines for progress bars and circles is that they are indeterminate by default unless there is a value given:
| * | ||
| * @property {string} static-color - Static color variant for use on different backgrounds. | ||
| * @property {number} progress - Progress value between 0 and 100. | ||
| * @property {boolean} indeterminate - Indeterminate state for loading. |
There was a problem hiding this comment.
This should be deprecated. A progress circle should be indeterminate until a value is given, so we can set indeterminate animation based on whether value is undefined.
New guidelines for progress bars and circles is that they are indeterminate by default unless there is a value given:
There was a problem hiding this comment.
There was a similar suggestion in the code styles PR, and also similarly, it feels out of scope for this ticket. Could we add another todo in the comments, referencing ticket 1891.
Does that work for now while the poc feature branch and epic are both still wrapping up?
| // non-text contrast (the track alone may not meet 3:1 against the background). | ||
| // Clamp to 98 to show a minimum 2-unit fill so the graphical element remains perceivable. | ||
| // aria-valuenow stays at 0 — this is a visual-only adjustment. | ||
| const dashOffset = this.indeterminate |
There was a problem hiding this comment.
This logic should be based on if value is undefined. We can deprecate indeterminate
| <div | ||
| class=${classMap({ | ||
| ['swc-ProgressCircle']: true, | ||
| [`swc-ProgressCircle--indeterminate`]: this.indeterminate, |
There was a problem hiding this comment.
Do this based on undefined value
| * <swc-progress-circle progress="75" label="Loading progress"></swc-progress-circle> | ||
| * | ||
| * @example | ||
| * <swc-progress-circle indeterminate label="Loading..."></swc-progress-circle> |
There was a problem hiding this comment.
| * <swc-progress-circle label="Loading..."></swc-progress-circle> |
|
|
||
| /** | ||
| * The indeterminate state shows an animated loading indicator when progress is unknown or cannot be determined. | ||
| * Set the `indeterminate` attribute to `true` to activate this state. |
There was a problem hiding this comment.
| * If no value is set, the progress circle is indeterminate. |
| * 3. **Progress state** (determinate): | ||
| * - Sets `aria-valuenow` with the current `progress` value | ||
| * 4. **Loading state** (indeterminate): | ||
| * - Removes `aria-valuenow` when `indeterminate="true"` |
There was a problem hiding this comment.
| * - Removes `aria-valuenow` when no value is given. |
| * - Use specific, meaningful labels (e.g., "Uploading profile photo" instead of "Loading") | ||
| * - Use determinate progress (`progress="50"`) when possible to give users a clear sense of completion | ||
| * - For determinate progress, ensure the `progress` value accurately reflects the actual progress | ||
| * - Use indeterminate progress only when duration is truly unknown |
There was a problem hiding this comment.
| * - Use indeterminate progress only when duration is truly unknown or when the wait is less than 3 seconds. |
| * - Ensure sufficient color contrast between the progress circle and its background | ||
| * - Use `static-color="white"` on dark backgrounds or `static-color="black"` on light backgrounds | ||
| * - Test with screen readers to verify progress announcements are clear and timely | ||
| * - Avoid updating progress values more frequently than every 1-2 seconds to prevent announcement overload |
There was a problem hiding this comment.
| * - Avoid updating progress values more frequently than every 1-2 seconds to prevent announcement overload | |
| * - Do not force live region announcements for progress durations that are 3 seconds or less. Instead, consider status messages when progress is complete or there is an error. |
to make sure a 0% fill progress circle meets contrast requirements, the dashOffset variable was created so that a small portion of the fill remains visible. this is a visual change only, and does not affect labels or aria-values
181944f to
b524d1f
Compare


Description
Cleans up progress circle styles in 2nd-gen. Also provides new accessibility improvements according to the a11y migration analysis.
ProgressCircleSizetype from@spectrum-web-components/core/components/progress-circleSizesstory with aPROGRESS_CIRCLE_VALID_SIZES.map()similar to badge and dividermas the default in the storybook control tableprefers-reduced-motionCSS media querydashOffsetvariable to ensure contrast is passed even when the progress is as 0% (notes in the code)Motivation and context
The
ProgressCircleSizetype was already inferred in the types file but not exported, making it unavailable for stories and external consumers. Aligns with the pattern established in the badge component. The accessibility improvements set the component up for immediate success instead of bandaging something together later in life.Related issue(s)
Screenshots (if appropriate)
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Use these tables as a quick reference when validating the token usage in the browser!
Track color —
--swc-progress-circle-track-border-colortrack-colorstatic-color="white"static-white-track-colorstatic-color="black"static-black-track-colorFill (indicator) color —
--swc-progress-circle-fill-border-coloraccent-content-color-defaultstatic-color="white"static-white-track-indicator-colorstatic-color="black"static-black-track-indicator-colorNote:
static-color="black"is new in S2. S1 only supportedstatic-color="white".Size-specific tokens
sm(default)l--swc-progress-circle-sizeprogress-circle-size-smallprogress-circle-size-mediumprogress-circle-size-large--swc-progress-circle-thicknessprogress-circle-thickness-smallprogress-circle-thickness-mediumprogress-circle-thickness-largeProgress circle stories render correctly at all sizes
Progress circle > Sizes)s,m,l) render with the appropriate custom properties listed above.Progress circle stories render correct color tokens
Indeterminate story renders with animation
Progress circle > IndeterminateProcessing requestlabel is setaria-valuenowattribute on the host elementWHCM styles in AssistivLabs
Reduced motion
indeterminatecontrol to trueScreen.Recording.2026-04-07.at.4.21.39.PM.mov
Device review
Accessibility testing checklist
Required: Complete each applicable item and document your testing steps (replace the placeholders with your component-specific instructions).
Progress circle > Playgroundin StorybookProgress circle > Playgroundin Storybook with a screen reader active (VoiceOver/NVDA)progressbarto be announced, thearia-labelto match thelabelarg (e.g. "Uploadingdocument"), and
aria-valuenow/aria-valuetextto reflect the current progress valueIndeterminatestory and navigate to the elementprogressbarto be announced with the label, but no value (noaria-valuenow)NOTE: reduced motion testing steps are in the default "Manual testing" section