Merge branch 'main' into caseyisonit/fafo-overlay

Author: caseyisonit

.changeset/odd-times-give.md

@@ -0,0 +1,5 @@
+---
+'@spectrum-web-components/close-button': minor
+---
+
+Added visually hidden default slot rendering to sp-close-button so text content is accessible to screen readers while remaining invisible to sighted users.

1st-gen/packages/action-menu/src/ActionMenu.ts

@@ -148,19 +148,21 @@ export class ActionMenu extends ObserveSlotPresence(
     }
 
     protected override warnNoLabel(): void {
-        window.__swc.warn(
-            this,
-            `<${this.localName}> needs one of the following to be accessible:`,
-            'https://opensource.adobe.com/spectrum-web-components/components/action-menu/#accessibility',
-            {
-                type: 'accessibility',
-                issues: [
-                    `an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
-                    'value supplied to the "label" attribute, which will be displayed visually as placeholder text',
-                    'text content supplied in a <span> with slot="label", or, text content supplied in a <span> with slot="label-only"',
-                    'which will also be displayed visually as placeholder text.',
-                ],
-            }
-        );
+        if (window.__swc?.DEBUG) {
+            window.__swc.warn(
+                this,
+                `<${this.localName}> needs one of the following to be accessible:`,
+                'https://opensource.adobe.com/spectrum-web-components/components/action-menu/#accessibility',
+                {
+                    type: 'accessibility',
+                    issues: [
+                        `an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
+                        'value supplied to the "label" attribute, which will be displayed visually as placeholder text',
+                        'text content supplied in a <span> with slot="label", or, text content supplied in a <span> with slot="label-only"',
+                        'which will also be displayed visually as placeholder text.',
+                    ],
+                }
+            );
+        }
     }
 }

1st-gen/packages/button/clear-button.md

@@ -25,34 +25,21 @@ import { ClearButton } from '@spectrum-web-components/button';
 
 ### Anatomy
 
-The clear button is a button with only close icon.
+The clear button is a button with only a close icon.
 
 ```html
-<sp-clear-button>Reset</sp-clear-button>
+<sp-clear-button label="Reset"></sp-clear-button>
 ```
 
 #### Label
 
-The label for an `<sp-clear-button>` element can be set via it's default slot or with the `label` attribute. With either method, the label will not be visible but still read by screen readers.
-
-<sp-tabs selected="attribute" auto label="Labelling a button">
-<sp-tab value="slot">Default slot</sp-tab>
-<sp-tab-panel value="slot">
-
-```html demo
-<sp-clear-button>Clear</sp-clear-button>
-```
-
-</sp-tab-panel>
-<sp-tab value="attribute">Label attribute</sp-tab>
-<sp-tab-panel value="attribute">
+An accessible label for an `<sp-clear-button>` must be provided using the `label` attribute. This sets the `aria-label` for screen readers. Unlike other button types, the clear button only displays an icon and does not render slot content, so the `label` attribute is the only way to provide an accessible name.
 
 ```html demo
-<sp-clear-button label="Clear">Clear</sp-clear-button>
+<sp-clear-button label="Clear"></sp-clear-button>
 ```
 
-</sp-tab-panel>
-</sp-tabs>
+The `label` attribute is required and will be set as the `aria-label` on the element.
 
 ### Options
 
@@ -63,31 +50,31 @@ The label for an `<sp-clear-button>` element can be set via it's default slot or
 <sp-tab-panel value="s">
 
 ```html demo
-<sp-clear-button size="s">Small</sp-clear-button>
+<sp-clear-button size="s" label="Clear"></sp-clear-button>
 ```
 
 </sp-tab-panel>
 <sp-tab value="m">Medium</sp-tab>
 <sp-tab-panel value="m">
 
 ```html demo
-<sp-clear-button size="m">Medium</sp-clear-button>
+<sp-clear-button size="m" label="Clear"></sp-clear-button>
 ```
 
 </sp-tab-panel>
 <sp-tab value="l">Large</sp-tab>
 <sp-tab-panel value="l">
 
 ```html demo
-<sp-clear-button size="l">Large</sp-clear-button>
+<sp-clear-button size="l" label="Clear"></sp-clear-button>
 ```
 
 </sp-tab-panel>
 <sp-tab value="xl">Extra Large</sp-tab>
 <sp-tab-panel value="xl">
 
 ```html demo
-<sp-clear-button size="xl">Extra Large</sp-clear-button>
+<sp-clear-button size="xl" label="Clear"></sp-clear-button>
 ```
 
 </sp-tab-panel>
@@ -119,8 +106,8 @@ While disabled, the `<sp-clear-button>` element will not respond to click events
 
 ```html
 <sp-button-group>
-    <sp-clear-button>Normal</sp-clear-button>
-    <sp-clear-button disabled>Disabled</sp-clear-button>
+    <sp-clear-button label="Clear"></sp-clear-button>
+    <sp-clear-button label="Clear" disabled></sp-clear-button>
 </sp-button-group>
 ```
 
