Skip to content

Commit

Permalink
fix(action-button,action-menu,picker,split-button): expand and update…
Browse files Browse the repository at this point in the history
… application of aria-* attributes
  • Loading branch information
majornista authored Jul 11, 2023
1 parent 3239f46 commit 52c0156
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 4 deletions.
18 changes: 17 additions & 1 deletion packages/action-button/src/ActionButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,13 @@ export class ActionButton extends SizedMixin(ButtonBase, {
protected override updated(changes: PropertyValues): void {
super.updated(changes);
const isButton = this.role === 'button';
const canBePressed = isButton && (this.selected || this.toggles);
const canBePressed =
isButton &&
(this.selected || this.toggles) &&
!(
this.hasAttribute('aria-haspopup') &&
this.hasAttribute('aria-expanded')
);
if (changes.has('selected') || changes.has('role')) {
// When role !== 'button' then the Action Button is within
// an Action Group that manages selects which means the
Expand All @@ -224,6 +230,16 @@ export class ActionButton extends SizedMixin(ButtonBase, {
} else {
// When !this.toggles the lack of "aria-pressed" is inconsequential.
this.removeAttribute('aria-pressed');
if (
isButton &&
this.toggles &&
this.hasAttribute('aria-expanded')
) {
this.setAttribute(
'aria-expanded',
this.selected ? 'true' : 'false'
);
}
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions packages/action-button/test/action-button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,48 @@ describe('ActionButton', () => {
expect(el.selected).to.be.true;
expect(button.getAttribute('aria-pressed')).to.equal('true');
});
it('toggles [aria-haspopup][aria-expanded]', async () => {
const el = await fixture<ActionButton>(
html`
<sp-action-button
toggles
aria-haspopup="true"
aria-expanded="false"
>
Button
</sp-action-button>
`
);

await elementUpdated(el);
const button = el.focusElement;

expect(el.toggles).to.be.true;
expect(el.selected).to.be.false;
expect(button).not.to.have.attribute('aria-pressed');
expect(button).to.have.attribute('aria-haspopup', 'true');
expect(button).to.have.attribute('aria-expanded', 'false');

el.focus();
await sendKeys({
press: 'Space',
});
await elementUpdated(el);

expect(el.toggles).to.be.true;
expect(el.selected).to.be.true;
expect(button).not.to.have.attribute('aria-pressed');
expect(button).to.have.attribute('aria-haspopup', 'true');
expect(button).to.have.attribute('aria-expanded', 'true');

el.addEventListener('change', (event: Event) => event.preventDefault());
el.click();
await elementUpdated(el);

expect(el.toggles).to.be.true;
expect(el.selected).to.be.true;
expect(button).not.to.have.attribute('aria-pressed');
expect(button).to.have.attribute('aria-haspopup', 'true');
expect(button).to.have.attribute('aria-expanded', 'true');
});
});
2 changes: 1 addition & 1 deletion packages/action-menu/src/ActionMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class ActionMenu extends ObserveSlotText(PickerBase, 'label') {
?quiet=${this.quiet}
?selected=${this.open}
aria-haspopup="true"
aria-controls="popover"
aria-controls=${ifDefined(this.open ? 'menu' : undefined)}
aria-expanded=${this.open ? 'true' : 'false'}
aria-label=${ifDefined(this.label || undefined)}
id="button"
Expand Down
17 changes: 17 additions & 0 deletions packages/action-menu/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
button.click();
await elementUpdated(el);
expect(el.open).to.be.true;
expect(button).to.have.attribute('aria-haspopup', 'true');
expect(button).to.have.attribute('aria-expanded', 'true');
expect(button).to.have.attribute('aria-controls', 'menu');
});
it('opens unmeasured with deprecated syntax', async () => {
const el = await deprecatedActionMenuFixture();
Expand All @@ -290,6 +293,12 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
const el = await actionMenuFixture();

await elementUpdated(el);

const button = el.button as HTMLButtonElement;
expect(button).to.have.attribute('aria-haspopup', 'true');
expect(button).to.have.attribute('aria-expanded', 'false');
expect(button).not.to.have.attribute('aria-controls');

let items = el.querySelectorAll('sp-menu-item');
const count = items.length;
expect(items.length).to.equal(count);
Expand All @@ -299,6 +308,8 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await opened;

expect(el.open).to.be.true;
expect(button).to.have.attribute('aria-expanded', 'true');
expect(button).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -307,6 +318,8 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await closed;

expect(el.open).to.be.false;
expect(button).to.have.attribute('aria-expanded', 'false');
expect(button).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(count);

Expand All @@ -315,6 +328,8 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await opened;

expect(el.open).to.be.true;
expect(button).to.have.attribute('aria-expanded', 'true');
expect(button).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -323,6 +338,8 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await closed;

expect(el.open).to.be.false;
expect(button).to.have.attribute('aria-expanded', 'false');
expect(button).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(count);
});
Expand Down
5 changes: 3 additions & 2 deletions packages/picker/src/Picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ export class PickerBase extends SizedMixin(Focusable) {
></span>
<button
aria-haspopup="true"
aria-controls=${ifDefined(this.open ? 'menu' : undefined)}
aria-expanded=${this.open ? 'true' : 'false'}
aria-labelledby="icon label applied-label"
id="button"
Expand Down Expand Up @@ -539,7 +540,7 @@ export class PickerBase extends SizedMixin(Focusable) {
return html`
<sp-tray
id="popover"
role="dialog"
role="presentation"
@sp-menu-item-added-or-updated=${this.updateMenuItems}
.overlayOpenCallback=${this.overlayOpenCallback}
.overlayCloseCallback=${this.overlayCloseCallback}
Expand All @@ -551,7 +552,7 @@ export class PickerBase extends SizedMixin(Focusable) {
return html`
<sp-popover
id="popover"
role="dialog"
role="presentation"
@sp-menu-item-added-or-updated=${this.updateMenuItems}
.overlayOpenCallback=${this.overlayOpenCallback}
.overlayCloseCallback=${this.overlayCloseCallback}
Expand Down
3 changes: 3 additions & 0 deletions packages/split-button/src/SplitButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ export class SplitButton extends SizedMixin(PickerBase) {
<sp-button
aria-haspopup="true"
aria-expanded=${this.open ? 'true' : 'false'}
aria-controls=${ifDefined(
this.open ? this.optionsMenu.id : undefined
)}
class="button trigger ${this.variant}"
@blur=${this.onButtonBlur}
@click=${this.onButtonClick}
Expand Down
26 changes: 26 additions & 0 deletions packages/split-button/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ export function runSplitButtonTests(
const el = test.querySelector('sp-split-button') as SplitButton;

await elementUpdated(el);

const trigger = el.shadowRoot?.querySelector('.trigger');
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');

let items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);

Expand All @@ -117,6 +122,8 @@ export function runSplitButtonTests(
await opened;

expect(el.open).to.be.true;
expect(trigger).to.have.attribute('aria-expanded', 'true');
expect(trigger).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -125,6 +132,8 @@ export function runSplitButtonTests(
await closed;

expect(el.open).to.be.false;
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);

Expand All @@ -133,6 +142,8 @@ export function runSplitButtonTests(
await opened;

expect(el.open).to.be.true;
expect(trigger).to.have.attribute('aria-expanded', 'true');
expect(trigger).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -141,6 +152,8 @@ export function runSplitButtonTests(
await closed;

expect(el.open).to.be.false;
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);
});
Expand All @@ -151,6 +164,11 @@ export function runSplitButtonTests(
const el = test.querySelector('sp-split-button') as SplitButton;

await elementUpdated(el);

const trigger = el.shadowRoot?.querySelector('.trigger');
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');

let items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);

Expand All @@ -159,6 +177,8 @@ export function runSplitButtonTests(
await opened;

expect(el.open).to.be.true;
expect(trigger).to.have.attribute('aria-expanded', 'true');
expect(trigger).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -167,6 +187,8 @@ export function runSplitButtonTests(
await closed;

expect(el.open).to.be.false;
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);

Expand All @@ -175,6 +197,8 @@ export function runSplitButtonTests(
await opened;

expect(el.open).to.be.true;
expect(trigger).to.have.attribute('aria-expanded', 'true');
expect(trigger).to.have.attribute('aria-controls', 'menu');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(0);

Expand All @@ -183,6 +207,8 @@ export function runSplitButtonTests(
await closed;

expect(el.open).to.be.false;
expect(trigger).to.have.attribute('aria-expanded', 'false');
expect(trigger).not.to.have.attribute('aria-controls');
items = el.querySelectorAll('sp-menu-item');
expect(items.length).to.equal(3);
});
Expand Down

0 comments on commit 52c0156

Please sign in to comment.