From 9988d76c3ff664636660400586e7677a1fcc75ad Mon Sep 17 00:00:00 2001
From: Cory LaViska <cory@abeautifulsite.net>
Date: Thu, 30 Dec 2021 12:14:39 -0500
Subject: [PATCH] fix ids

---
 docs/resources/changelog.md              |  2 ++
 src/components/details/details.ts        | 16 +++-------------
 src/components/dialog/dialog.ts          |  7 ++-----
 src/components/drawer/drawer.ts          |  9 +++------
 src/components/dropdown/dropdown.ts      |  7 ++-----
 src/components/tooltip/tooltip.styles.ts |  4 ++++
 src/components/tooltip/tooltip.ts        | 21 +++++----------------
 7 files changed, 21 insertions(+), 45 deletions(-)

diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md
index f31bc22f7..e2ea2dd27 100644
--- a/docs/resources/changelog.md
+++ b/docs/resources/changelog.md
@@ -10,7 +10,9 @@ _During the beta period, these restrictions may be relaxed in the event of a mis
 
 - Added `role="status"` to `<sl-spinner>`
 - Fixed broken spinner animation in Safari [#633](https://github.com/shoelace-style/shoelace/issues/633)
+- Fixed an a11y bug in `<sl-tooltip>` where `aria-describedby` referenced an id in the shadow root
 - Improved `<sl-spinner>` track color when used on various backgrounds
+- Refactored internal id usage in `<sl-details>`, `<sl-dialog>`, `<sl-drawer>`, and `<sl-dropdow>`
 - Removed `position: relative` from the common component stylesheet
 
 ## 2.0.0-beta.63
diff --git a/src/components/details/details.ts b/src/components/details/details.ts
index 2ad02ce9d..d23790821 100644
--- a/src/components/details/details.ts
+++ b/src/components/details/details.ts
@@ -10,8 +10,6 @@ import styles from './details.styles';
 
 import '../icon/icon';
 
-let id = 0;
-
 /**
  * @since 2.0
  * @status stable
@@ -43,8 +41,6 @@ export default class SlDetails extends LitElement {
   @query('.details__header') header: HTMLElement;
   @query('.details__body') body: HTMLElement;
 
-  private componentId = `details-${++id}`;
-
   /** Indicates whether or not the details is open. You can use this in lieu of the show/hide methods. */
   @property({ type: Boolean, reflect: true }) open = false;
 
@@ -144,11 +140,11 @@ export default class SlDetails extends LitElement {
       >
         <header
           part="header"
-          id=${`${this.componentId}-header`}
+          id="header"
           class="details__header"
           role="button"
           aria-expanded=${this.open ? 'true' : 'false'}
-          aria-controls=${`${this.componentId}-content`}
+          aria-controls="content"
           aria-disabled=${this.disabled ? 'true' : 'false'}
           tabindex=${this.disabled ? '-1' : '0'}
           @click=${this.handleSummaryClick}
@@ -164,13 +160,7 @@ export default class SlDetails extends LitElement {
         </header>
 
         <div class="details__body">
-          <div
-            part="content"
-            id=${`${this.componentId}-content`}
-            class="details__content"
-            role="region"
-            aria-labelledby=${`${this.componentId}-header`}
-          >
+          <div part="content" id="content" class="details__content" role="region" aria-labelledby="header">
             <slot></slot>
           </div>
         </div>
diff --git a/src/components/dialog/dialog.ts b/src/components/dialog/dialog.ts
index 4393d62ad..506b36776 100644
--- a/src/components/dialog/dialog.ts
+++ b/src/components/dialog/dialog.ts
@@ -17,8 +17,6 @@ import '../icon-button/icon-button';
 
 const hasPreventScroll = isPreventScrollSupported();
 
-let id = 0;
-
 /**
  * @since 2.0
  * @status stable
@@ -67,7 +65,6 @@ export default class SlDialog extends LitElement {
   @query('.dialog__panel') panel: HTMLElement;
   @query('.dialog__overlay') overlay: HTMLElement;
 
-  private componentId = `dialog-${++id}`;
   private modal: Modal;
   private originalTrigger: HTMLElement | null;
 
@@ -235,13 +232,13 @@ export default class SlDialog extends LitElement {
           aria-modal="true"
           aria-hidden=${this.open ? 'false' : 'true'}
           aria-label=${ifDefined(this.noHeader ? this.label : undefined)}
-          aria-labelledby=${ifDefined(!this.noHeader ? `${this.componentId}-title` : undefined)}
+          aria-labelledby=${ifDefined(!this.noHeader ? 'title' : undefined)}
           tabindex="0"
         >
           ${!this.noHeader
             ? html`
                 <header part="header" class="dialog__header">
-                  <span part="title" class="dialog__title" id=${`${this.componentId}-title`}>
+                  <span part="title" class="dialog__title" id="title">
                     <slot name="label"> ${this.label || String.fromCharCode(65279)} </slot>
                   </span>
                   <sl-icon-button
diff --git a/src/components/drawer/drawer.ts b/src/components/drawer/drawer.ts
index 442ac8df1..3386a5fd7 100644
--- a/src/components/drawer/drawer.ts
+++ b/src/components/drawer/drawer.ts
@@ -18,8 +18,6 @@ import '../icon-button/icon-button';
 
 const hasPreventScroll = isPreventScrollSupported();
 
-let id = 0;
-
 /**
  * @since 2.0
  * @status stable
@@ -75,7 +73,6 @@ export default class SlDrawer extends LitElement {
   @query('.drawer__panel') panel: HTMLElement;
   @query('.drawer__overlay') overlay: HTMLElement;
 
-  private componentId = `drawer-${++id}`;
   private modal: Modal;
   private originalTrigger: HTMLElement | null;
 
@@ -261,14 +258,14 @@ export default class SlDrawer extends LitElement {
           aria-modal="true"
           aria-hidden=${this.open ? 'false' : 'true'}
           aria-label=${ifDefined(this.noHeader ? this.label : undefined)}
-          aria-labelledby=${ifDefined(!this.noHeader ? `${this.componentId}-title` : undefined)}
+          aria-labelledby=${ifDefined(!this.noHeader ? 'title' : undefined)}
           tabindex="0"
         >
           ${!this.noHeader
             ? html`
                 <header part="header" class="drawer__header">
-                  <span part="title" class="drawer__title" id=${`${this.componentId}-title`}>
-                    <!-- If there's no label, use an invisible character to prevent the heading from collapsing -->
+                  <span part="title" class="drawer__title" id="title">
+                    <!-- If there's no label, use an invisible character to prevent the header from collapsing -->
                     <slot name="label"> ${this.label || String.fromCharCode(65279)} </slot>
                   </span>
                   <sl-icon-button
diff --git a/src/components/dropdown/dropdown.ts b/src/components/dropdown/dropdown.ts
index 7d2c25a4e..6486be676 100644
--- a/src/components/dropdown/dropdown.ts
+++ b/src/components/dropdown/dropdown.ts
@@ -13,8 +13,6 @@ import type SlMenu from '../menu/menu';
 import type SlMenuItem from '../menu-item/menu-item';
 import styles from './dropdown.styles';
 
-let id = 0;
-
 /**
  * @since 2.0
  * @status stable
@@ -42,7 +40,6 @@ export default class SlDropdown extends LitElement {
   @query('.dropdown__panel') panel: HTMLElement;
   @query('.dropdown__positioner') positioner: HTMLElement;
 
-  private componentId = `dropdown-${++id}`;
   private popover: PopperInstance;
 
   /** Indicates whether or not the dropdown is open. You can use this in lieu of the show/hide methods. */
@@ -406,7 +403,7 @@ export default class SlDropdown extends LitElement {
     return html`
       <div
         part="base"
-        id=${this.componentId}
+        id="dropdown"
         class=${classMap({
           dropdown: true,
           'dropdown--open': this.open
@@ -429,7 +426,7 @@ export default class SlDropdown extends LitElement {
             part="panel"
             class="dropdown__panel"
             aria-hidden=${this.open ? 'false' : 'true'}
-            aria-labelledby=${this.componentId}
+            aria-labelledby="dropdown"
           >
             <slot></slot>
           </div>
diff --git a/src/components/tooltip/tooltip.styles.ts b/src/components/tooltip/tooltip.styles.ts
index c7010861b..a01d5bd7e 100644
--- a/src/components/tooltip/tooltip.styles.ts
+++ b/src/components/tooltip/tooltip.styles.ts
@@ -12,6 +12,10 @@ export default css`
     display: contents;
   }
 
+  .tooltip-content {
+    display: contents;
+  }
+
   .tooltip-positioner {
     position: absolute;
     z-index: var(--sl-z-index-tooltip);
diff --git a/src/components/tooltip/tooltip.ts b/src/components/tooltip/tooltip.ts
index 8b952d622..e964553a8 100644
--- a/src/components/tooltip/tooltip.ts
+++ b/src/components/tooltip/tooltip.ts
@@ -38,7 +38,6 @@ export default class SlTooltip extends LitElement {
   @query('.tooltip-positioner') positioner: HTMLElement;
   @query('.tooltip') tooltip: HTMLElement;
 
-  private componentId = `tooltip-${++id}`;
   private target: HTMLElement;
   private popover: PopperInstance;
   private hoverTimeout: any;
@@ -283,18 +282,6 @@ export default class SlTooltip extends LitElement {
     }
   }
 
-  handleSlotChange() {
-    const oldTarget = this.target;
-    const newTarget = this.getTarget();
-
-    if (newTarget !== oldTarget) {
-      if (oldTarget) {
-        oldTarget.removeAttribute('aria-describedby');
-      }
-      newTarget.setAttribute('aria-describedby', this.componentId);
-    }
-  }
-
   hasTrigger(triggerType: string) {
     const triggers = this.trigger.split(' ');
     return triggers.includes(triggerType);
@@ -325,12 +312,14 @@ export default class SlTooltip extends LitElement {
 
   render() {
     return html`
-      <slot @slotchange=${this.handleSlotChange}></slot>
+      <div class="tooltip-content" aria-described-by="tooltip">
+        <slot></slot>
+      </div>
 
       <div class="tooltip-positioner">
         <div
           part="base"
-          id=${this.componentId}
+          id="tooltip"
           class=${classMap({
             tooltip: true,
             'tooltip--open': this.open
@@ -338,7 +327,7 @@ export default class SlTooltip extends LitElement {
           role="tooltip"
           aria-hidden=${this.open ? 'false' : 'true'}
         >
-          <slot name="content" @slotchange=${this.handleContentChange}> ${this.content} </slot>
+          <slot name="content"> ${this.content} </slot>
         </div>
       </div>
     `;