@@ -132,9 +119,10 @@ Events handlers for clicks and other user actions can be registered on a
 `<sp-clear-button>` as one would on a standard HTML `<button>` element.
 
 ```html
-<sp-clear-button onclick="spAlert(this, '<sp-clear-button> clicked!')">
-    Click me
-</sp-clear-button>
+<sp-clear-button
+    label="Click me"
+    onclick="spAlert(this, '<sp-clear-button> clicked!')"
+></sp-clear-button>
 ```
 
 #### Autofocus
@@ -148,7 +136,7 @@ popover or dialog opens.
 <sp-overlay trigger="trigger@click" placement="bottom">
     <sp-popover>
         <!-- Button will autofocus when open -->
-        <sp-clear-button autofocus>Clear</sp-clear-button>
+        <sp-clear-button label="Clear" autofocus></sp-clear-button>
     </sp-popover>
 </sp-overlay>
 ```
@@ -157,4 +145,4 @@ popover or dialog opens.
 
 #### Include a label
 
-A button is required to have either text in the default slot or a `label` attribute on the `<sp-clear-button>`.
+A button is required to have a `label` attribute on the `<sp-clear-button>` to provide an accessible name for screen readers. The `label` attribute sets the `aria-label` property, ensuring the button is properly announced to assistive technologies.

1st-gen/packages/button/src/ClearButton.ts

@@ -55,7 +55,7 @@ const crossIcon: Record<string, () => TemplateResult> = {
 /**
  * @element sp-clear-button
  *
- * @slot - text label of the Clear Button
+ * @attr {string} label - Required accessible label set as aria-label
  */
 export class ClearButton extends SizedMixin(StyledButton, {
     noDefaultSize: true,
@@ -64,6 +64,14 @@ export class ClearButton extends SizedMixin(StyledButton, {
         return [...super.styles, buttonStyles, crossMediumStyles];
     }
 
+    /**
+     * An accessible label that describes the component.
+     * It will be applied to aria-label, but not visually rendered.
+     * This attribute is required for clear buttons.
+     */
+    @property()
+    public override label!: string;
+
     @property({ type: Boolean, reflect: true })
     public quiet = false;
 
@@ -121,4 +129,28 @@ export class ClearButton extends SizedMixin(StyledButton, {
             <div class="fill">${super.render()}</div>
         `;
     }
+
+    public override connectedCallback(): void {
+        super.connectedCallback();
+
+        // Deprecation warning for default slot when content is provided
+        if (window.__swc.DEBUG && this.textContent?.trim()) {
+            window.__swc.warn(
+                this,
+                `The default slot for text content in <${this.localName}> has been deprecated and will be removed in a future release. The clear button is icon-only and does not render slot content. Use the "label" attribute instead to provide an accessible name.`,
+                'https://opensource.adobe.com/spectrum-web-components/components/button/#clear-button',
+                { level: 'deprecation' }
+            );
+        }
+
+        // Warning for missing label attribute
+        if (window.__swc.DEBUG && !this.label) {
+            window.__swc.warn(
+                this,
+                `The "label" attribute is required on <${this.localName}> to provide an accessible name for screen readers. Please add a label attribute, e.g., <${this.localName} label="Clear">.`,
+                'https://opensource.adobe.com/spectrum-web-components/components/button/#clear-button',
+                { level: 'high' }
+            );
+        }
+    }
 }

1st-gen/packages/button/src/CloseButton.ts

