From fe583501c8224a911854103e0550fd36a40f068b Mon Sep 17 00:00:00 2001
From: Westbrook Johnson <westbrook.johnson@gmail.com>
Date: Tue, 21 May 2024 12:33:24 -0400
Subject: [PATCH] fix(overlay): ensure that passing "open" to the directive
 manages a single strategy

---
 packages/overlay/src/InteractionController.ts |  7 +-
 .../overlay/src/overlay-trigger-directive.ts  |  4 +-
 .../stories/overlay-directive.stories.ts      | 97 ++++++++++++++++++-
 3 files changed, 100 insertions(+), 8 deletions(-)

diff --git a/packages/overlay/src/InteractionController.ts b/packages/overlay/src/InteractionController.ts
index de23a9063c..f256daa1a1 100644
--- a/packages/overlay/src/InteractionController.ts
+++ b/packages/overlay/src/InteractionController.ts
@@ -34,14 +34,19 @@ export class InteractionController implements ReactiveController {
 
     private handleOverlayReady?: (overlay: AbstractOverlay) => void;
 
+    // Holds optimistic open state when an Overlay is not yet present
+    private isLazilyOpen = false;
+
     public get open(): boolean {
-        return this.overlay?.open ?? false;
+        return this.overlay?.open ?? this.isLazilyOpen;
     }
 
     /**
      * Set `open` against the associated Overlay lazily.
      */
     public set open(open: boolean) {
+        if (open === this.open) return;
+        this.isLazilyOpen = open;
         if (this.overlay) {
             // If there already is an Overlay, apply the value of `open` directly.
             this.overlay.open = open;
diff --git a/packages/overlay/src/overlay-trigger-directive.ts b/packages/overlay/src/overlay-trigger-directive.ts
index a50b7c00b5..6901dc61fa 100644
--- a/packages/overlay/src/overlay-trigger-directive.ts
+++ b/packages/overlay/src/overlay-trigger-directive.ts
@@ -28,6 +28,7 @@ import {
 } from './slottable-request-event.js';
 import { SlottableRequestDirective } from './slottable-request-directive.js';
 import { AbstractOverlay } from './AbstractOverlay.js';
+import { InteractionTypes } from './InteractionController.js';
 
 export type InsertionOptions = {
     el: HTMLElement | (() => HTMLElement);
@@ -84,8 +85,7 @@ export class OverlayTriggerDirective extends SlottableRequestDirective {
         const triggerInteraction = (options?.triggerInteraction ||
             this.defaultOptions.triggerInteraction) as TriggerInteraction;
         const newStrategy =
-            (this.strategy?.type as unknown as TriggerInteraction) !==
-            triggerInteraction;
+            InteractionTypes[this.strategy?.type] !== triggerInteraction;
         if (this.target !== part.element) {
             this.target = part.element as HTMLElement;
             newTarget = true;
diff --git a/packages/overlay/stories/overlay-directive.stories.ts b/packages/overlay/stories/overlay-directive.stories.ts
index 18741651b7..a18e019e18 100644
--- a/packages/overlay/stories/overlay-directive.stories.ts
+++ b/packages/overlay/stories/overlay-directive.stories.ts
@@ -8,9 +8,14 @@ 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 { html, TemplateResult } from '@spectrum-web-components/base';
+import {
+    html,
+    LitElement,
+    TemplateResult,
+} from '@spectrum-web-components/base';
 import {
     OverlayContentTypes,
+    OverlayOpenCloseDetail,
     Placement,
     TriggerInteractions,
 } from '@spectrum-web-components/overlay';
@@ -46,6 +51,7 @@ import '../../../projects/story-decorator/src/types.js';
 import './overlay-story-components.js';
 import { tooltip } from '@spectrum-web-components/tooltip/src/tooltip-directive.js';
 import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
+import { state } from '@spectrum-web-components/base/src/decorators.js';
 
 const storyStyles = html`
     <style>
@@ -180,10 +186,9 @@ const template = ({
                     </div>
                     <sp-button
                         ${tooltip(
-                            () =>
-                                html`
-                                    Click to open another popover.
-                                `
+                            () => html`
+                                Click to open another popover.
+                            `
                         )}
                         ${trigger(
                             () => html`
@@ -266,3 +271,85 @@ insertionOptions.args = {
 insertionOptions.swc_vrt = {
     skip: true,
 };
+
+class ManagedOverlayTrigger extends LitElement {
+    @state()
+    private isRenderOverlay = false;
+
+    @state()
+    private isOpenState = false;
+
+    protected override render(): TemplateResult {
+        return html`
+            <sp-button
+                @click=${() => {
+                    this.isRenderOverlay = !this.isRenderOverlay;
+                }}
+            >
+                Toggle Overlay Render Button
+            </sp-button>
+
+            <sp-button
+                @click=${() => {
+                    this.isRenderOverlay = true;
+                    this.isOpenState = true;
+                }}
+            >
+                Create Overlay Render Button And Open Overlay
+            </sp-button>
+
+            ${this.isRenderOverlay ? this.renderOverlayButton() : html``}
+        `;
+    }
+
+    private renderOverlayButton(): TemplateResult {
+        return html`
+            <sp-button
+                ?selected=${this.isOpenState}
+                ${trigger(
+                    () => html`
+                        <sp-popover
+                            @sp-opened=${(
+                                event: CustomEvent<OverlayOpenCloseDetail>
+                            ) => {
+                                if (event.target !== event.currentTarget) {
+                                    return;
+                                }
+                                console.log('sp-opened');
+                                this.isOpenState = true;
+                            }}
+                            @sp-closed=${(
+                                event: CustomEvent<OverlayOpenCloseDetail>
+                            ) => {
+                                if (event.target !== event.currentTarget) {
+                                    return;
+                                }
+                                console.log('sp-closed');
+                                this.isOpenState = false;
+                            }}
+                        >
+                            <h1>My Test Popover</h1>
+                        </sp-popover>
+                    `,
+                    {
+                        triggerInteraction: 'click',
+                        overlayOptions: { placement: 'bottom-end' },
+                        open: this.isOpenState,
+                    }
+                )}
+            >
+                Toggle Popover
+            </sp-button>
+        `;
+    }
+}
+
+customElements.define('managed-overlay-trigger', ManagedOverlayTrigger);
+
+export const managedOverlayTrigger = (): TemplateResult => html`
+    <managed-overlay-trigger></managed-overlay-trigger>
+`;
+
+managedOverlayTrigger.swc_vrt = {
+    skip: true,
+};