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

@@ -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`
       <div class="btn">
         ${this._renderBefore()}
-        ${this.constructor.__isIE11()
+        ${browserDetection.isIE11
           ? html`
               <div id="${this._buttonId}"><slot></slot></div>
             `
@@ -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;
-  }
 }

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(`<lion-button>foo</lion-button>`);
@@ -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(`<lion-button>foo</lion-button>`);
       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(
         `<div id="${wrapperId}"><slot></slot></div>`,
       );
-      restoreMockIsIE11();
+      browserDetectionStub.restore();
     });
 
     it('has a native button node with aria-hidden set to true', async () => {

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';

packages/core/src/browserDetection.js

@@ -0,0 +1,5 @@
+const isIE11 = /Trident/.test(window.navigator.userAgent);
+
+export const browserDetection = {
+  isIE11,
+};

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

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`
+      <div>
+        <div id="a"></div>
+        <div></div>
+        <div id="b">
+          <div></div>
+          <div id="b-child"></div>
+        </div>
+        <div></div>
+        <div id="c"></div>
+        <div></div>
+      </div>
+    `);
+    // 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();
+  });
 });