Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/seven-nails-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@spectrum-web-components/button': patch
'@spectrum-web-components/reactive-controllers': patch
Comment on lines +2 to +3
Copy link
Contributor

@Rajdeepc Rajdeepc Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Will this be read with our current changelog script? Would you prefer adding separate changeset for each package instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are other changesets with mulitple packages, i dont think it should be an issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what Rajdeep means is, even though this works, our change log script does not pick up multiple packages (only the first one).

---

**Fixed** aria-label handling in button components and pending state controller.

**Button component changes:**

- Fixed timing of aria-label updates to occur after slot content changes are processed
- Added `label` property support for programmatic aria-label control
- Added comprehensive tests for aria-label behavior during content changes

**PendingState controller changes:**

- Improved aria-label caching logic to better handle dynamic label changes
- Enhanced caching mechanism to preserve user-set aria-labels during pending states

These changes ensure that aria-labels are properly managed and preserved across component state changes, improving accessibility for screen reader users.
26 changes: 9 additions & 17 deletions packages/button/src/ButtonBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -256,14 +258,4 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [
this.anchorElement.addEventListener('focus', this.proxyFocus);
}
}
protected override update(changes: PropertyValues): void {
super.update(changes);
if (changes.has('label')) {
if (this.label) {
this.setAttribute('aria-label', this.label);
} else {
this.removeAttribute('aria-label');
}
}
}
}
12 changes: 12 additions & 0 deletions packages/button/stories/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,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 =
Expand Down Expand Up @@ -122,6 +133,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>
Expand Down
2 changes: 1 addition & 1 deletion packages/button/stories/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import { html, TemplateResult } from '@spectrum-web-components/base';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
import {
Expand All @@ -30,6 +29,7 @@ export interface Properties {
target?: '_blank' | '_parent' | '_self' | '_top';
noWrap?: boolean;
iconOnly?: boolean;
label?: string;
}

export const Template = ({
Expand Down
56 changes: 43 additions & 13 deletions packages/button/test/button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,32 +384,62 @@ describe('Button', () => {

it('manages aria-label from pending state', async () => {
const el = await fixture<Button>(html`
<sp-button
href="test_url"
target="_blank"
label="clickable"
pending
>
<sp-button href="test_url" target="_blank" label="clickable">
Click me
</sp-button>
`);
await elementUpdated(el);
expect(el.getAttribute('aria-label')).to.equal('Pending');
expect(el.getAttribute('aria-label')).to.equal('clickable');

// button set to disabled while pending is true and the aria-label should be original
el.disabled = true;
// button set to pending and aria-label should update
el.pending = true;
await elementUpdated(el);
expect(el.getAttribute('aria-label')).to.equal('clickable');
expect(el.pending).to.be.true;
expect(el.getAttribute('aria-label')).to.equal('Pending');

// pending is removed and the aria-label should not change as the button is disabled
el.pending = false;
await elementUpdated(el);
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>
`);

// button is enabled and the aria-label should not change
el.disabled = false;
await elementUpdated(el);
expect(el.getAttribute('aria-label')).to.equal('clickable');
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 () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/picker/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2117,12 +2117,12 @@ export function runPickerTests(): void {
};

expect(
findAccessibilityNode<NamedNode>(
snapshot,
(node) =>
findAccessibilityNode<NamedNode>(snapshot, (node) => {
return (
node.name ===
'Pending Choose your neighborhood Where do you live?'
)
);
})
).to.not.be.null;
});
});
Expand Down
11 changes: 6 additions & 5 deletions packages/progress-circle/src/ProgressCircle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import {
property,
query,
} from '@spectrum-web-components/base/src/decorators.js';
import { getLabelFromSlot } from '@spectrum-web-components/shared/src/get-label-from-slot.js';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
import { getLabelFromSlot } from '@spectrum-web-components/shared/src/get-label-from-slot.js';

import progressCircleStyles from './progress-circle.css.js';

Expand Down Expand Up @@ -119,10 +119,11 @@ export class ProgressCircle extends SizedMixin(SpectrumElement, {

if (window.__swc.DEBUG) {
if (
!this.label &&
!this.getAttribute('aria-label') &&
!this.getAttribute('aria-labelledby') &&
!this.slotEl.assignedNodes().length
(!this.label ||
!this.getAttribute('aria-label') ||
!this.getAttribute('aria-labelledby') ||
!this.slotEl.assignedNodes().length) &&
this.getAttribute('role') === 'progressbar'
) {
window.__swc.warn(
this,
Expand Down
37 changes: 29 additions & 8 deletions tools/reactive-controllers/src/PendingState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -49,17 +49,18 @@ export class PendingStateController<T extends HostWithPendingState>
/**
* Renders the pending state UI.
* @returns A TemplateResult representing the pending state UI.
*
* @TODO: [SWC-1119, SWC-1255, SWC-459] Confirm the accessibility warning and a11y dom tree are accurate for the pending state in button, combobox, and picker components.
*/
public renderPendingState(): TemplateResult {
const pendingLabel = this.host.pendingLabel || 'Pending';
return this.host.pending
? html`
<sp-progress-circle
id="loader"
size="s"
indeterminate
aria-valuetext=${pendingLabel}
class="progress-circle"
role="presentation"
></sp-progress-circle>
`
: html``;
Expand All @@ -73,17 +74,37 @@ export class PendingStateController<T extends HostWithPendingState>
const { pending, disabled, pendingLabel } = this.host;
const currentAriaLabel = this.host.getAttribute('aria-label');

if (pending && !disabled && currentAriaLabel !== pendingLabel) {
// Cache the current `aria-label` to be restored when no longer `pending`
function shouldCacheAriaLabel(
cached: string | null,
current: string | null,
pending: string | undefined
): boolean {
return (
(!cached && current !== pending) ||
(cached !== 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 (
shouldCacheAriaLabel(
this.cachedAriaLabel,
currentAriaLabel,
pendingLabel
)
) {
this.cachedAriaLabel = currentAriaLabel;
}

if (pending && !disabled) {
// Since it is pending, we set the aria-label to `pendingLabel` or "Pending"
this.host.setAttribute('aria-label', pendingLabel || 'Pending');
} else if (!pending || disabled) {
} else {
// Restore the cached `aria-label` if it exists
if (this.cachedAriaLabel) {
this.host.setAttribute('aria-label', this.cachedAriaLabel);
} else if (!pending) {
// If no cached `aria-label` and not `pending`, remove the `aria-label`
} else {
this.host.removeAttribute('aria-label');
}
}
Expand Down
2 changes: 1 addition & 1 deletion tools/reactive-controllers/test/pending-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import {
PendingStateController,
} from '@spectrum-web-components/reactive-controllers/src/PendingState.js';

import '@spectrum-web-components/progress-circle/sp-progress-circle.js';
import '@spectrum-web-components/picker/sp-picker.js';
import '@spectrum-web-components/progress-circle/sp-progress-circle.js';

describe('PendingStateController', () => {
let host: HostWithPendingState;
Expand Down
Loading