fix: base-button, update aria label on updated event. This also updated attr on slotchange event #5674

tsavka-adobe · 2025-08-06T03:40:57Z

Description

Currently the aria-label attribute gets set on firstUpdate. This causes an issue when the button is rendered and then the prop is updated. The change is not getting reflected. This makes sure the change actually gets reflected.

Update:
aria-label gets removed on slotChange event as well. This PR sets aria-label on any change based on label prop value.

Motivation and context

Adobe Expres A11Y critical bug

Related issue(s)

fixes [5620]

Screenshots (if appropriate)

Author's checklist

I have read the CONTRIBUTING and PULL_REQUESTS documents.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
I have added automated tests to cover my changes.
I have included a well-written changeset if my change needs to be published.
I have included updated documentation if my change required it.

Reviewer's checklist

Includes a Github Issue with appropriate flag or Jira ticket number without a link
Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
Automated tests cover all use cases and follow best practices for writing
Validated on all supported browsers
All VRTs are approved before the author can update Golden Hash

Manual review test cases

Descriptive Test Statement
1. Go here
2. Do this action
3. Expect this result
Descriptive Test Statement
1. Go here
2. Do this action
3. Expect this result

Device review

Did it pass in Desktop?
Did it pass in (emulated) Mobile?
Did it pass in (emulated) iPad?

…ed attr on slotchange event

changeset-bot · 2025-08-06T03:41:01Z

⚠️ No Changeset found

Latest commit: 2d2515f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Rajdeepc

A little test I have added. Please verify this from your end and let me know when you are ready for a re-review. Thanks for catching the bug and working on this. I will create a ticket for this.

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');
        });

Rajdeepc · 2025-08-06T06:17:23Z

packages/button/src/ButtonBase.ts

-        if (changes.has('label')) {
-            if (this.label) {
-                this.setAttribute('aria-label', this.label);
-            } else {
-                this.removeAttribute('aria-label');
-            }
-        }


In update it is still useful to track label changes.

Rajdeepc · 2025-08-06T06:17:48Z

packages/button/src/ButtonBase.ts

@@ -243,6 +264,7 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [
            this.manageAnchor();
        }

+        this.updateAriaLabel();


Suggested change

this.updateAriaLabel();

if (changed.has('label')) {

this.updateAriaLabel();

}

Rajdeepc · 2025-08-06T06:22:05Z

packages/button/src/ButtonBase.ts

+     * @label property is empty within changed propertyValues on slot change event.
+     * Need to check actualy @label property value instead.
+     */
+    protected updateAriaLabel(): void {


Only update aria-label if we have a label property and we're not in a pending state. You need to account for these scenarios too. A minor update to your code would be.

- `<slot @slotchange=${this.manageTextObservedSlot}></slot>` + `<slot @slotchange=${this.handleSlotChange}></slot>` private handleSlotChange(): void { // Call the original manageTextObservedSlot method this.manageTextObservedSlot(); // Only update aria-label if we have a label property and we're not in a pending state if (this.label && !('pending' in this && (this as { pending: boolean }).pending === true)) { this.updateAriaLabel(); } }

This seem to cause the issue. I reverted my original solution.

…m/tsavka-adobe/spectrum-web-components into tsavka-button-base-aria-label-fix

tsavka-adobe · 2025-08-06T07:51:41Z

Thank you @Rajdeepc for recommendations. I updated another test that was failing as well.

Rajdeepc · 2025-08-06T07:51:46Z

@tsavka-adobe Can you add some testing criteria too so that other reviewers can verify?

…account for pending state

tsavka-adobe · 2025-08-07T06:13:23Z

packages/button/src/ButtonBase.ts

+        if (
+            'pending' in this &&
+            (this as { pending: boolean }).pending === true
+        ) {
+            return;
+        }


Checking for pending state

tsavka-adobe added 3 commits August 5, 2025 15:17

fix: base-button, update aria label on updated event. This also updat…

58799cc

…ed attr on slotchange event

fix: button-base do not remove aria label

87f684e

fix: button-base update message

44902dd

tsavka-adobe requested a review from a team as a code owner August 6, 2025 03:40

Merge branch 'main' into tsavka-button-base-aria-label-fix

d125203

Rajdeepc requested changes Aug 6, 2025

View reviewed changes

tsavka-adobe added 4 commits August 6, 2025 00:22

fix: button-base add pending state and add update life cycle check

2efcb88

test: test-button update test to check for change and fix pending test

fca159c

Merge branch 'tsavka-button-base-aria-label-fix' of https://github.co…

100ab97

…m/tsavka-adobe/spectrum-web-components into tsavka-button-base-aria-label-fix

test: button fix typo

4143a71

Rajdeepc added bug Something isn't working a11y Issues related to accessibility SEV 3 labels Aug 6, 2025

tsavka-adobe added 2 commits August 6, 2025 14:52

fix: button-base fix aria label update reflection on slot change and …

ec3bf52

…account for pending state

fix: button-base remove not needed logic

ed134ec

tsavka-adobe commented Aug 7, 2025

View reviewed changes

fix: button-base simplify check

2d2515f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: base-button, update aria label on updated event. This also updated attr on slotchange event #5674

fix: base-button, update aria label on updated event. This also updated attr on slotchange event #5674

tsavka-adobe commented Aug 6, 2025

Uh oh!

changeset-bot bot commented Aug 6, 2025 •

edited

Loading

Uh oh!

Rajdeepc left a comment

Uh oh!

Rajdeepc Aug 6, 2025

Uh oh!

tsavka-adobe Aug 6, 2025

Uh oh!

Rajdeepc Aug 6, 2025

Uh oh!

tsavka-adobe Aug 6, 2025

Uh oh!

Rajdeepc Aug 6, 2025

Uh oh!

tsavka-adobe Aug 6, 2025

Uh oh!

tsavka-adobe Aug 7, 2025

Uh oh!

tsavka-adobe commented Aug 6, 2025

Uh oh!

Rajdeepc commented Aug 6, 2025

Uh oh!

tsavka-adobe Aug 7, 2025

Uh oh!

Uh oh!

fix: base-button, update aria label on updated event. This also updated attr on slotchange event #5674

Are you sure you want to change the base?

fix: base-button, update aria label on updated event. This also updated attr on slotchange event #5674

Conversation

tsavka-adobe commented Aug 6, 2025

Description

Motivation and context

Related issue(s)

Screenshots (if appropriate)

Author's checklist

Reviewer's checklist

Manual review test cases

Device review

Uh oh!

changeset-bot bot commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Rajdeepc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsavka-adobe commented Aug 6, 2025

Uh oh!

Rajdeepc commented Aug 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

changeset-bot bot commented Aug 6, 2025 •

edited

Loading