@@ -75,6 +75,13 @@ export class CloseButton extends SizedMixin(StyledButton, {
     public staticColor?: 'black' | 'white';
 
     protected override get buttonContent(): TemplateResult[] {
-        return [crossIcon[this.size]()];
+        return [
+            crossIcon[this.size](),
+            html`
+                <span id="label" class="visually-hidden">
+                    <slot @slotchange=${this.manageTextObservedSlot}></slot>
+                </span>
+            `,
+        ];
     }
 }

1st-gen/packages/button/stories/template.ts

@@ -16,6 +16,9 @@ import {
     ButtonVariants,
 } from '@spectrum-web-components/button/src/Button.js';
 
+import '@spectrum-web-components/button/sp-clear-button.js';
+import '@spectrum-web-components/button/sp-close-button.js';
+
 export interface Properties {
     staticColor?: 'white' | 'black';
     variant?: ButtonVariants;
@@ -30,6 +33,7 @@ export interface Properties {
     noWrap?: boolean;
     iconOnly?: boolean;
     label?: string;
+    componentName?: string;
 }
 
 export const Template = ({
@@ -38,14 +42,47 @@ export const Template = ({
     size,
     treatment,
     variant,
-}: Properties): TemplateResult => html`
-    <sp-button
-        ?disabled=${disabled}
-        ?pending=${pending}
-        size=${ifDefined(size)}
-        treatment=${ifDefined(treatment)}
-        variant=${ifDefined(variant)}
-    >
-        Test Button
-    </sp-button>
-`;
+    label = 'Clear',
+    quiet,
+    staticColor,
+    componentName,
+}: Properties): TemplateResult => {
+    // Render clear-button for clear-button docs
+    if (componentName === 'clear-button') {
+        return html`
+            <sp-clear-button
+                label=${label}
+                ?disabled=${!!disabled}
+                ?quiet=${!!quiet}
+                size=${ifDefined(size)}
+                static-color=${ifDefined(staticColor)}
+            ></sp-clear-button>
+        `;
+    }
+
+    // Render close-button for close-button docs
+    if (componentName === 'close-button') {
+        return html`
+            <sp-close-button
+                label=${label}
+                ?disabled=${!!disabled}
+                ?quiet=${!!quiet}
+                size=${ifDefined(size)}
+                static-color=${ifDefined(staticColor)}
+            ></sp-close-button>
+        `;
+    }
+
+    // Default: render standard button
+    return html`
+        <sp-button
+            ?disabled=${disabled}
+            ?pending=${pending}
+            size=${ifDefined(size)}
+            treatment=${ifDefined(treatment)}
+            variant=${ifDefined(variant)}
+        >
+            Test Button
+        </sp-button>
+    `;
+};

1st-gen/packages/button/test/clear-button.test.ts

@@ -15,7 +15,7 @@ import { ElementSize } from '@spectrum-web-components/base';
 import { ClearButton } from '@spectrum-web-components/button';
 import '@spectrum-web-components/button/sp-clear-button.js';
 import { SinonStub, stub } from 'sinon';
-import { testForLitDevWarnings } from '../../../test/testing-helpers';
+import { testForLitDevWarnings } from '../../../test/testing-helpers.js';
 
 describe('Clear Button', () => {
     testForLitDevWarnings(async () =>
@@ -33,6 +33,40 @@ describe('Clear Button', () => {
         });
     });
 
+    it('has accessible name when label attribute is provided', async () => {
+        const el = await fixture<ClearButton>(html`
+            <sp-clear-button label="Clear field"></sp-clear-button>
+        `);
+
+        await elementUpdated(el);
+        expect(el.getAttribute('aria-label')).to.equal('Clear field');
+        await expect(el).to.be.accessible();
+    });
+
+    it('sets aria-label from label property', async () => {
+        const el = await fixture<ClearButton>(html`
+            <sp-clear-button></sp-clear-button>
+        `);
+
+        await elementUpdated(el);
+        expect(el.hasAttribute('aria-label')).to.be.false;
+
+        el.label = 'Remove item';
+        await elementUpdated(el);
+        expect(el.getAttribute('aria-label')).to.equal('Remove item');
+    });
+
+    it('maintains accessible name in disabled state', async () => {
+        const el = await fixture<ClearButton>(html`
+            <sp-clear-button label="Clear" disabled></sp-clear-button>
+        `);
+
+        await elementUpdated(el);
+        expect(el.getAttribute('aria-label')).to.equal('Clear');
+        expect(el.hasAttribute('aria-disabled')).to.be.true;
+        await expect(el).to.be.accessible();
+    });
+
     describe('dev mode', () => {
         let consoleStub: SinonStub;
         beforeEach(() => {
@@ -78,5 +112,45 @@ describe('Clear Button', () => {
             expect(consoleStub).to.be.calledOnce;
             expect(warning.includes(expectedContent)).to.be.true;
         });
+
+        it('should log deprecation warning when slot content is provided', async () => {
+            const el = await fixture<ClearButton>(html`
+                <sp-clear-button label="Clear">Clear</sp-clear-button>
+            `);
+
+            await elementUpdated(el);
+
+            const warning = consoleStub.getCall(0).args.at(0);
+            const expectedContent =
+                'The default slot for text content in <sp-clear-button> has been deprecated';
+
+            expect(consoleStub).to.be.calledOnce;
+            expect(warning.includes(expectedContent)).to.be.true;
+        });
+
+        it('should log warning when label attribute is missing', async () => {
+            const el = await fixture<ClearButton>(html`
+                <sp-clear-button></sp-clear-button>
+            `);
+
+            await elementUpdated(el);
+
+            const warning = consoleStub.getCall(0).args.at(0);
+            const expectedContent =
+                'The "label" attribute is required on <sp-clear-button>';
+
+            expect(consoleStub).to.be.calledOnce;
+            expect(warning.includes(expectedContent)).to.be.true;
+        });
+
+        it('should not log warning when label attribute is provided without slot content', async () => {
+            const el = await fixture<ClearButton>(html`
+                <sp-clear-button label="Clear"></sp-clear-button>
+            `);
+
+            await elementUpdated(el);
+
+            expect(consoleStub).to.not.be.called;
+        });
     });
 });