Implement roving tabindex for tabs (#2041)

* implement roving tabindex tabs

* implement roving tabindex tabs

* prettier

* implement roving tabindex tabs

* prettier

* remove test.only

* remove unncessary syncing

* Fix manual tab activations not working with roving tabindex

* prettier

* remove unnecessary extra code

* update changelog

* update changelog

* prettier

docs/pages/components/tab.md

@@ -5,26 +5,6 @@ meta:
 layout: component
 ---
 
-```html:preview
-<sl-tab>Tab</sl-tab>
-<sl-tab active>Active</sl-tab>
-<sl-tab closable>Closable</sl-tab>
-<sl-tab disabled>Disabled</sl-tab>
-```
-
-```jsx:react
-import SlTab from '@shoelace-style/shoelace/dist/react/tab';
-
-const App = () => (
-  <>
-    <SlTab>Tab</SlTab>
-    <SlTab active>Active</SlTab>
-    <SlTab closable>Closable</SlTab>
-    <SlTab disabled>Disabled</SlTab>
-  </>
-);
-```
-
 :::tip
 Additional demonstrations can be found in the [tab group examples](/components/tab-group).
 :::

docs/pages/resources/changelog.md

@@ -14,6 +14,8 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 
 ## Next
 
+- `<sl-tab>` `closable` property now reflects. [#2041]
+- `<sl-tab-group>` now implements a proper "roving tabindex" and `<sl-tab>` is no longer tabbable by default. This aligns closer to the APG pattern for tabs. [#2041]
 - Fixed a bug in the submenu controller that prevented submenus from rendering in RTL without explicitly setting `dir` on the parent menu item [#1992]
 
 ## 2.15.1

src/components/tab-group/tab-group.component.ts

@@ -206,10 +206,16 @@ export default class SlTabGroup extends ShoelaceElement {
           index = 0;
         }
 
-        this.tabs[index].focus({ preventScroll: true });
+        const currentTab = this.tabs[index];
+        currentTab.tabIndex = 0;
+        currentTab.focus({ preventScroll: true });
 
         if (this.activation === 'auto') {
-          this.setActiveTab(this.tabs[index], { scrollBehavior: 'smooth' });
+          this.setActiveTab(currentTab, { scrollBehavior: 'smooth' });
+        } else {
+          this.tabs.forEach(tabEl => {
+            tabEl.tabIndex = tabEl === currentTab ? 0 : -1;
+          });
         }
 
         if (['top', 'bottom'].includes(this.placement)) {
@@ -253,7 +259,10 @@ export default class SlTabGroup extends ShoelaceElement {
       this.activeTab = tab;
 
       // Sync active tab and panel
-      this.tabs.forEach(el => (el.active = el === this.activeTab));
+      this.tabs.forEach(el => {
+        el.active = el === this.activeTab;
+        el.tabIndex = el === this.activeTab ? 0 : -1;
+      });
       this.panels.forEach(el => (el.active = el.name === this.activeTab?.panel));
       this.syncIndicator();
 
@@ -326,6 +335,7 @@ export default class SlTabGroup extends ShoelaceElement {
   // This stores tabs and panels so we can refer to a cache instead of calling querySelectorAll() multiple times.
   private syncTabsAndPanels() {
     this.tabs = this.getAllTabs({ includeDisabled: false });
+
     this.panels = this.getAllPanels();
     this.syncIndicator();
 

src/components/tab/tab.component.ts

@@ -45,11 +45,13 @@ export default class SlTab extends ShoelaceElement {
   @property({ type: Boolean, reflect: true }) active = false;
 
   /** Makes the tab closable and shows a close button. */
-  @property({ type: Boolean }) closable = false;
+  @property({ type: Boolean, reflect: true }) closable = false;
 
   /** Disables the tab and prevents selection. */
   @property({ type: Boolean, reflect: true }) disabled = false;
 
+  tabIndex = -1;
+
   connectedCallback() {
     super.connectedCallback();
     this.setAttribute('role', 'tab');
@@ -68,16 +70,7 @@ export default class SlTab extends ShoelaceElement {
   @watch('disabled')
   handleDisabledChange() {
     this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false');
-  }
-
-  /** Sets focus to the tab. */
-  focus(options?: FocusOptions) {
-    this.tab.focus(options);
-  }
-
-  /** Removes focus from the tab. */
-  blur() {
-    this.tab.blur();
+    this.tabIndex = -1;
   }
 
   render() {
@@ -93,7 +86,6 @@ export default class SlTab extends ShoelaceElement {
           'tab--closable': this.closable,
           'tab--disabled': this.disabled
         })}
-        tabindex=${this.disabled ? '-1' : '0'}
       >
         <slot></slot>
         ${this.closable

src/components/tab/tab.styles.ts

@@ -27,15 +27,15 @@ export default css`
     color: var(--sl-color-primary-600);
   }
 
-  .tab:focus {
-    outline: none;
+  :host(:focus) {
+    outline: transparent;
   }
 
-  .tab:focus-visible:not(.tab--disabled) {
+  :host(:focus-visible):not([disabled]) {
     color: var(--sl-color-primary-600);
   }
 
-  .tab:focus-visible {
+  :host(:focus-visible) {
     outline: var(--sl-focus-ring);
     outline-offset: calc(-1 * var(--sl-focus-ring-width) - var(--sl-focus-ring-offset));
   }

src/components/tab/tab.test.ts

@@ -12,6 +12,7 @@ describe('<sl-tab>', () => {
         <sl-tab slot="nav">Test</sl-tab>
       </sl-tab-group>
     `);
+
     await expect(el).to.be.accessible();
   });
 
@@ -23,7 +24,7 @@ describe('<sl-tab>', () => {
     expect(el.getAttribute('role')).to.equal('tab');
     expect(el.getAttribute('aria-disabled')).to.equal('false');
     expect(el.getAttribute('aria-selected')).to.equal('false');
-    expect(base.getAttribute('tabindex')).to.equal('0');
+    expect(el.getAttribute('tabindex')).to.equal('-1');
     expect(base.getAttribute('class')).to.equal(' tab ');
     expect(el.active).to.equal(false);
     expect(el.closable).to.equal(false);
@@ -38,7 +39,7 @@ describe('<sl-tab>', () => {
     expect(el.disabled).to.equal(true);
     expect(el.getAttribute('aria-disabled')).to.equal('true');
     expect(base.getAttribute('class')).to.equal(' tab tab--disabled ');
-    expect(base.getAttribute('tabindex')).to.equal('-1');
+    expect(el.getAttribute('tabindex')).to.equal('-1');
   });
 
   it('should set active tab by attribute', async () => {
@@ -49,7 +50,7 @@ describe('<sl-tab>', () => {
     expect(el.active).to.equal(true);
     expect(el.getAttribute('aria-selected')).to.equal('true');
     expect(base.getAttribute('class')).to.equal(' tab tab--active ');
-    expect(base.getAttribute('tabindex')).to.equal('0');
+    expect(el.getAttribute('tabindex')).to.equal('-1');
   });
 
   it('should set closable by attribute', async () => {
@@ -59,34 +60,34 @@ describe('<sl-tab>', () => {
     const closeButton = el.shadowRoot!.querySelector('[part~="close-button"]');
 
     expect(el.closable).to.equal(true);
-    expect(base.getAttribute('class')).to.equal(' tab tab--closable ');
+    expect(base.getAttribute('class')).to.match(/tab tab--closable/);
     expect(closeButton).not.to.be.null;
   });
 
   describe('focus', () => {
-    it('should focus inner div', async () => {
+    it('should focus itself', async () => {
       const el = await fixture<SlTab>(html` <sl-tab>Test</sl-tab> `);
 
-      const base = el.shadowRoot!.querySelector<HTMLElement>('[part~="base"]')!;
-
       el.focus();
       await el.updateComplete;
 
-      expect(el.shadowRoot!.activeElement).to.equal(base);
+      expect(document.activeElement).to.equal(el);
     });
   });
 
   describe('blur', () => {
-    it('should blur inner div', async () => {
+    it('should blur itself', async () => {
       const el = await fixture<SlTab>(html` <sl-tab>Test</sl-tab> `);
 
       el.focus();
       await el.updateComplete;
 
+      expect(document.activeElement).to.equal(el);
+
       el.blur();
       await el.updateComplete;
 
-      expect(el.shadowRoot!.activeElement).to.equal(null);
+      expect(document.activeElement).to.not.equal(el);
     });
   });