From dd0c1193fb410b21a86eb82fad3866f5eb5b7977 Mon Sep 17 00:00:00 2001 From: Ruben Carvalho Date: Tue, 22 Jul 2025 10:55:18 +0100 Subject: [PATCH 1/8] chore: testing overlay fixes --- packages/overlay/src/Overlay.ts | 2 +- packages/overlay/src/OverlayStack.ts | 23 +++- packages/overlay/stories/overlay.stories.ts | 115 ++++++++++++++++++++ packages/picker/src/Picker.ts | 2 +- 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 8535f4db638..0fbef33e2e7 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -431,7 +431,7 @@ export class Overlay extends ComputedOverlayBase { switch (this.type) { case 'modal': - return 'auto'; + return 'manual'; case 'page': return 'manual'; case 'hint': diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index 4c174840f20..b85978b8604 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ /** * Copyright 2025 Adobe. All rights reserved. * This file is licensed to you under the Apache License, Version 2.0 (the "License"); @@ -151,14 +152,14 @@ class OverlayStack { private handleKeydown = (event: KeyboardEvent): void => { if (event.code !== 'Escape') return; + if (event.defaultPrevented) return; // Don't handle if already handled if (!this.stack.length) return; const last = this.stack[this.stack.length - 1]; if (last?.type === 'page') { event.preventDefault(); return; } - if (last?.type === 'manual') { - // Manual overlays should close on "Escape" key, but not when losing focus or interacting with other parts of the page. + if (last?.type === 'manual' || last?.type === 'modal') { this.closeOverlay(last); return; } @@ -213,11 +214,29 @@ class OverlayStack { const path = event.composedPath(); this.stack.forEach((overlayEl) => { const inPath = path.find((el) => el === overlayEl); + + // Check if the trigger element is inside this overlay + const triggerInOverlay = + overlay.triggerElement && + overlay.triggerElement instanceof HTMLElement && + overlayEl.contains && + overlayEl.contains(overlay.triggerElement); + console.log( + 'overlayEl.type:', + overlayEl.type, + 'triggerInOverlay:', + triggerInOverlay, + 'inPath:', + !!inPath + ); + if ( !inPath && + !triggerInOverlay && overlayEl.type !== 'manual' && overlayEl.type !== 'modal' ) { + console.log('Closing overlay:', overlayEl); this.closeOverlay(overlayEl); } }); diff --git a/packages/overlay/stories/overlay.stories.ts b/packages/overlay/stories/overlay.stories.ts index 52031d0cefe..814bd698f47 100644 --- a/packages/overlay/stories/overlay.stories.ts +++ b/packages/overlay/stories/overlay.stories.ts @@ -33,6 +33,7 @@ import '@spectrum-web-components/overlay/overlay-trigger.js'; import '@spectrum-web-components/accordion/sp-accordion-item.js'; import '@spectrum-web-components/accordion/sp-accordion.js'; +import '@spectrum-web-components/action-menu/sp-action-menu.js'; import '@spectrum-web-components/button-group/sp-button-group.js'; import '@spectrum-web-components/menu/sp-menu-divider.js'; import '@spectrum-web-components/menu/sp-menu-group.js'; @@ -634,6 +635,120 @@ export const deepChildTooltip = (): TemplateResult => html` `; +export const debug = (): TemplateResult => { + return html` + + + Button popover + + + + + Deselect + + Select inverse + + Feather... + + Select and mask... + + + + Save selection + + + Make work path + + + + + + I'm a tooltip in a different direction + + + + Open menu + + + Item 1 + Item 2 + + + More Actions + Deselect + Select inverse + Feather... + Select and mask... + + Save selection + Make work path + + + +
+ + Modal overlay + + `; +}; + +// Helper functions for the overlay functionality +const getModalOverlayContent = (): HTMLElement => { + const fragment = document.createDocumentFragment(); + render( + html` + + Modal overlay content + openAutoOverlay(event)}> + Auto overlay + + + `, + fragment + ); + return fragment.children[0] as HTMLElement; +}; + +const getAutoOverlayContent = (): HTMLElement => { + const fragment = document.createDocumentFragment(); + render( + html` + Auto overlay + `, + fragment + ); + return fragment.children[0] as HTMLElement; +}; + +const openModalOverlay = async (event: Event) => { + const trigger = event.target as HTMLElement; + const overlay = await Overlay.open(getModalOverlayContent(), { + trigger, + type: 'modal', + placement: 'bottom', + }); + const container = document.querySelector('.container'); + container?.insertAdjacentElement('afterend', overlay); +}; + +const openAutoOverlay = async (event: Event) => { + const trigger = event.target as HTMLElement; + const overlay = await Overlay.open(getAutoOverlayContent(), { + trigger, + type: 'auto', + placement: 'bottom', + }); + const container = document.querySelector('.container'); + container?.insertAdjacentElement('afterend', overlay); +}; export const deepNesting = (): TemplateResult => { const color = window.__swc_hack_knobs__.defaultColor; diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts index 9c48aaf6452..6daf136c917 100644 --- a/packages/picker/src/Picker.ts +++ b/packages/picker/src/Picker.ts @@ -285,7 +285,7 @@ export class PickerBase extends SizedMixin(SpectrumElement, { protected handleEscape = ( event: MenuItemKeydownEvent | KeyboardEvent ): void => { - if (event.key === 'Escape') { + if (event.key === 'Escape' && this.open) { event.stopPropagation(); event.preventDefault(); this.toggle(false); From e87567cd94d7b675fd2749173ad8c1dc050c647d Mon Sep 17 00:00:00 2001 From: Ruben Carvalho Date: Tue, 22 Jul 2025 11:27:42 +0100 Subject: [PATCH 2/8] chore: allow outside click --- packages/overlay/src/Overlay.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 0fbef33e2e7..9f128c6bcf0 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -542,6 +542,9 @@ export class Overlay extends ComputedOverlayBase { const focusTrap = await import('focus-trap'); this._focusTrap = focusTrap.createFocusTrap(this.dialogEl, { initialFocus: focusEl || undefined, + allowOutsideClick: (event) => { + return !event.isTrusted; + }, tabbableOptions: { getShadowRoot: true, }, From 87868a857a0d2b9e084269ae7ca1f3cd40e5a600 Mon Sep 17 00:00:00 2001 From: Ruben Carvalho Date: Wed, 23 Jul 2025 12:34:05 +0100 Subject: [PATCH 3/8] chore: make hint overlays render hint popovers --- packages/overlay/src/Overlay.ts | 6 ++-- packages/overlay/stories/overlay.stories.ts | 32 ++++++--------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 9f128c6bcf0..38b82a0ddea 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -420,9 +420,9 @@ export class Overlay extends ComputedOverlayBase { * Determines the value for the popover attribute based on the overlay type. * * @private - * @returns {'auto' | 'manual' | undefined} The popover value or undefined if not applicable. + * @returns {'auto' | 'manual' | 'hint' | undefined} The popover value or undefined if not applicable. */ - private get popoverValue(): 'auto' | 'manual' | undefined { + private get popoverValue(): 'auto' | 'manual' | 'hint' | undefined { const hasPopoverAttribute = 'popover' in this; if (!hasPopoverAttribute) { @@ -434,8 +434,6 @@ export class Overlay extends ComputedOverlayBase { return 'manual'; case 'page': return 'manual'; - case 'hint': - return 'manual'; default: return this.type; } diff --git a/packages/overlay/stories/overlay.stories.ts b/packages/overlay/stories/overlay.stories.ts index 814bd698f47..19a9f542a4d 100644 --- a/packages/overlay/stories/overlay.stories.ts +++ b/packages/overlay/stories/overlay.stories.ts @@ -697,6 +697,14 @@ export const debug = (): TemplateResult => { Modal overlay + + Open menu + + + Max 20 + + + `; }; @@ -1183,30 +1191,6 @@ export const modalManaged = (): TemplateResult => { `; }; -export const modalWithinNonModal = (): TemplateResult => { - return html` - - - Open inline overlay - - - - - - Open modal overlay - - - - Modal overlay - - - - - - - `; -}; - export const noCloseOnResize = (args: Properties): TemplateResult => html`