update PR link

.eslintrc.cjs

@@ -92,8 +92,7 @@ module.exports = {
         '@typescript-eslint/member-delimiter-style': 'warn',
         '@typescript-eslint/method-signature-style': 'warn',
         '@typescript-eslint/no-extraneous-class': 'error',
-        '@typescript-eslint/no-redundant-type-constituents': 'off',
-        '@typescript-eslint/parameter-properties': 'error',
+        '@typescript-eslint/no-parameter-properties': 'error',
         '@typescript-eslint/strict-boolean-expressions': 'off'
       }
     },

docs/pages/resources/changelog.md

@@ -15,19 +15,15 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 ## Next
 
 - Added tests for `<sl-qr-code>` [#1416]
-- Added support for pressing [[Space]] to select/toggle selected `<sl-menu-item>` elements [#1429]
-- Fixed a bug in focus trapping of modal elements like `<sl-dialog>`. We now manually handle focus ordering as well as added `offsetParent()` check for tabbable boundaries in Safari. Test cases added for `<sl-dialog>` inside a shadowRoot [#1403]
-- Fixed a bug in `valueAsDate` on `<sl-input>` where it would always set `type="date"` for the underlying `<input>` element. It now falls back to the native browser implementation for the in-memory input. This may cause unexpected behavior if you're using `valueAsDate` on any input elements that aren't `type="date"`. [#1399]
 - Fixed a bug in `<sl-qr-code>` where the `background` attribute was never passed to the QR code [#1416]
 - Fixed a bug in `<sl-dropdown>` where aria attributes were incorrectly applied to the default `<slot>` causing Lighthouse errors [#1417]
 - Fixed a bug in `<sl-carousel>` that caused navigation to work incorrectly in some case [#1420]
-- Fixed a number of slots that incorrectly had aria- and/or role attributes directly on them [#1422]
 - Fixed a bug in `<sl-tree>` that caused focus to be stolen when removing focused tree items [#1430]
-- Updated ESLint and related plugins to the latest versions
 
 ## 2.5.2
 
 - Fixed broken source buttons in the docs [#1401]
+- Fixed broken links in the docs [#1407]
 
 ## 2.5.1
 

package.json

@@ -79,8 +79,8 @@
     "@open-wc/testing": "^3.1.7",
     "@types/mocha": "^10.0.1",
     "@types/react": "^18.0.26",
-    "@typescript-eslint/eslint-plugin": "^6.0.0",
-    "@typescript-eslint/parser": "^6.0.0",
+    "@typescript-eslint/eslint-plugin": "^5.48.1",
+    "@typescript-eslint/parser": "^5.48.1",
     "@web/dev-server-esbuild": "^0.3.3",
     "@web/test-runner": "^0.15.0",
     "@web/test-runner-commands": "^0.6.5",
@@ -96,15 +96,15 @@
     "del": "^7.0.0",
     "download": "^8.0.0",
     "esbuild": "^0.18.2",
-    "eslint": "^8.44.0",
+    "eslint": "^8.31.0",
     "eslint-plugin-chai-expect": "^3.0.0",
     "eslint-plugin-chai-friendly": "^0.7.2",
-    "eslint-plugin-import": "^2.27.5",
-    "eslint-plugin-lit": "^1.8.3",
-    "eslint-plugin-lit-a11y": "^4.1.0",
+    "eslint-plugin-import": "^2.27.4",
+    "eslint-plugin-lit": "^1.8.2",
+    "eslint-plugin-lit-a11y": "^2.3.0",
     "eslint-plugin-markdown": "^3.0.0",
     "eslint-plugin-sort-imports-es6-autofix": "^0.6.0",
-    "eslint-plugin-wc": "^1.5.0",
+    "eslint-plugin-wc": "^1.4.0",
     "front-matter": "^4.0.2",
     "get-port": "^7.0.0",
     "globby": "^13.1.3",

src/components/alert/alert.ts

@@ -199,13 +199,9 @@ export default class SlAlert extends ShoelaceElement {
         aria-hidden=${this.open ? 'false' : 'true'}
         @mousemove=${this.handleMouseMove}
       >
-        <div part="icon" class="alert__icon">
-          <slot name="icon"></slot>
-        </div>
+        <slot name="icon" part="icon" class="alert__icon"></slot>
 
-        <div part="message" class="alert__message" aria-live="polite">
-          <slot></slot>
-        </div>
+        <slot part="message" class="alert__message" aria-live="polite"></slot>
 
         ${this.closable
           ? html`

src/components/avatar/avatar.ts

@@ -69,11 +69,9 @@ export default class SlAvatar extends ShoelaceElement {
       avatarWithoutImage = html`<div part="initials" class="avatar__initials">${this.initials}</div>`;
     } else {
       avatarWithoutImage = html`
-        <div part="icon" class="avatar__icon" aria-hidden="true">
-          <slot name="icon">
-            <sl-icon name="person-fill" library="system"></sl-icon>
-          </slot>
-        </div>
+        <slot name="icon" part="icon" class="avatar__icon" aria-hidden="true">
+          <sl-icon name="person-fill" library="system"></sl-icon>
+        </slot>
       `;
     }
 

src/components/badge/badge.test.ts

@@ -2,10 +2,6 @@ import '../../../dist/shoelace.js';
 import { expect, fixture, html } from '@open-wc/testing';
 import type SlBadge from './badge.js';
 
-// The default badge background just misses AA contrast, but the next step up is way too dark. We're going to relax this
-// rule for now.
-const ignoredRules = ['color-contrast'];
-
 describe('<sl-badge>', () => {
   let el: SlBadge;
 
@@ -15,7 +11,7 @@ describe('<sl-badge>', () => {
     });
 
     it('should pass accessibility tests with a role of status on the base part.', async () => {
-      await expect(el).to.be.accessible({ ignoredRules });
+      await expect(el).to.be.accessible();
 
       const part = el.shadowRoot!.querySelector('[part~="base"]')!;
       expect(part.getAttribute('role')).to.eq('status');
@@ -37,7 +33,7 @@ describe('<sl-badge>', () => {
     });
 
     it('should pass accessibility tests', async () => {
-      await expect(el).to.be.accessible({ ignoredRules });
+      await expect(el).to.be.accessible();
     });
 
     it('should append the pill class to the classlist to render a pill', () => {
@@ -52,7 +48,7 @@ describe('<sl-badge>', () => {
     });
 
     it('should pass accessibility tests', async () => {
-      await expect(el).to.be.accessible({ ignoredRules });
+      await expect(el).to.be.accessible();
     });
 
     it('should append the pulse class to the classlist to render a pulse', () => {
@@ -68,7 +64,7 @@ describe('<sl-badge>', () => {
       });
 
       it('should pass accessibility tests', async () => {
-        await expect(el).to.be.accessible({ ignoredRules });
+        await expect(el).to.be.accessible();
       });
 
       it('should default to square styling, with the primary color', () => {

src/components/badge/badge.ts

@@ -30,7 +30,7 @@ export default class SlBadge extends ShoelaceElement {
 
   render() {
     return html`
-      <span
+      <slot
         part="base"
         class=${classMap({
           badge: true,
@@ -43,9 +43,7 @@ export default class SlBadge extends ShoelaceElement {
           'badge--pulse': this.pulse
         })}
         role="status"
-      >
-        <slot></slot>
-      </span>
+      ></slot>
     `;
   }
 }

src/components/breadcrumb-item/breadcrumb-item.ts

@@ -55,9 +55,7 @@ export default class SlBreadcrumbItem extends ShoelaceElement {
           'breadcrumb-item--has-suffix': this.hasSlotController.test('suffix')
         })}
       >
-        <span part="prefix" class="breadcrumb-item__prefix">
-          <slot name="prefix"></slot>
-        </span>
+        <slot name="prefix" part="prefix" class="breadcrumb-item__prefix"></slot>
 
         ${isLink
           ? html`
@@ -77,13 +75,9 @@ export default class SlBreadcrumbItem extends ShoelaceElement {
               </button>
             `}
 
-        <span part="suffix" class="breadcrumb-item__suffix">
-          <slot name="suffix"></slot>
-        </span>
+        <slot name="suffix" part="suffix" class="breadcrumb-item__suffix"></slot>
 
-        <span part="separator" class="breadcrumb-item__separator" aria-hidden="true">
-          <slot name="separator"></slot>
-        </span>
+        <slot name="separator" part="separator" class="breadcrumb-item__separator" aria-hidden="true"></slot>
       </div>
     `;
   }

src/components/breadcrumb/breadcrumb.ts

@@ -90,11 +90,9 @@ export default class SlBreadcrumb extends ShoelaceElement {
         <slot @slotchange=${this.handleSlotChange}></slot>
       </nav>
 
-      <span hidden aria-hidden="true">
-        <slot name="separator">
-          <sl-icon name=${this.localize.dir() === 'rtl' ? 'chevron-left' : 'chevron-right'} library="system"></sl-icon>
-        </slot>
-      </span>
+      <slot name="separator" hidden aria-hidden="true">
+        <sl-icon name=${this.localize.dir() === 'rtl' ? 'chevron-left' : 'chevron-right'} library="system"></sl-icon>
+      </slot>
     `;
   }
 }

src/components/button-group/button-group.ts

@@ -68,7 +68,7 @@ export default class SlButtonGroup extends ShoelaceElement {
   render() {
     // eslint-disable-next-line lit-a11y/mouse-events-have-key-events
     return html`
-      <div
+      <slot
         part="base"
         class="button-group"
         role="${this.disableRole ? 'presentation' : 'group'}"
@@ -77,9 +77,8 @@ export default class SlButtonGroup extends ShoelaceElement {
         @focusin=${this.handleFocus}
         @mouseover=${this.handleMouseOver}
         @mouseout=${this.handleMouseOut}
-      >
-        <slot @slotchange=${this.handleSlotChange}></slot>
-      </div>
+        @slotchange=${this.handleSlotChange}
+      ></slot>
     `;
   }
 }

src/components/dialog/dialog.test.ts

@@ -1,7 +1,6 @@
 import '../../../dist/shoelace.js';
 // cspell:dictionaries lorem-ipsum
-import { aTimeout, elementUpdated, expect, fixture, html, waitUntil } from '@open-wc/testing';
-import { LitElement } from 'lit';
+import { expect, fixture, html, waitUntil } from '@open-wc/testing';
 import { sendKeys } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import type SlDialog from './dialog';
@@ -147,124 +146,4 @@ describe('<sl-dialog>', () => {
 
     expect(el.open).to.be.false;
   });
-
-  // https://github.com/shoelace-style/shoelace/issues/1382
-  it('should properly cycle through tabbable elements when sl-dialog is used in a shadowRoot', async () => {
-    class AContainer extends LitElement {
-      get dialog() {
-        return this.shadowRoot?.querySelector('sl-dialog');
-      }
-
-      openDialog() {
-        this.dialog?.show();
-      }
-
-      render() {
-        return html`
-          <h1>Dialog Example</h1>
-          <sl-dialog label="Dialog" class="dialog-overview">
-            Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-            <br />
-            <label><input type="checkbox" />A</label>
-            <label><input type="checkbox" />B</label>
-            <button>Button</button>
-          </sl-dialog>
-
-          <sl-button @click=${this.openDialog}>Open Dialog</sl-button>
-        `;
-      }
-    }
-
-    if (!window.customElements.get('a-container')) {
-      window.customElements.define('a-container', AContainer);
-    }
-
-    const testCase = await fixture(html`
-      <div>
-        <a-container></a-container>
-
-        <p>
-          Open the dialog, then use <kbd>Tab</kbd> to cycle through the inputs. Focus should be trapped, but it reaches
-          things outside the dialog.
-        </p>
-      </div>
-    `);
-
-    const container = testCase.querySelector('a-container');
-
-    if (!container) {
-      throw Error('Could not find <a-container> element.');
-    }
-
-    await elementUpdated(container);
-    const dialog = container.shadowRoot?.querySelector('sl-dialog');
-
-    if (!dialog) {
-      throw Error('Could not find <sl-dialog> element.');
-    }
-
-    const closeButton = dialog.shadowRoot?.querySelector('sl-icon-button');
-    const checkbox1 = dialog.querySelector("input[type='checkbox']");
-    const checkbox2 = dialog.querySelectorAll("input[type='checkbox']")[1];
-    const button = dialog.querySelector('button');
-
-    // Opens modal.
-    const openModalButton = container.shadowRoot?.querySelector('sl-button');
-
-    if (openModalButton) openModalButton.click();
-
-    // Test tab cycling
-    await pressTab();
-
-    expect(container.shadowRoot?.activeElement).to.equal(dialog);
-    expect(dialog.shadowRoot?.activeElement).to.equal(closeButton);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(checkbox1);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(checkbox2);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(button);
-
-    await pressTab();
-    expect(dialog.shadowRoot?.activeElement).to.equal(closeButton);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(checkbox1);
-
-    // Test Shift+Tab cycling
-
-    // I found these timeouts were needed for WebKit locally.
-    await aTimeout(10);
-    await sendKeys({ down: 'Shift' });
-    await aTimeout(10);
-
-    await pressTab();
-    expect(dialog.shadowRoot?.activeElement).to.equal(closeButton);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(button);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(checkbox2);
-
-    await pressTab();
-    expect(container.shadowRoot?.activeElement).to.equal(checkbox1);
-
-    await pressTab();
-    expect(dialog.shadowRoot?.activeElement).to.equal(closeButton);
-
-    // End shift+tab cycling
-    await sendKeys({ up: 'Shift' });
-  });
 });
-
-// We wait 50ms just to give the browser some time to figure out the current focus.
-// 50 was the magic number I found locally :shrug:
-async function pressTab() {
-  await aTimeout(50);
-  await sendKeys({ press: 'Tab' });
-  await aTimeout(50);
-}

src/components/dialog/dialog.ts

@@ -104,7 +104,6 @@ export default class SlDialog extends ShoelaceElement {
 
   disconnectedCallback() {
     super.disconnectedCallback();
-    this.modal.deactivate();
     unlockBodyScrolling(this);
   }
 
@@ -270,7 +269,7 @@ export default class SlDialog extends ShoelaceElement {
           aria-hidden=${this.open ? 'false' : 'true'}
           aria-label=${ifDefined(this.noHeader ? this.label : undefined)}
           aria-labelledby=${ifDefined(!this.noHeader ? 'title' : undefined)}
-          tabindex="-1"
+          tabindex="0"
         >
           ${!this.noHeader
             ? html`
@@ -293,10 +292,8 @@ export default class SlDialog extends ShoelaceElement {
                 </header>
               `
             : ''}
-          ${
-            '' /* The tabindex="-1" is here because the body is technically scrollable if overflowing. However, if there's no focusable elements inside, you won't actually be able to scroll it via keyboard. */
-          }
-          <slot part="body" class="dialog__body" tabindex="-1"></slot>
+
+          <slot part="body" class="dialog__body"></slot>
 
           <footer part="footer" class="dialog__footer">
             <slot name="footer"></slot>

src/components/image-comparer/image-comparer.ts

@@ -108,19 +108,16 @@ export default class SlImageComparer extends ShoelaceElement {
         @keydown=${this.handleKeyDown}
       >
         <div class="image-comparer__image">
-          <div part="before" class="image-comparer__before">
-            <slot name="before"></slot>
-          </div>
+          <slot name="before" part="before" class="image-comparer__before"></slot>
 
-          <div
+          <slot
+            name="after"
             part="after"
             class="image-comparer__after"
             style=${styleMap({
               clipPath: isRtl ? `inset(0 0 0 ${100 - this.position}%)` : `inset(0 ${100 - this.position}% 0 0)`
             })}
-          >
-            <slot name="after"></slot>
-          </div>
+          ></slot>
         </div>
 
         <div
@@ -132,7 +129,8 @@ export default class SlImageComparer extends ShoelaceElement {
           @mousedown=${this.handleDrag}
           @touchstart=${this.handleDrag}
         >
-          <div
+          <slot
+            name="handle"
             part="handle"
             class="image-comparer__handle"
             role="scrollbar"
@@ -142,10 +140,8 @@ export default class SlImageComparer extends ShoelaceElement {
             aria-controls="image-comparer"
             tabindex="0"
           >
-            <slot name="handle">
-              <sl-icon library="system" name="grip-vertical"></sl-icon>
-            </slot>
-          </div>
+            <sl-icon library="system" name="grip-vertical"></sl-icon>
+          </slot>
         </div>
       </div>
     `;

src/components/input/input.styles.ts

@@ -147,8 +147,8 @@ export default css`
     cursor: default;
   }
 
-  .input__prefix ::slotted(sl-icon),
-  .input__suffix ::slotted(sl-icon) {
+  .input__prefix::slotted(sl-icon),
+  .input__suffix::slotted(sl-icon) {
     color: var(--sl-input-icon-color);
   }
 
@@ -172,11 +172,11 @@ export default css`
     width: calc(1em + var(--sl-input-spacing-small) * 2);
   }
 
-  .input--small .input__prefix ::slotted(*) {
+  .input--small .input__prefix::slotted(*) {
     margin-inline-start: var(--sl-input-spacing-small);
   }
 
-  .input--small .input__suffix ::slotted(*) {
+  .input--small .input__suffix::slotted(*) {
     margin-inline-end: var(--sl-input-spacing-small);
   }
 
@@ -196,11 +196,11 @@ export default css`
     width: calc(1em + var(--sl-input-spacing-medium) * 2);
   }
 
-  .input--medium .input__prefix ::slotted(*) {
+  .input--medium .input__prefix::slotted(*) {
     margin-inline-start: var(--sl-input-spacing-medium);
   }
 
-  .input--medium .input__suffix ::slotted(*) {
+  .input--medium .input__suffix::slotted(*) {
     margin-inline-end: var(--sl-input-spacing-medium);
   }
 
@@ -220,11 +220,11 @@ export default css`
     width: calc(1em + var(--sl-input-spacing-large) * 2);
   }
 
-  .input--large .input__prefix ::slotted(*) {
+  .input--large .input__prefix::slotted(*) {
     margin-inline-start: var(--sl-input-spacing-large);
   }
 
-  .input--large .input__suffix ::slotted(*) {
+  .input--large .input__suffix::slotted(*) {
     margin-inline-end: var(--sl-input-spacing-large);
   }
 

src/components/input/input.ts

@@ -198,17 +198,13 @@ export default class SlInput extends ShoelaceElement implements ShoelaceFormCont
   // can be set before the component is rendered.
   //
 
-  /**
-   * Gets or sets the current value as a `Date` object. Returns `null` if the value can't be converted. This will use the native `<input type="{{type}}">` implementation and may result in an error.
-   */
+  /** Gets or sets the current value as a `Date` object. Returns `null` if the value can't be converted. */
   get valueAsDate() {
-    this.__dateInput.type = this.type;
     this.__dateInput.value = this.value;
     return this.input?.valueAsDate || this.__dateInput.valueAsDate;
   }
 
   set valueAsDate(newValue: Date | null) {
-    this.__dateInput.type = this.type;
     this.__dateInput.valueAsDate = newValue;
     this.value = this.__dateInput.value;
   }
@@ -451,10 +447,7 @@ export default class SlInput extends ShoelaceElement implements ShoelaceFormCont
               'input--no-spin-buttons': this.noSpinButtons
             })}
           >
-            <span part="prefix" class="input__prefix">
-              <slot name="prefix"></slot>
-            </span>
-
+            <slot name="prefix" part="prefix" class="input__prefix"></slot>
             <input
               part="input"
               id="input"
@@ -489,60 +482,64 @@ export default class SlInput extends ShoelaceElement implements ShoelaceFormCont
               @blur=${this.handleBlur}
             />
 
-            ${hasClearIcon
-              ? html`
-                  <button
-                    part="clear-button"
-                    class="input__clear"
-                    type="button"
-                    aria-label=${this.localize.term('clearEntry')}
-                    @click=${this.handleClearClick}
-                    tabindex="-1"
-                  >
-                    <slot name="clear-icon">
-                      <sl-icon name="x-circle-fill" library="system"></sl-icon>
-                    </slot>
-                  </button>
-                `
-              : ''}
-            ${this.passwordToggle && !this.disabled
-              ? html`
-                  <button
-                    part="password-toggle-button"
-                    class="input__password-toggle"
-                    type="button"
-                    aria-label=${this.localize.term(this.passwordVisible ? 'hidePassword' : 'showPassword')}
-                    @click=${this.handlePasswordToggle}
-                    tabindex="-1"
-                  >
-                    ${this.passwordVisible
-                      ? html`
-                          <slot name="show-password-icon">
-                            <sl-icon name="eye-slash" library="system"></sl-icon>
-                          </slot>
-                        `
-                      : html`
-                          <slot name="hide-password-icon">
-                            <sl-icon name="eye" library="system"></sl-icon>
-                          </slot>
-                        `}
-                  </button>
-                `
-              : ''}
+            ${
+              hasClearIcon
+                ? html`
+                    <button
+                      part="clear-button"
+                      class="input__clear"
+                      type="button"
+                      aria-label=${this.localize.term('clearEntry')}
+                      @click=${this.handleClearClick}
+                      tabindex="-1"
+                    >
+                      <slot name="clear-icon">
+                        <sl-icon name="x-circle-fill" library="system"></sl-icon>
+                      </slot>
+                    </button>
+                  `
+                : ''
+            }
+            ${
+              this.passwordToggle && !this.disabled
+                ? html`
+                    <button
+                      part="password-toggle-button"
+                      class="input__password-toggle"
+                      type="button"
+                      aria-label=${this.localize.term(this.passwordVisible ? 'hidePassword' : 'showPassword')}
+                      @click=${this.handlePasswordToggle}
+                      tabindex="-1"
+                    >
+                      ${this.passwordVisible
+                        ? html`
+                            <slot name="show-password-icon">
+                              <sl-icon name="eye-slash" library="system"></sl-icon>
+                            </slot>
+                          `
+                        : html`
+                            <slot name="hide-password-icon">
+                              <sl-icon name="eye" library="system"></sl-icon>
+                            </slot>
+                          `}
+                    </button>
+                  `
+                : ''
+            }
 
-            <span part="suffix" class="input__suffix">
-              <slot name="suffix"></slot>
-            </span>
+            <slot name="suffix" part="suffix" class="input__suffix"></slot>
           </div>
         </div>
 
-        <div
+        <slot
+          name="help-text"
           part="form-control-help-text"
           id="help-text"
           class="form-control__help-text"
           aria-hidden=${hasHelpText ? 'false' : 'true'}
         >
-          <slot name="help-text">${this.helpText}</slot>
+          ${this.helpText}
+        </slot>
         </div>
       </div>
     `;

src/components/menu/menu.ts

@@ -45,8 +45,8 @@ export default class SlMenu extends ShoelaceElement {
   }
 
   private handleKeyDown(event: KeyboardEvent) {
-    // Make a selection when pressing enter or space
-    if (event.key === 'Enter' || event.key === ' ') {
+    // Make a selection when pressing enter
+    if (event.key === 'Enter') {
       const item = this.getCurrentItem();
       event.preventDefault();
 
@@ -54,6 +54,11 @@ export default class SlMenu extends ShoelaceElement {
       item?.click();
     }
 
+    // Prevent scrolling when space is pressed
+    if (event.key === ' ') {
+      event.preventDefault();
+    }
+
     // Move the selection when pressing down or up
     if (['ArrowDown', 'ArrowUp', 'Home', 'End'].includes(event.key)) {
       const items = this.getAllItems();

src/components/radio-group/radio-group.ts

@@ -329,9 +329,12 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor
     const hasHelpText = this.helpText ? true : !!hasHelpTextSlot;
 
     const defaultSlot = html`
-      <span @click=${this.handleRadioClick} @keydown=${this.handleKeyDown} role="presentation">
-        <slot @slotchange=${this.syncRadios}></slot>
-      </span>
+      <slot
+        @click=${this.handleRadioClick}
+        @keydown=${this.handleKeyDown}
+        @slotchange=${this.syncRadios}
+        role="presentation"
+      ></slot>
     `;
 
     return html`
@@ -385,14 +388,15 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor
             : defaultSlot}
         </div>
 
-        <div
+        <slot
+          name="help-text"
           part="form-control-help-text"
           id="help-text"
           class="form-control__help-text"
           aria-hidden=${hasHelpText ? 'false' : 'true'}
         >
-          <slot name="help-text">${this.helpText}</slot>
-        </div>
+          ${this.helpText}
+        </slot>
       </fieldset>
     `;
     /* eslint-enable lit-a11y/click-events-have-key-events */

src/components/range/range.ts

@@ -343,14 +343,15 @@ export default class SlRange extends ShoelaceElement implements ShoelaceFormCont
           </div>
         </div>
 
-        <div
+        <slot
+          name="help-text"
           part="form-control-help-text"
           id="help-text"
           class="form-control__help-text"
           aria-hidden=${hasHelpText ? 'false' : 'true'}
         >
-          <slot name="help-text">${this.helpText}</slot>
-        </div>
+          ${this.helpText}
+        </slot>
       </div>
     `;
   }

src/components/select/select.ts

@@ -835,14 +835,15 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
           </sl-popup>
         </div>
 
-        <div
+        <slot
+          name="help-text"
           part="form-control-help-text"
           id="help-text"
           class="form-control__help-text"
           aria-hidden=${hasHelpText ? 'false' : 'true'}
         >
-          <slot name="help-text">${this.helpText}</slot>
-        </div>
+          ${this.helpText}
+        </slot>
       </div>
     `;
   }

src/components/spinner/spinner.ts

@@ -26,7 +26,7 @@ export default class SlSpinner extends ShoelaceElement {
 
   render() {
     return html`
-      <svg part="base" class="spinner" role="progressbar" aria-label=${this.localize.term('loading')}>
+      <svg part="base" class="spinner" role="progressbar" aria-valuetext=${this.localize.term('loading')}>
         <circle class="spinner__track"></circle>
         <circle class="spinner__indicator"></circle>
       </svg>

src/components/textarea/textarea.ts

@@ -372,14 +372,15 @@ export default class SlTextarea extends ShoelaceElement implements ShoelaceFormC
           </div>
         </div>
 
-        <div
+        <slot
+          name="help-text"
           part="form-control-help-text"
           id="help-text"
           class="form-control__help-text"
           aria-hidden=${hasHelpText ? 'false' : 'true'}
         >
-          <slot name="help-text">${this.helpText}</slot>
-        </div>
+          ${this.helpText}
+        </slot>
       </div>
     `;
   }

src/components/tooltip/tooltip.ts

@@ -241,12 +241,6 @@ export default class SlTooltip extends ShoelaceElement {
     return waitForEvent(this, 'sl-after-hide');
   }
 
-  //
-  // NOTE: Tooltip is a bit unique in that we're using aria-live instead of aria-labelledby to trick screen readers into
-  // announcing the content. It works really well, but it violates an accessibility rule. We're also adding the
-  // aria-describedby attribute to a slot, which is required by <sl-popup> to correctly locate the first assigned
-  // element, otherwise positioning is incorrect.
-  //
   render() {
     return html`
       <sl-popup
@@ -267,13 +261,18 @@ export default class SlTooltip extends ShoelaceElement {
         shift
         arrow
       >
-        ${'' /* eslint-disable-next-line lit-a11y/no-aria-slot */}
         <slot slot="anchor" aria-describedby="tooltip"></slot>
 
-        ${'' /* eslint-disable-next-line lit-a11y/accessible-name */}
-        <div part="body" id="tooltip" class="tooltip__body" role="tooltip" aria-live=${this.open ? 'polite' : 'off'}>
-          <slot name="content">${this.content}</slot>
-        </div>
+        <slot
+          name="content"
+          part="body"
+          id="tooltip"
+          class="tooltip__body"
+          role="tooltip"
+          aria-live=${this.open ? 'polite' : 'off'}
+        >
+          ${this.content}
+        </slot>
       </sl-popup>
     `;
   }

src/components/tree-item/tree-item.ts

@@ -288,9 +288,13 @@ export default class SlTreeItem extends ShoelaceElement {
           <slot class="tree-item__label" part="label"></slot>
         </div>
 
-        <div class="tree-item__children" part="children" role="group">
-          <slot name="children" @slotchange="${this.handleChildrenSlotChange}"></slot>
-        </div>
+        <slot
+          name="children"
+          class="tree-item__children"
+          part="children"
+          role="group"
+          @slotchange="${this.handleChildrenSlotChange}"
+        ></slot>
       </div>
     `;
   }

src/components/tree/tree.ts

@@ -403,8 +403,8 @@ export default class SlTree extends ShoelaceElement {
         @mousedown=${this.handleMouseDown}
       >
         <slot @slotchange=${this.handleSlotChange}></slot>
-        <span hidden aria-hidden="true"><slot name="expand-icon"></slot></span>
-        <span hidden aria-hidden="true"><slot name="collapse-icon"></slot></span>
+        <slot name="expand-icon" hidden aria-hidden="true"> </slot>
+        <slot name="collapse-icon" hidden aria-hidden="true"> </slot>
       </div>
     `;
   }

src/internal/modal.ts

@@ -1,11 +1,10 @@
-import { getTabbableElements } from './tabbable.js';
+import { getTabbableBoundary } from './tabbable.js';
 
 let activeModals: HTMLElement[] = [];
 
 export default class Modal {
   element: HTMLElement;
   tabDirection: 'forward' | 'backward' = 'forward';
-  currentFocus: HTMLElement | null;
 
   constructor(element: HTMLElement) {
     this.element = element;
@@ -23,7 +22,6 @@ export default class Modal {
 
   deactivate() {
     activeModals = activeModals.filter(modal => modal !== this.element);
-    this.currentFocus = null;
     document.removeEventListener('focusin', this.handleFocusIn);
     document.removeEventListener('keydown', this.handleKeyDown);
     document.removeEventListener('keyup', this.handleKeyUp);
@@ -36,14 +34,11 @@ export default class Modal {
 
   checkFocus() {
     if (this.isActive()) {
-      const tabbableElements = getTabbableElements(this.element);
       if (!this.element.matches(':focus-within')) {
-        const start = tabbableElements[0];
-        const end = tabbableElements[tabbableElements.length - 1];
+        const { start, end } = getTabbableBoundary(this.element);
         const target = this.tabDirection === 'forward' ? start : end;
 
         if (typeof target?.focus === 'function') {
-          this.currentFocus = target;
           target.focus({ preventScroll: true });
         }
       }
@@ -54,45 +49,13 @@ export default class Modal {
     this.checkFocus();
   }
 
-  get currentFocusIndex() {
-    return getTabbableElements(this.element).findIndex(el => el === this.currentFocus);
-  }
-
   handleKeyDown(event: KeyboardEvent) {
-    if (event.key !== 'Tab') return;
-
-    if (event.shiftKey) {
+    if (event.key === 'Tab' && event.shiftKey) {
       this.tabDirection = 'backward';
-    } else {
-      this.tabDirection = 'forward';
+
+      // Ensure focus remains trapped after the key is pressed
+      requestAnimationFrame(() => this.checkFocus());
     }
-
-    event.preventDefault();
-
-    const tabbableElements = getTabbableElements(this.element);
-    const start = tabbableElements[0];
-    let focusIndex = this.currentFocusIndex;
-
-    if (focusIndex === -1) {
-      this.currentFocus = start;
-      this.currentFocus.focus({ preventScroll: true });
-      return;
-    }
-
-    const addition = this.tabDirection === 'forward' ? 1 : -1;
-
-    if (focusIndex + addition >= tabbableElements.length) {
-      focusIndex = 0;
-    } else if (this.currentFocusIndex + addition < 0) {
-      focusIndex = tabbableElements.length - 1;
-    } else {
-      focusIndex += addition;
-    }
-
-    this.currentFocus = tabbableElements[focusIndex];
-    this.currentFocus?.focus({ preventScroll: true });
-
-    setTimeout(() => this.checkFocus());
   }
 
   handleKeyUp() {

src/internal/tabbable.ts

@@ -1,5 +1,3 @@
-import { offsetParent } from 'composed-offset-position';
-
 /** Determines if the specified element is tabbable using heuristics inspired by https://github.com/focus-trap/tabbable */
 function isTabbable(el: HTMLElement) {
   const tag = el.tagName.toLowerCase();
@@ -25,8 +23,7 @@ function isTabbable(el: HTMLElement) {
   }
 
   // Elements that are hidden have no offsetParent and are not tabbable
-  // offsetParent() is added because otherwise it misses elements in Safari
-  if (el.offsetParent === null && offsetParent(el) === null) {
+  if (el.offsetParent === null) {
     return false;
   }
 
@@ -59,20 +56,10 @@ function isTabbable(el: HTMLElement) {
  * element because it short-circuits after finding the first and last ones.
  */
 export function getTabbableBoundary(root: HTMLElement | ShadowRoot) {
-  const tabbableElements = getTabbableElements(root);
-
-  // Find the first and last tabbable elements
-  const start = tabbableElements[0] ?? null;
-  const end = tabbableElements[tabbableElements.length - 1] ?? null;
-
-  return { start, end };
-}
-
-export function getTabbableElements(root: HTMLElement | ShadowRoot) {
   const allElements: HTMLElement[] = [];
 
   function walk(el: HTMLElement | ShadowRoot) {
-    if (el instanceof Element) {
+    if (el instanceof HTMLElement) {
       allElements.push(el);
 
       if (el.shadowRoot !== null && el.shadowRoot.mode === 'open') {
@@ -86,10 +73,9 @@ export function getTabbableElements(root: HTMLElement | ShadowRoot) {
   // Collect all elements including the root
   walk(root);
 
-  return allElements.filter(isTabbable).sort((a, b) => {
-    // Make sure we sort by tabindex.
-    const aTabindex = Number(a.getAttribute('tabindex')) || 0;
-    const bTabindex = Number(b.getAttribute('tabindex')) || 0;
-    return bTabindex - aTabindex;
-  });
+  // Find the first and last tabbable elements
+  const start = allElements.find(el => isTabbable(el)) ?? null;
+  const end = allElements.reverse().find(el => isTabbable(el)) ?? null;
+
+  return { start, end };
 }