fix: separated interaction strategy for picker

TarunAdobe · TarunAdobe · commit 73473ab10440 · 2024-05-30T18:20:36.000+05:30
diff --git a/packages/picker/src/DesktopController.ts b/packages/picker/src/DesktopController.ts
@@ -16,25 +16,14 @@ import {
 } from './InteractionController.js';
 
 export class DesktopController extends InteractionController {
-    override type = InteractionTypes.click;
+    override type = InteractionTypes.desktop;
 
-    /**
-     * An overlay with a `click` interaction should not close on click `triggerElement`.
-     * When a click is initiated (`pointerdown`), apply `preventNextToggle` when the
-     * overlay is `open` to prevent from toggling the overlay when the click event
-     * propagates later in the interaction.
-     */
-    // private preventNextToggle: 'no' | 'maybe' | 'yes' = 'no';
-    // private pointerdownState = false;
-
-    protected handlePointerdown(event: PointerEvent): void {
+    public override handlePointerdown(event: PointerEvent): void {
         if (event.button !== 0) {
             return;
         }
-        this.host.pointerdownState = this.open;
-        // this.pointerdownState = this.open;
-        // this.preventNextToggle = 'maybe';
-        this.host.preventNextToggle = 'maybe';
+        this.pointerdownState = this.open;
+        this.preventNextToggle = 'maybe';
         let cleanupAction = 0;
         const cleanup = (): void => {
             cancelAnimationFrame(cleanupAction);
@@ -45,7 +34,7 @@ export class DesktopController extends InteractionController {
                 requestAnimationFrame(() => {
                     // Complete cleanup on the second animation frame so that `click` can go first.
                     // this.preventNextToggle = 'no';
-                    this.host.preventNextToggle = 'no';
+                    this.preventNextToggle = 'no';
                 });
             });
         };
@@ -56,34 +45,19 @@ export class DesktopController extends InteractionController {
         this.handleActivate();
     }
 
-    protected handleButtonFocus(event: FocusEvent): void {
-        // When focus comes from a pointer event, and the related target is the Menu,
-        // we don't want to reopen the Menu.
-        if (
-            this.host.preventNextToggle === 'maybe' &&
-            event.relatedTarget === this.host.optionsMenu
-        ) {
-            // this.preventNextToggle = 'yes';
-            this.host.preventNextToggle = 'yes';
-        }
-    }
-
-    protected handleActivate(event?: Event): void {
-        if (this.enterKeydownOn && this.enterKeydownOn !== this.button) {
+    public override handleActivate(event?: Event): void {
+        if (this.enterKeydownOn && this.enterKeydownOn !== this.target) {
             return;
         }
-        if (this.host.preventNextToggle === 'yes') {
+        if (this.preventNextToggle === 'yes') {
             return;
         }
-        if (
-            event?.type === 'click' &&
-            this.open !== this.host.pointerdownState
-        ) {
+        if (event?.type === 'click' && this.open !== this.pointerdownState) {
             // When activation comes from a `click` event ensure that the `pointerup`
             // event didn't already toggle the Picker state before doing so.
             return;
         }
-        this.host.toggle();
+        this.open = !this.open;
     }
 
     override init(): void {
@@ -93,7 +67,7 @@ export class DesktopController extends InteractionController {
         const { signal } = this.abortController;
         this.target.addEventListener(
             'click',
-            (event: PointerEvent) => this.handleActivate(event),
+            (event: Event) => this.handleActivate(event),
             {
                 signal,
             }
diff --git a/packages/picker/src/InteractionController.ts b/packages/picker/src/InteractionController.ts
@@ -10,9 +10,13 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */
 
-import type { ReactiveController } from '@spectrum-web-components/base';
+import {
+    ReactiveController,
+    TemplateResult,
+} from '@spectrum-web-components/base';
 import { AbstractOverlay } from '@spectrum-web-components/overlay/src/AbstractOverlay';
-import { PickerBase } from './PickerBase.js';
+import { Overlay } from '@spectrum-web-components/overlay/src/Overlay.js';
+import { PickerBase } from './Picker.js';
 
 export enum InteractionTypes {
     'desktop',
@@ -22,47 +26,143 @@ export enum InteractionTypes {
 export class InteractionController implements ReactiveController {
     abortController!: AbortController;
 
+    public preventNextToggle: 'no' | 'maybe' | 'yes' = 'no';
+    public pointerdownState = false;
+    public enterKeydownOn: EventTarget | null = null;
+
+    public container!: TemplateResult;
+
     get activelyOpening(): boolean {
         return false;
     }
 
-    private handleOverlayReady?: (overlay: AbstractOverlay) => void;
+    private _open = false;
 
     public get open(): boolean {
-        return this.host.open;
+        return this._open;
     }
 
     /**
      * Set `open`
      */
     public set open(open: boolean) {
-        this.host.open = open;
+        this._open = open;
+        if (this.overlay) {
+            // If there already is an Overlay, apply the value of `open` directly.
+            this.overlay.open = open;
+            this.host.open = open;
+            return;
+        }
+        if (!open) {
+            this.host.open = open;
+            // When `open` moves to `false` and there is not yet an Overlay,
+            // assume that no Overlay and a closed Overlay are the same and return early.
+            return;
+        }
+        // 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.overlay.open = true;
+                this.host.open = true;
+            });
+        import('@spectrum-web-components/overlay/sp-overlay.js');
+    }
+
+    private _overlay!: AbstractOverlay;
+
+    public get overlay(): AbstractOverlay {
+        return this._overlay;
     }
 
-    toggle(target?: boolean): void {
-        this.host.toggle(target);
+    public set overlay(overlay: AbstractOverlay | undefined) {
+        if (!overlay) return;
+        if (this.overlay === overlay) return;
+        this._overlay = overlay;
+        this.initOverlay();
     }
 
     type!: InteractionTypes;
 
     constructor(
         public target: HTMLElement,
-        public overlay: AbstractOverlay | undefined,
         public host: PickerBase
     ) {
         this.target = target;
-        this.overlay = overlay;
         this.host = host;
         this.init();
     }
 
     releaseDescription(): void {}
 
-    /* c8 ignore next 3 */
-    init(): void {
-        // Abstract init() method.
+    protected handleBeforetoggle(
+        event: Event & {
+            target: Overlay;
+            newState: 'open' | 'closed';
+        }
+    ): void {
+        if (event.composedPath()[0] !== event.target) {
+            return;
+        }
+        if (event.newState === 'closed') {
+            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();
+            }
+        }
+        if (!this.open) {
+            this.host.optionsMenu.updateSelectedItemIndex();
+            this.host.optionsMenu.closeDescendentOverlays();
+        }
     }
 
+    initOverlay(): void {
+        if (this.overlay) {
+            this.overlay.addEventListener('beforetoggle', (event: Event) => {
+                this.handleBeforetoggle(
+                    event as Event & {
+                        target: Overlay;
+                        newState: 'open' | 'closed';
+                    }
+                );
+            });
+
+            this.overlay.triggerElement = this.host as HTMLElement;
+            this.overlay.placement = this.host.isMobile.matches
+                ? undefined
+                : this.host.placement;
+            this.overlay.receivesFocus = 'true';
+            this.overlay.willPreventClose =
+                this.preventNextToggle !== 'no' && this.open;
+        }
+    }
+
+    public handlePointerdown(_event: PointerEvent): void {}
+
+    public handleButtonFocus(event: FocusEvent): void {
+        // When focus comes from a pointer event, and the related target is the Menu,
+        // we don't want to reopen the Menu.
+        if (
+            this.preventNextToggle === 'maybe' &&
+            event.relatedTarget === this.host.optionsMenu
+        ) {
+            this.preventNextToggle = 'yes';
+        }
+    }
+
+    public handleActivate(_event: Event): void {}
+
+    /* c8 ignore next 3 */
+    init(): void {}
+
     abort(): void {
         this.releaseDescription();
         this.abortController?.abort();
@@ -73,8 +173,6 @@ export class InteractionController implements ReactiveController {
     }
 
     hostDisconnected(): void {
-        if (!this.isPersistent) {
-            this.abort();
-        }
+        this.abortController?.abort();
     }
 }
diff --git a/packages/picker/src/MobileController.ts b/packages/picker/src/MobileController.ts
@@ -15,25 +15,17 @@ import {
 } from './InteractionController.js';
 
 export class MobileController extends InteractionController {
-    override type = InteractionTypes.click;
-
-    /**
-     * An overlay with a `click` interaction should not close on click `triggerElement`.
-     * When a click is initiated (`pointerdown`), apply `preventNextToggle` when the
-     * overlay is `open` to prevent from toggling the overlay when the click event
-     * propagates later in the interaction.
-     */
-    private preventNextToggle = false;
+    override type = InteractionTypes.mobile;
 
     handleClick(): void {
-        if (!this.preventNextToggle) {
-            this.host.open = !this.host.open;
+        if (this.preventNextToggle == 'no') {
+            this.open = !this.open;
         }
-        this.preventNextToggle = false;
+        this.preventNextToggle = 'no';
     }
 
-    handlePointerdown(): void {
-        this.preventNextToggle = this.host.open;
+    public override handlePointerdown(): void {
+        this.preventNextToggle = this.open ? 'yes' : 'no';
     }
 
     override init(): void {
@@ -49,5 +41,12 @@ export class MobileController extends InteractionController {
             () => this.handlePointerdown(),
             { signal }
         );
+        this.target.addEventListener(
+            'focus',
+            (event: FocusEvent) => this.handleButtonFocus(event),
+            {
+                signal,
+            }
+        );
     }
 }
diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts
