From 954adc56596f2d9244baf48889d6b338b2a12ac4 Mon Sep 17 00:00:00 2001 From: Wessel Loth Date: Wed, 5 Feb 2020 15:01:20 +0100 Subject: [PATCH] fix(button): run regexp once instead of every render cycle chore: add browserDetection utility to @lion/core Use it to fix test involving checking the userAgent in LionButton chore: add browserDetection util to aria element sorting --- packages/button/src/LionButton.js | 21 ++++++++-------- packages/button/test/lion-button.test.js | 16 +++--------- packages/core/index.js | 1 + packages/core/src/browserDetection.js | 5 ++++ .../utils/getAriaElementsInRightDomOrder.js | 6 ++--- .../getAriaElementsInRightDomOrder.test.js | 25 +++++++++++++++++++ 6 files changed, 47 insertions(+), 27 deletions(-) create mode 100644 packages/core/src/browserDetection.js diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index ab6857ba8..d19c09c43 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -1,8 +1,13 @@ -import { css, html, SlotMixin, DisabledWithTabIndexMixin, LitElement } from '@lion/core'; +import { + css, + html, + browserDetection, + SlotMixin, + DisabledWithTabIndexMixin, + LitElement, +} from '@lion/core'; -// eslint-disable-next-line class-methods-use-this const isKeyboardClickEvent = e => e.keyCode === 32 /* space */ || e.keyCode === 13; /* enter */ -// eslint-disable-next-line class-methods-use-this const isSpaceKeyboardClickEvent = e => e.keyCode === 32; /* space */ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement)) { @@ -27,7 +32,7 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement)) return html`
${this._renderBefore()} - ${this.constructor.__isIE11() + ${browserDetection.isIE11 ? html`
` @@ -153,7 +158,7 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement)) this.active = false; this.__setupDelegationInConstructor(); - if (this.constructor.__isIE11()) { + if (browserDetection.isIE11) { this._buttonId = `button-${Math.random() .toString(36) .substr(2, 10)}`; @@ -255,10 +260,4 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement)) this.click(); } } - - static __isIE11() { - const ua = window.navigator.userAgent; - const result = /Trident/.test(ua); - return result; - } } diff --git a/packages/button/test/lion-button.test.js b/packages/button/test/lion-button.test.js index 3010579b1..8c43082d2 100644 --- a/packages/button/test/lion-button.test.js +++ b/packages/button/test/lion-button.test.js @@ -9,7 +9,7 @@ import { keyDownOn, keyUpOn, } from '@polymer/iron-test-helpers/mock-interactions.js'; -import { LionButton } from '../src/LionButton.js'; +import { browserDetection } from '@lion/core'; import '../lion-button.js'; @@ -21,16 +21,6 @@ function getTopElement(el) { return crossBrowserRoot.elementFromPoint(left + width / 2, top + height / 2); } -let originalIsIE11Method; -function mockIsIE11() { - originalIsIE11Method = LionButton.__isIE11; - LionButton.__isIE11 = () => true; -} - -function restoreMockIsIE11() { - LionButton.__isIE11 = originalIsIE11Method; -} - describe('lion-button', () => { it('behaves like native `button` in terms of a11y', async () => { const el = await fixture(`foo`); @@ -212,7 +202,7 @@ describe('lion-button', () => { }); it('has an aria-labelledby and wrapper element in IE11', async () => { - mockIsIE11(); + const browserDetectionStub = sinon.stub(browserDetection, 'isIE11').value(true); const el = await fixture(`foo`); expect(el.hasAttribute('aria-labelledby')).to.be.true; const wrapperId = el.getAttribute('aria-labelledby'); @@ -220,7 +210,7 @@ describe('lion-button', () => { expect(el.shadowRoot.querySelector(`#${wrapperId}`)).dom.to.equal( `
`, ); - restoreMockIsIE11(); + browserDetectionStub.restore(); }); it('has a native button node with aria-hidden set to true', async () => { diff --git a/packages/core/index.js b/packages/core/index.js index de1162c39..b51dde4a0 100644 --- a/packages/core/index.js +++ b/packages/core/index.js @@ -59,3 +59,4 @@ export { DisabledWithTabIndexMixin } from './src/DisabledWithTabIndexMixin.js'; export { LionSingleton } from './src/LionSingleton.js'; export { SlotMixin } from './src/SlotMixin.js'; export { UpdateStylesMixin } from './src/UpdateStylesMixin.js'; +export { browserDetection } from './src/browserDetection.js'; diff --git a/packages/core/src/browserDetection.js b/packages/core/src/browserDetection.js new file mode 100644 index 000000000..1a2b89745 --- /dev/null +++ b/packages/core/src/browserDetection.js @@ -0,0 +1,5 @@ +const isIE11 = /Trident/.test(window.navigator.userAgent); + +export const browserDetection = { + isIE11, +}; diff --git a/packages/field/src/utils/getAriaElementsInRightDomOrder.js b/packages/field/src/utils/getAriaElementsInRightDomOrder.js index 549f4ba04..00bdacd16 100644 --- a/packages/field/src/utils/getAriaElementsInRightDomOrder.js +++ b/packages/field/src/utils/getAriaElementsInRightDomOrder.js @@ -1,4 +1,4 @@ -const isIE11 = /Trident/.test(window.navigator.userAgent); +import { browserDetection } from '@lion/core'; /** * @desc Let the order of adding ids to aria element by DOM order, so that the screen reader @@ -14,9 +14,9 @@ export function getAriaElementsInRightDomOrder(descriptionElements, { reverse } const pos = a.compareDocumentPosition(b); // Unfortunately, for IE, we have to switch the order (?) if (pos === Node.DOCUMENT_POSITION_PRECEDING || pos === Node.DOCUMENT_POSITION_CONTAINED_BY) { - return isIE11 ? -1 : 1; + return browserDetection.isIE11 ? -1 : 1; } - return isIE11 ? 1 : -1; + return browserDetection.isIE11 ? 1 : -1; }; const descriptionEls = descriptionElements.filter(el => el); // filter out null references diff --git a/packages/field/test/utils/getAriaElementsInRightDomOrder.test.js b/packages/field/test/utils/getAriaElementsInRightDomOrder.test.js index 00187a7b1..c3f012733 100644 --- a/packages/field/test/utils/getAriaElementsInRightDomOrder.test.js +++ b/packages/field/test/utils/getAriaElementsInRightDomOrder.test.js @@ -1,4 +1,6 @@ import { expect, fixture, html } from '@open-wc/testing'; +import sinon from 'sinon'; +import { browserDetection } from '@lion/core'; import { getAriaElementsInRightDomOrder } from '../../src/utils/getAriaElementsInRightDomOrder.js'; describe('getAriaElementsInRightDomOrder', () => { @@ -43,4 +45,27 @@ describe('getAriaElementsInRightDomOrder', () => { const result = getAriaElementsInRightDomOrder(unorderedNodes, { reverse: true }); expect(result).to.eql([c, bChild, b, a]); }); + + it('orders in reverse for IE11', async () => { + const browserDetectionStub = sinon.stub(browserDetection, 'isIE11').value(true); + const el = await fixture(html` +
+
+
+
+
+
+
+
+
+
+
+ `); + // eslint-disable-next-line no-unused-vars + const [a, _1, b, _2, bChild, _3, c, _4] = el.querySelectorAll('div'); + const unorderedNodes = [bChild, c, a, b]; + const result = getAriaElementsInRightDomOrder(unorderedNodes); + expect(result).to.eql([c, bChild, b, a]); + browserDetectionStub.restore(); + }); });