Skip to content

Commit

Permalink
fix!: aria-labels announcing twice with "group" on components
Browse files Browse the repository at this point in the history
BREAKING CHANGE: `querySelector` for `[role]` and `[aria-*]` attributes may no longer work. See `@material/web/migrations/v2/README.md` and `@material/web/migrations/v2/query-selector-aria.ts`.

Browser/SR test results (go/mwc-double-aria-test-results)
  - ✅ VoiceOver on Chrome
  - ✅ VoiceOver on iOS Safari
  - ✅ TalkBack on Chrome
  - ✅ ChromeVox on Chrome
  - ✅ NVDA on Chrome
  - ✅ NVDA on Firefox
  - ✅ JAWS on Chrome
  - ✅ JAWS on Firefox
  (Opt)
  - ❓ VoiceOver on Safari
  - ❓ VoiceOver on Firefox

PiperOrigin-RevId: 632611852
  • Loading branch information
asyncLiz authored and copybara-github committed Jul 1, 2024
1 parent 352607d commit aebbfc9
Show file tree
Hide file tree
Showing 28 changed files with 815 additions and 203 deletions.
5 changes: 2 additions & 3 deletions button/internal/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {html, isServer, LitElement, nothing} from 'lit';
import {property, query, queryAssignedElements} from 'lit/decorators.js';

