style(progress-circle): clean up code styles (#6146)

Author: rise-erpelding

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

@@ -10,15 +10,14 @@
  * governing permissions and limitations under the License.
  */
 import { PropertyValues } from 'lit';
-import { property, query } from 'lit/decorators.js';
+import { property } from 'lit/decorators.js';
 
 import {
   LanguageResolutionController,
   languageResolverUpdatedSymbol,
 } from '@spectrum-web-components/core/controllers/language-resolution.js';
 import { SpectrumElement } from '@spectrum-web-components/core/element/index.js';
 import { SizedMixin } from '@spectrum-web-components/core/mixins/index.js';
-import { getLabelFromSlot } from '@spectrum-web-components/core/utils/get-label-from-slot.js';
 
 import {
   PROGRESS_CIRCLE_VALID_SIZES,
@@ -30,14 +29,6 @@ import {
  * Can be used in both determinate (with specific progress value) and indeterminate (loading) states.
  *
  * @attribute {ElementSize} size - The size of the progress circle.
- *
- * @todo Why do we support both the slot and the label attribute? Should we deprecate the slot?
- *
- * @todo Figure out why our tool chain doesn't respect the line breaks in this slot description.
- *
- * @slot - Accessible label for the progress circle.
- *
- *   Used to provide context about what is loading or progressing.
  */
 export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
   validSizes: PROGRESS_CIRCLE_VALID_SIZES,
@@ -85,6 +76,8 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
   // ──────────────────
 
   /**
+   * @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.
@@ -103,7 +96,8 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
   /**
    * Progress value from 0 to 100.
    *
-   * Only relevant when indeterminate is false.
+   * Only relevant when indeterminate is false. Values outside that range or
+   * non-finite numbers are clamped to 0–100 (non-finite becomes 0).
    */
   @property({ type: Number })
   public progress = 0;
@@ -114,23 +108,49 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
   //     IMPLEMENTATION
   // ──────────────────────
 
-  /**
-   * @internal
-   */
-  @query('slot')
-  private slotEl!: HTMLSlotElement;
+  /** 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) {
+      if (
+        node.nodeType === Node.ELEMENT_NODE ||
+        (node.nodeType === Node.TEXT_NODE && node.textContent?.trim())
+      ) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private warnDeprecatedLightDomChildren(): void {
+    if (!window.__swc?.DEBUG) {
+      return;
+    }
+    if (!ProgressCircleBase.hasMeaningfulLightDomChildren(this)) {
+      return;
+    }
+    window.__swc.warn(
+      this,
+      `<${this.localName}> no longer has a default slot. Light DOM children are not rendered and are not used for an accessible name. Use the "label" attribute or property, or "aria-label" / "aria-labelledby" on the host instead.`,
+      'https://opensource.adobe.com/spectrum-web-components/second-gen/?path=/docs/components-progress-circle--docs',
+      { level: 'deprecation' }
+    );
+  }
 
-  protected makeRotation(rotation: number): string | undefined {
-    return this.indeterminate
-      ? undefined
-      : `transform: rotate(${rotation}deg);`;
+  private static clampProgress(value: number): number {
+    if (!Number.isFinite(value)) {
+      return 0;
+    }
+    return Math.min(100, Math.max(0, value));
   }
 
-  protected handleSlotchange(): void {
-    const labelFromSlot = getLabelFromSlot(this.label, this.slotEl);
-    if (labelFromSlot) {
-      this.label = labelFromSlot;
+  protected override willUpdate(changes: PropertyValues): void {
+    if (changes.has('progress')) {
+      const clamped = ProgressCircleBase.clampProgress(this.progress);
+      if (clamped !== this.progress) {
+        this.progress = clamped;
+      }
     }
+    super.willUpdate(changes);
   }
 
   protected override firstUpdated(changes: PropertyValues): void {
@@ -158,12 +178,12 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
       } else {
         this.setAttribute('aria-valuemin', '0');
         this.setAttribute('aria-valuemax', '100');
-        this.setAttribute('aria-valuenow', '' + this.progress);
+        this.setAttribute('aria-valuenow', String(this.progress));
         this.setAttribute('aria-valuetext', this.formatProgress());
       }
     }
     if (!this.indeterminate && changes.has('progress')) {
-      this.setAttribute('aria-valuenow', '' + this.progress);
+      this.setAttribute('aria-valuenow', String(this.progress));
       this.setAttribute('aria-valuetext', this.formatProgress());
     }
     if (!this.indeterminate && changes.has(languageResolverUpdatedSymbol)) {
@@ -181,22 +201,21 @@ export abstract class ProgressCircleBase extends SizedMixin(SpectrumElement, {
       return Boolean(
         this.label ||
         this.getAttribute('aria-label') ||
-        this.getAttribute('aria-labelledby') ||
-        this.slotEl.assignedNodes().length
+        this.getAttribute('aria-labelledby')
       );
     };
 
     if (window.__swc?.DEBUG) {
+      this.warnDeprecatedLightDomChildren();
       if (!hasAccessibleName() && this.getAttribute('role') === 'progressbar') {
         window.__swc?.warn(
           this,
-          '<sp-progress-circle> elements need one of the following to be accessible:',
-          'https://opensource.adobe.com/spectrum-web-components/components/progress-circle/#accessibility',
+          `<${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',
-              'text content supplied directly to the <sp-progress-circle> 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.',
             ],

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

@@ -36,6 +36,8 @@ import styles from './progress-circle.css';
  *
  * @example
  * <swc-progress-circle indeterminate label="Loading..."></swc-progress-circle>
+ *
+ * Light DOM children are not projected into the shadow tree. Use the `label` attribute or property, or `aria-label` / `aria-labelledby` on the host, for an accessible name.
  */
 export class ProgressCircle extends ProgressCircleBase {
   // ────────────────────
@@ -74,20 +76,18 @@ export class ProgressCircle extends ProgressCircleBase {
       <div
         class=${classMap({
           ['swc-ProgressCircle']: true,
-          [`swc-ProgressCircle--indeterminate`]: this.indeterminate,
+          ['swc-ProgressCircle--indeterminate']: this.indeterminate,
           [`swc-ProgressCircle--static${capitalize(this.staticColor)}`]:
             typeof this.staticColor !== 'undefined',
           [`swc-ProgressCircle--size${this.size?.toUpperCase()}`]:
             typeof this.size !== 'undefined',
         })}
       >
-        <slot @slotchange=${this.handleSlotchange}></slot>
-        <svg fill="none" width="100%" height="100%" class="swc-outerCircle">
+        <svg fill="none" width="100%" height="100%">
           <circle
-            class="swc-innerCircle"
             cx="50%"
             cy="50%"
-            r=${`calc(50% - ${strokeWidth / 1}px)`}
+            r=${`calc(50% - ${strokeWidth}px)`}
             stroke-width=${strokeWidth}
           />
           <circle

2nd-gen/packages/swc/components/progress-circle/progress-circle.css

@@ -100,10 +100,6 @@
   --swc-progress-circle-fill-border-color: token("static-black-track-indicator-color");
 }
 
-slot {
-  display: none;
-}
-
 @media (forced-colors: active) {
   .swc-ProgressCircle {
     --swc-progress-circle-fill-border-color: Highlight;

2nd-gen/packages/swc/components/progress-circle/stories/progress-circle.stories.ts

@@ -103,10 +103,10 @@ export const Overview: Story = {
  *
  * 1. **Track** - Background ring showing the full progress range
  * 2. **Fill** - Colored ring segment showing current progress
- * 3. **Label** - Accessible text describing the operation (not visually rendered)
+ * 3. **Label** - Accessible text describing the operation (not visually rendered), provided via the `label` attribute or property, or `aria-label` / `aria-labelledby` on the host
  *
  * ### Content
- * - **Default slot**: Alternative way to provide an accessible label (the `label` attribute is preferred)
+ *
  * - **Label**: Accessible text describing what is loading or progressing (not visually rendered)
  */
 export const Anatomy: Story = {
@@ -246,9 +246,7 @@ export const Indeterminate: Story = {
  * #### ARIA implementation
  *
  * 1. **ARIA role**: Automatically sets `role="progressbar"` for proper semantic meaning
- * 2. **Labeling**:
- *     - Uses the `label` attribute value as `aria-label`
- *     - Alternative: Content in the default slot can provide the label
+ * 2. **Labeling**: Uses the `label` attribute value as `aria-label`, or rely on `aria-label` / `aria-labelledby` you set on the host
  * 3. **Progress state** (determinate):
  *     - Sets `aria-valuenow` with the current `progress` value
  * 4. **Loading state** (indeterminate):

2nd-gen/packages/swc/components/progress-circle/test/progress-circle.test.ts

@@ -202,10 +202,10 @@ export const AriaLabelledbyAccessibleNameTest: Story = {
 };
 
 // ──────────────────────────────────────────────────────────────
-// TEST: Slots
+// TEST: Light DOM (no default slot)
 // ──────────────────────────────────────────────────────────────
 
-export const SlotLabelTest: Story = {
+export const LightDomChildrenDoNotSetLabelTest: Story = {
   render: () => html`
     <swc-progress-circle>Loading data</swc-progress-circle>
   `,
@@ -215,10 +215,62 @@ export const SlotLabelTest: Story = {
       'swc-progress-circle'
     );
 
-    await step('uses slot content as the label', async () => {
-      expect(progressCircle.label).toBe('Loading data');
-      expect(progressCircle.getAttribute('aria-label')).toBe('Loading data');
-    });
+    await step(
+      'does not use light DOM text as the label or accessible name',
+      async () => {
+        expect(progressCircle.label).toBe('');
+        expect(progressCircle.getAttribute('aria-label')).toBeNull();
+      }
+    );
+
+    await step(
+      'warns in dev mode: deprecation for light DOM children and accessibility when there is no name',
+      () =>
+        withWarningSpy(async (warnCalls) => {
+          progressCircle.progress = 10;
+          await progressCircle.updateComplete;
+
+          expect(warnCalls.length).toBeGreaterThanOrEqual(2);
+          expect(
+            warnCalls.some((call) =>
+              String(call[1] ?? '').includes('no longer has a default slot')
+            )
+          ).toBe(true);
+          expect(
+            warnCalls.some((call) =>
+              String(call[1] ?? '').includes('accessible')
+            )
+          ).toBe(true);
+        })
+    );
+  },
+};
+
+export const LightDomWithLabelDeprecationOnlyTest: Story = {
+  render: () => html`
+    <swc-progress-circle label="Uploading" progress="5">
+      Ignored slot content
+    </swc-progress-circle>
+  `,
+  play: async ({ canvasElement, step }) => {
+    const progressCircle = await getComponent<ProgressCircle>(
+      canvasElement,
+      'swc-progress-circle'
+    );
+
+    await step(
+      'still deprecates light DOM when label provides the accessible name',
+      () =>
+        withWarningSpy(async (warnCalls) => {
+          progressCircle.progress = 6;
+          await progressCircle.updateComplete;
+
+          expect(warnCalls.length).toBe(1);
+          expect(String(warnCalls[0]?.[1] ?? '')).toContain(
+            'no longer has a default slot'
+          );
+        })
+    );
   },
 };
 
@@ -243,6 +295,35 @@ export const ProgressValuesTest: Story = {
   },
 };
 
+export const ProgressClampTest: Story = {
+  render: () => html`
+    <swc-progress-circle
+      progress="150"
+      label="Clamped high"
+    ></swc-progress-circle>
+    <swc-progress-circle
+      progress="-20"
+      label="Clamped low"
+    ></swc-progress-circle>
+  `,
+  play: async ({ canvasElement, step }) => {
+    const circles = await getComponents<ProgressCircle>(
+      canvasElement,
+      'swc-progress-circle'
+    );
+
+    await step('clamps progress above 100 to 100', async () => {
+      expect(circles[0].progress).toBe(100);
+      expect(circles[0].getAttribute('aria-valuenow')).toBe('100');
+    });
+
+    await step('clamps progress below 0 to 0', async () => {
+      expect(circles[1].progress).toBe(0);
+      expect(circles[1].getAttribute('aria-valuenow')).toBe('0');
+    });
+  },
+};
+
 export const IndeterminateTest: Story = {
   ...Indeterminate,
   play: async ({ canvasElement, step }) => {

CONTRIBUTOR-DOCS/02_style-guide/02_typescript/06_method-patterns.md

@@ -27,24 +27,24 @@ This guide explains how to order and name methods in 2nd-gen component classes.
 
 ## Method ordering
 
-Within the [IMPLEMENTATION section](02_class-structure.md#section-implementation) of a base class, or the [RENDERING & STYLING section](02_class-structure.md#section-rendering-and-styling) of a concrete class, order methods by access level:
+Within the [IMPLEMENTATION section](02_class-structure.md#section-implementation) of a base class, or the [RENDERING & STYLING section](02_class-structure.md#section-rendering-and-styling) of a concrete class, use this **default** order by access level:
 
 1. **public** methods first
-2. **protected** methods second (including lifecycle)
+2. **protected** methods second (including lifecycle overrides)
 3. **private** methods last
 
-**Example from ProgressCircle.base.ts — IMPLEMENTATION section:**
+**Exception:** Keep lifecycle overrides in **Lit order** (e.g. `firstUpdated` before `updated`). You may place a **private method** **between** those hooks **when only those hooks call it** (and nothing else in the class does), so the file reads in execution order; if the method is also used elsewhere, put it **after** all protected members instead.
+
+**Example from [ProgressCircle.base.ts](../../../2nd-gen/packages/core/components/progress-circle/ProgressCircle.base.ts) — IMPLEMENTATION section:**
 
 ```ts
-// Protected methods (lifecycle and helpers)
-protected makeRotation(rotation: number): string | undefined { ... }
-protected handleSlotchange(): void { ... }
+// Protected — firstUpdated
 protected override firstUpdated(changes: PropertyValues): void { ... }
 
-// Private methods
+// Private — used by updated()
 private formatProgress(): string { ... }
 
-// Protected lifecycle (later in call order)
+// Protected — updated
 protected override updated(changes: PropertyValues): void { ... }
 ```