Improve the ObserveAttributesMixin so there is less duplication

.changeset/silent-hands-pump.md

@@ -0,0 +1,12 @@
+---
+'@sl-design-system/text-field': patch
+'@sl-design-system/text-area': patch
+'@sl-design-system/checkbox': patch
+'@sl-design-system/select': patch
+'@sl-design-system/shared': patch
+'@sl-design-system/switch': patch
+'@sl-design-system/menu': patch
+'@sl-design-system/tree': patch
+---
+
+Remove duplication of `observedAttributes` from the components and into the `ObserveAttributesMixin`

packages/components/checkbox/src/checkbox.spec.ts

@@ -110,6 +110,30 @@ describe('sl-checkbox', () => {
       expect(label).to.have.attribute('for', input.id);
     });
 
+    it('should proxy the aria-disabled attribute to the input element', async () => {
+      el.setAttribute('aria-disabled', 'true');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-disabled');
+      expect(el.input).to.have.attribute('aria-disabled', 'true');
+    });
+
+    it('should proxy the aria-label attribute to the input element', async () => {
+      el.setAttribute('aria-label', 'Label');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-label');
+      expect(el.input).to.have.attribute('aria-label', 'Label');
+    });
+
+    it('should proxy the aria-labelledby attribute to the input element', async () => {
+      el.setAttribute('aria-labelledby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-labelledby');
+      expect(el.input).to.have.attribute('aria-labelledby', 'id');
+    });
+
     it('should be pristine', () => {
       expect(el.dirty).not.to.be.true;
     });
@@ -307,23 +331,6 @@ describe('sl-checkbox', () => {
     });
   });
 
