fix(tabs): do not focus tabs when selectedIndex is set (#729)

packages/tabs/src/LionTabs.js

@@ -34,9 +34,8 @@ const cleanButton = ({ element, clickHandler, keydownHandler, keyupHandler }) =>
   element.removeEventListener('keydown', keydownHandler);
 };
 
-const selectButton = (element, firstUpdate = false) => {
+const selectButton = (element, withFocus = false) => {
-  // Don't focus on first update, as the component might be lower on the page
+  if (withFocus) {
-  if (!firstUpdate) {
     element.focus();
   }
 
@@ -123,7 +122,6 @@ export class LionTabs extends LitElement {
 
   firstUpdated() {
     super.firstUpdated();
-    this.__firstUpdate = true;
     this.__setupSlots();
   }
 
@@ -132,7 +130,7 @@ export class LionTabs extends LitElement {
     const handleSlotChange = () => {
       this.__cleanStore();
       this.__setupStore();
-      this.__updateSelected();
+      this.__updateSelected(false);
     };
     tabSlot.addEventListener('slotchange', handleSlotChange);
   }
@@ -178,7 +176,7 @@ export class LionTabs extends LitElement {
 
   __createButtonClickHandler(index) {
     return () => {
-      this.selectedIndex = index;
+      this._setSelectedIndexWithFocus(index);
     };
   }
 
@@ -187,34 +185,41 @@ export class LionTabs extends LitElement {
       case 'ArrowDown':
       case 'ArrowRight':
         if (this.selectedIndex + 1 >= this._pairCount) {
-          this.selectedIndex = 0;
+          this._setSelectedIndexWithFocus(0);
         } else {
-          this.selectedIndex += 1;
+          this._setSelectedIndexWithFocus(this.selectedIndex + 1);
         }
         break;
       case 'ArrowUp':
       case 'ArrowLeft':
         if (this.selectedIndex <= 0) {
-          this.selectedIndex = this._pairCount - 1;
+          this._setSelectedIndexWithFocus(this._pairCount - 1);
         } else {
-          this.selectedIndex -= 1;
+          this._setSelectedIndexWithFocus(this.selectedIndex - 1);
         }
         break;
       case 'Home':
-        this.selectedIndex = 0;
+        this._setSelectedIndexWithFocus(0);
         break;
       case 'End':
-        this.selectedIndex = this._pairCount - 1;
+        this._setSelectedIndexWithFocus(this._pairCount - 1);
         break;
       /* no default */
     }
   }
 
   set selectedIndex(value) {
-    this.__firstUpdate = false;
     const stale = this.__selectedIndex;
     this.__selectedIndex = value;
-    this.__updateSelected();
+    this.__updateSelected(false);
+    this.dispatchEvent(new Event('selected-changed'));
+    this.requestUpdate('selectedIndex', stale);
+  }
+
+  _setSelectedIndexWithFocus(value) {
+    const stale = this.__selectedIndex;
+    this.__selectedIndex = value;
+    this.__updateSelected(true);
     this.dispatchEvent(new Event('selected-changed'));
     this.requestUpdate('selectedIndex', stale);
   }
@@ -227,7 +232,7 @@ export class LionTabs extends LitElement {
     return this.__store.length;
   }
 
-  __updateSelected() {
+  __updateSelected(withFocus = false) {
     if (!(this.__store && this.__store[this.selectedIndex])) {
       return;
     }
@@ -245,7 +250,7 @@ export class LionTabs extends LitElement {
     }
     const { button: currentButton, panel: currentPanel } = this.__store[this.selectedIndex];
     if (currentButton) {
-      selectButton(currentButton, this.__firstUpdate);
+      selectButton(currentButton, withFocus);
     }
     if (currentPanel) {
       selectPanel(currentPanel);

packages/tabs/test/lion-tabs.test.js

@@ -244,7 +244,7 @@ describe('<lion-tabs>', () => {
   });
 
   describe('Initializing without Focus', () => {
-    it('keeps track of when the component is updated', async () => {
+    it('does not focus a tab when setting selectedIndex property', async () => {
       const el = await fixture(html`
         <lion-tabs>
           <button slot="tab">tab 1</button>
@@ -254,9 +254,9 @@ describe('<lion-tabs>', () => {
         </lion-tabs>
       `);
 
-      expect(el.__firstUpdate).to.be.true;
       el.selectedIndex = 1;
-      expect(el.__firstUpdate).to.be.false;
+      expect(el.querySelector('[slot="tab"]:nth-of-type(2)') === document.activeElement).to.be
+        .false;
     });
 
     it('does not focus a tab on firstUpdate', async () => {
@@ -272,7 +272,7 @@ describe('<lion-tabs>', () => {
       expect(tabs.some(tab => tab === document.activeElement)).to.be.false;
     });
 
-    it('focuses on a tab when switching the selectedIndex', async () => {
+    it('focuses on a tab when setting with _setSelectedIndexWithFocus method', async () => {
       const el = await fixture(html`
         <lion-tabs>
           <button slot="tab">tab 1</button>
@@ -281,12 +281,25 @@ describe('<lion-tabs>', () => {
           <div slot="panel">panel 2</div>
         </lion-tabs>
       `);
-      el.selectedIndex = 1;
+      el._setSelectedIndexWithFocus(1);
-      const tab = Array.from(el.children).filter(child => child.slot === 'tab')[1];
+      expect(el.querySelector('[slot="tab"]:nth-of-type(2)') === document.activeElement).to.be.true;
-      expect(tab).to.equal(document.activeElement);
     });
   });
 
+  it('focuses on a tab when the selected tab is changed by user interaction', async () => {
+    const el = await fixture(html`
+      <lion-tabs>
+        <button slot="tab">tab 1</button>
+        <div slot="panel">panel 1</div>
+        <button slot="tab">tab 2</button>
+        <div slot="panel">panel 2</div>
+      </lion-tabs>
+    `);
+    const secondTab = el.querySelector('[slot="tab"]:nth-of-type(2)');
+    secondTab.dispatchEvent(new MouseEvent('click'));
+    expect(secondTab === document.activeElement).to.be.true;
+  });
+
   describe('Accessibility', () => {
     it('does not make panels focusable', async () => {
       const el = await fixture(html`