Merge branch 'main' into ruben/picker-value-items-lazy-loaded

Author: rubencarvalho

.changeset/fix-safari-voiceover-crash.md

@@ -0,0 +1,6 @@
+---
+'@spectrum-web-components/picker': patch
+'@spectrum-web-components/action-menu': patch
+---
+
+**Fixed**: Safari + VoiceOver crash when opening Picker and ActionMenu. The issue was caused by an imperative `render()` call that mutated the DOM during the render cycle, causing Safari to crash while VoiceOver scanned the accessibility tree.

.changeset/good-humans-trade.md

@@ -0,0 +1,5 @@
+---
+'@spectrum-web-components/shared': patch
+---
+
+Improve reliability when composing components and mixins that observe slot presence.

1st-gen/packages/action-button/src/ActionButton.ts

@@ -179,7 +179,6 @@ export class ActionButton extends SizedMixin(ButtonBase, {
         if (code === 'Space' || (altKey && code === 'ArrowDown')) {
             event.preventDefault();
             if (code === 'ArrowDown') {
-                event.stopPropagation();
                 event.stopImmediatePropagation();
             }
             this.addEventListener('keyup', this.handleKeyup);

1st-gen/packages/button/src/ButtonBase.ts

@@ -89,7 +89,6 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [
         if (this.disabled) {
             event.preventDefault();
             event.stopImmediatePropagation();
-            event.stopPropagation();
             return false;
         }
 

1st-gen/packages/menu/src/MenuItem.ts

@@ -331,7 +331,6 @@ export class MenuItem extends LikeAnchor(
         if (this.disabled) {
             event.preventDefault();
             event.stopImmediatePropagation();
-            event.stopPropagation();
             return false;
         }
 

1st-gen/packages/overlay/src/LongpressController.ts

@@ -95,7 +95,6 @@ export class LongpressController extends InteractionController {
     private handleKeydown(event: KeyboardEvent): void {
         const { code, altKey } = event;
         if (altKey && code === 'ArrowDown') {
-            event.stopPropagation();
             event.stopImmediatePropagation();
         }
     }

1st-gen/packages/overlay/src/OverlayStack.ts

@@ -163,23 +163,22 @@ class OverlayStack {
      * @param event {PointerEvent}
      */
     handlePointerdown = (event: Event): void => {
+        if (!this.stack.length) return;
+
         const pointerPath = event.composedPath();
 
         // For page overlays only: block clicks outside (no light dismiss)
         // Modal overlays have light dismiss handled by handlePointerup
-        if (this.stack.length) {
-            const pageOverlays = this.stack.filter(
-                (o) => o.open && o.type === 'page'
-            );
-            if (
-                pageOverlays.length > 0 &&
-                !this.isEventInsideModal(pointerPath, pageOverlays)
-            ) {
-                event.preventDefault();
-                event.stopPropagation();
-                event.stopImmediatePropagation();
-                return;
-            }
+        const pageOverlays = this.stack.filter(
+            (o) => o.open && o.type === 'page'
+        );
+        if (
+            pageOverlays.length > 0 &&
+            !this.isEventInsideModal(pointerPath, pageOverlays)
+        ) {
+            event.preventDefault();
+            event.stopImmediatePropagation();
+            return;
         }
 
         this.pointerdownPath = pointerPath;
@@ -210,7 +209,6 @@ class OverlayStack {
             // If click is outside all page overlays, prevent it from reaching the target
             // This replicates the behavior that showModal() provided automatically
             event.stopImmediatePropagation();
-            event.stopPropagation();
             event.preventDefault();
         }
     };
@@ -232,19 +230,30 @@ class OverlayStack {
         this.lastOverlay = undefined;
 
         const lastIndex = this.stack.length - 1;
-        const clickedBackdrop = composedPath.some(
-            (el) =>
-                el instanceof HTMLElement &&
-                el.classList.contains('modal-backdrop')
+
+        // Avoid the cost of event.composedPath() unless we actually have a modal/page
+        // overlay open. composedPath() walks and allocates the full event path and can
+        // be surprisingly expensive in hot paths.
+        const hasModalOrPageOverlay = this.stack.some(
+            (overlay) =>
+                overlay.open &&
+                (overlay.type === 'modal' || overlay.type === 'page')
         );
-        if (clickedBackdrop) {
-            const topOverlay = this.stack[this.stack.length - 1];
-            // Only modal overlays close on backdrop click.
-            // Page overlays are blocking and should not be light-dismissable.
-            if (topOverlay?.type === 'modal') {
-                this.closeOverlay(topOverlay);
+        if (hasModalOrPageOverlay) {
+            const clickedBackdrop = composedPath.some(
+                (el) =>
+                    el instanceof HTMLDivElement &&
+                    el.classList.contains('modal-backdrop')
+            );
+            if (clickedBackdrop) {
+                const topOverlay = this.stack[this.stack.length - 1];
+                // Only modal overlays close on backdrop click.
+                // Page overlays are blocking and should not be light-dismissable.
+                if (topOverlay?.type === 'modal') {
+                    this.closeOverlay(topOverlay);
+                }
+                return;
             }
-            return;
         }
         const nonAncestorOverlays = this.stack.filter((overlay, i) => {
             const inStack = composedPath.find(

1st-gen/packages/picker/src/InteractionController.ts

@@ -50,24 +50,11 @@ export class InteractionController implements ReactiveController {
         if (this._open === open) return;
         this._open = open;
 
-        if (this.overlay) {
-            this.host.open = open;
-            return;
-        }
+        this.host.open = open;
 
-        // When there is no Overlay and `open` is moving to `true`, lazily import/create
-        // an Overlay and apply that state to it.
-        customElements
-            .whenDefined('sp-overlay')
-            .then(async (): Promise<void> => {
-                const { Overlay } = await import(
-                    '@spectrum-web-components/overlay/src/Overlay.js'
-                );
-                this.overlay = new Overlay();
-                this.host.open = true;
-                this.host.requestUpdate();
-            });
-        import('@spectrum-web-components/overlay/sp-overlay.js');
+        if (!this.overlay && this.host.overlayElement) {
+            this.overlay = this.host.overlayElement;
+        }
     }
 
     private _overlay!: AbstractOverlay;
@@ -110,8 +97,6 @@ export class InteractionController implements ReactiveController {
             if (this.preventNextToggle === 'no') {
                 this.open = false;
             } else if (!this.pointerdownState) {
-                // Prevent browser driven closure while opening the Picker
-                // and the expected event series has not completed.
                 this.overlay?.manuallyKeepOpen();
             }
         }
@@ -195,13 +180,12 @@ export class InteractionController implements ReactiveController {
     }
 
     public hostUpdated(): void {
-        if (
-            this.overlay &&
-            this.host.dependencyManager.loaded &&
-            this.host.open !== this.overlay.open
-        ) {
+        if (!this.overlay && this.host.overlayElement) {
+            this.overlay = this.host.overlayElement;
+        }
+
+        if (this.overlay && this.host.dependencyManager.loaded) {
             this.overlay.willPreventClose = this.preventNextToggle !== 'no';
-            this.overlay.open = this.host.open;
         }
     }
 }

1st-gen/packages/picker/src/Picker.ts

@@ -16,7 +16,6 @@ import {
     html,
     nothing,
     PropertyValues,
-    render,
     SizedMixin,
     SpectrumElement,
     TemplateResult,
@@ -438,6 +437,30 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
 
     public handleSlottableRequest = (_event: SlottableRequestEvent): void => {};
 
+    protected handleBeforetoggle = (
+        event: Event & {
+            target: Overlay;
+            newState: 'open' | 'closed';
+        }
+    ): void => {
+        if (event.composedPath()[0] !== event.target) {
+            return;
+        }
+        if (event.newState === 'closed') {
+            if (this.strategy?.preventNextToggle === 'no') {
+                this.open = false;
+            } else if (!this.strategy?.pointerdownState) {
+                // Prevent browser driven closure while opening the Picker
+                // and the expected event series has not completed.
+                this.overlayElement?.manuallyKeepOpen();
+            }
+        }
+        if (!this.open) {
+            this.optionsMenu.updateSelectedItemIndex();
+            this.optionsMenu.closeDescendentOverlays();
+        }
+    };
+
     protected renderLabelContent(content: Node[]): TemplateResult | Node[] {
         if (this.value && this.selectedItem) {
             return content;
@@ -577,14 +600,30 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
     }
 
     protected renderOverlay(menu: TemplateResult): TemplateResult {
-        if (this.strategy?.overlay === undefined) {
-            return menu;
-        }
         const container = this.renderContainer(menu);
-        render(container, this.strategy?.overlay as unknown as HTMLElement, {
-            host: this,
-        });
-        return this.strategy?.overlay as unknown as TemplateResult;
+        this.dependencyManager.add('sp-overlay');
+        import('@spectrum-web-components/overlay/sp-overlay.js');
+        return html`
+            <sp-overlay
+                @slottable-request=${this.handleSlottableRequest}
+                @beforetoggle=${this.handleBeforetoggle}
+                .triggerElement=${this as HTMLElement}
+                .offset=${0}
+                ?open=${this.open && this.dependencyManager.loaded}
+                .placement=${this.isMobile.matches && !this.forcePopover
+                    ? undefined
+                    : this.placement}
+                .type=${this.isMobile.matches && !this.forcePopover
+                    ? 'modal'
+                    : 'auto'}
+                .receivesFocus=${'false'}
+                .willPreventClose=${this.strategy?.preventNextToggle !== 'no' &&
+                this.open &&
+                this.dependencyManager.loaded}
+            >
+                ${container}
+            </sp-overlay>
+        `;
     }
 
     protected get renderDescriptionSlot(): TemplateResult {
@@ -662,7 +701,7 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
             // await the same here.
             this.shouldScheduleManageSelection();
         }
-        // Maybe it's finally time to remove this support?s
+        // Maybe it's finally time to remove this support?
         if (!this.hasUpdated) {
             this.deprecatedMenu = this.querySelector(':scope > sp-menu');
             this.deprecatedMenu?.toggleAttribute('ignore', true);
@@ -697,15 +736,26 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
 
     protected override updated(changes: PropertyValues<this>): void {
         super.updated(changes);
-        if (changes.has('open')) {
-            this.strategy.open = this.open;
+        if (
+            changes.has('open') &&
+            this.overlayElement &&
+            !this.strategy.overlay
+        ) {
+            this.strategy.overlay = this.overlayElement;
         }
     }
 
-    protected override firstUpdated(changes: PropertyValues<this>): void {
+    protected override async firstUpdated(
+        changes: PropertyValues<this>
+    ): Promise<void> {
         super.firstUpdated(changes);
         this.bindButtonKeydownListener();
         this.bindEvents();
+
+        await this.updateComplete;
+        if (this.overlayElement && !this.strategy.overlay) {
+            this.strategy.overlay = this.overlayElement;
+        }
     }
 
     protected get dismissHelper(): TemplateResult {
@@ -815,7 +865,6 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
                 ((event.target as HTMLElement).getRootNode() as ShadowRoot)
                     .host === this)
         ) {
-            //s set a flag to manage selection on the next frame
             this.willManageSelection = true;
             requestAnimationFrame(() => {
                 requestAnimationFrame(() => {
@@ -889,9 +938,6 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
     protected override async getUpdateComplete(): Promise<boolean> {
         const complete = (await super.getUpdateComplete()) as boolean;
         await this.selectionPromise;
-        // if (this.overlayElement) {
-        //     await this.overlayElement.updateComplete;
-        // }
         return complete;
     }