chore: cache aria helper, label to updated lifecycle

Author: caseyisonit

packages/button/src/ButtonBase.ts

@@ -225,13 +225,7 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [
         if (!this.hasAttribute('tabindex')) {
             this.setAttribute('tabindex', '0');
         }
-        if (changed.has('label')) {
-            if (this.label) {
-                this.setAttribute('aria-label', this.label);
-            } else {
-                this.removeAttribute('aria-label');
-            }
-        }
+
         this.manageAnchor();
         this.addEventListener('keydown', this.handleKeydown);
         this.addEventListener('keypress', this.handleKeypress);
@@ -243,6 +237,14 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [
             this.manageAnchor();
         }
 
+        if (changed.has('label')) {
+            if (this.label) {
+                this.setAttribute('aria-label', this.label);
+            } else {
+                this.removeAttribute('aria-label');
+            }
+        }
+
         if (this.anchorElement) {
             // Ensure the anchor element is not focusable directly via tab
             this.anchorElement.tabIndex = -1;

packages/button/stories/index.ts

@@ -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.
  */
+/* eslint-disable import/no-extraneous-dependencies */
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
 
@@ -86,6 +87,17 @@ export const argTypes = {
             type: 'boolean',
         },
     },
+    label: {
+        name: 'label',
+        type: { name: 'string', required: false },
+        description: 'The label to apply to the aria-label of the button.',
+        table: {
+            type: { summary: 'string' },
+        },
+        control: {
+            type: 'text',
+        },
+    },
 };
 
 export const makeOverBackground =
@@ -122,6 +134,7 @@ export function renderButton(properties: Properties): TemplateResult {
             treatment=${ifDefined(properties.treatment)}
             variant=${ifDefined(properties.variant)}
             static-color=${ifDefined(properties.staticColor)}
+            label=${ifDefined(properties.label)}
         >
             ${properties.content || 'Click Me'}
         </sp-button>

packages/button/stories/template.ts

@@ -9,7 +9,7 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-
+/* eslint-disable import/no-extraneous-dependencies */
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
 import {
@@ -30,6 +30,7 @@ export interface Properties {
     target?: '_blank' | '_parent' | '_self' | '_top';
     noWrap?: boolean;
     iconOnly?: boolean;
+    label?: string;
 }
 
 export const Template = ({

packages/button/test/button.test.ts

@@ -423,6 +423,45 @@ describe('Button', () => {
             expect(el.getAttribute('aria-label')).to.equal('clickable');
         });
 
+        it('updates aria-label when label changes', async () => {
+            const el = await fixture<Button>(html`
+                <sp-button label="Initial label">Button</sp-button>
+            `);
+
+            await elementUpdated(el);
+            expect(el.getAttribute('aria-label')).to.equal('Initial label');
+
+            // Change the label
+            el.label = 'New Label';
+            await elementUpdated(el);
+
+            // The aria-label should also update
+            expect(el.getAttribute('aria-label')).to.equal('New Label');
+        });
+
+        it('preserves aria-label when slot content changes', async () => {
+            const el = await fixture<Button>(html`
+                <sp-button label="Test label">Initial Content</sp-button>
+            `);
+
+            await elementUpdated(el);
+            expect(el.getAttribute('aria-label')).to.equal('Test label');
+
+            // Change the slot content
+            el.textContent = 'Updated content';
+            await elementUpdated(el);
+
+            // The aria-label should still be preserved
+            expect(el.getAttribute('aria-label')).to.equal('Test label');
+
+            // Change slot content again
+            el.innerHTML = '<span>New content</span>';
+            await elementUpdated(el);
+
+            // The aria-label should still be preserved
+            expect(el.getAttribute('aria-label')).to.equal('Test label');
+        });
+
         it('manages aria-label set from outside', async () => {
             const el = await fixture<Button>(html`
                 <sp-button

tools/reactive-controllers/src/PendingState.ts

@@ -10,8 +10,8 @@
  * governing permissions and limitations under the License.
  */
 
-import { html, LitElement, ReactiveController, TemplateResult } from 'lit';
 import '@spectrum-web-components/progress-circle/sp-progress-circle.js';
+import { html, LitElement, ReactiveController, TemplateResult } from 'lit';
 
 /**
  * Represents a host element with pending state.
@@ -58,7 +58,7 @@ export class PendingStateController<T extends HostWithPendingState>
                       id="loader"
                       size="s"
                       indeterminate
-                      aria-valuetext=${pendingLabel}
+                      aria-label=${pendingLabel}
                       class="progress-circle"
                   ></sp-progress-circle>
               `
@@ -73,15 +73,25 @@ export class PendingStateController<T extends HostWithPendingState>
         const { pending, disabled, pendingLabel } = this.host;
         const currentAriaLabel = this.host.getAttribute('aria-label');
 
+        function shouldCacheAriaLabel(
+            cached: string | null,
+            current: string | null,
+            pending: string | undefined
+        ): string | boolean | null {
+            return (
+                (!cached && current && current !== pending) ||
+                (cached !== current && current && current !== pending)
+            );
+        }
+
         // If the current `aria-label` is different from the pending label, cache it
         // or if the cached `aria-label` is different from the current `aria-label`, cache it
         if (
-            (!this.cachedAriaLabel &&
-                currentAriaLabel &&
-                currentAriaLabel !== pendingLabel) ||
-            (this.cachedAriaLabel !== currentAriaLabel &&
-                currentAriaLabel &&
-                currentAriaLabel !== pendingLabel)
+            shouldCacheAriaLabel(
+                this.cachedAriaLabel,
+                currentAriaLabel,
+                pendingLabel
+            )
         ) {
             this.cachedAriaLabel = currentAriaLabel;
         }