Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion 2nd-gen/packages/core/components/asset/Asset.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export abstract class AssetBase extends SpectrumElement {
/**
* Accessible label for the asset’s file or folder variant.
*/
@property()
@property({ type: String })
public label = '';

// ──────────────────────
Expand Down
2 changes: 1 addition & 1 deletion 2nd-gen/packages/core/components/icon/Icon.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export abstract class IconBase extends SizedMixin(SpectrumElement, {
/**
* Accessible label for the icon.
*/
@property()
@property({ type: String })
public label = '';

@queryAssignedElements({ flatten: true })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import { PropertyValues } from 'lit';
import { property } from 'lit/decorators.js';

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

/**
* @todo SWC-1891 Extract shared progress logic (ARIA, label, clamping, formatting,
* indeterminate derivation) into a `ProgressBase` mixin or abstract class in
* `core/components/progress/` so that both `ProgressCircleBase` and a future
* `ProgressBarBase` can extend it. Also add `formatOptions` support for
* progress-bar's custom value labels (e.g. "3 of 10", "45 MB / 100 MB").
*/

/**
* A progress circle component that visually represents the completion progress of a task.
* Can be used in both determinate (with specific progress value) and indeterminate (loading) states.
Expand Down Expand Up @@ -75,39 +84,44 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
// SHARED API
// ──────────────────

/**
* @todo Revisit the default API for `indeterminate` and `progress`. SWC-1891
*
* Whether the progress circle shows indeterminate progress (loading state).
*
* When true, displays an animated loading indicator instead of a specific progress value.
*/
@property({ type: Boolean, reflect: true })
public indeterminate = false;

/**
* Accessible label for the progress circle.
*
* Used to provide context about what is loading or progressing.
* When no accessible name is provided (no label, aria-label, or
* aria-labelledby), a default "Loading" label is applied.
*
* @todo Localize the default "Loading" fallback via LanguageResolutionController
* once a runtime i18n system for static strings is available.
*/
@property({ type: String })
public label = '';

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

private languageResolver = new LanguageResolutionController(this);

// ──────────────────────
// IMPLEMENTATION
// ──────────────────────

/**
* @todo Localize via LanguageResolutionController once a runtime i18n
* system for static strings is available.
*/
private static readonly DEFAULT_LABEL = 'Loading';

/** True when light DOM has element nodes or non-whitespace text (no default slot). */
private static hasMeaningfulLightDomChildren(host: HTMLElement): boolean {
for (const node of host.childNodes) {
Expand All @@ -121,6 +135,28 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
return false;
}

private hasAccessibleName(): boolean {
return Boolean(
this.label ||
this.getAttribute('aria-label') ||
this.getAttribute('aria-labelledby')
);
}

private static clampProgress(value: number): number {
if (!Number.isFinite(value)) {
return 0;
}
return Math.min(100, Math.max(0, value));
}

private formatProgress(): string {
return new Intl.NumberFormat(this.languageResolver.language, {
style: 'percent',
unitDisplay: 'narrow',
}).format((this.progress ?? 0) / 100);
}

private warnDeprecatedLightDomChildren(): void {
if (!window.__swc?.DEBUG) {
return;
Expand All @@ -136,15 +172,27 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
);
}

private static clampProgress(value: number): number {
if (!Number.isFinite(value)) {
return 0;
private warnMissingAccessibleName(): void {
if (!window.__swc?.DEBUG) {
return;
}
return Math.min(100, Math.max(0, value));
window.__swc?.warn(
this,
`<${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:`,
'https://opensource.adobe.com/spectrum-web-components/second-gen/?path=/docs/components-progress-circle--docs',
{
type: 'accessibility',
issues: [
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
'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.',
],
}
);
}

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

protected override firstUpdated(changes: PropertyValues): void {
super.firstUpdated(changes);
if (!this.hasAttribute('role')) {
this.setAttribute('role', 'progressbar');
}
}

private formatProgress(): string {
return new Intl.NumberFormat(this.languageResolver.language, {
style: 'percent',
unitDisplay: 'narrow',
}).format(this.progress / 100);
this.setAttribute('role', 'progressbar');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we removed the guard here? Would this remove whatever the user/consumer sets? ✨

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch this was not intended to be removed, i will add this back in!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh after looking at this, the format progess is handled in the updated() to make sure it stays in sync with the reflective attribute which with trigger an updated lifecycle. the formatProgress method was moved up to the methods section since it was sandwiched between two lifecycles. its still present

}

protected override updated(changes: PropertyValues): void {
super.updated(changes);
if (changes.has('indeterminate')) {
if (this.indeterminate) {
this.removeAttribute('aria-valuemin');
this.removeAttribute('aria-valuemax');
this.removeAttribute('aria-valuenow');
this.removeAttribute('aria-valuetext');
} else {

if (changes.has('progress')) {
if (this.progress !== null && this.progress >= 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify this ✨

Suggested change
if (this.progress !== null && this.progress >= 0) {
if (this.progress !== null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it is needed because setting this.progree === 0 also can compile to null so this makes sure its reading zero as a value and not as a falsey statement

this.setAttribute('aria-valuemin', '0');
this.setAttribute('aria-valuemax', '100');
this.setAttribute('aria-valuenow', String(this.progress));
this.setAttribute('aria-valuetext', this.formatProgress());
} else {
this.removeAttribute('aria-valuemin');
this.removeAttribute('aria-valuemax');
this.removeAttribute('aria-valuenow');
this.removeAttribute('aria-valuetext');
}
}
if (!this.indeterminate && changes.has('progress')) {
this.setAttribute('aria-valuenow', String(this.progress));
this.setAttribute('aria-valuetext', this.formatProgress());
}
if (!this.indeterminate && changes.has(languageResolverUpdatedSymbol)) {

if (this.progress !== null && changes.has(languageResolverUpdatedSymbol)) {
this.setAttribute('aria-valuetext', this.formatProgress());
}

if (changes.has('label')) {
if (this.label.length) {
this.setAttribute('aria-label', this.label);
Expand All @@ -197,31 +235,14 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
}
}

const hasAccessibleName = (): boolean => {
return Boolean(
this.label ||
this.getAttribute('aria-label') ||
this.getAttribute('aria-labelledby')
);
};
// Apply default accessible name fallback after handling explicit label changes.
if (changes.has('label') && !this.hasAccessibleName()) {
this.setAttribute('aria-label', ProgressCircleBase.DEFAULT_LABEL);
this.warnMissingAccessibleName();
}

if (window.__swc?.DEBUG) {
this.warnDeprecatedLightDomChildren();
if (!hasAccessibleName() && this.getAttribute('role') === 'progressbar') {
window.__swc?.warn(
this,
`<${this.localName}> elements need one of the following to be accessible:`,
'https://opensource.adobe.com/spectrum-web-components/second-gen/?path=/docs/components-progress-circle--docs',
{
type: 'accessibility',
issues: [
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
'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.',
],
}
);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ export const PROGRESS_CIRCLE_VALID_SIZES = [
's',
'm',
'l',
] as const satisfies ElementSize[];
] as const satisfies readonly ElementSize[];
export const PROGRESS_CIRCLE_STATIC_COLORS = ['white', 'black'] as const;

export type ProgressCircleStaticColor =
(typeof PROGRESS_CIRCLE_STATIC_COLORS)[number];
export type ProgressCircleSize = (typeof PROGRESS_CIRCLE_VALID_SIZES)[number];
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { SpectrumElement } from '@spectrum-web-components/core/element/index.js'
import { SizedMixin } from '@spectrum-web-components/core/mixins/index.js';

import {
STATUSLIGHT_VALID_SIZES,
STATUS_LIGHT_VALID_SIZES,
type StatusLightVariant,
} from './StatusLight.types.js';

Expand All @@ -27,7 +27,7 @@ import {
* @attribute {ElementSize} size - The size of the status light.
*/
export abstract class StatusLightBase extends SizedMixin(SpectrumElement, {
validSizes: STATUSLIGHT_VALID_SIZES,
validSizes: STATUS_LIGHT_VALID_SIZES,
noDefaultSize: true,
}) {
// ─────────────────────────
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,24 @@
* governing permissions and limitations under the License.
*/

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

export const STATUSLIGHT_VALID_SIZES = [
export const STATUS_LIGHT_VALID_SIZES = [
's',
'm',
'l',
'xl',
] as const satisfies readonly ElementSize[];

export const STATUSLIGHT_VARIANTS_SEMANTIC = [
export const STATUS_LIGHT_VARIANTS_SEMANTIC = [
'neutral',
'info',
'positive',
'negative',
'notice',
] as const;

export const STATUSLIGHT_VARIANTS_COLOR = [
export const STATUS_LIGHT_VARIANTS_COLOR = [
'fuchsia',
'indigo',
'magenta',
Expand All @@ -48,15 +44,15 @@ export const STATUSLIGHT_VARIANTS_COLOR = [
'silver',
] as const;

export const STATUSLIGHT_VARIANTS = [
...STATUSLIGHT_VARIANTS_SEMANTIC,
...STATUSLIGHT_VARIANTS_COLOR,
export const STATUS_LIGHT_VARIANTS = [
...STATUS_LIGHT_VARIANTS_SEMANTIC,
...STATUS_LIGHT_VARIANTS_COLOR,
] as const;

export type StatusLightSemanticVariant =
(typeof STATUSLIGHT_VARIANTS_SEMANTIC)[number];
(typeof STATUS_LIGHT_VARIANTS_SEMANTIC)[number];

export type StatusLightColorVariant =
(typeof STATUSLIGHT_VARIANTS_COLOR)[number];
(typeof STATUS_LIGHT_VARIANTS_COLOR)[number];

export type StatusLightVariant = (typeof STATUSLIGHT_VARIANTS)[number];
export type StatusLightVariant = (typeof STATUS_LIGHT_VARIANTS)[number];
2 changes: 1 addition & 1 deletion 2nd-gen/packages/swc/.storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ const preview = {
},
source: {
excludeDecorators: true,
type: 'auto',
type: 'dynamic',
language: 'html',
transform: async (source: string) => {
try {
Expand Down
4 changes: 4 additions & 0 deletions 2nd-gen/packages/swc/components/avatar/Avatar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ import styles from './avatar.css';
* <swc-avatar src="/path/to/image.jpg" alt="Jane Doe" outline></swc-avatar>
*/
export class Avatar extends AvatarBase {
// ──────────────────────────────
// RENDERING & STYLING
// ──────────────────────────────

public static override get styles(): CSSResultArray {
return [styles];
}
Expand Down
Loading
Loading