Skip to content

Commit 63a0728

Browse files
marissahuysentruytcdransf
authored andcommitted
feat(progresscircle): ensure s2 visual fidelity (#6151)
* feat(progresscircle): ensure s2 visual fidelity - Exports `ProgressCircleSize` type from core - Adds `prefers-reduced-motion` CSS media query - Adds `dashOffset` variable to ensure contrast at 0% progress - Replaces hardcoded size templates with `PROGRESS_CIRCLE_VALID_SIZES.map()` - Removes sizing antipattern in render - Removes canvas WHCM track color - Implements a11y improvements per migration analysis Also includes minor cross-component cleanup: - Renames `STATUSLIGHT_*` constants to `STATUS_LIGHT_*` convention - Adds `{ type: String }` to `@property()` decorators in Asset and Icon base classes - Fixes `customElements` path in swc package.json - Tags icon internal stories as migrated - Updates Storybook source type from auto to dynamic - Minor avatar stories and status-light stories updates fixes swc-1670
1 parent 6f47489 commit 63a0728

20 files changed

Lines changed: 395 additions & 280 deletions

File tree

2nd-gen/packages/core/components/asset/Asset.base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export abstract class AssetBase extends SpectrumElement {
4444
/**
4545
* Accessible label for the asset’s file or folder variant.
4646
*/
47-
@property()
47+
@property({ type: String })
4848
public label = '';
4949

5050
// ──────────────────────

2nd-gen/packages/core/components/icon/Icon.base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export abstract class IconBase extends SizedMixin(SpectrumElement, {
4040
/**
4141
* Accessible label for the icon.
4242
*/
43-
@property()
43+
@property({ type: String })
4444
public label = '';
4545

4646
@queryAssignedElements({ flatten: true })

2nd-gen/packages/core/components/progress-circle/ProgressCircle.base.ts

Lines changed: 84 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12+
1213
import { PropertyValues } from 'lit';
1314
import { property } from 'lit/decorators.js';
1415

@@ -24,6 +25,14 @@ import {
2425
ProgressCircleStaticColor,
2526
} from './ProgressCircle.types.js';
2627

28+
/**
29+
* @todo SWC-1891 Extract shared progress logic (ARIA, label, clamping, formatting,
30+
* indeterminate derivation) into a `ProgressBase` mixin or abstract class in
31+
* `core/components/progress/` so that both `ProgressCircleBase` and a future
32+
* `ProgressBarBase` can extend it. Also add `formatOptions` support for
33+
* progress-bar's custom value labels (e.g. "3 of 10", "45 MB / 100 MB").
34+
*/
35+
2736
/**
2837
* A progress circle component that visually represents the completion progress of a task.
2938
* Can be used in both determinate (with specific progress value) and indeterminate (loading) states.
@@ -75,39 +84,44 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
7584
// SHARED API
7685
// ──────────────────
7786

78-
/**
79-
* @todo Revisit the default API for `indeterminate` and `progress`. SWC-1891
80-
*
81-
* Whether the progress circle shows indeterminate progress (loading state).
82-
*
83-
* When true, displays an animated loading indicator instead of a specific progress value.
84-
*/
85-
@property({ type: Boolean, reflect: true })
86-
public indeterminate = false;
87-
8887
/**
8988
* Accessible label for the progress circle.
9089
*
9190
* Used to provide context about what is loading or progressing.
91+
* When no accessible name is provided (no label, aria-label, or
92+
* aria-labelledby), a default "Loading" label is applied.
93+
*
94+
* @todo Localize the default "Loading" fallback via LanguageResolutionController
95+
* once a runtime i18n system for static strings is available.
9296
*/
9397
@property({ type: String })
9498
public label = '';
9599

96100
/**
97101
* Progress value from 0 to 100.
98102
*
99-
* Only relevant when indeterminate is false. Values outside that range or
100-
* non-finite numbers are clamped to 0–100 (non-finite becomes 0).
103+
* When `null` (indeterminate), the component shows a loading animation.
104+
* Setting a number switches to determinate mode. Removing the `progress`
105+
* attribute or setting this property to `null` returns to indeterminate.
106+
* Values outside 0–100 or non-finite numbers are clamped (non-finite becomes 0).
107+
*
108+
* Reflected to the `progress` attribute when set; the attribute is omitted when indeterminate.
101109
*/
102-
@property({ type: Number })
103-
public progress = 0;
110+
@property({ type: Number, reflect: true })
111+
public progress: number | null = null;
104112

105113
private languageResolver = new LanguageResolutionController(this);
106114

107115
// ──────────────────────
108116
// IMPLEMENTATION
109117
// ──────────────────────
110118

119+
/**
120+
* @todo Localize via LanguageResolutionController once a runtime i18n
121+
* system for static strings is available.
122+
*/
123+
private static readonly DEFAULT_LABEL = 'Loading';
124+
111125
/** True when light DOM has element nodes or non-whitespace text (no default slot). */
112126
private static hasMeaningfulLightDomChildren(host: HTMLElement): boolean {
113127
for (const node of host.childNodes) {
@@ -121,6 +135,28 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
121135
return false;
122136
}
123137

138+
private hasAccessibleName(): boolean {
139+
return Boolean(
140+
this.label ||
141+
this.getAttribute('aria-label') ||
142+
this.getAttribute('aria-labelledby')
143+
);
144+
}
145+
146+
private static clampProgress(value: number): number {
147+
if (!Number.isFinite(value)) {
148+
return 0;
149+
}
150+
return Math.min(100, Math.max(0, value));
151+
}
152+
153+
private formatProgress(): string {
154+
return new Intl.NumberFormat(this.languageResolver.language, {
155+
style: 'percent',
156+
unitDisplay: 'narrow',
157+
}).format((this.progress ?? 0) / 100);
158+
}
159+
124160
private warnDeprecatedLightDomChildren(): void {
125161
if (!window.__swc?.DEBUG) {
126162
return;
@@ -136,15 +172,27 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
136172
);
137173
}
138174

139-
private static clampProgress(value: number): number {
140-
if (!Number.isFinite(value)) {
141-
return 0;
175+
private warnMissingAccessibleName(): void {
176+
if (!window.__swc?.DEBUG) {
177+
return;
142178
}
143-
return Math.min(100, Math.max(0, value));
179+
window.__swc?.warn(
180+
this,
181+
`<${this.localName}> requires an accessible name. A default label of "${ProgressCircleBase.DEFAULT_LABEL}" has been applied, but a more specific label should be provided via:`,
182+
'https://opensource.adobe.com/spectrum-web-components/second-gen/?path=/docs/components-progress-circle--docs',
183+
{
184+
type: 'accessibility',
185+
issues: [
186+
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
187+
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
188+
'an element ID reference supplied to the "aria-labelledby" attribute, which will be provided by screen readers and will need to be managed manually by the parent application.',
189+
],
190+
}
191+
);
144192
}
145193

146194
protected override willUpdate(changes: PropertyValues): void {
147-
if (changes.has('progress')) {
195+
if (changes.has('progress') && this.progress !== null) {
148196
const clamped = ProgressCircleBase.clampProgress(this.progress);
149197
if (clamped !== this.progress) {
150198
this.progress = clamped;
@@ -155,40 +203,30 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
155203

156204
protected override firstUpdated(changes: PropertyValues): void {
157205
super.firstUpdated(changes);
158-
if (!this.hasAttribute('role')) {
159-
this.setAttribute('role', 'progressbar');
160-
}
161-
}
162-
163-
private formatProgress(): string {
164-
return new Intl.NumberFormat(this.languageResolver.language, {
165-
style: 'percent',
166-
unitDisplay: 'narrow',
167-
}).format(this.progress / 100);
206+
this.setAttribute('role', 'progressbar');
168207
}
169208

170209
protected override updated(changes: PropertyValues): void {
171210
super.updated(changes);
172-
if (changes.has('indeterminate')) {
173-
if (this.indeterminate) {
174-
this.removeAttribute('aria-valuemin');
175-
this.removeAttribute('aria-valuemax');
176-
this.removeAttribute('aria-valuenow');
177-
this.removeAttribute('aria-valuetext');
178-
} else {
211+
212+
if (changes.has('progress')) {
213+
if (this.progress !== null && this.progress >= 0) {
179214
this.setAttribute('aria-valuemin', '0');
180215
this.setAttribute('aria-valuemax', '100');
181216
this.setAttribute('aria-valuenow', String(this.progress));
182217
this.setAttribute('aria-valuetext', this.formatProgress());
218+
} else {
219+
this.removeAttribute('aria-valuemin');
220+
this.removeAttribute('aria-valuemax');
221+
this.removeAttribute('aria-valuenow');
222+
this.removeAttribute('aria-valuetext');
183223
}
184224
}
185-
if (!this.indeterminate && changes.has('progress')) {
186-
this.setAttribute('aria-valuenow', String(this.progress));
187-
this.setAttribute('aria-valuetext', this.formatProgress());
188-
}
189-
if (!this.indeterminate && changes.has(languageResolverUpdatedSymbol)) {
225+
226+
if (this.progress !== null && changes.has(languageResolverUpdatedSymbol)) {
190227
this.setAttribute('aria-valuetext', this.formatProgress());
191228
}
229+
192230
if (changes.has('label')) {
193231
if (this.label.length) {
194232
this.setAttribute('aria-label', this.label);
@@ -197,31 +235,14 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
197235
}
198236
}
199237

200-
const hasAccessibleName = (): boolean => {
201-
return Boolean(
202-
this.label ||
203-
this.getAttribute('aria-label') ||
204-
this.getAttribute('aria-labelledby')
205-
);
206-
};
238+
// Apply default accessible name fallback after handling explicit label changes.
239+
if (changes.has('label') && !this.hasAccessibleName()) {
240+
this.setAttribute('aria-label', ProgressCircleBase.DEFAULT_LABEL);
241+
this.warnMissingAccessibleName();
242+
}
207243

208244
if (window.__swc?.DEBUG) {
209245
this.warnDeprecatedLightDomChildren();
210-
if (!hasAccessibleName() && this.getAttribute('role') === 'progressbar') {
211-
window.__swc?.warn(
212-
this,
213-
`<${this.localName}> elements need one of the following to be accessible:`,
214-
'https://opensource.adobe.com/spectrum-web-components/second-gen/?path=/docs/components-progress-circle--docs',
215-
{
216-
type: 'accessibility',
217-
issues: [
218-
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
219-
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
220-
'an element ID reference supplied to the "aria-labelledby" attribute, which will be provided by screen readers and will need to be managed manually by the parent application.',
221-
],
222-
}
223-
);
224-
}
225246
}
226247
}
227248
}

2nd-gen/packages/core/components/progress-circle/ProgressCircle.types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ export const PROGRESS_CIRCLE_VALID_SIZES = [
1616
's',
1717
'm',
1818
'l',
19-
] as const satisfies ElementSize[];
19+
] as const satisfies readonly ElementSize[];
2020
export const PROGRESS_CIRCLE_STATIC_COLORS = ['white', 'black'] as const;
2121

2222
export type ProgressCircleStaticColor =
2323
(typeof PROGRESS_CIRCLE_STATIC_COLORS)[number];
24+
export type ProgressCircleSize = (typeof PROGRESS_CIRCLE_VALID_SIZES)[number];

2nd-gen/packages/core/components/status-light/StatusLight.base.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { SpectrumElement } from '@spectrum-web-components/core/element/index.js'
1616
import { SizedMixin } from '@spectrum-web-components/core/mixins/index.js';
1717

1818
import {
19-
STATUSLIGHT_VALID_SIZES,
19+
STATUS_LIGHT_VALID_SIZES,
2020
type StatusLightVariant,
2121
} from './StatusLight.types.js';
2222

@@ -27,7 +27,7 @@ import {
2727
* @attribute {ElementSize} size - The size of the status light.
2828
*/
2929
export abstract class StatusLightBase extends SizedMixin(SpectrumElement, {
30-
validSizes: STATUSLIGHT_VALID_SIZES,
30+
validSizes: STATUS_LIGHT_VALID_SIZES,
3131
noDefaultSize: true,
3232
}) {
3333
// ─────────────────────────

2nd-gen/packages/core/components/status-light/StatusLight.types.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,24 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
/**
14-
* @todo Rename STATUSLIGHT_ prefix to STATUS_LIGHT_ to align with type prefix
15-
* naming convention (use underscore separators for multi-word names).
16-
*/
1713
import type { ElementSize } from '@spectrum-web-components/core/mixins/index.js';
1814

19-
export const STATUSLIGHT_VALID_SIZES = [
15+
export const STATUS_LIGHT_VALID_SIZES = [
2016
's',
2117
'm',
2218
'l',
2319
'xl',
2420
] as const satisfies readonly ElementSize[];
2521

26-
export const STATUSLIGHT_VARIANTS_SEMANTIC = [
22+
export const STATUS_LIGHT_VARIANTS_SEMANTIC = [
2723
'neutral',
2824
'info',
2925
'positive',
3026
'negative',
3127
'notice',
3228
] as const;
3329

34-
export const STATUSLIGHT_VARIANTS_COLOR = [
30+
export const STATUS_LIGHT_VARIANTS_COLOR = [
3531
'fuchsia',
3632
'indigo',
3733
'magenta',
@@ -48,15 +44,15 @@ export const STATUSLIGHT_VARIANTS_COLOR = [
4844
'silver',
4945
] as const;
5046

51-
export const STATUSLIGHT_VARIANTS = [
52-
...STATUSLIGHT_VARIANTS_SEMANTIC,
53-
...STATUSLIGHT_VARIANTS_COLOR,
47+
export const STATUS_LIGHT_VARIANTS = [
48+
...STATUS_LIGHT_VARIANTS_SEMANTIC,
49+
...STATUS_LIGHT_VARIANTS_COLOR,
5450
] as const;
5551

5652
export type StatusLightSemanticVariant =
57-
(typeof STATUSLIGHT_VARIANTS_SEMANTIC)[number];
53+
(typeof STATUS_LIGHT_VARIANTS_SEMANTIC)[number];
5854

5955
export type StatusLightColorVariant =
60-
(typeof STATUSLIGHT_VARIANTS_COLOR)[number];
56+
(typeof STATUS_LIGHT_VARIANTS_COLOR)[number];
6157

62-
export type StatusLightVariant = (typeof STATUSLIGHT_VARIANTS)[number];
58+
export type StatusLightVariant = (typeof STATUS_LIGHT_VARIANTS)[number];

2nd-gen/packages/swc/.storybook/preview.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ const preview = {
197197
},
198198
source: {
199199
excludeDecorators: true,
200-
type: 'auto',
200+
type: 'dynamic',
201201
language: 'html',
202202
transform: async (source: string) => {
203203
try {

2nd-gen/packages/swc/components/avatar/Avatar.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ import styles from './avatar.css';
3636
* <swc-avatar src="/path/to/image.jpg" alt="Jane Doe" outline></swc-avatar>
3737
*/
3838
export class Avatar extends AvatarBase {
39+
// ──────────────────────────────
40+
// RENDERING & STYLING
41+
// ──────────────────────────────
42+
3943
public static override get styles(): CSSResultArray {
4044
return [styles];
4145
}

0 commit comments

Comments
 (0)