Skip to content

Commit

Permalink
fix: remove non-standard aria props from polyfill
Browse files Browse the repository at this point in the history
Fixes #2733

W-10820032
  • Loading branch information
nolanlawson committed Mar 10, 2022
1 parent 2a9db6d commit c8abfbc
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { AriaPropNameToAttrNameMap, keys } from '@lwc/shared';
import { StandardAriaPropNameToAttrNameMap, keys } from '@lwc/shared';
import { detect } from './detect';
import { patch } from './polyfill';

const ElementPrototypeAriaPropertyNames = keys(AriaPropNameToAttrNameMap);
const ElementPrototypeAriaPropertyNames = keys(StandardAriaPropNameToAttrNameMap);

for (let i = 0, len = ElementPrototypeAriaPropertyNames.length; i < len; i += 1) {
const propName = ElementPrototypeAriaPropertyNames[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { hasOwnProperty, AriaPropNameToAttrNameMap } from '@lwc/shared';
import { hasOwnProperty, StandardAriaPropNameToAttrNameMap } from '@lwc/shared';

type NormalizedAttributeValue = string | null;
type AriaPropMap = Record<string, NormalizedAttributeValue>;
Expand Down Expand Up @@ -63,7 +63,7 @@ export function patch(propName: string) {
// Typescript is inferring the wrong function type for this particular
// overloaded method: https://github.com/Microsoft/TypeScript/issues/27972
// @ts-ignore type-mismatch
const attrName = AriaPropNameToAttrNameMap[propName];
const attrName = StandardAriaPropNameToAttrNameMap[propName];
const descriptor = createAriaPropertyPropertyDescriptor(propName, attrName);
Object.defineProperty(Element.prototype, propName, descriptor);
}
49 changes: 40 additions & 9 deletions packages/@lwc/shared/src/aria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { create, forEach, StringReplace, StringToLowerCase } from './language';
import {
ArrayFilter,
ArrayIndexOf,
create,
forEach,
StringReplace,
StringToLowerCase,
} from './language';

/**
* According to the following list, there are 48 aria attributes of which two (ariaDropEffect and
Expand All @@ -16,7 +23,7 @@ import { create, forEach, StringReplace, StringToLowerCase } from './language';
* https://github.com/w3c/aria/pull/708/files#diff-eacf331f0ffc35d4b482f1d15a887d3bR11060
* https://wicg.github.io/aom/spec/aria-reflection.html
*/
const AriaPropertyNames = [
const ariaPropertyNames = [
'ariaActiveDescendant',
'ariaAtomic',
'ariaAutoComplete',
Expand Down Expand Up @@ -66,28 +73,52 @@ const AriaPropertyNames = [
'role',
] as const;

// The non-standard list includes prop->attr mappings that we have added in the
// past, but which are not part of AOM ARIA reflection as supported in browsers.
// https://github.com/salesforce/lwc/issues/2733
const nonStandardAriaPropertyNames = [
'ariaActiveDescendant',
'ariaControls',
'ariaDescribedBy',
'ariaDetails',
'ariaErrorMessage',
'ariaFlowTo',
'ariaInvalid',
'ariaLabelledBy',
'ariaOwns',
];

const standardAriaPropertyNames = /*@__PURE__*/ ArrayFilter.call(
ariaPropertyNames,
(item) => ArrayIndexOf.call(nonStandardAriaPropertyNames, item) === -1
);

export type AccessibleElementProperties = {
[prop in typeof AriaPropertyNames[number]]: string | null;
[prop in typeof ariaPropertyNames[number]]: string | null;
};

const { AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap } = /*@__PURE__*/ (() => {
function createMappings(propertyNames: string[]) {
const AriaAttrNameToPropNameMap: Record<string, string> = create(null);
const AriaPropNameToAttrNameMap: Record<string, string> = create(null);

// Synthetic creation of all AOM property descriptors for Custom Elements
forEach.call(AriaPropertyNames, (propName) => {
forEach.call(propertyNames, (propName) => {
const attrName = StringToLowerCase.call(
StringReplace.call(propName, /^aria/, () => 'aria-')
);
AriaAttrNameToPropNameMap[attrName] = propName;
AriaPropNameToAttrNameMap[propName] = attrName;
});

return { AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap };
})();
return [AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap];
}

export const [AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap] = /*@__PURE__*/ createMappings(
ariaPropertyNames as unknown as string[]
);
export const [StandardAriaAttrNameToPropNameMap, StandardAriaPropNameToAttrNameMap] =
/*@__PURE__*/ createMappings(standardAriaPropertyNames);

export function isAriaAttribute(attrName: string): boolean {
return attrName in AriaAttrNameToPropNameMap;
}

export { AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap };
Original file line number Diff line number Diff line change
Expand Up @@ -54,35 +54,26 @@ testInvalidComponentConstructor('Class not extending LightningElement', class Co

const GLOBAL_HTML_ATTRIBUTES = [
'accessKey',
'ariaActiveDescendant',
'ariaAtomic',
'ariaAutoComplete',
'ariaBusy',
'ariaChecked',
'ariaColCount',
'ariaColIndex',
'ariaColSpan',
'ariaControls',
'ariaCurrent',
'ariaDescribedBy',
'ariaDetails',
'ariaDisabled',
'ariaErrorMessage',
'ariaExpanded',
'ariaFlowTo',
'ariaHasPopup',
'ariaHidden',
'ariaInvalid',
'ariaKeyShortcuts',
'ariaLabel',
'ariaLabelledBy',
'ariaLevel',
'ariaLive',
'ariaModal',
'ariaMultiLine',
'ariaMultiSelectable',
'ariaOrientation',
'ariaOwns',
'ariaPlaceholder',
'ariaPosInSet',
'ariaPressed',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,35 +7,26 @@ const expectedEnumerableProps = [
'accessKey',
'accessKeyLabel',
'addEventListener',
'ariaActiveDescendant',
'ariaAtomic',
'ariaAutoComplete',
'ariaBusy',
'ariaChecked',
'ariaColCount',
'ariaColIndex',
'ariaColSpan',
'ariaControls',
'ariaCurrent',
'ariaDescribedBy',
'ariaDetails',
'ariaDisabled',
'ariaErrorMessage',
'ariaExpanded',
'ariaFlowTo',
'ariaHasPopup',
'ariaHidden',
'ariaInvalid',
'ariaKeyShortcuts',
'ariaLabel',
'ariaLabelledBy',
'ariaLevel',
'ariaLive',
'ariaModal',
'ariaMultiLine',
'ariaMultiSelectable',
'ariaOrientation',
'ariaOwns',
'ariaPlaceholder',
'ariaPosInSet',
'ariaPressed',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,46 @@
function testAriaProperty(property, attribute) {
function testAriaProperty(property, attribute, standard) {
describe(property, () => {
it(`should assign property ${property} to Element prototype`, () => {
expect(Object.prototype.hasOwnProperty.call(Element.prototype, property)).toBe(true);
it(`should ${
standard ? '' : 'not'
} assign property ${property} to Element prototype`, () => {
expect(Object.prototype.hasOwnProperty.call(Element.prototype, property)).toBe(
standard
);
});

it(`should return null if the value is not set`, () => {
it(`should ${standard ? '' : 'not'} return null if the value is not set`, () => {
const el = document.createElement('div');
expect(el[property]).toBe(null);
expect(el[property]).toBe(standard ? null : undefined);
});

it('should return the right value from the getter', () => {
it(`should return the right value from the getter`, () => {
const el = document.createElement('div');
el[property] = 'foo';
expect(el[property]).toBe('foo');
});

it('should reflect the property to the associated attribute', () => {
it(`should ${
standard ? '' : 'not'
} reflect the property to the associated attribute`, () => {
const el = document.createElement('div');
el[property] = 'foo';
expect(el.getAttribute(attribute)).toBe('foo');
expect(el.getAttribute(attribute)).toBe(standard ? 'foo' : null);
});

it('should reflect the attribute to the property', () => {
it(`should ${standard ? '' : 'not'} reflect the attribute to the property`, () => {
const el = document.createElement('div');
el.setAttribute(attribute, 'foo');
expect(el[property]).toBe('foo');
expect(el[property]).toBe(standard ? 'foo' : undefined);
});

it('should remove the attribute if the property is set to null', () => {
it(`should ${
standard ? '' : 'not'
} remove the attribute if the property is set to null`, () => {
const el = document.createElement('div');
el.setAttribute(attribute, 'foo');

el[property] = null;
expect(el.hasAttribute(attribute)).toBe(false);
expect(el.hasAttribute(attribute)).toBe(!standard);
});
});
}
Expand Down Expand Up @@ -87,6 +95,22 @@ const ariaPropertiesMapping = {
role: 'role',
};

// The non-standard list includes prop->attr mappings that we have added in the
// past, but which are not part of AOM ARIA reflection as supported in browsers.
// https://github.com/salesforce/lwc/issues/2733
const nonStandardAriaProps = [
'ariaActiveDescendant',
'ariaControls',
'ariaDescribedBy',
'ariaDetails',
'ariaErrorMessage',
'ariaFlowTo',
'ariaInvalid',
'ariaLabelledBy',
'ariaOwns',
];

for (const [ariaProperty, ariaAttribute] of Object.entries(ariaPropertiesMapping)) {
testAriaProperty(ariaProperty, ariaAttribute);
const standard = !nonStandardAriaProps.includes(ariaProperty);
testAriaProperty(ariaProperty, ariaAttribute, standard);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ it('should handle multiple idrefs when set dynamically', () => {
expect(hokkaido.id).toMatch(/^hokkaido-\w+/);
}

expect(input.ariaLabelledBy).toContain(aomori.id);
expect(input.ariaLabelledBy).toContain(hokkaido.id);
expect(input.getAttribute('aria-labelledby')).toContain(aomori.id);
expect(input.getAttribute('aria-labelledby')).toContain(hokkaido.id);
});

it('should handle multiple idrefs when set statically', () => {
Expand All @@ -38,6 +38,6 @@ it('should handle multiple idrefs when set statically', () => {
expect(iwate.id).toMatch(/^iwate-\w+/);
}

expect(input.ariaLabelledBy).toContain(aomori.id);
expect(input.ariaLabelledBy).toContain(iwate.id);
expect(input.getAttribute('aria-labelledby')).toContain(aomori.id);
expect(input.getAttribute('aria-labelledby')).toContain(iwate.id);
});

0 comments on commit c8abfbc

Please sign in to comment.