fix(overlay): handle hover/longpress more directly via the "open" att…

…ribute

packages/overlay/src/OverlayTrigger.ts

@@ -77,9 +77,13 @@ export class OverlayTrigger extends LitElement {
     private targetContent?: HTMLElement;
 
     private handleClose(event?: CustomEvent<OverlayOpenCloseDetail>): void {
-        if (!event || this.open === event.detail.interaction) {
-            this.removeAttribute('open');
+        if (
+            event?.detail.interaction !== this.open &&
+            event?.detail.interaction !== this.type
+        ) {
+            return;
         }
+        this.removeAttribute('open');
     }
 
     protected render(): TemplateResult {
@@ -200,7 +204,7 @@ export class OverlayTrigger extends LitElement {
         switch (event.type) {
             case 'mouseenter':
             case 'focusin':
-                if (this.hoverContent) {
+                if (!this.open && this.hoverContent) {
                     this.open = 'hover';
                 }
                 return;
@@ -219,6 +223,7 @@ export class OverlayTrigger extends LitElement {
                 return;
             case 'longpress':
                 if (this.longpressContent) {
+                    this._longpressEvent = event;
                     this.open = event.type;
                 }
                 return;
@@ -251,26 +256,27 @@ export class OverlayTrigger extends LitElement {
         );
     }
 
-    private async onTriggerLongpress(
-        event?: CustomEvent<LongpressEvent>
-    ): Promise<void> {
+    private _longpressEvent?: CustomEvent<LongpressEvent>;
+
+    private async onTriggerLongpress(): Promise<void> {
         if (!this.targetContent || !this.longpressContent) {
             return;
         }
         const { targetContent, longpressContent } = this;
         this.prepareToFocusOverlayContent(longpressContent);
-        const type =
-            event && event.detail.source === 'keyboard' ? 'click' : 'longpress';
-        const interaction = this.type ? this.type : type || 'longpress';
+        const notImmediatelyClosable =
+            this._longpressEvent?.detail.source !== 'keyboard';
         this.closeLongpressOverlay = await OverlayTrigger.openOverlay(
             targetContent,
-            interaction,
+            this.type ? this.type : 'longpress',
             longpressContent,
             {
                 ...this.overlayOptions,
                 receivesFocus: 'auto',
+                notImmediatelyClosable,
             }
         );
+        this._longpressEvent = undefined;
     }
 
     private hoverOverlayReady = Promise.resolve();

packages/overlay/src/overlay-stack.ts

@@ -162,8 +162,8 @@ export class OverlayStack {
         if (this.findOverlayForContent(details.content)) {
             return false;
         }
-        if (details.interaction === 'longpress') {
-            this.initialLongpressClick = true;
+        if (details.notImmediatelyClosable) {
+            this._doesNotCloseOnFirstClick = true;
         }
         if (details.interaction === 'modal') {
             this.startTabTrapping();
@@ -409,11 +409,16 @@ export class OverlayStack {
         return this.hideAndCloseOverlay(this.topOverlay);
     }
 
-    private initialLongpressClick = false;
+    /**
+     * A "longpress" occurs before the "click" that creates it has occured.
+     * In that way the first click will still be part of the "longpress" and
+     * not part of closing the overlay.
+     */
+    private _doesNotCloseOnFirstClick = false;
 
     private handleMouse = (event: Event): void => {
-        if (this.initialLongpressClick) {
-            this.initialLongpressClick = false;
+        if (this._doesNotCloseOnFirstClick) {
+            this._doesNotCloseOnFirstClick = false;
             return;
         }
         if (this.preventMouseRootClose || event.defaultPrevented) {

packages/overlay/src/overlay-types.ts

@@ -32,6 +32,7 @@ export interface OverlayOpenDetail {
     trigger: HTMLElement;
     interaction: TriggerInteractions;
     theme: ThemeData;
+    notImmediatelyClosable?: boolean;
 }
 
 export interface OverlayOpenCloseDetail {
@@ -55,6 +56,7 @@ export type OverlayOptions = {
     placement?: Placement;
     offset?: number;
     receivesFocus?: 'auto';
+    notImmediatelyClosable?: boolean;
 };
 
 declare global {

packages/overlay/src/overlay.ts

@@ -94,6 +94,7 @@ export class Overlay {
         offset = 0,
         placement = 'top',
         receivesFocus,
+        notImmediatelyClosable,
     }: OverlayOptions): Promise<boolean> {
         /* c8 ignore next */
         if (this.isOpen) return true;
@@ -115,14 +116,15 @@ export class Overlay {
         this.owner.dispatchEvent(queryThemeEvent);
 
         const overlayDetailQuery: OverlayDisplayQueryDetail = {};
-        const queryOverlayDetailEvent = new CustomEvent<
-            OverlayDisplayQueryDetail
-        >('sp-overlay-query', {
-            bubbles: true,
-            composed: true,
-            detail: overlayDetailQuery,
-            cancelable: true,
-        });
+        const queryOverlayDetailEvent = new CustomEvent<OverlayDisplayQueryDetail>(
+            'sp-overlay-query',
+            {
+                bubbles: true,
+                composed: true,
+                detail: overlayDetailQuery,
+                cancelable: true,
+            }
+        );
         this.overlayElement.dispatchEvent(queryOverlayDetailEvent);
 
         await Overlay.overlayStack.openOverlay({
@@ -135,6 +137,7 @@ export class Overlay {
             interaction: this.interaction,
             theme: queryThemeDetail,
             receivesFocus,
+            notImmediatelyClosable,
             ...overlayDetailQuery,
         });
         this.isOpen = true;
@@ -150,15 +153,15 @@ export class Overlay {
 }
 
 /**
-* Announces that an overlay-based UI element has opened
-* @event sp-open
-* @type {object}
-* @property {TriggerInteractions} interaction type of interaction that triggered the opening
-*/
+ * Announces that an overlay-based UI element has opened
+ * @event sp-open
+ * @type {object}
+ * @property {TriggerInteractions} interaction type of interaction that triggered the opening
+ */
 
 /**
-* Announces that an overlay-based UI element has opened
-* @event sp-close
-* @type {object}
-* @property {TriggerInteractions} interaction type of interaction that triggered the closing
-*/
+ * Announces that an overlay-based UI element has opened
+ * @event sp-close
+ * @type {object}
+ * @property {TriggerInteractions} interaction type of interaction that triggered the closing
+ */

packages/overlay/test/overlay-trigger-click.test.ts

@@ -9,14 +9,21 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { fixture, elementUpdated, waitUntil, html } from '@open-wc/testing';
+import {
+    fixture,
+    elementUpdated,
+    waitUntil,
+    html,
+    expect,
+} from '@open-wc/testing';
 import '@spectrum-web-components/popover/sp-popover.js';
 import '@spectrum-web-components/action-button/sp-action-button.js';
 import '@spectrum-web-components/icons-workflow/icons/sp-icon-magnify.js';
 import '@spectrum-web-components/popover/sp-popover.js';
 import { OverlayTrigger } from '..';
 import '@spectrum-web-components/overlay/overlay-trigger.js';
 import { spy } from 'sinon';
+import { ActionButton } from '@spectrum-web-components/action-button';
 
 describe('Overlay Trigger - Click', () => {
     it('displays `click` declaratively', async () => {
@@ -51,4 +58,46 @@ describe('Overlay Trigger - Click', () => {
             timeout: 2000,
         });
     });
+    it('opens a second time', async () => {
+        const openedSpy = spy();
+        const closedSpy = spy();
+        const el = await fixture<OverlayTrigger>(html`
+            <overlay-trigger placement="right-start" type="modal" open="click">
+                <sp-action-button
+                    slot="trigger"
+                    @sp-opened=${() => openedSpy()}
+                    @sp-closed=${() => closedSpy()}
+                >
+                    <sp-icon-magnify slot="icon"></sp-icon-magnify>
+                </sp-action-button>
+                <sp-popover slot="click-content" tip></sp-popover>
+            </overlay-trigger>
+        `);
+        await elementUpdated(el);
+        const trigger = el.querySelector('[slot=trigger]') as ActionButton;
+
+        await waitUntil(
+            () => openedSpy.calledOnce,
+            'click content projected to overlay',
+            { timeout: 2000 }
+        );
+        expect(el.open).to.equal('click');
+
+        el.removeAttribute('open');
+        await elementUpdated(el);
+
+        await waitUntil(() => closedSpy.calledOnce, 'click content returned', {
+            timeout: 2000,
+        });
+
+        expect(el.open).to.be.null;
+
+        trigger.click();
+        await waitUntil(
+            () => openedSpy.callCount === 2,
+            'click content projected to overlay, again',
+            { timeout: 2000 }
+        );
+        expect(el.open).to.equal('click');
+    });
 });

packages/overlay/test/overlay-trigger-longpress.test.ts

@@ -76,9 +76,8 @@ describe('Overlay Trigger - Longpress', () => {
             () => !(content.parentElement instanceof OverlayTrigger)
         );
         await waitUntil(() => content.open, 'opens for `Space`');
-        await executeServerCommand('send-keys', {
-            press: 'Escape',
-        });
+        document.body.click();
+
         await waitUntil(() => !content.open, 'closes for `Space`');
         await elementUpdated(el);