import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';
import {
FormSubmitter,
setupFormSubmitter,
Expand All @@ -27,14 +27,13 @@ import {
} from '../../labs/behaviors/element-internals.js';

// Separate variable needed for closure.
const buttonBaseClass = mixinElementInternals(LitElement);
const buttonBaseClass = mixinDelegatesAria(mixinElementInternals(LitElement));

/**
* A button component.
*/
export abstract class Button extends buttonBaseClass implements FormSubmitter {
static {
requestUpdateOnAriaChange(Button);
setupFormSubmitter(Button);
}

Expand Down
12 changes: 5 additions & 7 deletions checkbox/internal/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {property, query, state} from 'lit/decorators.js';
import {classMap} from 'lit/directives/class-map.js';

import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';
import {
dispatchActivationClick,
isActivationClick,
Expand All @@ -32,8 +32,10 @@ import {
import {CheckboxValidator} from '../../labs/behaviors/validators/checkbox-validator.js';

// Separate variable needed for closure.
const checkboxBaseClass = mixinConstraintValidation(
mixinFormAssociated(mixinElementInternals(LitElement)),
const checkboxBaseClass = mixinDelegatesAria(
mixinConstraintValidation(
mixinFormAssociated(mixinElementInternals(LitElement)),
),
);

/**
Expand All @@ -48,10 +50,6 @@ const checkboxBaseClass = mixinConstraintValidation(
* --bubbles --composed
*/
export class Checkbox extends checkboxBaseClass {
static {
requestUpdateOnAriaChange(Checkbox);
}

/** @nocollapse */
static override shadowRootOptions = {
...LitElement.shadowRootOptions,
Expand Down
11 changes: 5 additions & 6 deletions chips/internal/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,17 @@ import {html, LitElement, PropertyValues, TemplateResult} from 'lit';
import {property} from 'lit/decorators.js';
import {ClassInfo, classMap} from 'lit/directives/class-map.js';

import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';

// Separate variable needed for closure.
const chipBaseClass = mixinDelegatesAria(LitElement);

/**
* A chip component.
*
* @fires update-focus {Event} Dispatched when `disabled` is toggled. --bubbles
*/
export abstract class Chip extends LitElement {
static {
requestUpdateOnAriaChange(Chip);
}

export abstract class Chip extends chipBaseClass {
/** @nocollapse */
static override shadowRootOptions = {
...LitElement.shadowRootOptions,
Expand Down
11 changes: 5 additions & 6 deletions dialog/internal/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {property, query, state} from 'lit/decorators.js';
import {classMap} from 'lit/directives/class-map.js';

import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';
import {redispatchEvent} from '../../internal/events/redispatch-event.js';

import {
Expand All @@ -21,6 +21,9 @@ import {
DialogAnimationArgs,
} from './animations.js';

// Separate variable needed for closure.
const dialogBaseClass = mixinDelegatesAria(LitElement);

/**
* A dialog component.
*
Expand All @@ -31,11 +34,7 @@ import {
* @fires cancel {Event} Dispatched when the dialog has been canceled by clicking
* on the scrim or pressing Escape.
*/
export class Dialog extends LitElement {
static {
requestUpdateOnAriaChange(Dialog);
}

export class Dialog extends dialogBaseClass {
// We do not use `delegatesFocus: true` due to a Chromium bug with
// selecting text.
// See https://bugs.chromium.org/p/chromium/issues/detail?id=950357
Expand Down
11 changes: 5 additions & 6 deletions fab/internal/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,18 @@ import {property} from 'lit/decorators.js';
import {classMap} from 'lit/directives/class-map.js';

import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';

/**
* Sizes variants available to non-extended FABs.
*/
export type FabSize = 'medium' | 'small' | 'large';

// tslint:disable-next-line:enforce-comments-on-exported-symbols
export abstract class SharedFab extends LitElement {
static {
requestUpdateOnAriaChange(SharedFab);
}
// Separate variable needed for closure.
const fabBaseClass = mixinDelegatesAria(LitElement);

// tslint:disable-next-line:enforce-comments-on-exported-symbols
export abstract class SharedFab extends fabBaseClass {
/** @nocollapse */
static override shadowRootOptions: ShadowRootInit = {
mode: 'open' as const,
Expand Down
7 changes: 4 additions & 3 deletions iconbutton/internal/icon-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {classMap} from 'lit/directives/class-map.js';
import {literal, html as staticHtml} from 'lit/static-html.js';

import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {mixinDelegatesAria} from '../../internal/aria/delegate.js';
import {
FormSubmitter,
setupFormSubmitter,
Expand All @@ -28,7 +28,9 @@ import {
type LinkTarget = '_blank' | '_parent' | '_self' | '_top';

// Separate variable needed for closure.
const iconButtonBaseClass = mixinElementInternals(LitElement);
const iconButtonBaseClass = mixinDelegatesAria(
mixinElementInternals(LitElement),
);

/**
* A button for rendering icons.
Expand All @@ -39,7 +41,6 @@ const iconButtonBaseClass = mixinElementInternals(LitElement);
*/
export class IconButton extends iconButtonBaseClass implements FormSubmitter {
static {
requestUpdateOnAriaChange(IconButton);
setupFormSubmitter(IconButton);
}

Expand Down
2 changes: 1 addition & 1 deletion internal/aria/aria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const ARIA_ATTRIBUTES = ARIA_PROPERTIES.map(ariaPropertyToAttribute);
* @return True if the attribute is an aria attribute, or false if not.
*/
export function isAriaAttribute(attribute: string): attribute is ARIAAttribute {
return attribute.startsWith('aria-') || attribute === 'role';
return ARIA_ATTRIBUTES.includes(attribute as ARIAAttribute);
}

/**
Expand Down
10 changes: 8 additions & 2 deletions internal/aria/aria_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ describe('aria', () => {
.toBeTrue();
});

it('should return true for aria idref attributes', () => {
it('should return false for aria idref attributes', () => {
expect(isAriaAttribute('aria-labelledby'))
.withContext('aria-labelledby input')
.toBeTrue();
.toBeFalse();
});

it('should return true for role', () => {
Expand All @@ -29,6 +29,12 @@ describe('aria', () => {
it('should return false for non-aria attributes', () => {
expect(isAriaAttribute('label')).withContext('label input').toBeFalse();
});

it('should return false for custom aria-* attributes', () => {
expect(isAriaAttribute('aria-label-custom'))
.withContext('aria-label-custom input')
.toBeFalse();
});
});

describe('ariaPropertyToAttribute()', () => {
Expand Down
Loading

0 comments on commit aebbfc9

Please sign in to comment.