Merge branch 'next' into current

docs/pages/frameworks/react.md

@@ -21,7 +21,7 @@ Next, [include a theme](/getting-started/themes) and set the [base path](/gettin
 ```jsx
 // App.jsx
 import '@shoelace-style/shoelace/%NPMDIR%/themes/light.css';
-import { setBasePath } from '@shoelace-style/shoelace/%NPMDIR%/utilities/base-path';
+import { setBasePath } from '@shoelace-style/shoelace/%NPMDIR%/utilities/base-path.js';
 
 setBasePath('https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@%VERSION%/%CDNDIR%/');
 ```
@@ -43,7 +43,7 @@ Preact users facing type errors using components may benefit from setting "paths
 Every Shoelace component is available to import as a React component. Note that we're importing the `<SlButton>` _React component_ instead of the `<sl-button>` _custom element_ in the example below.
 
 ```jsx
-import SlButton from '@shoelace-style/shoelace/%NPMDIR%/react/button';
+import SlButton from '@shoelace-style/shoelace/%NPMDIR%/react/button/index.js';
 
 const MyComponent = () => <SlButton variant="primary">Click me</SlButton>;
 
@@ -62,7 +62,7 @@ However, tree-shaking extra Shoelace components proved to be a challenge. As a r
 
 ```diff
 - import { SlButton } from '@shoelace-style/shoelace/%NPMDIR%/react';
-+ import SlButton from '@shoelace-style/shoelace/%NPMDIR%/react/button';
++ import SlButton from '@shoelace-style/shoelace/%NPMDIR%/react/button/index.js';
 ```
 
 You can find a copy + paste import for each component in the "importing" section of its documentation.
@@ -75,7 +75,7 @@ Here's how you can bind the input's value to a state variable.
 
 ```jsx
 import { useState } from 'react';
-import SlInput from '@shoelace-style/shoelace/%NPMDIR%/react/input';
+import SlInput from '@shoelace-style/shoelace/%NPMDIR%/react/input/index.js';
 
 function MyComponent() {
   const [value, setValue] = useState('');
@@ -90,8 +90,8 @@ If you're using TypeScript, it's important to note that `event.target` will be a
 
 ```tsx
 import { useState } from 'react';
-import SlInput from '@shoelace-style/shoelace/%NPMDIR%/react/input';
-import type SlInputElement from '@shoelace-style/shoelace/%NPMDIR%/components/input/input';
+import SlInput from '@shoelace-style/shoelace/%NPMDIR%/react/input/index.js';
+import type SlInputElement from '@shoelace-style/shoelace/%NPMDIR%/components/input/input.js';
 
 function MyComponent() {
   const [value, setValue] = useState('');
@@ -106,8 +106,8 @@ You can also import the event type for use in your callbacks, shown below.
 
 ```tsx
 import { useCallback, useState } from 'react';
-import SlInput, { type SlInputEvent } from '@shoelace-style/shoelace/%NPMDIR%/react/input';
-import type SlInputElement from '@shoelace-style/shoelace/%NPMDIR%/components/input/input';
+import SlInput, { type SlInputEvent } from '@shoelace-style/shoelace/%NPMDIR%/react/input/index.js';
+import type SlInputElement from '@shoelace-style/shoelace/%NPMDIR%/components/input/input.js';
 
 function MyComponent() {
   const [value, setValue] = useState('');

docs/pages/resources/changelog.md

@@ -12,6 +12,15 @@ Components with the <sl-badge variant="warning" pill>Experimental</sl-badge> bad
 
 New versions of Shoelace are released as-needed and generally occur when a critical mass of changes have accumulated. At any time, you can see what's coming in the next release by visiting [next.shoelace.style](https://next.shoelace.style).
 
+## 2.20.1
+
+- Fixed a bug that prevented `<sl-tab-group>` to be activated properly when rendered in another `<sl-tab-group>` [#2367]
+- Fixed a bug that in `<sl-dropdown>` that prevented tab from working properly in some cases [#2371]
+- Fixed the guard on popover to allow virtual elements [#2399]
+- Fixed the close button in `<sl-alert>` so clicking above/below it doesn't inadvertently close it [#2375]
+- Fixed accessibility issues for elements that are closed while having slotted focused children. [#2383]
+- Improved accessibility of `<sl-carousel>` [#2364]
+
 ## 2.20.0
 
 - Added the ability to set a custom snap function and use `repeat(n)` to `<sl-split-panel>` [#2340]

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "@shoelace-style/shoelace",
-  "version": "2.20.0",
+  "version": "2.20.1",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "@shoelace-style/shoelace",
-      "version": "2.20.0",
+      "version": "2.20.1",
       "license": "MIT",
       "dependencies": {
         "@ctrl/tinycolor": "^4.1.0",

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@shoelace-style/shoelace",
   "description": "A forward-thinking library of web components.",
-  "version": "2.20.0",
+  "version": "2.20.1",
   "homepage": "https://github.com/shoelace-style/shoelace",
   "author": "Cory LaViska",
   "license": "MIT",

src/components/alert/alert.component.ts

@@ -1,4 +1,5 @@
 import { animateTo, stopAnimations } from '../../internal/animate.js';
+import { blurActiveElement } from '../../internal/closeActiveElement.js';
 import { classMap } from 'lit/directives/class-map.js';
 import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry.js';
 import { HasSlotController } from '../../internal/slot.js';
@@ -157,6 +158,7 @@ export default class SlAlert extends ShoelaceElement {
       this.emit('sl-after-show');
     } else {
       // Hide
+      blurActiveElement(this);
       this.emit('sl-hide');
 
       clearTimeout(this.autoHideTimeout);

src/components/alert/alert.styles.ts

@@ -94,7 +94,8 @@ export default css`
     display: flex;
     align-items: center;
     font-size: var(--sl-font-size-medium);
-    padding-inline-end: var(--sl-spacing-medium);
+    margin-inline-end: var(--sl-spacing-medium);
+    align-self: center;
   }
 
   .alert__countdown {

src/components/alert/alert.test.ts

@@ -123,14 +123,14 @@ describe('<sl-alert>', () => {
   });
 
   describe('close button', () => {
-    it('shows a close button if the alert has the closable attribute', () => async () => {
+    it('shows a close button if the alert has the closable attribute', async () => {
       const alert = await fixture<SlAlert>(html` <sl-alert open closable>I am an alert</sl-alert> `);
       const closeButton = getCloseButton(alert);
 
       expect(closeButton).to.be.visible;
     });
 
-    it('clicking the close button closes the alert', () => async () => {
+    it('clicking the close button closes the alert', async () => {
       const alert = await fixture<SlAlert>(html` <sl-alert open closable>I am an alert</sl-alert> `);
       const closeButton = getCloseButton(alert);
 
@@ -138,6 +138,83 @@ describe('<sl-alert>', () => {
         clickOnElement(closeButton!);
       });
     });
+
+    it('clicking above close button does not close the alert', async () => {
+      const wrapper = await fixture<HTMLDivElement>(
+        html`<div class="wrapper" style="padding: 24px; background-color:red;">
+          <sl-alert open closable>I am an alert</sl-alert>
+        </div>`
+      );
+      const alert = wrapper.querySelector('sl-alert')!;
+
+      const clickTargetPromise = new Promise<HTMLElement>(resolve => {
+        const clickHandler = sinon.spy((event: MouseEvent) => {
+          resolve(event.target as HTMLElement);
+        });
+        alert.shadowRoot!.addEventListener('click', clickHandler);
+        wrapper.addEventListener('click', clickHandler);
+      });
+
+      const closeButton = getCloseButton(alert);
+      await clickOnElement(closeButton!, 'top', 0, -4);
+      const clickTarget = await clickTargetPromise;
+      await expect(clickTarget.tagName.toLowerCase()).to.not.be.equal('sl-icon-button');
+      expect(clickTarget.classList.contains('alert')).to.be.true;
+      expect(clickTarget.classList.contains('wrapper'), 'The click should happen in the alert and not outside of it').to
+        .be.false;
+    });
+
+    it('clicking under close button does not close the alert', async () => {
+      const wrapper = await fixture<HTMLDivElement>(
+        html`<div class="wrapper" style="padding: 24px; background-color:red;">
+          <sl-alert open closable>I am an alert</sl-alert>
+        </div>`
+      );
+      const alert = wrapper.querySelector('sl-alert')!;
+
+      const clickTargetPromise = new Promise<HTMLElement>(resolve => {
+        const clickHandler = sinon.spy((event: MouseEvent) => {
+          resolve(event.target as HTMLElement);
+        });
+        alert.shadowRoot!.addEventListener('click', clickHandler);
+        wrapper.addEventListener('click', clickHandler);
+      });
+
+      const closeButton = getCloseButton(alert);
+      await clickOnElement(closeButton!, 'bottom', 0, 4);
+      const clickTarget = await clickTargetPromise;
+
+      await expect(clickTarget.tagName.toLowerCase()).to.not.be.equal('sl-icon-button');
+      expect(clickTarget.classList.contains('alert')).to.be.true;
+      expect(clickTarget.classList.contains('wrapper'), 'The click should happen in the alert and not outside of it').to
+        .be.false;
+    });
+
+    it('clicking on the right side of the close button does not close the alert', async () => {
+      const wrapper = await fixture<HTMLDivElement>(
+        html`<div class="wrapper" style="padding: 24px; background-color:red;">
+          <sl-alert open closable>I am an alert</sl-alert>
+        </div>`
+      );
+      const alert = wrapper.querySelector('sl-alert')!;
+
+      const clickTargetPromise = new Promise<HTMLElement>(resolve => {
+        const clickHandler = sinon.spy((event: MouseEvent) => {
+          resolve(event.target as HTMLElement);
+        });
+        alert.shadowRoot!.addEventListener('click', clickHandler);
+        wrapper.addEventListener('click', clickHandler);
+      });
+
+      const closeButton = getCloseButton(alert);
+      await clickOnElement(closeButton!, 'right', 4, 0);
+      const clickTarget = await clickTargetPromise;
+
+      await expect(clickTarget.tagName.toLowerCase()).to.not.be.equal('sl-icon-button');
+      expect(clickTarget.classList.contains('alert')).to.be.true;
+      expect(clickTarget.classList.contains('wrapper'), 'The click should happen in the alert and not outside of it').to
+        .be.false;
+    });
   });
 
   describe('toast', () => {

src/components/carousel-item/carousel-item.component.ts

@@ -20,7 +20,6 @@ export default class SlCarouselItem extends ShoelaceElement {
 
   connectedCallback() {
     super.connectedCallback();
-    this.setAttribute('role', 'group');
   }
 
   render() {

src/components/carousel/carousel.component.ts

@@ -367,8 +367,16 @@ export default class SlCarousel extends ShoelaceElement {
     this.getSlides({ excludeClones: false }).forEach((slide, index) => {
       slide.classList.remove('--in-view');
       slide.classList.remove('--is-active');
+      slide.setAttribute('role', 'group');
       slide.setAttribute('aria-label', this.localize.term('slideNum', index + 1));
 
+      if (this.pagination) {
+        slide.setAttribute('id', `slide-${index + 1}`);
+        slide.setAttribute('role', 'tabpanel');
+        slide.removeAttribute('aria-label');
+        slide.setAttribute('aria-labelledby', `tab-${index + 1}`);
+      }
+
       if (slide.hasAttribute('data-clone')) {
         slide.remove();
       }
@@ -611,7 +619,7 @@ export default class SlCarousel extends ShoelaceElement {
           : ''}
         ${this.pagination
           ? html`
-              <div part="pagination" role="tablist" class="carousel__pagination" aria-controls="scroll-container">
+              <div part="pagination" role="tablist" class="carousel__pagination">
                 ${map(range(pagesCount), index => {
                   const isActive = index === currentPage;
                   return html`
@@ -622,8 +630,12 @@ export default class SlCarousel extends ShoelaceElement {
                         'carousel__pagination-item--active': isActive
                       })}"
                       role="tab"
+                      id="tab-${index + 1}"
+                      aria-controls="slide-${index + 1}"
                       aria-selected="${isActive ? 'true' : 'false'}"
-                      aria-label="${this.localize.term('goToSlide', index + 1, pagesCount)}"
+                      aria-label="${isActive
+                        ? this.localize.term('slideNum', index + 1)
+                        : this.localize.term('goToSlide', index + 1, pagesCount)}"
                       tabindex=${isActive ? '0' : '-1'}
                       @click=${() => this.goToSlide(index * slidesPerMove)}
                       @keydown=${this.handleKeyDown}

src/components/carousel/carousel.test.ts

@@ -768,11 +768,14 @@ describe('<sl-carousel>', () => {
       expect(el.scrollContainer).to.have.attribute('aria-atomic', 'true');
 
       expect(pagination).to.have.attribute('role', 'tablist');
-      expect(pagination).to.have.attribute('aria-controls', el.scrollContainer.id);
+      let paginationItemIndex = 0;
       for (const paginationItem of pagination.querySelectorAll('.carousel__pagination-item')) {
+        expect(paginationItem).to.have.attribute('id', `tab-${paginationItemIndex + 1}`);
         expect(paginationItem).to.have.attribute('role', 'tab');
+        expect(paginationItem).to.have.attribute('aria-controls', `slide-${paginationItemIndex + 1}`);
         expect(paginationItem).to.have.attribute('aria-selected');
         expect(paginationItem).to.have.attribute('aria-label');
+        paginationItemIndex++;
       }
 
       for (const navigationItem of navigation.querySelectorAll('.carousel__navigation-item')) {
@@ -781,7 +784,7 @@ describe('<sl-carousel>', () => {
         expect(navigationItem).to.have.attribute('aria-label');
       }
 
-      await expect(el).to.be.accessible();
+      await expect(el).to.be.accessible({ ignoredRules: ['aria-valid-attr-value'] });
     });
 
     describe('when scrolling', () => {

src/components/dialog/dialog.component.ts

@@ -1,4 +1,5 @@
 import { animateTo, stopAnimations } from '../../internal/animate.js';
+import { blurActiveElement } from '../../internal/closeActiveElement.js';
 import { classMap } from 'lit/directives/class-map.js';
 import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry.js';
 import { HasSlotController } from '../../internal/slot.js';
@@ -208,6 +209,7 @@ export default class SlDialog extends ShoelaceElement {
       this.emit('sl-after-show');
     } else {
       // Hide
+      blurActiveElement(this);
       this.emit('sl-hide');
       this.removeOpenListeners();
       this.modal.deactivate();

src/components/drawer/drawer.component.ts

@@ -1,4 +1,5 @@
 import { animateTo, stopAnimations } from '../../internal/animate.js';
+import { blurActiveElement } from '../../internal/closeActiveElement.js';
 import { classMap } from 'lit/directives/class-map.js';
 import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry.js';
 import { HasSlotController } from '../../internal/slot.js';
@@ -237,6 +238,7 @@ export default class SlDrawer extends ShoelaceElement {
       this.emit('sl-after-show');
     } else {
       // Hide
+      blurActiveElement(this);
       this.emit('sl-hide');
       this.removeOpenListeners();
 

src/components/dropdown/dropdown.component.ts

@@ -1,6 +1,7 @@
 import { animateTo, stopAnimations } from '../../internal/animate.js';
 import { classMap } from 'lit/directives/class-map.js';
 import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry.js';
+import { getDeepestActiveElement } from '../../internal/active-elements.js';
 import { getTabbableBoundary } from '../../internal/tabbable.js';
 import { html } from 'lit';
 import { ifDefined } from 'lit/directives/if-defined.js';
@@ -175,6 +176,20 @@ export default class SlDropdown extends ShoelaceElement {
         return;
       }
 
+      const computeClosestContaining = (element: Element | null | undefined, tagName: string): Element | null => {
+        if (!element) return null;
+
+        const closest = element.closest(tagName);
+        if (closest) return closest;
+
+        const rootNode = element.getRootNode();
+        if (rootNode instanceof ShadowRoot) {
+          return computeClosestContaining(rootNode.host, tagName);
+        }
+
+        return null;
+      };
+
       // Tabbing outside of the containing element closes the panel
       //
       // If the dropdown is used within a shadow DOM, we need to obtain the activeElement within that shadowRoot,
@@ -182,12 +197,13 @@ export default class SlDropdown extends ShoelaceElement {
       setTimeout(() => {
         const activeElement =
           this.containingElement?.getRootNode() instanceof ShadowRoot
-            ? document.activeElement?.shadowRoot?.activeElement
+            ? getDeepestActiveElement()
             : document.activeElement;
 
         if (
           !this.containingElement ||
-          activeElement?.closest(this.containingElement.tagName.toLowerCase()) !== this.containingElement
+          computeClosestContaining(activeElement, this.containingElement.tagName.toLowerCase()) !==
+            this.containingElement
         ) {
           this.hide();
         }

src/components/dropdown/dropdown.test.ts

@@ -1,6 +1,8 @@
 import '../../../dist/shoelace.js';
 import { clickOnElement } from '../../internal/test.js';
+import { customElement } from 'lit/decorators.js';
 import { expect, fixture, html, waitUntil } from '@open-wc/testing';
+import { LitElement } from 'lit';
 import { sendKeys, sendMouse } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import type SlDropdown from './dropdown.js';
@@ -354,4 +356,99 @@ describe('<sl-dropdown>', () => {
 
     expect(el.open).to.be.false;
   });
+
+  describe('when a sl-menu is provided and the dropdown is opened', () => {
+    before(() => {
+      @customElement('custom-wrapper')
+      class Wrapper extends LitElement {
+        render() {
+          return html`<nested-dropdown></nested-dropdown>`;
+        }
+      }
+      // eslint-disable-next-line chai-friendly/no-unused-expressions
+      Wrapper;
+
+      @customElement('nested-dropdown')
+      class NestedDropdown extends LitElement {
+        render() {
+          return html`
+            <sl-dropdown>
+              <sl-button slot="trigger" caret>Toggle</sl-button>
+              <sl-menu>
+                <sl-menu-item>Item 1</sl-menu-item>
+              </sl-menu>
+            </sl-dropdown>
+          `;
+        }
+      }
+      // eslint-disable-next-line chai-friendly/no-unused-expressions
+      NestedDropdown;
+    });
+
+    it('should remain open on tab key', async () => {
+      const el = await fixture<SlDropdown>(html`<custom-wrapper></custom-wrapper>`);
+
+      const dropdown = el.shadowRoot!.querySelector('nested-dropdown')!.shadowRoot!.querySelector('sl-dropdown')!;
+
+      const trigger = dropdown.querySelector('sl-button')!;
+
+      trigger.focus();
+      await dropdown.updateComplete;
+      await sendKeys({ press: 'Enter' });
+      await dropdown.updateComplete;
+      await sendKeys({ press: 'Tab' });
+      await dropdown.updateComplete;
+
+      expect(dropdown.open).to.be.true;
+    });
+  });
+
+  describe('when arbitrary content is provided and the dropdown is opened', () => {
+    before(() => {
+      @customElement('custom-wrapper-arbitrary')
+      class WrapperArbitrary extends LitElement {
+        render() {
+          return html`<nested-dropdown-arbitrary></nested-dropdown-arbitrary>`;
+        }
+      }
+      // eslint-disable-next-line chai-friendly/no-unused-expressions
+      WrapperArbitrary;
+
+      @customElement('nested-dropdown-arbitrary')
+      class NestedDropdownArbitrary extends LitElement {
+        render() {
+          return html`
+            <sl-dropdown>
+              <sl-button slot="trigger" caret>Toggle</sl-button>
+              <ul>
+                <li><a href="/settings">Settings</a></li>
+                <li><a href="/profile">Profile</a></li>
+              </ul>
+            </sl-dropdown>
+          `;
+        }
+      }
+      // eslint-disable-next-line chai-friendly/no-unused-expressions
+      NestedDropdownArbitrary;
+    });
+
+    it('should remain open on tab key', async () => {
+      const el = await fixture<SlDropdown>(html`<custom-wrapper-arbitrary></custom-wrapper-arbitrary>`);
+
+      const dropdown = el
+        .shadowRoot!.querySelector('nested-dropdown-arbitrary')!
+        .shadowRoot!.querySelector('sl-dropdown')!;
+
+      const trigger = dropdown.querySelector('sl-button')!;
+
+      trigger.focus();
+      await dropdown.updateComplete;
+      await sendKeys({ press: 'Enter' });
+      await dropdown.updateComplete;
+      await sendKeys({ press: 'Tab' });
+      await dropdown.updateComplete;
+
+      expect(dropdown.open).to.be.true;
+    });
+  });
 });

src/components/popup/popup.component.ts

@@ -19,7 +19,7 @@ function isVirtualElement(e: unknown): e is VirtualElement {
     e !== null &&
     typeof e === 'object' &&
     'getBoundingClientRect' in e &&
-    ('contextElement' in e ? e instanceof Element : true)
+    ('contextElement' in e ? e.contextElement instanceof Element : true)
   );
 }
 

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

@@ -93,17 +93,33 @@ export default class SlTabGroup extends ShoelaceElement {
     });
 
     this.mutationObserver = new MutationObserver(mutations => {
+      // Make sure to only observe the direct children of the tab group
+      // instead of other sub elements that might be slotted in.
+      // @see https://github.com/shoelace-style/shoelace/issues/2320
+      const instanceMutations = mutations.filter(({ target }) => {
+        if (target === this) return true; // Allow self updates
+        if ((target as HTMLElement).closest('sl-tab-group') !== this) return false; // We are not direct children
+
+        // We should only care about changes to the tab or tab panel
+        const tagName = (target as HTMLElement).tagName.toLowerCase();
+        return tagName === 'sl-tab' || tagName === 'sl-tab-panel';
+      });
+
+      if (instanceMutations.length === 0) {
+        return;
+      }
+
       // Update aria labels when the DOM changes
-      if (mutations.some(m => !['aria-labelledby', 'aria-controls'].includes(m.attributeName!))) {
+      if (instanceMutations.some(m => !['aria-labelledby', 'aria-controls'].includes(m.attributeName!))) {
         setTimeout(() => this.setAriaLabels());
       }
 
       // Sync tabs when disabled states change
-      if (mutations.some(m => m.attributeName === 'disabled')) {
+      if (instanceMutations.some(m => m.attributeName === 'disabled')) {
         this.syncTabsAndPanels();
         // sync tabs when active state on tab changes
-      } else if (mutations.some(m => m.attributeName === 'active')) {
-        const tabs = mutations
+      } else if (instanceMutations.some(m => m.attributeName === 'active')) {
+        const tabs = instanceMutations
           .filter(m => m.attributeName === 'active' && (m.target as HTMLElement).tagName.toLowerCase() === 'sl-tab')
           .map(m => m.target as SlTab);
         const newActiveTab = tabs.find(tab => tab.active);
@@ -117,7 +133,14 @@ export default class SlTabGroup extends ShoelaceElement {
     // After the first update...
     this.updateComplete.then(() => {
       this.syncTabsAndPanels();
-      this.mutationObserver.observe(this, { attributes: true, childList: true, subtree: true });
+
+      this.mutationObserver.observe(this, {
+        attributes: true,
+        attributeFilter: ['active', 'disabled', 'name', 'panel'],
+        childList: true,
+        subtree: true
+      });
+
       this.resizeObserver.observe(this.nav);
 
       // Wait for tabs and tab panels to be registered

src/internal/closeActiveElement.ts

@@ -0,0 +1,12 @@
+/**
+ * Calls the blur method on the current active element if it is a child of the provided element.
+ * Needed for fixing a11y errors in console.
+ * @see https://github.com/shoelace-style/shoelace/issues/2283
+ * @param elm The element to check
+ */
+export const blurActiveElement = (elm: HTMLElement) => {
+  const { activeElement } = document;
+  if (activeElement && elm.contains(activeElement)) {
+    (document.activeElement as HTMLElement)?.blur();
+  }
+};

src/internal/test.ts

@@ -47,7 +47,7 @@ export async function clickOnElement(
 ) {
   const { clickX, clickY } = determineMousePosition(el, position, offsetX, offsetY);
 
-  await sendMouse({ type: 'click', position: [clickX, clickY] });
+  await sendMouse({ type: 'click', position: [Math.round(clickX), Math.round(clickY)] });
 }
 
 /** A testing utility that moves the mouse onto an element. */