refactor: update tabindex logic to work with Lit, add sync: true (#9119)

Author: web-padawan

packages/a11y-base/src/delegate-focus-mixin.js

@@ -42,6 +42,7 @@ export const DelegateFocusMixin = dedupeMixin(
             type: Object,
             readOnly: true,
             observer: '_focusElementChanged',
+            sync: true,
           },
 
           /**
@@ -228,6 +229,11 @@ export const DelegateFocusMixin = dedupeMixin(
           }
           this.tabindex = undefined;
         }
+
+        // Lit does not remove attribute when setting property to undefined
+        if (tabindex === undefined && this.hasAttribute('tabindex')) {
+          this.removeAttribute('tabindex');
+        }
       }
     },
 );

packages/a11y-base/src/tabindex-mixin.js

@@ -27,6 +27,7 @@ export const TabindexMixin = (superclass) =>
           type: Number,
           reflectToAttribute: true,
           observer: '_tabindexChanged',
+          sync: true,
         },
 
         /**
@@ -60,9 +61,13 @@ export const TabindexMixin = (superclass) =>
         if (this.tabindex !== undefined) {
           this._lastTabIndex = this.tabindex;
         }
-        this.tabindex = -1;
+        this.setAttribute('tabindex', '-1');
       } else if (oldDisabled) {
-        this.tabindex = this._lastTabIndex;
+        if (this._lastTabIndex !== undefined) {
+          this.setAttribute('tabindex', this._lastTabIndex);
+        } else {
+          this.tabindex = undefined;
+        }
       }
     }
 
@@ -80,7 +85,7 @@ export const TabindexMixin = (superclass) =>
 
       if (this.disabled && tabindex !== -1) {
         this._lastTabIndex = tabindex;
-        this.tabindex = -1;
+        this.setAttribute('tabindex', '-1');
       }
     }
 

packages/a11y-base/test/delegate-focus-mixin.test.js

@@ -1,47 +1,46 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync, focusin, focusout, nextFrame } from '@vaadin/testing-helpers';
+import { defineLit, definePolymer, fixtureSync, focusin, focusout, nextFrame } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
-import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
+import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
+import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
 import { DelegateFocusMixin } from '../src/delegate-focus-mixin.js';
 
-describe('delegate-focus-mixin', () => {
-  let element, input, setFocusedSpy;
-
-  customElements.define(
-    'delegate-focus-mixin-element',
-    class extends DelegateFocusMixin(PolymerElement) {
-      static get template() {
-        return html`
-          <slot name="input"></slot>
-          <slot name="suffix"></slot>
-        `;
-      }
-
-      /** @protected */
-      ready() {
-        super.ready();
-
-        const input = this.querySelector('input');
-        this._setFocusElement(input);
-      }
-
-      _setFocused(focused) {
-        super._setFocused(focused);
-        setFocusedSpy?.(focused);
-      }
-    },
+const runTests = (defineHelper, baseMixin) => {
+  let setFocusedSpy;
+
+  const tag = defineHelper(
+    'delegate-focus-mixin',
+    `
+      <slot name="input"></slot>
+      <slot name="suffix"></slot>
+    `,
+    (Base) =>
+      class extends DelegateFocusMixin(baseMixin(Base)) {
+        ready() {
+          super.ready();
+          const input = this.querySelector('input');
+          this._setFocusElement(input);
+        }
+
+        _setFocused(focused) {
+          super._setFocused(focused);
+          setFocusedSpy?.(focused);
+        }
+      },
   );
 
+  let element, input;
+
   describe('default', () => {
     let button;
 
     beforeEach(() => {
       setFocusedSpy = sinon.spy();
       element = fixtureSync(`
-        <delegate-focus-mixin-element>
+        <${tag}>
           <input slot="input" />
           <button slot="suffix"></button>
-        </delegate-focus-mixin-element>
+        </${tag}>
       `);
       input = element.querySelector('input');
       button = element.querySelector('button');
@@ -143,9 +142,9 @@ describe('delegate-focus-mixin', () => {
 
     beforeEach(() => {
       element = fixtureSync(`
-        <delegate-focus-mixin-element>
+        <${tag}>
           <input slot="input" />
-        </delegate-focus-mixin-element>
+        </${tag}>
       `);
       input = element.querySelector('input');
       spy = sinon.spy();
@@ -188,7 +187,7 @@ describe('delegate-focus-mixin', () => {
 
   describe('autofocus', () => {
     beforeEach(() => {
-      element = document.createElement('delegate-focus-mixin-element');
+      element = document.createElement(tag);
       element.autofocus = true;
 
       const input = document.createElement('input');
@@ -246,9 +245,9 @@ describe('delegate-focus-mixin', () => {
     describe('default', () => {
       beforeEach(() => {
         element = fixtureSync(`
-          <delegate-focus-mixin-element>
+          <${tag}>
             <input slot="input" />
-          </delegate-focus-mixin-element>
+          </${tag}>
         `);
         input = element.querySelector('input');
       });
@@ -285,9 +284,9 @@ describe('delegate-focus-mixin', () => {
     describe('attribute', () => {
       beforeEach(() => {
         element = fixtureSync(`
-          <delegate-focus-mixin-element tabindex="-1">
+          <${tag} tabindex="-1">
             <input slot="input" />
-          </delegate-focus-mixin-element>
+          </${tag}>
         `);
         input = element.querySelector('input');
       });
@@ -307,4 +306,12 @@ describe('delegate-focus-mixin', () => {
       });
     });
   });
+};
+
+describe('DelegateFocusMixin + Polymer', () => {
+  runTests(definePolymer, ControllerMixin);
+});
+
+describe('DelegateFocusMixin + Lit', () => {
+  runTests(defineLit, PolylitMixin);
 });

packages/a11y-base/test/tabindex-mixin.test.js

@@ -1,23 +1,21 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync } from '@vaadin/testing-helpers';
-import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
+import { defineLit, definePolymer, fixtureSync } from '@vaadin/testing-helpers';
+import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
+import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
 import { TabindexMixin } from '../src/tabindex-mixin.js';
 
-customElements.define(
-  'tabindex-mixin-element',
-  class extends TabindexMixin(PolymerElement) {
-    static get template() {
-      return html`<div></div>`;
-    }
-  },
-);
+const runTests = (defineHelper, baseMixin) => {
+  const tag = defineHelper(
+    'tabindex-mixin',
+    '<slot></slot>',
+    (Base) => class extends TabindexMixin(baseMixin(Base)) {},
+  );
 
-describe('tabindex-mixin', () => {
   let element;
 
   describe('default', () => {
     beforeEach(() => {
-      element = fixtureSync(`<tabindex-mixin-element></tabindex-mixin-element>`);
+      element = fixtureSync(`<${tag}></${tag}>`);
     });
 
     it('should not have tabindex attribute by default', () => {
@@ -84,11 +82,19 @@ describe('tabindex-mixin', () => {
 
   describe('custom', () => {
     beforeEach(() => {
-      element = fixtureSync(`<tabindex-mixin-element tabindex="1"></tabindex-mixin-element>`);
+      element = fixtureSync(`<${tag} tabindex="1"></${tag}>`);
     });
 
     it('should set tabindex property to the custom value', () => {
       expect(element.tabindex).to.equal(1);
     });
   });
+};
+
+describe('TabindexMixin + Polymer', () => {
+  runTests(definePolymer, ControllerMixin);
+});
+
+describe('TabindexMixin + Lit', () => {
+  runTests(defineLit, PolylitMixin);
 });