-  describe('aria attributes', () => {
-    beforeEach(async () => {
-      el = await fixture(html`<sl-checkbox aria-label="my checkbox label" aria-disabled="true"></sl-checkbox>`);
-      input = el.querySelector('input')!;
-
-      // Give time to rewrite arias
-      await new Promise(resolve => setTimeout(resolve, 100));
-    });
-
-    it('should have an input with proper arias', () => {
-      expect(el).not.to.have.attribute('aria-label', 'my checkbox label');
-      expect(el).not.to.have.attribute('aria-disabled', 'true');
-      expect(input).to.have.attribute('aria-label', 'my checkbox label');
-      expect(input).to.have.attribute('aria-disabled', 'true');
-    });
-  });
-
   describe('validation', () => {
     beforeEach(async () => {
       el = await fixture(html`<sl-checkbox>Hello world</sl-checkbox>`);

packages/components/checkbox/src/checkbox.stories.ts

@@ -217,6 +217,18 @@ export const Indeterminate: StoryObj = {
   }
 };
 
+export const NoVisibleLabel: StoryObj = {
+  render: () => {
+    return html`
+      <p style="margin: 0 0 1rem 0">
+        This checkbox has no internal or external label. It only has an <code>aria-label</code> attribute. That
+        attribute is automatically applied to the <code>input</code> element.
+      </p>
+      <sl-checkbox aria-label="Check me"></sl-checkbox>
+    `;
+  }
+};
+
 export const Overflow: Story = {
   args: {
     hint: 'The checkbox should be aligned with the first row of text',

packages/components/checkbox/src/checkbox.ts

@@ -34,11 +34,6 @@ export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(L
   'aria-label',
   'aria-labelledby'
 ]) {
-  /** @internal */
-  static override get observedAttributes(): string[] {
-    return [...super.observedAttributes, 'aria-disabled', 'aria-label', 'aria-labelledby'];
-  }
-
   /** @internal */
   static override shadowRootOptions: ShadowRootInit = { ...LitElement.shadowRootOptions, delegatesFocus: true };
 

packages/components/menu/src/menu-button.spec.ts

@@ -33,6 +33,30 @@ describe('sl-menu-button', () => {
       expect(el.disabled).not.to.be.true;
     });
 
+    it('should proxy the aria-disabled attribute to the input element', async () => {
+      el.setAttribute('aria-disabled', 'true');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-disabled');
+      expect(el.button).to.have.attribute('aria-disabled', 'true');
+    });
+
+    it('should proxy the aria-label attribute to the input element', async () => {
+      el.setAttribute('aria-label', 'Label');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-label');
+      expect(el.button).to.have.attribute('aria-label', 'Label');
+    });
+
+    it('should proxy the aria-labelledby attribute to the input element', async () => {
+      el.setAttribute('aria-labelledby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-labelledby');
+      expect(el.button).to.have.attribute('aria-labelledby', 'id');
+    });
+
     describe('button', () => {
       it('should have a button', () => {
         expect(button).to.exist;
@@ -273,30 +297,4 @@ describe('sl-menu-button', () => {
       expect(button.querySelector('.selected')).not.to.exist;
     });
   });
-
-  describe('aria attributes', () => {
-    beforeEach(async () => {
-      el = await fixture(html`
-        <sl-menu-button aria-label="my label" aria-disabled="true">
-          <span slot="button">Button</span>
-
-          <sl-menu-item>Item 1</sl-menu-item>
-          <sl-menu-item>Item 2</sl-menu-item>
-        </sl-menu-button>
-      `);
-
-      button = el.renderRoot.querySelector('sl-button') as Button;
-      menu = el.renderRoot.querySelector('sl-menu') as Menu;
-
-      // Give time to rewrite arias
-      await new Promise(resolve => setTimeout(resolve, 100));
-    });
-
-    it('should have a button with proper arias', () => {
-      expect(el).not.to.have.attribute('aria-label', 'my label');
-      expect(el).not.to.have.attribute('aria-disabled', 'true');
-      expect(button).to.have.attribute('aria-label', 'my label');
-      expect(button).to.have.attribute('aria-disabled', 'true');
-    });
-  });
 });

packages/components/menu/src/menu-button.ts

@@ -35,13 +35,9 @@ declare global {
 @localized()
 export class MenuButton extends ObserveAttributesMixin(ScopedElementsMixin(LitElement), [
   'aria-disabled',
-  'aria-label'
+  'aria-label',
+  'aria-labelledby'
 ]) {
-  /** @internal */
-  static override get observedAttributes(): string[] {
-    return [...super.observedAttributes, 'aria-disabled', 'aria-label'];
-  }
-
   /** @internal */
   static get scopedElements(): ScopedElementsMap {
     return {

packages/components/select/src/select.spec.ts

@@ -93,6 +93,30 @@ describe('sl-select', () => {
       expect(el.value).to.be.undefined;
     });
 
+    it('should proxy the aria-describedby attribute to the button element', async () => {
+      el.setAttribute('aria-describedby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-describedby');
+      expect(el.button).to.have.attribute('aria-describedby', 'id');
+    });
+
+    it('should proxy the aria-label attribute to the button element', async () => {
+      el.setAttribute('aria-label', 'Label');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-label');
+      expect(el.button).to.have.attribute('aria-label', 'Label');
+    });
+
+    it('should proxy the aria-labelledby attribute to the button element', async () => {
+      el.setAttribute('aria-labelledby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-labelledby');
+      expect(el.button).to.have.attribute('aria-labelledby', 'id');
+    });
+
     it('should have set aria-selected to false on all options', () => {
       const allNotSelected = Array.from(el.querySelectorAll('sl-option')).every(
         option => option.getAttribute('aria-selected') === 'false'

packages/components/select/src/select.stories.ts

@@ -175,6 +175,22 @@ export const Groups: Story = {
   }
 };
 
+export const NoVisibleLabel: StoryObj = {
+  render: () => {
+    return html`
+      <p style="margin: 0 0 1rem 0">
+        This select has no internal or external label. It only has an <code>aria-label</code> attribute. That attribute
+        is automatically applied to the <code>sl-select-button</code> element.
+      </p>
+      <sl-select aria-label="Select an option">
+        <sl-option value="1">Option 1</sl-option>
+        <sl-option value="2">Option 2</sl-option>
+        <sl-option value="3">Option 3</sl-option>
+      </sl-select>
+    `;
+  }
+};
+
 export const OptionOverflow: Story = {
   args: {
     hint: 'This field has a lot of options, try scrolling through them.',

packages/components/select/src/select.ts

@@ -51,11 +51,6 @@ export class Select<T = any> extends ObserveAttributesMixin(FormControlMixin(Sco
   /** @internal The default offset of the listbox to the button. */
   static offset = 6;
 
-  /** @internal */
-  static override get observedAttributes(): string[] {
-    return [...super.observedAttributes, 'aria-describedby', 'aria-label', 'aria-labelledby'];
-  }
-
   /** @internal */
   static get scopedElements(): ScopedElementsMap {
     return {

packages/components/shared/src/mixins/observe-attributes-mixin.ts

@@ -8,13 +8,17 @@ export interface ObserveAttributesMixinInterface {
 /**
  * Mixin that is used to rewrite aria attributes in the component (based on the observedAttributes) to the focusable target element.
  */
-export function ObserveAttributesMixin<T extends Constructor<ReactiveElement>>(
+export function ObserveAttributesMixin<T extends Constructor<ReactiveElement> & { observedAttributes?: string[] }>(
   constructor: T,
   observedAttributes: string[] = []
 ): T & Constructor<ObserveAttributesMixinInterface> {
   class ObserveAttributesImpl extends constructor {
     #targetElement?: Element;
 
+    static override get observedAttributes(): string[] {
+      return [...(super.observedAttributes ?? []), ...observedAttributes];
+    }
+
     /** @internal */
     setAttributesTarget(target: Element): void {
       this.#targetElement = target;
@@ -26,6 +30,7 @@ export function ObserveAttributesMixin<T extends Constructor<ReactiveElement>>(
       requestAnimationFrame(() => {
         if (this.#targetElement && observedAttributes.includes(name)) {
           const value = this.getAttribute(name);
+
           if (value !== null) {
             this.#targetElement.setAttribute(name, value);
             this.removeAttribute(name);

packages/components/switch/src/switch.spec.ts

@@ -79,6 +79,30 @@ describe('sl-switch', () => {
       expect(el.renderRoot.querySelector('sl-icon')).to.exist;
     });
 
+    it('should proxy the aria-disabled attribute to the input element', async () => {
+      el.setAttribute('aria-disabled', 'true');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-disabled');
+      expect(el.input).to.have.attribute('aria-disabled', 'true');
+    });
+
+    it('should proxy the aria-label attribute to the input element', async () => {
+      el.setAttribute('aria-label', 'Label');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-label');
+      expect(el.input).to.have.attribute('aria-label', 'Label');
+    });
+
+    it('should proxy the aria-labelledby attribute to the input element', async () => {
+      el.setAttribute('aria-labelledby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-labelledby');
+      expect(el.input).to.have.attribute('aria-labelledby', 'id');
+    });
+
     it('should be pristine', () => {
       expect(el.dirty).not.to.be.true;
     });
@@ -321,23 +345,6 @@ describe('sl-switch', () => {
     });
   });
 
-  describe('aria attributes', () => {
-    beforeEach(async () => {
-      el = await fixture(html`<sl-switch checked aria-label="my label" aria-disabled="true"></sl-switch>`);
-      input = el.querySelector('input')!;
-
-      // Give time to rewrite arias
-      await new Promise(resolve => setTimeout(resolve, 100));
-    });
-
-    it('should have an input with proper arias', () => {
-      expect(el).not.to.have.attribute('aria-label', 'my label');
-      expect(el).not.to.have.attribute('aria-disabled', 'true');
-      expect(input).to.have.attribute('aria-label', 'my label');
-      expect(input).to.have.attribute('aria-disabled', 'true');
-    });
-  });
-
   describe('form reset', () => {
     let form: HTMLFormElement;
 

packages/components/switch/src/switch.ts

@@ -36,11 +36,6 @@ export class Switch<T = any> extends ObserveAttributesMixin(FormControlMixin(Sco
   /** @internal */
   static formAssociated = true;
 
-  /** @internal */
-  static override get observedAttributes(): string[] {
-    return [...super.observedAttributes, 'aria-disabled', 'aria-label', 'aria-labelledby'];
-  }
-
   /** @internal */
   static get scopedElements(): ScopedElementsMap {
     return {

packages/components/text-area/src/text-area.spec.ts

@@ -172,6 +172,38 @@ describe('sl-text-area', () => {
       expect(el).to.have.attribute('resize', 'auto');
     });
 
+    it('should proxy the aria-disabled attribute to the textarea element', async () => {
+      el.setAttribute('aria-disabled', 'true');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-disabled');
+      expect(el.textarea).to.have.attribute('aria-disabled', 'true');
+    });
+
+    it('should proxy the aria-label attribute to the textarea element', async () => {
+      el.setAttribute('aria-label', 'Label');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-label');
+      expect(el.textarea).to.have.attribute('aria-label', 'Label');
+    });
+
+    it('should proxy the aria-labelledby attribute to the textarea element', async () => {
+      el.setAttribute('aria-labelledby', 'id');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-labelledby');
+      expect(el.textarea).to.have.attribute('aria-labelledby', 'id');
+    });
+
+    it('should proxy the aria-required attribute to the textarea element', async () => {
+      el.setAttribute('aria-required', 'true');
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(el).to.not.have.attribute('aria-required');
+      expect(el.textarea).to.have.attribute('aria-required', 'true');
+    });
+
     it('should be pristine', () => {
       expect(el.dirty).not.to.be.true;
     });
@@ -325,23 +357,6 @@ describe('sl-text-area', () => {
     });
   });
 
-  describe('aria attributes', () => {
-    beforeEach(async () => {
-      el = await fixture(html`<sl-text-area aria-label="my label" aria-disabled="true"></sl-checkbox>`);
-      textArea = el.querySelector('textarea')!;
-
-      // Give time to rewrite arias
-      await new Promise(resolve => setTimeout(resolve, 100));
-    });
-
-    it('should have an input with proper arias', () => {
-      expect(el).not.to.have.attribute('aria-label', 'my label');
-      expect(el).not.to.have.attribute('aria-disabled', 'true');
-      expect(textArea).to.have.attribute('aria-label', 'my label');
-      expect(textArea).to.have.attribute('aria-disabled', 'true');
-    });
-  });
-
   describe('maxlength', () => {
     beforeEach(async () => {
       el = await fixture(html`<sl-text-area maxlength="3"></sl-text-area>`);