From 864d56757259d7af734d56630f91deef6e6956a7 Mon Sep 17 00:00:00 2001
From: Cory LaViska <cory@abeautifulsite.net>
Date: Wed, 20 Jul 2022 15:29:19 -0400
Subject: [PATCH] improve tab accessibility

---
 docs/resources/changelog.md           |  1 +
 src/components/tab-group/tab-group.ts | 11 +++--------
 src/components/tab-panel/tab-panel.ts |  9 +++++++--
 src/components/tab/tab.ts             | 21 +++++++++++++++++----
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md
index d96aabc43..c18d4a6e0 100644
--- a/docs/resources/changelog.md
+++ b/docs/resources/changelog.md
@@ -14,6 +14,7 @@ _During the beta period, these restrictions may be relaxed in the event of a mis
 - Fixed a bug in `<sl-tab-group>` where the divider was on the wrong side when using `placement="end"`
 - Fixed a bug in `<sl-tab-group>` that caused nested tab groups to scroll when using `placement="start|end"` [#815](https://github.com/shoelace-style/shoelace/issues/815)
 - Fixed a bug in `<sl-tooltip>` that caused the target to be lost after a slot change [#831](https://github.com/shoelace-style/shoelace/pull/831)
+- Improved accessibility of `<sl-tab-group>`, `<sl-tab>`, and `<sl-tab-panel>` to work better with screen readers
 - Updated Bootstrap Icons to 1.9.1
 - Updated Floating UI to 1.0.0
 
diff --git a/src/components/tab-group/tab-group.ts b/src/components/tab-group/tab-group.ts
index af0a730c4..c2a0122fc 100644
--- a/src/components/tab-group/tab-group.ts
+++ b/src/components/tab-group/tab-group.ts
@@ -122,14 +122,9 @@ export default class SlTabGroup extends LitElement {
     }
   }
 
-  getAllTabs(includeDisabled = false) {
+  getAllTabs() {
     const slot = this.shadowRoot!.querySelector<HTMLSlotElement>('slot[name="nav"]')!;
-
-    return [...(slot.assignedElements() as SlTab[])].filter(el => {
-      return includeDisabled
-        ? el.tagName.toLowerCase() === 'sl-tab'
-        : el.tagName.toLowerCase() === 'sl-tab' && !el.disabled;
-    });
+    return [...(slot.assignedElements() as SlTab[])].filter(el => el.tagName.toLowerCase() === 'sl-tab');
   }
 
   getAllPanels() {
@@ -318,7 +313,7 @@ export default class SlTabGroup extends LitElement {
 
     // We can't used offsetLeft/offsetTop here due to a shadow parent issue where neither can getBoundingClientRect
     // because it provides invalid values for animating elements: https://bugs.chromium.org/p/chromium/issues/detail?id=920069
-    const allTabs = this.getAllTabs(true);
+    const allTabs = this.getAllTabs();
     const precedingTabs = allTabs.slice(0, allTabs.indexOf(currentTab));
     const offset = precedingTabs.reduce(
       (previous, current) => ({
diff --git a/src/components/tab-panel/tab-panel.ts b/src/components/tab-panel/tab-panel.ts
index 1740a64e3..3dcbe0832 100644
--- a/src/components/tab-panel/tab-panel.ts
+++ b/src/components/tab-panel/tab-panel.ts
@@ -2,6 +2,7 @@ import { html, LitElement } from 'lit';
 import { customElement, property } from 'lit/decorators.js';
 import { classMap } from 'lit/directives/class-map.js';
 import { autoIncrement } from '../../internal/auto-increment';
+import { watch } from '../../internal/watch';
 import styles from './tab-panel.styles';
 import type { CSSResultGroup } from 'lit';
 
@@ -31,6 +32,12 @@ export default class SlTabPanel extends LitElement {
   connectedCallback() {
     super.connectedCallback();
     this.id = this.id.length > 0 ? this.id : this.componentId;
+    this.setAttribute('role', 'tabpanel');
+  }
+
+  @watch('active')
+  handleActiveChange() {
+    this.setAttribute('aria-hidden', this.active ? 'false' : 'true');
   }
 
   render() {
@@ -41,8 +48,6 @@ export default class SlTabPanel extends LitElement {
           'tab-panel': true,
           'tab-panel--active': this.active
         })}
-        role="tabpanel"
-        aria-hidden=${this.active ? 'false' : 'true'}
       >
         <slot></slot>
       </div>
diff --git a/src/components/tab/tab.ts b/src/components/tab/tab.ts
index 8fd1a352f..bca400e5f 100644
--- a/src/components/tab/tab.ts
+++ b/src/components/tab/tab.ts
@@ -4,6 +4,7 @@ import { classMap } from 'lit/directives/class-map.js';
 import '../../components/icon-button/icon-button';
 import { autoIncrement } from '../../internal/auto-increment';
 import { emit } from '../../internal/event';
+import { watch } from '../../internal/watch';
 import { LocalizeController } from '../../utilities/localize';
 import styles from './tab.styles';
 import type { CSSResultGroup } from 'lit';
@@ -47,6 +48,11 @@ export default class SlTab extends LitElement {
   /** The locale to render the component in. */
   @property() lang: string;
 
+  connectedCallback() {
+    super.connectedCallback();
+    this.setAttribute('role', 'tab');
+  }
+
   /** Sets focus to the tab. */
   focus(options?: FocusOptions) {
     this.tab.focus(options);
@@ -61,6 +67,16 @@ export default class SlTab extends LitElement {
     emit(this, 'sl-close');
   }
 
+  @watch('active')
+  handleActiveChange() {
+    this.setAttribute('aria-selected', this.active ? 'true' : 'false');
+  }
+
+  @watch('disabled')
+  handleDisabledChange() {
+    this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false');
+  }
+
   render() {
     // If the user didn't provide an ID, we'll set one so we can link tabs and tab panels with aria labels
     this.id = this.id.length > 0 ? this.id : this.componentId;
@@ -74,10 +90,7 @@ export default class SlTab extends LitElement {
           'tab--closable': this.closable,
           'tab--disabled': this.disabled
         })}
-        role="tab"
-        aria-disabled=${this.disabled ? 'true' : 'false'}
-        aria-selected=${this.active ? 'true' : 'false'}
-        tabindex=${this.disabled || !this.active ? '-1' : '0'}
+        tabindex="0"
       >
         <slot></slot>
         ${this.closable