fix(picker): disable drag and select for mobile picker

.changeset/bright-brooms-remain.md

@@ -0,0 +1,9 @@
+---
+'@spectrum-web-components/picker': minor
+'@spectrum-web-components/menu': minor
+---
+
+Disabled drag and select functionality of picker in mobile devices. This is done to prevent click event being captured behind the menu-tray combination because the menu was closing immediately on pointerup.
+
+-   Fixed a bug where the picker in a dialog was not closing when clicking outside the dialog. ([#5111](https://github.com/adobe/spectrum-web-components/issues/5111))
+-   Fixed another bug where the elements behind the menu were receiving click events. ([#5060](https://github.com/adobe/spectrum-web-components/issues/5060))

packages/combobox/test/combobox.test.ts

@@ -499,9 +499,11 @@ describe('Combobox', () => {
             await elementUpdated(el);
             expect(
                 el.activeDescendant?.value,
-                'activeDscendant after open?'
+                'activeDescendant after open?'
             ).to.equal('apple');
+            const changed = oneEvent(el, 'change');
             el.focusElement.dispatchEvent(enterEvent());
+            await changed;
 
             await elementUpdated(el);
             expect(el.open, 'open?').to.be.false;
@@ -659,8 +661,9 @@ describe('Combobox', () => {
 
             const itemValue = item.itemText;
 
+            const closed = oneEvent(el, 'sp-closed');
             item.click();
-
+            await closed;
             await elementUpdated(el);
 
             expect(el.value).to.equal(itemValue);

packages/menu/src/Menu.ts

@@ -149,6 +149,13 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
     public focusedItemIndex = 0;
     public focusInItemIndex = 0;
 
+    /**
+     * whether or not to support pointerdown - drag - pointerup selection strategy
+     * default is true
+     * should be false for mobile to prevent click event being captured behind the menu-tray (cz menu immediately closes on pointerup)
+     */
+    public shouldSupportDragAndSelect = true;
+
     public get focusInItem(): MenuItem | undefined {
         return this.rovingTabindexController?.focusInElement;
     }
@@ -464,11 +471,18 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
     }
 
     private handlePointerup(event: Event): void {
+        /*
+         * early return if drag and select is not supported
+         * in this case, selection will be handled by the click event
+         */
+        if (!this.shouldSupportDragAndSelect) {
+            return;
+        }
         this.pointerUpTarget = event.target;
         this.handlePointerBasedSelection(event);
     }
 
-    private handlePointerBasedSelection(event: Event): void {
+    private async handlePointerBasedSelection(event: Event): Promise<void> {
         // Only handle left clicks
         if (event instanceof MouseEvent && event.button !== 0) {
             return;
@@ -638,6 +652,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
                 composed: true,
             })
         );
+
         if (!applyDefault) {
             // Cancel the event & don't apply the selection
             this._selected = oldSelected;

packages/menu/test/menu-group.test.ts

@@ -252,8 +252,9 @@ describe('Menu group', () => {
 
         expect(firstItem.getAttribute('aria-checked')).to.equal('true');
         expect(secondItem.getAttribute('aria-checked')).to.equal('false');
-
+        let change = oneEvent(el, 'change');
         secondItem.click();
+        await change;
         await elementUpdated(el);
         await elementUpdated(firstItem);
         await elementUpdated(secondItem);
@@ -263,8 +264,9 @@ describe('Menu group', () => {
         expect(secondItem.getAttribute('aria-checked')).to.equal('true');
         expect(el.value).to.equal('Second');
         expect(el.selectedItems.length).to.equal(1);
-
+        change = oneEvent(el, 'change');
         inheritItem1.click();
+        await change;
         await elementUpdated(el);
         await elementUpdated(inheritItem1);
         await elementUpdated(secondItem);
@@ -275,17 +277,19 @@ describe('Menu group', () => {
         expect(inheritItem1.getAttribute('aria-checked')).to.equal('true');
         expect(el.value).to.equal('Inherit1');
         expect(el.selectedItems.length).to.equal(1);
-
+        change = oneEvent(el, 'change');
         noneItem2.click();
+        await change;
         await elementUpdated(el);
         await elementUpdated(noneGroup);
         await elementUpdated(noneItem2);
         expect(inheritItem1.selected).to.be.true;
         expect(noneItem2.selected, 'none item not selected').to.be.false;
         expect(el.value).to.equal('Inherit1');
         expect(el.selectedItems.length).to.equal(1);
-
+        change = oneEvent(el, 'change');
         singleItem2.click();
+        await change;
         await elementUpdated(singleGroup);
         await elementUpdated(singleItem1);
         await elementUpdated(singleItem2);
@@ -298,8 +302,9 @@ describe('Menu group', () => {
         expect(el.selectedItems.length).to.equal(1);
         //expect(singleGroup.value).to.equal('Inherit1')
         expect(singleGroup.selectedItems.length).to.equal(1);
-
+        change = oneEvent(el, 'change');
         multiItem2.click();
+        await change;
         await elementUpdated(el);
         await elementUpdated(multiItem2);
         expect(multiItem1.selected).to.be.true;

packages/menu/test/menu-selects.test.ts

@@ -480,8 +480,10 @@ describe('Menu w/ groups [selects]', () => {
                 event.preventDefault();
                 preventSpy();
             });
-            const change = oneEvent(el, 'change');
+            let change = oneEvent(el, 'change');
             item1a.click();
+            await change;
+            change = oneEvent(el, 'change');
             item1b.click();
             await change;
             expect(preventSpy.callCount).to.equal(1);

packages/menu/test/menu.test.ts

@@ -16,6 +16,7 @@ import {
     expect,
     html,
     nextFrame,
+    oneEvent,
     waitUntil,
 } from '@open-wc/testing';
 import { Menu, MenuItem } from '@spectrum-web-components/menu';
@@ -543,7 +544,9 @@ describe('Menu', () => {
         expect(secondItem.getAttribute('aria-checked')).to.equal('false');
         expect(el.value).to.equal('First');
 
+        const change = oneEvent(el, 'change');
         secondItem.click();
+        await change;
 
         await elementUpdated(el);
         await elementUpdated(firstItem);
@@ -655,7 +658,9 @@ describe('Menu', () => {
         expect(el.value).to.equal('First');
         expect(el.selectedItems.length).to.equal(1);
 
+        let change = oneEvent(el, 'change');
         secondItem.click();
+        await change;
 
         await elementUpdated(el);
         await elementUpdated(firstItem);
@@ -669,7 +674,9 @@ describe('Menu', () => {
         expect(el.value).to.equal('First,Second');
         expect(el.selectedItems.length).to.equal(2);
 
+        change = oneEvent(el, 'change');
         firstItem.click();
+        await change;
 
         await elementUpdated(el);
         await elementUpdated(firstItem);

packages/overlay/src/AbstractOverlay.ts

@@ -339,37 +339,7 @@ export class AbstractOverlay extends SpectrumElement {
         overlay.willPreventClose = !!options.notImmediatelyClosable;
     }
 
-    private iosEventPropagationController?: AbortController;
-
-    protected setupIOSEventManagement(): void {
-        // This is a workaround for a bug in iOS <17 where the event leaks out of the dialog to the element below it.
-        // Issue - https://github.com/adobe/spectrum-web-components/issues/5042
-        this.iosEventPropagationController = new AbortController();
-        this.dialogEl.addEventListener(
-            'pointerup',
-            (event) => event.stopPropagation(),
-            {
-                capture: true,
-                signal: this.iosEventPropagationController.signal,
-            }
-        );
-        this.dialogEl.addEventListener(
-            'touchend',
-            (event) => event.stopPropagation(),
-            {
-                capture: true,
-                signal: this.iosEventPropagationController.signal,
-            }
-        );
-    }
-
-    protected cleanupIOSEventManagement(): void {
-        this.iosEventPropagationController?.abort();
-        this.iosEventPropagationController = undefined;
-    }
-
     override disconnectedCallback(): void {
-        this.cleanupIOSEventManagement();
         super.disconnectedCallback();
     }
 }

packages/overlay/src/OverlayDialog.ts

@@ -54,7 +54,6 @@ export function OverlayDialog<T extends Constructor<AbstractOverlay>>(
                     if (!targetOpenState) {
                         const close = (): void => {
                             el.removeEventListener('close', close);
-                            this.cleanupIOSEventManagement();
                             finish(el, index);
                         };
                         el.addEventListener('close', close);
@@ -89,7 +88,6 @@ export function OverlayDialog<T extends Constructor<AbstractOverlay>>(
                         return;
                     }
                     this.dialogEl.showModal();
-                    this.setupIOSEventManagement();
                 };
             const finish = (el: OpenableElement, index: number) => (): void => {
                 if (this.open !== targetOpenState) {

packages/overlay/stories/overlay.stories.ts

@@ -47,6 +47,8 @@ import '@spectrum-web-components/slider/sp-slider.js';
 import '@spectrum-web-components/theme/sp-theme.js';
 import '@spectrum-web-components/theme/src/themes.js';
 import '@spectrum-web-components/tooltip/sp-tooltip.js';
+import '@spectrum-web-components/dialog/sp-dialog.js';
+
 import '../../../projects/story-decorator/src/types.js';
 
 import { Button } from '@spectrum-web-components/button';
@@ -1612,6 +1614,51 @@ export const triggeredByOptimization = (): TemplateResult => {
     `;
 };
 
+export const pickerInDialog = (): TemplateResult => {
+    return html`
+        <sp-button variant="primary" id="mybutton">Button popover</sp-button>
+        <sp-overlay trigger="mybutton@click" type="modal" placement="bottom">
+            <sp-popover tip>
+                <sp-dialog no-divider>
+                    <sp-field-label for="picker-value">
+                        Open picker, then try clicking outside to close it:
+                    </sp-field-label>
+                    <sp-picker
+                        label="Select a Country with a very long label, too long in fact"
+                        value="item-2"
+                        id="picker-value"
+                    >
+                        <sp-menu-item value="item-1">Deselect</sp-menu-item>
+                        <sp-menu-item value="item-2">
+                            Select inverse
+                        </sp-menu-item>
+                        <sp-menu-item value="item-3">Feather...</sp-menu-item>
+                        <sp-menu-item value="item-4">
+                            Select and mask...
+                        </sp-menu-item>
+                        <sp-menu-divider></sp-menu-divider>
+                        <sp-menu-item value="item-5">
+                            Save selection
+                        </sp-menu-item>
+                        <sp-menu-item disabled value="item-6">
+                            Make work path
+                        </sp-menu-item>
+                    </sp-picker>
+                </sp-dialog>
+            </sp-popover>
+        </sp-overlay>
+    `;
+};
+
+pickerInDialog.swc_vrt = {
+    skip: true,
+};
+
+pickerInDialog.args = {
+    // Disables Chromatic's snapshotting on a global level
+    chromatic: { disableSnapshot: true },
+};
+
 export const disabledOverlayTrigger = (): TemplateResult => {
     return html`
         ${storyStyles}

packages/picker/src/InteractionController.ts

@@ -176,8 +176,21 @@ export class InteractionController implements ReactiveController {
 
     hostConnected(): void {
         this.init();
-        this.host.addEventListener('sp-closed', ()=> {
-            if(!this.open && this.host.optionsMenu.matches(':focus-within') && !this.host.button.matches(':focus')) {
+        this.host.addEventListener('sp-opened', () => {
+            /**
+             * set shouldSupportDragAndSelect to false for mobile
+             * to prevent click event being captured behind the menu-tray
+             * we do this here because the menu gets reinitialized on overlay open
+             */
+            this.host.optionsMenu.shouldSupportDragAndSelect =
+                !this.host.isMobile.matches;
+        });
+        this.host.addEventListener('sp-closed', () => {
+            if (
+                !this.open &&
+                this.host.optionsMenu.matches(':focus-within') &&
+                !this.host.button.matches(':focus')
+            ) {
                 this.host.button.focus();
             }
         });

packages/picker/src/Picker.ts

@@ -320,9 +320,6 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
     ): Promise<void> {
         this.open = false;
         // should always close when "setting" a value
-        if (this.strategy) {
-            this.strategy.open = false;
-        }
         const oldSelectedItem = this.selectedItem;
         const oldValue = this.value;
 

packages/picker/stories/picker.stories.ts

@@ -772,8 +772,11 @@ export const BackgroundClickTest = (): TemplateResult => {
             <div style="position: absolute; bottom: 50px;">
                 <sp-button
                     @click=${() => {
+                        alert(
+                            'this button should not receive a click event on menu-item selection'
+                        );
                         console.log(
-                            'this button should not have been clicked...'
+                            'this button should not receive a click event on menu-item selection'
                         );
                     }}
                     size="l"