From 9dcf37a6f3006fca4fdb8a3fb2114cdeada59d9f Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 9 Jul 2025 09:20:20 -0700 Subject: [PATCH 01/27] fix(tabs,textfield): patch --- packages/number-field/src/NumberField.ts | 1 + packages/tabs/src/Tabs.ts | 6 +- packages/tabs/src/TabsOverflow.ts | 2 +- packages/textfield/src/Textfield.ts | 140 +++++++++++++++++++++++ packages/textfield/src/textfield.css | 11 ++ 5 files changed, 156 insertions(+), 4 deletions(-) diff --git a/packages/number-field/src/NumberField.ts b/packages/number-field/src/NumberField.ts index 44dbca28959..0b408128aca 100644 --- a/packages/number-field/src/NumberField.ts +++ b/packages/number-field/src/NumberField.ts @@ -817,6 +817,7 @@ export class NumberField extends TextfieldBase { } protected override updated(changes: PropertyValues): void { + super.updated(changes); if (!this.inputElement || !this.isConnected) { // Prevent race conditions if inputElement is removed from DOM while a queued update is still running. return; diff --git a/packages/tabs/src/Tabs.ts b/packages/tabs/src/Tabs.ts index a48c849c2b0..3c410ee7492 100644 --- a/packages/tabs/src/Tabs.ts +++ b/packages/tabs/src/Tabs.ts @@ -161,7 +161,7 @@ export class Tabs extends SizedMixin(Focusable, { noDefaultSize: true }) { private slotEl!: HTMLSlotElement; @query('#list') - private tabList!: HTMLDivElement; + protected tabList!: HTMLDivElement; @property({ reflect: true }) selected = ''; @@ -178,7 +178,7 @@ export class Tabs extends SizedMixin(Focusable, { noDefaultSize: true }) { this.rovingTabindexController.clearElementCache(); } - private get tabs(): Tab[] { + protected get tabs(): Tab[] { return this._tabs; } @@ -300,7 +300,7 @@ export class Tabs extends SizedMixin(Focusable, { noDefaultSize: true }) { return complete; } - private getNecessaryAutoScroll(index: number): number { + protected getNecessaryAutoScroll(index: number): number { const selectedTab = this.tabs[index]; const selectionEnd = selectedTab.offsetLeft + selectedTab.offsetWidth; const viewportEnd = this.tabList.scrollLeft + this.tabList.offsetWidth; diff --git a/packages/tabs/src/TabsOverflow.ts b/packages/tabs/src/TabsOverflow.ts index bd261144998..5725b2d1a8e 100644 --- a/packages/tabs/src/TabsOverflow.ts +++ b/packages/tabs/src/TabsOverflow.ts @@ -101,7 +101,7 @@ export class TabsOverflow extends SizedMixin(SpectrumElement) { this._updateScrollState(); } - private _updateScrollState(): void { + protected _updateScrollState(): void { const { scrollContent, overflowState } = this; if (scrollContent) { diff --git a/packages/textfield/src/Textfield.ts b/packages/textfield/src/Textfield.ts index 44476c9fa3d..16caa01434e 100644 --- a/packages/textfield/src/Textfield.ts +++ b/packages/textfield/src/Textfield.ts @@ -27,6 +27,7 @@ import { query, state, } from '@spectrum-web-components/base/src/decorators.js'; +import { isWebKit } from '@spectrum-web-components/shared'; import { ManageHelpText } from '@spectrum-web-components/help-text/src/manage-help-text.js'; import { Focusable } from '@spectrum-web-components/shared/src/focusable.js'; @@ -55,6 +56,14 @@ export class TextfieldBase extends ManageHelpText( @state() protected appliedLabel?: string; + private _firstUpdateAfterConnected = true; + + private _inputRef?: HTMLInputElement | HTMLTextAreaElement; + private _formRef?: HTMLFormElement; + + @query('#form-wrapper') + protected formElement!: HTMLFormElement; + /** * A regular expression outlining the keys that will be allowed to update the value of the form control. */ @@ -203,6 +212,9 @@ export class TextfieldBase extends ManageHelpText( | HTMLTextAreaElement['autocomplete']; public override get focusElement(): HTMLInputElement | HTMLTextAreaElement { + if (isWebKit()) { + return this._inputRef ?? this.inputElement; + } return this.inputElement; } @@ -272,6 +284,104 @@ export class TextfieldBase extends ManageHelpText( protected handleInputElementPointerdown(): void {} + protected handleInputSubmit(event: Event): void { + this.dispatchEvent( + new Event('submit', { + cancelable: true, + bubbles: true, + }) + ); + event.preventDefault(); + } + + private _eventHandlers = { + input: this.handleInput.bind(this), + change: this.handleChange.bind(this), + focus: this.onFocus.bind(this), + blur: this.onBlur.bind(this), + submit: this.handleInputSubmit.bind(this), + }; + + protected firstUpdateAfterConnected(): void { + this._inputRef = this.inputElement; + if (this.formElement) { + this._formRef = this.formElement; + this.formElement.addEventListener( + 'submit', + this._eventHandlers.submit + ); + this.inputElement.addEventListener( + 'change', + this._eventHandlers['change'] + ); + this.inputElement.addEventListener( + 'input', + this._eventHandlers['input'] + ); + this.inputElement.addEventListener( + 'focus', + this._eventHandlers['focus'] + ); + this.inputElement.addEventListener( + 'blur', + this._eventHandlers['blur'] as EventListener + ); + } + } + + protected override updated(changes: PropertyValues): void { + super.updated(changes); + if (isWebKit() && this._firstUpdateAfterConnected) { + this._firstUpdateAfterConnected = false; + this.firstUpdateAfterConnected(); + } + } + + override connectedCallback(): void { + super.connectedCallback(); + if (isWebKit()) { + this._firstUpdateAfterConnected = true; + if (this._formRef) { + const formContainer = + this.shadowRoot.querySelector('#form-container'); + if (formContainer) { + formContainer.appendChild(this._formRef); + this.requestUpdate(); + } + this._formRef = undefined; + } + } + } + + override disconnectedCallback(): void { + if (isWebKit()) { + this._inputRef?.removeEventListener( + 'change', + this._eventHandlers['change'] + ); + this._inputRef?.removeEventListener( + 'input', + this._eventHandlers['input'] + ); + this._inputRef?.removeEventListener( + 'focus', + this._eventHandlers['focus'] + ); + this._inputRef?.removeEventListener( + 'blur', + this._eventHandlers['blur'] as EventListener + ); + if (this._formRef) { + this._formRef.remove(); + this._formRef.removeEventListener( + 'submit', + this._eventHandlers.submit + ); + } + } + super.disconnectedCallback(); + } + protected renderStateIcons(): TemplateResult | typeof nothing { if (this.invalid) { return html` @@ -334,6 +444,36 @@ export class TextfieldBase extends ManageHelpText( } private get renderInput(): TemplateResult { + if (isWebKit()) { + return html` + +
+
+ -1 ? this.maxlength : undefined + )} + minlength=${ifDefined( + this.minlength > -1 ? this.minlength : undefined + )} + pattern=${ifDefined(this.pattern)} + placeholder=${this.placeholder} + .value=${live(this.displayValue)} + ?disabled=${this.disabled} + ?required=${this.required} + ?readonly=${this.readonly} + autocomplete=${ifDefined(this.autocomplete)} + /> +
+
+ `; + } return html` Date: Thu, 10 Jul 2025 14:45:42 -0700 Subject: [PATCH 02/27] fix(typography): delete heading margin --- tools/styles/src/spectrum-heading.css | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/styles/src/spectrum-heading.css b/tools/styles/src/spectrum-heading.css index a2e68138fda..eca19b62de8 100644 --- a/tools/styles/src/spectrum-heading.css +++ b/tools/styles/src/spectrum-heading.css @@ -69,8 +69,6 @@ governing permissions and limitations under the License. --spectrum-heading-cjk-font-family: var(--spectrum-cjk-font-family-stack); --spectrum-heading-cjk-letter-spacing: var(--spectrum-cjk-letter-spacing); --spectrum-heading-font-color: var(--spectrum-heading-color); - --spectrum-heading-margin-start: calc(var(--mod-heading-font-size, var(--spectrum-heading-font-size)) * var(--spectrum-heading-margin-top-multiplier)); - --spectrum-heading-margin-end: calc(var(--mod-heading-font-size, var(--spectrum-heading-font-size)) * var(--spectrum-heading-margin-bottom-multiplier)); font-family: var(--mod-heading-sans-serif-font-family, var(--spectrum-heading-sans-serif-font-family)); font-style: var(--mod-heading-sans-serif-font-style, var(--spectrum-heading-sans-serif-font-style)); font-weight: var(--mod-heading-sans-serif-font-weight, var(--spectrum-heading-sans-serif-font-weight)); From 31d074c7c34eca5cadcb4c87864af24d4e1b17ca Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 14 Jul 2025 14:50:26 -0700 Subject: [PATCH 03/27] fix(overlay): use manual popover for modal overlays --- packages/overlay/src/Overlay.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 9ec533d19a0..8f912044019 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': From e26cabf265a3798a976d897161a82ab00c4f13c2 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 16 Jul 2025 11:34:23 -0700 Subject: [PATCH 04/27] fix(card): revert style --- packages/card/src/spectrum-card.css | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/card/src/spectrum-card.css b/packages/card/src/spectrum-card.css index 2c1a7a08a0a..70dae996f06 100644 --- a/packages/card/src/spectrum-card.css +++ b/packages/card/src/spectrum-card.css @@ -68,7 +68,6 @@ governing permissions and limitations under the License. :host { box-sizing: border-box; min-inline-size: var(--mod-card-minimum-width, var(--spectrum-card-minimum-width)); - block-size: 100%; border: var(--spectrum-card-border-width) solid transparent; border-radius: var(--spectrum-card-corner-radius); border-color: var(--mod-card-border-color, var(--spectrum-card-border-color)); From dfbf32cac56c7e586a1f42203ffda24ba31fe40c Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 16 Jul 2025 13:31:50 -0700 Subject: [PATCH 05/27] fix(textfield): fix patch --- packages/textfield/src/Textfield.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/textfield/src/Textfield.ts b/packages/textfield/src/Textfield.ts index 16caa01434e..aa60220c790 100644 --- a/packages/textfield/src/Textfield.ts +++ b/packages/textfield/src/Textfield.ts @@ -76,7 +76,7 @@ export class TextfieldBase extends ManageHelpText( @property({ type: Boolean, reflect: true }) public focused = false; - @query('.input:not(#sizer)') + @query('input, input.input:not(#sizer), textarea.input:not(#sizer)') protected inputElement!: HTMLInputElement | HTMLTextAreaElement; /** From fc7d69c5c9493234868f434d0e17aafbf73d42eb Mon Sep 17 00:00:00 2001 From: Helen Le Date: Thu, 17 Jul 2025 16:21:32 -0700 Subject: [PATCH 06/27] fix(tooltip): fix mod token --- packages/tooltip/src/tooltip.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tooltip/src/tooltip.css b/packages/tooltip/src/tooltip.css index 2771d3b81e3..2d7ad53d616 100644 --- a/packages/tooltip/src/tooltip.css +++ b/packages/tooltip/src/tooltip.css @@ -20,7 +20,7 @@ governing permissions and limitations under the License. #tooltip { width: fit-content; white-space: initial; - max-width: var(--spectrum-tooltip-max-inline-size); + max-width: var(--mod-tooltip-max-inline-size, var(--spectrum-tooltip-max-inline-size)); } #tip { From 5a8c1a8790e53321b9553b8d6b7df5e0de95e8d5 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 21 Jul 2025 14:22:26 -0700 Subject: [PATCH 07/27] fix(overlay): close modal overlays on esc --- packages/overlay/src/OverlayStack.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index 71c4d9b0446..389f170a2c2 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -156,8 +156,8 @@ class OverlayStack { 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') { + // Manual and modal overlays should close on "Escape" key, but not when losing focus or interacting with other parts of the page. this.closeOverlay(last); return; } From 4ad32e80e4a8cc52b863be4c57fb9a104a3888f2 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Tue, 22 Jul 2025 09:10:00 -0700 Subject: [PATCH 08/27] fix(overlay): allow programmatic clicks --- packages/overlay/src/Overlay.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 8f912044019..f83178e7ec8 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -552,6 +552,10 @@ export class Overlay extends ComputedOverlayBase { }, // disable escape key capture to close the overlay, the focus-trap library captures it otherwise escapeDeactivates: false, + // allow events generated by programmatic methods, such as .click() + allowOutsideClick: (event: Event) => { + return !event.isTrusted; + }, }); if (this.type === 'modal' || this.type === 'page') { From 7f17e8506abc73751793a9d973541f189f881fd4 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Tue, 22 Jul 2025 11:54:00 -0700 Subject: [PATCH 09/27] chore(picker): patch --- packages/picker/src/Picker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts index 3715dc4b5d5..6d61c9180f3 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 69e9f856ab05ba3838a80037e857648d3c210a1e Mon Sep 17 00:00:00 2001 From: Helen Le Date: Tue, 22 Jul 2025 12:00:43 -0700 Subject: [PATCH 10/27] fix(overlay): more patches --- packages/overlay/src/OverlayStack.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index 389f170a2c2..ca563380d87 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -150,6 +150,7 @@ 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') { @@ -157,7 +158,6 @@ class OverlayStack { return; } if (last?.type === 'manual' || last?.type === 'modal') { - // Manual and modal overlays should close on "Escape" key, but not when losing focus or interacting with other parts of the page. this.closeOverlay(last); return; } @@ -212,8 +212,17 @@ 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); + if ( !inPath && + !triggerInOverlay && overlayEl.type !== 'manual' && overlayEl.type !== 'modal' ) { From 37b76792e551780144cd7643ea0fafc5349d35b1 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 23 Jul 2025 12:57:01 -0700 Subject: [PATCH 11/27] fix(typography): patch body --- tools/styles/src/spectrum-detail.css | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/styles/src/spectrum-detail.css b/tools/styles/src/spectrum-detail.css index 6fc55f888d3..8e1d86ef59c 100644 --- a/tools/styles/src/spectrum-detail.css +++ b/tools/styles/src/spectrum-detail.css @@ -43,8 +43,6 @@ governing permissions and limitations under the License. --spectrum-detail-sans-serif-font-family: var(--spectrum-sans-font-family-stack); --spectrum-detail-serif-font-family: var(--spectrum-serif-font-family-stack); --spectrum-detail-cjk-font-family: var(--spectrum-cjk-font-family-stack); - --spectrum-detail-margin-start: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-top-multiplier)); - --spectrum-detail-margin-end: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-bottom-multiplier)); --spectrum-detail-font-color: var(--spectrum-detail-color); font-family: var(--mod-detail-sans-serif-font-family, var(--spectrum-detail-sans-serif-font-family)); font-style: var(--mod-detail-sans-serif-font-style, var(--spectrum-detail-sans-serif-font-style)); From 909fe6096cb3159fcf296ad2f14f769616181c45 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 23 Jul 2025 13:16:26 -0700 Subject: [PATCH 12/27] fix(overlay): apply more patches --- packages/overlay/package.json | 1 + packages/overlay/src/Overlay.ts | 22 +++++++++++++++++----- yarn.lock | 1 + 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/overlay/package.json b/packages/overlay/package.json index b0500e5a3a8..14d78ac0b74 100644 --- a/packages/overlay/package.json +++ b/packages/overlay/package.json @@ -171,6 +171,7 @@ "@spectrum-web-components/reactive-controllers": "1.7.0", "@spectrum-web-components/shared": "1.7.0", "@spectrum-web-components/theme": "1.7.0", + "@spectrum-web-components/underlay": "1.7.0", "focus-trap": "^7.6.4" }, "types": "./src/index.d.ts", diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index f83178e7ec8..8b543e2fb2a 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -55,6 +55,7 @@ import { import styles from './overlay.css.js'; import { FocusTrap } from 'focus-trap'; +import '@spectrum-web-components/underlay/sp-underlay.js'; const browserSupportsPopover = 'showPopover' in document.createElement('div'); @@ -420,9 +421,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 +435,6 @@ export class Overlay extends ComputedOverlayBase { return 'manual'; case 'page': return 'manual'; - case 'hint': - return 'manual'; default: return this.type; } @@ -558,7 +557,9 @@ export class Overlay extends ComputedOverlayBase { }, }); - if (this.type === 'modal' || this.type === 'page') { + if (this.type === 'modal' || this.type === 'page' && + this.receivesFocus !== 'false' + ) { this._focusTrap.activate(); } } @@ -1145,6 +1146,17 @@ export class Overlay extends ComputedOverlayBase { */ public override render(): TemplateResult { return html` + ${this.type === 'modal' || this.type === 'page' + ? html` + { + this.open = false; + }} + style="--spectrum-underlay-background-color: transparent" + > + ` + : ''} ${this.renderPopover()} `; diff --git a/yarn.lock b/yarn.lock index 8200cdf86aa..2c68d8beacb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6960,6 +6960,7 @@ __metadata: "@spectrum-web-components/reactive-controllers": "npm:1.7.0" "@spectrum-web-components/shared": "npm:1.7.0" "@spectrum-web-components/theme": "npm:1.7.0" + "@spectrum-web-components/underlay": "npm:1.7.0" focus-trap: "npm:^7.6.4" languageName: unknown linkType: soft From 8fbadec554ab3c24fc112c7ce0ef1b271514b5a8 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Thu, 24 Jul 2025 15:35:44 -0700 Subject: [PATCH 13/27] fix(styles): remove margin --- tools/styles/fonts.css | 4 ---- tools/styles/typography.css | 2 -- 2 files changed, 6 deletions(-) diff --git a/tools/styles/fonts.css b/tools/styles/fonts.css index 22270fbe70b..a7322d5e8ef 100755 --- a/tools/styles/fonts.css +++ b/tools/styles/fonts.css @@ -73,8 +73,6 @@ --spectrum-heading-cjk-font-family: var(--spectrum-cjk-font-family-stack); --spectrum-heading-cjk-letter-spacing: var(--spectrum-cjk-letter-spacing); --spectrum-heading-font-color: var(--spectrum-heading-color); - --spectrum-heading-margin-start: calc(var(--mod-heading-font-size, var(--spectrum-heading-font-size)) * var(--spectrum-heading-margin-top-multiplier)); - --spectrum-heading-margin-end: calc(var(--mod-heading-font-size, var(--spectrum-heading-font-size)) * var(--spectrum-heading-margin-bottom-multiplier)); font-family: var(--mod-heading-sans-serif-font-family, var(--spectrum-heading-sans-serif-font-family)); font-style: var(--mod-heading-sans-serif-font-style, var(--spectrum-heading-sans-serif-font-style)); font-weight: var(--mod-heading-sans-serif-font-weight, var(--spectrum-heading-sans-serif-font-weight)); @@ -502,8 +500,6 @@ --spectrum-detail-sans-serif-font-family: var(--spectrum-sans-font-family-stack); --spectrum-detail-serif-font-family: var(--spectrum-serif-font-family-stack); --spectrum-detail-cjk-font-family: var(--spectrum-cjk-font-family-stack); - --spectrum-detail-margin-start: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-top-multiplier)); - --spectrum-detail-margin-end: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-bottom-multiplier)); --spectrum-detail-font-color: var(--spectrum-detail-color); font-family: var(--mod-detail-sans-serif-font-family, var(--spectrum-detail-sans-serif-font-family)); font-style: var(--mod-detail-sans-serif-font-style, var(--spectrum-detail-sans-serif-font-style)); diff --git a/tools/styles/typography.css b/tools/styles/typography.css index 22270fbe70b..6d88c9803fd 100644 --- a/tools/styles/typography.css +++ b/tools/styles/typography.css @@ -502,8 +502,6 @@ --spectrum-detail-sans-serif-font-family: var(--spectrum-sans-font-family-stack); --spectrum-detail-serif-font-family: var(--spectrum-serif-font-family-stack); --spectrum-detail-cjk-font-family: var(--spectrum-cjk-font-family-stack); - --spectrum-detail-margin-start: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-top-multiplier)); - --spectrum-detail-margin-end: calc(var(--mod-detail-font-size, var(--spectrum-detail-font-size)) * var(--spectrum-detail-margin-bottom-multiplier)); --spectrum-detail-font-color: var(--spectrum-detail-color); font-family: var(--mod-detail-sans-serif-font-family, var(--spectrum-detail-sans-serif-font-family)); font-style: var(--mod-detail-sans-serif-font-style, var(--spectrum-detail-sans-serif-font-style)); From a9f50205ee624f35041b070d389f37715e6b9f69 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Thu, 24 Jul 2025 15:36:17 -0700 Subject: [PATCH 14/27] fix(dialog): dispatch close event instantly --- packages/dialog/src/DialogBase.ts | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/packages/dialog/src/DialogBase.ts b/packages/dialog/src/DialogBase.ts index 81eb5396a80..bca9a1b80a4 100644 --- a/packages/dialog/src/DialogBase.ts +++ b/packages/dialog/src/DialogBase.ts @@ -24,7 +24,7 @@ import '@spectrum-web-components/underlay/sp-underlay.js'; import '@spectrum-web-components/button/sp-button.js'; // Leveraged in build systems that use aliasing to prevent multiple registrations: https://github.com/adobe/spectrum-web-components/pull/3225 -import '@spectrum-web-components/dialog/sp-dialog.js'; +import '../sp-dialog.js'; import modalWrapperStyles from '@spectrum-web-components/modal/src/modal-wrapper.css.js'; import modalStyles from '@spectrum-web-components/modal/src/modal.css.js'; import { Dialog } from './Dialog.js'; @@ -155,41 +155,18 @@ export class DialogBase extends FocusVisiblePolyfillMixin(SpectrumElement) { this.handleTransitionEvent(event); } - private get hasTransitionDuration(): boolean { - const modal = this.shadowRoot.querySelector('.modal') as HTMLElement; - - const modalTransitionDurations = - window.getComputedStyle(modal).transitionDuration; - for (const duration of modalTransitionDurations.split(',')) - if (parseFloat(duration) > 0) return true; - - const underlay = this.shadowRoot.querySelector( - 'sp-underlay' - ) as HTMLElement; - - if (underlay) { - const underlayTransitionDurations = - window.getComputedStyle(underlay).transitionDuration; - for (const duration of underlayTransitionDurations.split(',')) - if (parseFloat(duration) > 0) return true; - } - - return false; - } - protected override update(changes: PropertyValues): void { if (changes.has('open') && changes.get('open') !== undefined) { - const hasTransitionDuration = this.hasTransitionDuration; this.animating = true; this.transitionPromise = new Promise((res) => { this.resolveTransitionPromise = () => { this.animating = false; - if (!this.open && hasTransitionDuration) - this.dispatchClosed(); res(); }; }); - if (!this.open && !hasTransitionDuration) this.dispatchClosed(); + if (!this.open) { + this.dispatchClosed(); + } } super.update(changes); } From ea996c7fdb098254668260ab15ab4dd5bc9ff0da Mon Sep 17 00:00:00 2001 From: Helen Le Date: Thu, 24 Jul 2025 15:36:57 -0700 Subject: [PATCH 15/27] chore(dialog): comment --- packages/dialog/src/DialogBase.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/dialog/src/DialogBase.ts b/packages/dialog/src/DialogBase.ts index bca9a1b80a4..decbdc1ff13 100644 --- a/packages/dialog/src/DialogBase.ts +++ b/packages/dialog/src/DialogBase.ts @@ -24,6 +24,7 @@ import '@spectrum-web-components/underlay/sp-underlay.js'; import '@spectrum-web-components/button/sp-button.js'; // Leveraged in build systems that use aliasing to prevent multiple registrations: https://github.com/adobe/spectrum-web-components/pull/3225 +// Get around lint error by importing locally for now. Not required for actual change. import '../sp-dialog.js'; import modalWrapperStyles from '@spectrum-web-components/modal/src/modal-wrapper.css.js'; import modalStyles from '@spectrum-web-components/modal/src/modal.css.js'; From fb7f00809a27a40f8f52369e98f976fcc19cc46b Mon Sep 17 00:00:00 2001 From: Helen Le Date: Fri, 25 Jul 2025 15:13:41 -0700 Subject: [PATCH 16/27] chore(overlay): fix patch --- packages/overlay/src/Overlay.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 8b543e2fb2a..d44c6b3a3ca 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -557,7 +557,8 @@ export class Overlay extends ComputedOverlayBase { }, }); - if (this.type === 'modal' || this.type === 'page' && + if ( + (this.type === 'modal' || this.type === 'page') && this.receivesFocus !== 'false' ) { this._focusTrap.activate(); From 87702a54ae4151e1fe83ce06cdde1f793b01243e Mon Sep 17 00:00:00 2001 From: Helen Le Date: Fri, 25 Jul 2025 15:13:52 -0700 Subject: [PATCH 17/27] fix(overlay): block scroll --- packages/overlay/src/OverlayStack.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index ca563380d87..258477d419e 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -26,6 +26,10 @@ class OverlayStack { stack: Overlay[] = []; + private originalBodyOverflow = ''; + + private bodyScrollBlocked = false; + constructor() { this.bindEvents(); } @@ -78,8 +82,27 @@ class OverlayStack { this.stack.splice(overlayIndex, 1); } overlay.open = false; + + this.manageBodyScroll(); } + /** + * Manage body scroll blocking based on modal/page overlays + */ + private manageBodyScroll(): void { + const shouldBlock = this.stack.some( + (overlay) => overlay.type === 'modal' || overlay.type === 'page' + ); + if (shouldBlock && !this.bodyScrollBlocked) { + this.originalBodyOverflow = document.body.style.overflow || ''; + document.body.style.overflow = 'hidden'; + this.bodyScrollBlocked = true; + } else if (!shouldBlock && this.bodyScrollBlocked) { + document.body.style.overflow = this.originalBodyOverflow; + this.bodyScrollBlocked = false; + } + } + /** * Cach the `pointerdownTarget` for later testing * @@ -256,6 +279,7 @@ class OverlayStack { overlay.addEventListener('beforetoggle', this.handleBeforetoggle, { once: true, }); + this.manageBodyScroll(); }); } From f8d2d6554d1818ca35b28e562ab3f67e46c7dbdb Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 08:47:18 -0700 Subject: [PATCH 18/27] chore(overlay): revert forward fixes for overlay --- packages/overlay/package.json | 1 - packages/overlay/src/Overlay.ts | 51 ++++++++---------------- packages/overlay/src/OverlayStack.ts | 58 +++++++--------------------- packages/picker/src/Picker.ts | 2 +- yarn.lock | 1 - 5 files changed, 31 insertions(+), 82 deletions(-) diff --git a/packages/overlay/package.json b/packages/overlay/package.json index 14d78ac0b74..b0500e5a3a8 100644 --- a/packages/overlay/package.json +++ b/packages/overlay/package.json @@ -171,7 +171,6 @@ "@spectrum-web-components/reactive-controllers": "1.7.0", "@spectrum-web-components/shared": "1.7.0", "@spectrum-web-components/theme": "1.7.0", - "@spectrum-web-components/underlay": "1.7.0", "focus-trap": "^7.6.4" }, "types": "./src/index.d.ts", diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index d44c6b3a3ca..8535f4db638 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -1,14 +1,14 @@ -/* -Copyright 2023 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { html, PropertyValues, @@ -55,7 +55,6 @@ import { import styles from './overlay.css.js'; import { FocusTrap } from 'focus-trap'; -import '@spectrum-web-components/underlay/sp-underlay.js'; const browserSupportsPopover = 'showPopover' in document.createElement('div'); @@ -421,9 +420,9 @@ export class Overlay extends ComputedOverlayBase { * Determines the value for the popover attribute based on the overlay type. * * @private - * @returns {'auto' | 'manual' | 'hint' | undefined} The popover value or undefined if not applicable. + * @returns {'auto' | 'manual' | undefined} The popover value or undefined if not applicable. */ - private get popoverValue(): 'auto' | 'manual' | 'hint' | undefined { + private get popoverValue(): 'auto' | 'manual' | undefined { const hasPopoverAttribute = 'popover' in this; if (!hasPopoverAttribute) { @@ -432,9 +431,11 @@ export class Overlay extends ComputedOverlayBase { switch (this.type) { case 'modal': - return 'manual'; + return 'auto'; case 'page': return 'manual'; + case 'hint': + return 'manual'; default: return this.type; } @@ -551,16 +552,9 @@ export class Overlay extends ComputedOverlayBase { }, // disable escape key capture to close the overlay, the focus-trap library captures it otherwise escapeDeactivates: false, - // allow events generated by programmatic methods, such as .click() - allowOutsideClick: (event: Event) => { - return !event.isTrusted; - }, }); - if ( - (this.type === 'modal' || this.type === 'page') && - this.receivesFocus !== 'false' - ) { + if (this.type === 'modal' || this.type === 'page') { this._focusTrap.activate(); } } @@ -1147,17 +1141,6 @@ export class Overlay extends ComputedOverlayBase { */ public override render(): TemplateResult { return html` - ${this.type === 'modal' || this.type === 'page' - ? html` - { - this.open = false; - }} - style="--spectrum-underlay-background-color: transparent" - > - ` - : ''} ${this.renderPopover()} `; diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index 258477d419e..4c174840f20 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -1,13 +1,14 @@ -/* -Copyright 2023 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { Overlay } from './Overlay.js'; @@ -26,10 +27,6 @@ class OverlayStack { stack: Overlay[] = []; - private originalBodyOverflow = ''; - - private bodyScrollBlocked = false; - constructor() { this.bindEvents(); } @@ -82,27 +79,8 @@ class OverlayStack { this.stack.splice(overlayIndex, 1); } overlay.open = false; - - this.manageBodyScroll(); } - /** - * Manage body scroll blocking based on modal/page overlays - */ - private manageBodyScroll(): void { - const shouldBlock = this.stack.some( - (overlay) => overlay.type === 'modal' || overlay.type === 'page' - ); - if (shouldBlock && !this.bodyScrollBlocked) { - this.originalBodyOverflow = document.body.style.overflow || ''; - document.body.style.overflow = 'hidden'; - this.bodyScrollBlocked = true; - } else if (!shouldBlock && this.bodyScrollBlocked) { - document.body.style.overflow = this.originalBodyOverflow; - this.bodyScrollBlocked = false; - } - } - /** * Cach the `pointerdownTarget` for later testing * @@ -173,14 +151,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' || last?.type === 'modal') { + if (last?.type === 'manual') { + // Manual overlays should close on "Escape" key, but not when losing focus or interacting with other parts of the page. this.closeOverlay(last); return; } @@ -235,17 +213,8 @@ 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); - if ( !inPath && - !triggerInOverlay && overlayEl.type !== 'manual' && overlayEl.type !== 'modal' ) { @@ -279,7 +248,6 @@ class OverlayStack { overlay.addEventListener('beforetoggle', this.handleBeforetoggle, { once: true, }); - this.manageBodyScroll(); }); } diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts index 6d61c9180f3..3715dc4b5d5 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' && this.open) { + if (event.key === 'Escape') { event.stopPropagation(); event.preventDefault(); this.toggle(false); diff --git a/yarn.lock b/yarn.lock index 2c68d8beacb..8200cdf86aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6960,7 +6960,6 @@ __metadata: "@spectrum-web-components/reactive-controllers": "npm:1.7.0" "@spectrum-web-components/shared": "npm:1.7.0" "@spectrum-web-components/theme": "npm:1.7.0" - "@spectrum-web-components/underlay": "npm:1.7.0" focus-trap: "npm:^7.6.4" languageName: unknown linkType: soft From dc3908d4ee77206a73f89126160456b6aec788ac Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 08:56:24 -0700 Subject: [PATCH 19/27] fix(overlay): revert changes --- .../action-group/test/action-group.test.ts | 2 +- packages/overlay/README.md | 151 ++++++++++- packages/overlay/package.json | 3 +- packages/overlay/src/AbstractOverlay.ts | 5 + packages/overlay/src/HoverController.ts | 13 + packages/overlay/src/InteractionController.ts | 2 + packages/overlay/src/Overlay.ts | 142 +++++------ packages/overlay/src/OverlayDialog.ts | 182 ++++++++++++++ packages/overlay/src/OverlayPopover.ts | 1 + packages/overlay/src/overlay.css | 3 +- .../stories/overlay-element.stories.ts | 237 ------------------ packages/overlay/test/overlay.test.ts | 150 +---------- packages/picker/src/InteractionController.ts | 5 +- packages/picker/stories/picker.stories.ts | 25 -- packages/picker/test/index.ts | 46 +--- .../picker/test/picker-responsive.test.ts | 1 - tools/shared/src/focusable-selectors.ts | 22 +- yarn.lock | 17 -- 18 files changed, 447 insertions(+), 560 deletions(-) create mode 100644 packages/overlay/src/OverlayDialog.ts diff --git a/packages/action-group/test/action-group.test.ts b/packages/action-group/test/action-group.test.ts index 22c97a60f76..72313927b47 100644 --- a/packages/action-group/test/action-group.test.ts +++ b/packages/action-group/test/action-group.test.ts @@ -261,7 +261,7 @@ describe('ActionGroup', () => { expect(el.children[3]).to.equal(document.activeElement); }); - it.skip('action-group with action-menu manages tabIndex correctly while using mouse', async () => { + it('action-group with action-menu manages tabIndex correctly while using mouse', async () => { const el = await fixture( HasActionMenuAsChild({ label: 'Action Group' }) ); diff --git a/packages/overlay/README.md b/packages/overlay/README.md index 78162fb6b74..295b0b4dbee 100644 --- a/packages/overlay/README.md +++ b/packages/overlay/README.md @@ -269,7 +269,7 @@ Some Overlays will always be passed focus (e.g. modal or page Overlays). When th The `trigger` option accepts an `HTMLElement` or a `VirtualTrigger` from which to position the Overlay. -- You can import the `VirtualTrigger` class from the overlay package to create a virtual trigger that can be used to position an Overlay. This is useful when you want to position an Overlay relative to a point on the screen that is not an element in the DOM, like the mouse cursor. +- You can import the `VirtualTrigger` class from the overlay package to create a virtual trigger that can be used to position an Overlay. This is useful when you want to position an Overlay relative to a point on the screen that is not an element in the DOM, like the mouse cursor. The `type` of an Overlay outlines a number of things about the interaction model within which it works: @@ -408,8 +408,8 @@ The `overlay` value in this case will hold a reference to the actual `` element have successfully dispatched their `transitionend` or `transitioncancel` event. Keep in mind the following: -- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account -- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard +- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account +- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard This means that in both cases, if the transition is meant to be a part of the opening or closing of the overlay in question you will need to redispatch the `transitionrun`, `transitionend`, and `transitioncancel` events from that transition from the closest direct child of the ``. @@ -573,7 +573,136 @@ Common in `modal`/`page` overlays for full-screen content -#### Styling +The `type` of an Overlay outlines a number of things about the interaction model within which is works. + +### Modal + +`'modal'` Overlays are opened with the `showModal()` method on a `` element, which causes the Overlay to accept focus and trap the tab stop within the content of said Overlay. + +They should be used when you need to ensure that the user has interacted with the content of the Overlay before continuing with their work. This is commonly used for dialogs that require a user to confirm or cancel an action before continuing. + +```html +open modal + + +

I am a modal type overlay.

+ Enter your email + + + Sign in + + + +``` + +### Page + +`'page'` Overlays will act in a similar manner to a `'modal'` Overlay, however they will not be allowed to close via the "light dismiss" algorithm (e.g. the Escape key). + +A page overlay could be used for a full-screen menu on a mobile website. When the user clicks on the menu button, the entire screen is covered with the menu options. + +```html +open page + + +

I am a page type overlay.

+ + +``` + +### Hint + +`'hint'` Overlays are much like tooltips so they are not just ephemeral, but they are delivered primarily as a visual helper and exist outside of the tab order. In this way, be sure _not_ to place interactive content within this type of Overlay. + +This overlay type does not accept focus and does not interfere with the user's interaction with the rest of the page. + +```html +open hint + + + I am a hint type overlay. I am not interactive and will close when the + user interacts with the page. + + +``` + +### Auto + +`'auto'` Overlays provide a place for content that is ephemeral _and_ interactive. These Overlays can accept focus and remain open while interacting with their content. They will close when focus moves outside the overlay or when clicking elsewhere on the page. + +```html +Open Overlay + + +

+ My slider in overlay element: + +

+ + +``` + +### Manual + +`'manual'` Overlays act much like `'auto'` Overlays, but do not close when losing focus or interacting with other parts of the page. + +Note: When a `'manual'` Overlay is at the top of the "overlay stack", it will still respond to the Escape key and close. + +```html + +open manual + + + + Chat Window + + Send + + + +``` + +### Events + +When fully open the `` element will dispatch the `sp-opened` event, and when fully closed the `sp-closed` event will be dispatched. Both of these events are of type: + +```ts +type OverlayStateEvent = Event & { + overlay: Overlay; +}; +``` + +The `overlay` value in this case will hold a reference to the actual `` that is opening or closing to trigger this event. Remember that some `` element (like those creates via the imperative API) can be transiently available in the DOM, so if you choose to build a cache of Overlay elements to some end, be sure to leverage a weak reference so that the `` can be garbage collected as desired by the browser. + +#### When it is "fully" open or closed? + +"Fully" in this context means that all CSS transitions that have dispatched `transitionrun` events on the direct children of the `` element have successfully dispatched their `transitionend` or `transitioncancel` event. Keep in mind the following: + +- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account +- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard + +This means that in both cases, if the transition is meant to be a part of the opening or closing of the overlay in question you will need to redispatch the `transitionrun`, `transitionend`, and `transitioncancel` events from that transition from the closest direct child of the ``. + +## Styling `` element will use the `` element or `popover` attribute to project your content onto the top-layer of the browser, without being moved in the DOM tree. That means that you can style your overlay content with whatever techniques you are already leveraging to style the content that doesn't get overlaid. This means standard CSS selectors, CSS Custom Properties, and CSS Parts applied in your parent context will always apply to your overlaid content. @@ -772,9 +901,9 @@ When nesting multiple overlays, it is important to ensure that the nested overla The overlay manages focus based on its type: -- For `modal` and `page` types, focus is always trapped within the overlay -- For `auto` and `manual` types, focus behavior is controlled by the `receives-focus` attribute -- For `hint` type, focus remains on the trigger element +- For `modal` and `page` types, focus is always trapped within the overlay +- For `auto` and `manual` types, focus behavior is controlled by the `receives-focus` attribute +- For `hint` type, focus remains on the trigger element Example of proper focus management: @@ -840,10 +969,10 @@ Example of proper focus management: #### Screen reader considerations -- Use `aria-haspopup` on trigger elements to indicate the type of overlay -- Provide descriptive labels using `aria-label` or `aria-labelledby` -- Use proper heading structure within overlays -- Ensure error messages are announced using `aria-live` +- Use `aria-haspopup` on trigger elements to indicate the type of overlay +- Provide descriptive labels using `aria-label` or `aria-labelledby` +- Use proper heading structure within overlays +- Ensure error messages are announced using `aria-live` Example of a tooltip with proper screen reader support: diff --git a/packages/overlay/package.json b/packages/overlay/package.json index b0500e5a3a8..1530b93e236 100644 --- a/packages/overlay/package.json +++ b/packages/overlay/package.json @@ -170,8 +170,7 @@ "@spectrum-web-components/base": "1.7.0", "@spectrum-web-components/reactive-controllers": "1.7.0", "@spectrum-web-components/shared": "1.7.0", - "@spectrum-web-components/theme": "1.7.0", - "focus-trap": "^7.6.4" + "@spectrum-web-components/theme": "1.7.0" }, "types": "./src/index.d.ts", "customElements": "custom-elements.json", diff --git a/packages/overlay/src/AbstractOverlay.ts b/packages/overlay/src/AbstractOverlay.ts index 50a0e1dc3de..5c3a4b50efd 100644 --- a/packages/overlay/src/AbstractOverlay.ts +++ b/packages/overlay/src/AbstractOverlay.ts @@ -162,6 +162,10 @@ export class AbstractOverlay extends SpectrumElement { return; } /* c8 ignore next 3 */ + protected async manageDialogOpen(): Promise { + return; + } + /* c8 ignore next 3 */ protected async managePopoverOpen(): Promise { return; } @@ -240,6 +244,7 @@ export class AbstractOverlay extends SpectrumElement { content?: HTMLElement, optionsV1?: OverlayOptionsV1 ): Promise void)> { + /* eslint-disable */ await import('@spectrum-web-components/overlay/sp-overlay.js'); const v2 = arguments.length === 2; const overlayContent = content || triggerOrContent; diff --git a/packages/overlay/src/HoverController.ts b/packages/overlay/src/HoverController.ts index 7f12a7e5c40..95ed877440f 100644 --- a/packages/overlay/src/HoverController.ts +++ b/packages/overlay/src/HoverController.ts @@ -18,6 +18,7 @@ import { InteractionController, InteractionTypes, lastInteractionType, + SAFARI_FOCUS_RING_CLASS, } from './InteractionController.js'; const HOVER_DELAY = 300; @@ -36,6 +37,7 @@ export class HoverController extends InteractionController { handleKeyup(event: KeyboardEvent): void { if (event.code === 'Tab' || event.code === 'Escape') { this.open = true; + this.removeSafariFocusRingClass(); } } @@ -48,14 +50,17 @@ export class HoverController extends InteractionController { isWebKit() && this.target[lastInteractionType] === InteractionTypes.click ) { + this.target.classList.add(SAFARI_FOCUS_RING_CLASS); return; } this.open = true; this.focusedin = true; + this.removeSafariFocusRingClass(); } handleTargetFocusout(): void { + this.removeSafariFocusRingClass(); this.focusedin = false; if (this.pointerentered) return; this.open = false; @@ -199,4 +204,12 @@ export class HoverController extends InteractionController { { signal } ); } + + private removeSafariFocusRingClass(): void { + if ( + isWebKit() && + this.target.classList.contains(SAFARI_FOCUS_RING_CLASS) + ) + this.target.classList.remove(SAFARI_FOCUS_RING_CLASS); + } } diff --git a/packages/overlay/src/InteractionController.ts b/packages/overlay/src/InteractionController.ts index 3584c3140c2..2c1b52ea83b 100644 --- a/packages/overlay/src/InteractionController.ts +++ b/packages/overlay/src/InteractionController.ts @@ -20,6 +20,7 @@ export enum InteractionTypes { } export const lastInteractionType = Symbol('lastInteractionType'); +export const SAFARI_FOCUS_RING_CLASS = 'remove-focus-ring-safari-hack'; export type ControllerOptions = { overlay?: AbstractOverlay; @@ -74,6 +75,7 @@ export class InteractionController implements ReactiveController { this.overlay.open = true; this.target[lastInteractionType] = this.type; }); + /* eslint-disable */ import('@spectrum-web-components/overlay/sp-overlay.js'); } diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 8535f4db638..11d88180c31 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -38,6 +38,7 @@ import type { TriggerInteraction, } from './overlay-types.js'; import { AbstractOverlay, nextFrame } from './AbstractOverlay.js'; +import { OverlayDialog } from './OverlayDialog.js'; import { OverlayPopover } from './OverlayPopover.js'; import { OverlayNoPopover } from './OverlayNoPopover.js'; import { overlayStack } from './OverlayStack.js'; @@ -54,14 +55,16 @@ import { } from './slottable-request-event.js'; import styles from './overlay.css.js'; -import { FocusTrap } from 'focus-trap'; const browserSupportsPopover = 'showPopover' in document.createElement('div'); // Start the base class and add the popover or no-popover functionality -let ComputedOverlayBase = OverlayPopover(AbstractOverlay); -if (!browserSupportsPopover) { - ComputedOverlayBase = OverlayNoPopover(AbstractOverlay); +let ComputedOverlayBase = OverlayDialog(AbstractOverlay); + +if (browserSupportsPopover) { + ComputedOverlayBase = OverlayPopover(ComputedOverlayBase); +} else { + ComputedOverlayBase = OverlayNoPopover(ComputedOverlayBase); } /** @@ -394,12 +397,6 @@ export class Overlay extends ComputedOverlayBase { */ protected wasOpen = false; - /** - * Focus trap to keep focus within the dialog - * @private - */ - private _focusTrap: FocusTrap | null = null; - /** * Provides an instance of the `ElementResolutionController` for managing the element * that the overlay should be associated with. If the instance does not already exist, @@ -416,6 +413,17 @@ export class Overlay extends ComputedOverlayBase { return this._elementResolver; } + /** + * Determines if the overlay uses a dialog. + * Returns `true` if the overlay type is "modal" or "page". + * + * @private + * @returns {boolean} `true` if the overlay uses a dialog, otherwise `false`. + */ + private get usesDialog(): boolean { + return this.type === 'modal' || this.type === 'page'; + } + /** * Determines the value for the popover attribute based on the overlay type. * @@ -431,9 +439,8 @@ export class Overlay extends ComputedOverlayBase { switch (this.type) { case 'modal': - return 'auto'; case 'page': - return 'manual'; + return undefined; case 'hint': return 'manual'; default: @@ -538,26 +545,7 @@ export class Overlay extends ComputedOverlayBase { if (this.open !== targetOpenState) { return; } - if (targetOpenState) { - const focusTrap = await import('focus-trap'); - this._focusTrap = focusTrap.createFocusTrap(this.dialogEl, { - initialFocus: focusEl || undefined, - tabbableOptions: { - getShadowRoot: true, - }, - fallbackFocus: () => { - // set tabIndex to -1 allow the focus-trap to still be applied - this.dialogEl.setAttribute('tabIndex', '-1'); - return this.dialogEl; - }, - // disable escape key capture to close the overlay, the focus-trap library captures it otherwise - escapeDeactivates: false, - }); - - if (this.type === 'modal' || this.type === 'page') { - this._focusTrap.activate(); - } - } + // Apply focus to the appropriate element after opening the popover. await this.applyFocus(targetOpenState, focusEl); } @@ -700,10 +688,6 @@ export class Overlay extends ComputedOverlayBase { event.relatedTarget.dispatchEvent(relationEvent); }; - private closeOnCancelEvent = (): void => { - this.open = false; - }; - /** * Manages the process of opening or closing the overlay. * @@ -716,7 +700,7 @@ export class Overlay extends ComputedOverlayBase { */ protected async manageOpen(oldOpen: boolean): Promise { // Prevent entering the manage workflow if the overlay is not connected to the DOM. - // The `.showPopover()` event will error on content that is not connected to the DOM. + // The `.showPopover()` and `.showModal()` events will error on content that is not connected to the DOM. if (!this.isConnected && this.open) return; // Wait for the component to finish updating if it has not already done so. @@ -748,8 +732,6 @@ export class Overlay extends ComputedOverlayBase { } } else { if (oldOpen) { - this._focusTrap?.deactivate(); - this._focusTrap = null; // Dispose of the overlay if it was previously open. this.dispose(); } @@ -765,11 +747,16 @@ export class Overlay extends ComputedOverlayBase { this.state = 'closing'; } - this.managePopoverOpen(); + // Manage the dialog or popover based on the overlay type. + if (this.usesDialog) { + this.manageDialogOpen(); + } else { + this.managePopoverOpen(); + } - const listenerRoot = this.getRootNode() as Document; // Handle focus events for auto type overlays. if (this.type === 'auto') { + const listenerRoot = this.getRootNode() as Document; if (this.open) { listenerRoot.addEventListener( 'focusout', @@ -784,27 +771,6 @@ export class Overlay extends ComputedOverlayBase { ); } } - - // Handle cancel events for modal and page type overlays. - if (this.type === 'modal' || this.type === 'page') { - if (this.open) { - listenerRoot.addEventListener( - 'cancel', - this.closeOnCancelEvent, - { - capture: true, - } - ); - } else { - listenerRoot.removeEventListener( - 'cancel', - this.closeOnCancelEvent, - { - capture: true, - } - ); - } - } } /** @@ -1081,6 +1047,45 @@ export class Overlay extends ComputedOverlayBase { }; } + /** + * Renders the dialog element for the overlay. + * + * This method returns a template result containing a dialog element. The dialog element + * includes various attributes and event listeners to manage the overlay's state and behavior. + * + * @protected + * @returns {TemplateResult} The template result containing the dialog element. + */ + protected renderDialog(): TemplateResult { + /** + * The `--swc-overlay-open-count` custom property is applied to mimic the single stack + * nature of the top layer in browsers that do not yet support it. + * + * The value should always represent the total number of overlays that have ever been opened. + * This value will be added to the `--swc-overlay-z-index-base` custom property, which can be + * provided by a consuming developer. By default, `--swc-overlay-z-index-base` is set to 1000 + * to ensure that the overlay stacks above most other elements during fallback delivery. + */ + return html` + + ${this.renderContent()} + + `; + } + /** * Renders the popover element for the overlay. * @@ -1104,16 +1109,6 @@ export class Overlay extends ComputedOverlayBase {
`; } diff --git a/packages/overlay/src/OverlayDialog.ts b/packages/overlay/src/OverlayDialog.ts new file mode 100644 index 00000000000..441a4d008d7 --- /dev/null +++ b/packages/overlay/src/OverlayDialog.ts @@ -0,0 +1,182 @@ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ +import type { SpectrumElement } from '@spectrum-web-components/base'; +import { + firstFocusableIn, + firstFocusableSlottedIn, +} from '@spectrum-web-components/shared/src/first-focusable-in.js'; +import { VirtualTrigger } from './VirtualTrigger.js'; +import { Constructor, OpenableElement } from './overlay-types.js'; +import { guaranteedAllTransitionend, nextFrame } from './AbstractOverlay.js'; +import { + BeforetoggleClosedEvent, + BeforetoggleOpenEvent, + OverlayStateEvent, +} from './events.js'; +import type { AbstractOverlay } from './AbstractOverlay.js'; +import { userFocusableSelector } from '@spectrum-web-components/shared'; + +export function OverlayDialog>( + constructor: T +): T & Constructor { + class OverlayWithDialog extends constructor { + protected override async manageDialogOpen(): Promise { + const targetOpenState = this.open; + await nextFrame(); + await this.managePosition(); + if (this.open !== targetOpenState) { + return; + } + const focusEl = await this.dialogMakeTransition(targetOpenState); + if (this.open !== targetOpenState) { + return; + } + await this.dialogApplyFocus(targetOpenState, focusEl); + } + + protected async dialogMakeTransition( + targetOpenState: boolean + ): Promise { + let focusEl = null as HTMLElement | null; + const start = + (el: OpenableElement, index: number) => + async (): Promise => { + el.open = targetOpenState; + if (!targetOpenState) { + const close = (): void => { + el.removeEventListener('close', close); + finish(el, index); + }; + el.addEventListener('close', close); + } + if (index > 0) { + // Announce workflow on the first element _only_. + return; + } + const event = targetOpenState + ? BeforetoggleOpenEvent + : BeforetoggleClosedEvent; + this.dispatchEvent(new event()); + if (!targetOpenState) { + // Show/focus workflow when opening _only_. + return; + } + if (el.matches(userFocusableSelector)) { + focusEl = el; + } + focusEl = focusEl || firstFocusableIn(el); + if (!focusEl) { + const childSlots = el.querySelectorAll('slot'); + childSlots.forEach((slot) => { + if (!focusEl) { + focusEl = firstFocusableSlottedIn(slot); + } + }); + } + if (!this.isConnected || this.dialogEl.open) { + // In both of these cases the browser will error. + // You can neither "reopen" a or open one that is not on the DOM. + return; + } + this.dialogEl.showModal(); + }; + const finish = (el: OpenableElement, index: number) => (): void => { + if (this.open !== targetOpenState) { + return; + } + const eventName = targetOpenState ? 'sp-opened' : 'sp-closed'; + if (index > 0) { + el.dispatchEvent( + new OverlayStateEvent(eventName, this, { + interaction: this.type, + publish: false, + }) + ); + return; + } + if (!this.isConnected || targetOpenState !== this.open) { + // Don't lead into the `.close()` workflow if not connected to the DOM. + // The browser will error in this case. + return; + } + const reportChange = async (): Promise => { + const hasVirtualTrigger = + this.triggerElement instanceof VirtualTrigger; + this.dispatchEvent( + new OverlayStateEvent(eventName, this, { + interaction: this.type, + publish: hasVirtualTrigger, + }) + ); + el.dispatchEvent( + new OverlayStateEvent(eventName, this, { + interaction: this.type, + publish: false, + }) + ); + if (this.triggerElement && !hasVirtualTrigger) { + (this.triggerElement as HTMLElement).dispatchEvent( + new OverlayStateEvent(eventName, this, { + interaction: this.type, + publish: true, + }) + ); + } + this.state = targetOpenState ? 'opened' : 'closed'; + this.returnFocus(); + // Ensure layout and paint are done and the Overlay is still closed before removing the slottable request. + await nextFrame(); + await nextFrame(); + if ( + targetOpenState === this.open && + targetOpenState === false + ) { + this.requestSlottable(); + } + }; + if (!targetOpenState && this.dialogEl.open) { + this.dialogEl.addEventListener( + 'close', + () => { + reportChange(); + }, + { once: true } + ); + this.dialogEl.close(); + } else { + reportChange(); + } + }; + this.elements.forEach((el, index) => { + guaranteedAllTransitionend( + el, + start(el, index), + finish(el, index) + ); + }); + return focusEl; + } + + protected async dialogApplyFocus( + targetOpenState: boolean, + focusEl: HTMLElement | null + ): Promise { + /** + * Focus should be handled natively in `` elements when leveraging `.showModal()`, but it's NOT. + * - webkit bug: https://bugs.webkit.org/show_bug.cgi?id=255507 + * - firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1828398 + **/ + this.applyFocus(targetOpenState, focusEl); + } + } + return OverlayWithDialog; +} diff --git a/packages/overlay/src/OverlayPopover.ts b/packages/overlay/src/OverlayPopover.ts index 76b381a91c8..5be607f092a 100644 --- a/packages/overlay/src/OverlayPopover.ts +++ b/packages/overlay/src/OverlayPopover.ts @@ -123,6 +123,7 @@ export function OverlayPopover>( protected override async ensureOnDOM( targetOpenState: boolean ): Promise { + await nextFrame(); if (!supportsOverlayAuto) { await this.shouldHidePopover(targetOpenState); } diff --git a/packages/overlay/src/overlay.css b/packages/overlay/src/overlay.css index 450576b1c83..eb6e49a14cc 100644 --- a/packages/overlay/src/overlay.css +++ b/packages/overlay/src/overlay.css @@ -185,7 +185,8 @@ slot[name="longpress-describedby-descriptor"] { transition-behavior: allow-discrete; } - .dialog:popover-open { + .dialog:popover-open, + .dialog:modal { display: flex; } } diff --git a/packages/overlay/stories/overlay-element.stories.ts b/packages/overlay/stories/overlay-element.stories.ts index 3c8e4252400..390da94541c 100644 --- a/packages/overlay/stories/overlay-element.stories.ts +++ b/packages/overlay/stories/overlay-element.stories.ts @@ -12,7 +12,6 @@ governing permissions and limitations under the License. import { html, render, TemplateResult } from '@spectrum-web-components/base'; import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; import '@spectrum-web-components/dialog/sp-dialog.js'; -import '@spectrum-web-components/dialog/sp-dialog-wrapper.js'; import '@spectrum-web-components/overlay/sp-overlay.js'; import '@spectrum-web-components/action-button/sp-action-button.js'; import '@spectrum-web-components/action-menu/sp-action-menu.js'; @@ -26,16 +25,6 @@ import '@spectrum-web-components/tooltip/sp-tooltip.js'; import '@spectrum-web-components/slider/sp-slider.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-anchor-select.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-polygon-select.js'; -import '@spectrum-web-components/textfield/sp-textfield.js'; -import '@spectrum-web-components/field-label/sp-field-label.js'; -import '@spectrum-web-components/table/sp-table.js'; -import '@spectrum-web-components/table/sp-table-checkbox-cell.js'; -import '@spectrum-web-components/table/sp-table-head.js'; -import '@spectrum-web-components/table/sp-table-head-cell.js'; -import '@spectrum-web-components/table/sp-table-body.js'; -import '@spectrum-web-components/table/sp-table-row.js'; -import '@spectrum-web-components/table/sp-table-cell.js'; - import '@spectrum-web-components/icons-workflow/icons/sp-icon-rect-select.js'; import { Placement } from '@floating-ui/dom'; import { OverlayTypes } from '../src/overlay-types.js'; @@ -177,183 +166,6 @@ page.args = { type: 'page', }; -export const complexSlowPage = (): TemplateResult => html` -
- -

- This is a complex slow page. It has a lot of content. Even with a lot of content on the page, - the overlay should still be able to open and close without extreme delay. -

- -
- - open modal - - - -

I am a modal type overlay.

- Enter your email - - - Sign in - - - - - open page - - -

I am a page type overlay.

- - - - - open manual - - - - Chat Window - - Send - - - - -
- - - ${Array(30) - .fill(0) - .map( - () => html` -
- - - - Column Title - - - Column Title - - - Column Title - - - - - - Row Item Alpha - - - Row Item Alpha - - - Row Item Alpha - - - - - Row Item Bravo - - - Row Item Bravo - - - Row Item Bravo - - - - - Row Item Charlie - - - Row Item Charlie - - - Row Item Charlie - - - - - Row Item Delta - - - Row Item Delta - - - Row Item Delta - - - - Row Item Echo - Row Item Echo - Row Item Echo - - - - - - - - - - - - - - Menu Group - Option 1 - Option 2 - - Option 3 - -
- ` - )} -
-`; - -complexSlowPage.swc_vrt = { - skip: true, -}; - -complexSlowPage.parameters = { - chromatic: { disableSnapshot: true }, -}; - export const click = (args: Properties): TemplateResult => Template(args); click.args = { interaction: 'click', @@ -930,52 +742,3 @@ lazyElements.parameters = { // Disables Chromatic's snapshotting on a global level chromatic: { disableSnapshot: true }, }; - -export const nestedModalOverlays = (): TemplateResult => html` -
- - Open Outer Modal - - - - - -

- This is the outer modal content. Press ESC to close it. -

- - Open Inner Modal - - - - -

- This is the inner modal content. Press ESC - to close this first, then the outer modal. -

- - - - - - -
-`; - -nestedModalOverlays.swc_vrt = { - skip: true, -}; - -nestedModalOverlays.parameters = { - chromatic: { disableSnapshot: true }, -}; diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts index 32d9d495bc4..38065b5d532 100644 --- a/packages/overlay/test/overlay.test.ts +++ b/packages/overlay/test/overlay.test.ts @@ -52,7 +52,8 @@ import { } from '../../../test/testing-helpers.js'; import { Menu } from '@spectrum-web-components/menu'; import { Button } from '@spectrum-web-components/button'; -// import { isWebKit } from '@spectrum-web-components/shared'; +import { isWebKit } from '@spectrum-web-components/shared'; +import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/overlay/src/InteractionController.js'; async function styledFixture( story: TemplateResult @@ -968,8 +969,12 @@ describe('Overlay - timing', () => { }); }); -describe('maintains focus consistency in all browsers', () => { - it('should not have a focus-visible on trigger when focus happens after click', async () => { +describe('maintains focus consistency in webkit', () => { + it('should apply remove-focus-ring class in webkit when focus happens after click', async () => { + if (!isWebKit()) { + return; + } + const overlayTrigger = await fixture( clickAndHoverTarget() ); @@ -1008,7 +1013,7 @@ describe('maintains focus consistency in all browsers', () => { }); await closed; - expect(trigger.matches(':focus-visible')).to.be.false; + expect(trigger.classList.contains(SAFARI_FOCUS_RING_CLASS)).to.be.true; }); }); @@ -1058,140 +1063,3 @@ describe('Overlay - Interactive Content', () => { expect(overlay.open).to.be.true; }); }); - -describe('Overlay should correctly trap focus', () => { - it('should trap focus when the overlay type is modal', async () => { - const el = await fixture(html` -
- Open Overlay - - -

Overlay content

- button 1 - button 2 - - -
- `); - - const trigger = el.querySelector('#trigger') as HTMLElement; - const overlay = el.querySelector('sp-overlay') as Overlay; - - await elementUpdated(overlay); - - const opened = oneEvent(overlay, 'sp-opened'); - // use keyboard to open the overlay - trigger.focus(); - await sendKeys({ - press: 'Enter', - }); - await opened; - - expect(overlay.open).to.be.true; - - const button1 = el.querySelector('#button-1') as HTMLElement; - const button2 = el.querySelector('#button-2') as HTMLElement; - - // expect button1 to be focused - expect(document.activeElement).to.equal(button1); - - // press tab to focus on button2 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button2); - - // press tab to focus on button1 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button1); - - // press tab to focus on button2 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button2); - }); - it('should trap focus when the overlay type is page', async () => { - const el = await fixture(html` -
- Open Overlay - - -

Overlay content

- button 1 - button 2 - - -
- `); - - const trigger = el.querySelector('#trigger') as HTMLElement; - const overlay = el.querySelector('sp-overlay') as Overlay; - - await elementUpdated(overlay); - - const opened = oneEvent(overlay, 'sp-opened'); - trigger.click(); - await opened; - - expect(overlay.open).to.be.true; - - const button1 = el.querySelector('#button-1') as HTMLElement; - const button2 = el.querySelector('#button-2') as HTMLElement; - - // expect button1 to be focused - expect(document.activeElement).to.equal(button1); - - // press tab to focus on button2 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button2); - - // press tab to focus on button1 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button1); - - // press tab to focus on button2 - await sendKeys({ - press: 'Tab', - }); - expect(document.activeElement).to.equal(button2); - }); - it('should not trap focus when the overlay type is auto', async () => { - const el = await fixture(html` -
- Open Overlay - - -

Overlay content

- test - - - -
- `); - - const trigger = el.querySelector('#trigger') as HTMLElement; - const overlay = el.querySelector('sp-overlay') as Overlay; - - await elementUpdated(overlay); - - const opened = oneEvent(overlay, 'sp-opened'); - trigger.click(); - await opened; - - expect(overlay.open).to.be.true; - - await sendKeys({ - press: 'Tab', - }); - - const input = el.querySelector('#input') as HTMLInputElement; - expect(document.activeElement).to.equal(input); - }); -}); diff --git a/packages/picker/src/InteractionController.ts b/packages/picker/src/InteractionController.ts index f3b9a731548..e1f66439e09 100644 --- a/packages/picker/src/InteractionController.ts +++ b/packages/picker/src/InteractionController.ts @@ -131,10 +131,7 @@ export class InteractionController implements ReactiveController { } ); }); - this.overlay.type = - this.host.isMobile.matches && !this.host.forcePopover - ? 'modal' - : 'auto'; + this.overlay.type = this.host.isMobile.matches ? 'modal' : 'auto'; this.overlay.triggerElement = this.host as HTMLElement; this.overlay.placement = this.host.isMobile.matches && !this.host.forcePopover diff --git a/packages/picker/stories/picker.stories.ts b/packages/picker/stories/picker.stories.ts index d42592d181a..d62a2f23824 100644 --- a/packages/picker/stories/picker.stories.ts +++ b/packages/picker/stories/picker.stories.ts @@ -22,8 +22,6 @@ import '@spectrum-web-components/link/sp-link.js'; import '@spectrum-web-components/menu/sp-menu-item.js'; import '@spectrum-web-components/picker/sp-picker.js'; import '@spectrum-web-components/tooltip/sp-tooltip.js'; -import '@spectrum-web-components/overlay/sp-overlay.js'; -import '@spectrum-web-components/popover/sp-popover.js'; import { ifDefined } from 'lit-html/directives/if-defined.js'; import { spreadProps } from '../../../test/lit-helpers.js'; import '../../overlay/stories/index.js'; @@ -792,26 +790,3 @@ export const BackgroundClickTest = (): TemplateResult => { BackgroundClickTest.swc_vrt = { skip: true, }; - -export const PickerInModal = (): TemplateResult => { - return html` - Overlay Trigger - - - - Save - Finish - Review - - - - `; -}; -PickerInModal.swc_vrt = { - skip: true, -}; diff --git a/packages/picker/test/index.ts b/packages/picker/test/index.ts index 4d58e553b32..6e0f1ceb1a3 100644 --- a/packages/picker/test/index.ts +++ b/packages/picker/test/index.ts @@ -1182,42 +1182,22 @@ export function runPickerTests(): void { input1.remove(); input2.remove(); }); - it('tabs forward through the element', async function () { - // Increase timeout for this test to avoid timeout failures in webkit - this.timeout(10000); - + it('tabs forward through the element', async () => { let focused: Promise>; - - // start at input1 - input1.focus(); - await nextFrame(); - expect(document.activeElement === input1, 'focuses input 1').to - .true; - // tab to the picker - focused = oneEvent(el, 'focus'); - await sendKeys({ press: 'Tab' }); - - // Increase timeout for focus event to prevent flakiness - try { - await Promise.race([ - focused, - new Promise((_, reject) => - setTimeout( - () => - reject(new Error('Focus event timed out')), - 5000 - ) - ), - ]); - } catch (error) { - console.error('Focus event timed out:', error); - el.focus(); + if (!isSafari) { + // start at input1 + input1.focus(); await nextFrame(); - expect( - document.activeElement === el, - 'element focused manually after timeout' - ).to.be.true; + expect(document.activeElement === input1, 'focuses input 1') + .to.true; + // tab to the picker + focused = oneEvent(el, 'focus'); + await sendKeys({ press: 'Tab' }); + } else { + focused = oneEvent(el, 'focus'); + el.focus(); } + await focused; expect(el.focused, 'focused').to.be.true; expect(el.open, 'closed').to.be.false; diff --git a/packages/picker/test/picker-responsive.test.ts b/packages/picker/test/picker-responsive.test.ts index 88d25c303ee..152e94ac663 100644 --- a/packages/picker/test/picker-responsive.test.ts +++ b/packages/picker/test/picker-responsive.test.ts @@ -120,7 +120,6 @@ describe('Picker, responsive', () => { */ el.isMobile.matches = true; el.bindEvents(); - await elementUpdated(el); // Wait until the element is fully updated after viewport change await waitUntil( diff --git a/tools/shared/src/focusable-selectors.ts b/tools/shared/src/focusable-selectors.ts index ba30fef8cec..eb06f3d90aa 100644 --- a/tools/shared/src/focusable-selectors.ts +++ b/tools/shared/src/focusable-selectors.ts @@ -10,21 +10,15 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -// Inspired from https://github.com/focus-trap/tabbable/blob/8acf516c29da42c928753950210b07ac32efc724/src/index.js#L6 const focusables = [ - 'input:not([inert])', - 'select:not([inert])', - 'textarea:not([inert])', - 'a[href]:not([inert])', - 'button:not([inert])', - 'label:not([inert])', - '[tabindex]:not([inert])', - 'audio[controls]:not([inert])', - 'video[controls]:not([inert])', - '[contenteditable]:not([contenteditable="false"]):not([inert])', - 'details>summary:first-of-type:not([inert])', - 'details:not([inert])', - '[focusable]:not([focusable="false"])', // custom dev use-case + 'button', + '[focusable]', + '[href]', + 'input', + 'label', + 'select', + 'textarea', + '[tabindex]', ]; const userFocuable = ':not([tabindex="-1"])'; diff --git a/yarn.lock b/yarn.lock index 8200cdf86aa..02612bb5a5f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6960,7 +6960,6 @@ __metadata: "@spectrum-web-components/reactive-controllers": "npm:1.7.0" "@spectrum-web-components/shared": "npm:1.7.0" "@spectrum-web-components/theme": "npm:1.7.0" - focus-trap: "npm:^7.6.4" languageName: unknown linkType: soft @@ -17939,15 +17938,6 @@ __metadata: languageName: node linkType: hard -"focus-trap@npm:^7.6.4": - version: 7.6.4 - resolution: "focus-trap@npm:7.6.4" - dependencies: - tabbable: "npm:^6.2.0" - checksum: 10c0/ed810d47fd904a5e0269e822d98e634c6cbdd7222046c712ef299bdd26a422db754e3cec04e6517065b12be4b47f65c21f6244e0c07a308b1060985463d518cb - languageName: node - linkType: hard - "focus-visible@npm:^5.1.0": version: 5.2.0 resolution: "focus-visible@npm:5.2.0" @@ -31569,13 +31559,6 @@ __metadata: languageName: node linkType: hard -"tabbable@npm:^6.2.0": - version: 6.2.0 - resolution: "tabbable@npm:6.2.0" - checksum: 10c0/ced8b38f05f2de62cd46836d77c2646c42b8c9713f5bd265daf0e78ff5ac73d3ba48a7ca45f348bafeef29b23da7187c72250742d37627883ef89cbd7fa76898 - languageName: node - linkType: hard - "table-layout@npm:^1.0.1": version: 1.0.2 resolution: "table-layout@npm:1.0.2" From dc05d7d72c78bf3d5c49b010738c9e249da7ba71 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 12:01:31 -0700 Subject: [PATCH 20/27] fix(overlay): revert some, save others --- .../action-group/test/action-group.test.ts | 2 +- .../test/action-menu-responsive.test.ts | 24 +- packages/overlay/README.md | 131 +-------- packages/overlay/src/Overlay.ts | 21 +- packages/overlay/src/OverlayStack.ts | 21 +- .../stories/overlay-element.stories.ts | 258 +++++++++++++++++- packages/overlay/test/overlay.test.ts | 172 ++++++++++-- packages/picker/stories/picker.stories.ts | 47 +++- packages/picker/test/index.ts | 77 ++++-- .../picker/test/picker-responsive.test.ts | 23 +- 10 files changed, 531 insertions(+), 245 deletions(-) diff --git a/packages/action-group/test/action-group.test.ts b/packages/action-group/test/action-group.test.ts index 72313927b47..22c97a60f76 100644 --- a/packages/action-group/test/action-group.test.ts +++ b/packages/action-group/test/action-group.test.ts @@ -261,7 +261,7 @@ describe('ActionGroup', () => { expect(el.children[3]).to.equal(document.activeElement); }); - it('action-group with action-menu manages tabIndex correctly while using mouse', async () => { + it.skip('action-group with action-menu manages tabIndex correctly while using mouse', async () => { const el = await fixture( HasActionMenuAsChild({ label: 'Action Group' }) ); diff --git a/packages/action-menu/test/action-menu-responsive.test.ts b/packages/action-menu/test/action-menu-responsive.test.ts index 169e65c6e79..a19fa3bbaee 100644 --- a/packages/action-menu/test/action-menu-responsive.test.ts +++ b/packages/action-menu/test/action-menu-responsive.test.ts @@ -1,14 +1,14 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { elementUpdated, @@ -82,7 +82,6 @@ describe('ActionMenu, responsive', () => { }); it('is a Popover in desktop', async () => { - el.open = true; // in this test we only need to wait to see if a popover opens @@ -140,7 +139,6 @@ describe('ActionMenu, responsive', () => { }); it('is a Popover in desktop', async () => { - el.open = true; // in this test we only need to wait to see if a popover opens diff --git a/packages/overlay/README.md b/packages/overlay/README.md index 295b0b4dbee..e136526e86c 100644 --- a/packages/overlay/README.md +++ b/packages/overlay/README.md @@ -573,136 +573,7 @@ Common in `modal`/`page` overlays for full-screen content -The `type` of an Overlay outlines a number of things about the interaction model within which is works. - -### Modal - -`'modal'` Overlays are opened with the `showModal()` method on a `` element, which causes the Overlay to accept focus and trap the tab stop within the content of said Overlay. - -They should be used when you need to ensure that the user has interacted with the content of the Overlay before continuing with their work. This is commonly used for dialogs that require a user to confirm or cancel an action before continuing. - -```html -open modal - - -

I am a modal type overlay.

- Enter your email - - - Sign in - - - -``` - -### Page - -`'page'` Overlays will act in a similar manner to a `'modal'` Overlay, however they will not be allowed to close via the "light dismiss" algorithm (e.g. the Escape key). - -A page overlay could be used for a full-screen menu on a mobile website. When the user clicks on the menu button, the entire screen is covered with the menu options. - -```html -open page - - -

I am a page type overlay.

- - -``` - -### Hint - -`'hint'` Overlays are much like tooltips so they are not just ephemeral, but they are delivered primarily as a visual helper and exist outside of the tab order. In this way, be sure _not_ to place interactive content within this type of Overlay. - -This overlay type does not accept focus and does not interfere with the user's interaction with the rest of the page. - -```html -open hint - - - I am a hint type overlay. I am not interactive and will close when the - user interacts with the page. - - -``` - -### Auto - -`'auto'` Overlays provide a place for content that is ephemeral _and_ interactive. These Overlays can accept focus and remain open while interacting with their content. They will close when focus moves outside the overlay or when clicking elsewhere on the page. - -```html -Open Overlay - - -

- My slider in overlay element: - -

- - -``` - -### Manual - -`'manual'` Overlays act much like `'auto'` Overlays, but do not close when losing focus or interacting with other parts of the page. - -Note: When a `'manual'` Overlay is at the top of the "overlay stack", it will still respond to the Escape key and close. - -```html - -open manual - - - - Chat Window - - Send - - - -``` - -### Events - -When fully open the `` element will dispatch the `sp-opened` event, and when fully closed the `sp-closed` event will be dispatched. Both of these events are of type: - -```ts -type OverlayStateEvent = Event & { - overlay: Overlay; -}; -``` - -The `overlay` value in this case will hold a reference to the actual `` that is opening or closing to trigger this event. Remember that some `` element (like those creates via the imperative API) can be transiently available in the DOM, so if you choose to build a cache of Overlay elements to some end, be sure to leverage a weak reference so that the `` can be garbage collected as desired by the browser. - -#### When it is "fully" open or closed? - -"Fully" in this context means that all CSS transitions that have dispatched `transitionrun` events on the direct children of the `` element have successfully dispatched their `transitionend` or `transitioncancel` event. Keep in mind the following: - -- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account -- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard - -This means that in both cases, if the transition is meant to be a part of the opening or closing of the overlay in question you will need to redispatch the `transitionrun`, `transitionend`, and `transitioncancel` events from that transition from the closest direct child of the ``. - -## Styling +#### Styling `` element will use the `` element or `popover` attribute to project your content onto the top-layer of the browser, without being moved in the DOM tree. That means that you can style your overlay content with whatever techniques you are already leveraging to style the content that doesn't get overlaid. This means standard CSS selectors, CSS Custom Properties, and CSS Parts applied in your parent context will always apply to your overlaid content. diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 11d88180c31..0a4dadc5b41 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2023 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import { html, PropertyValues, diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index 4c174840f20..71c4d9b0446 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2023 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import { Overlay } from './Overlay.js'; diff --git a/packages/overlay/stories/overlay-element.stories.ts b/packages/overlay/stories/overlay-element.stories.ts index 390da94541c..d94690c76d7 100644 --- a/packages/overlay/stories/overlay-element.stories.ts +++ b/packages/overlay/stories/overlay-element.stories.ts @@ -1,17 +1,19 @@ -/* -Copyright 2022 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { html, render, TemplateResult } from '@spectrum-web-components/base'; import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; import '@spectrum-web-components/dialog/sp-dialog.js'; +import '@spectrum-web-components/dialog/sp-dialog-wrapper.js'; import '@spectrum-web-components/overlay/sp-overlay.js'; import '@spectrum-web-components/action-button/sp-action-button.js'; import '@spectrum-web-components/action-menu/sp-action-menu.js'; @@ -25,6 +27,16 @@ import '@spectrum-web-components/tooltip/sp-tooltip.js'; import '@spectrum-web-components/slider/sp-slider.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-anchor-select.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-polygon-select.js'; +import '@spectrum-web-components/textfield/sp-textfield.js'; +import '@spectrum-web-components/field-label/sp-field-label.js'; +import '@spectrum-web-components/table/sp-table.js'; +import '@spectrum-web-components/table/sp-table-checkbox-cell.js'; +import '@spectrum-web-components/table/sp-table-head.js'; +import '@spectrum-web-components/table/sp-table-head-cell.js'; +import '@spectrum-web-components/table/sp-table-body.js'; +import '@spectrum-web-components/table/sp-table-row.js'; +import '@spectrum-web-components/table/sp-table-cell.js'; + import '@spectrum-web-components/icons-workflow/icons/sp-icon-rect-select.js'; import { Placement } from '@floating-ui/dom'; import { OverlayTypes } from '../src/overlay-types.js'; @@ -166,6 +178,183 @@ page.args = { type: 'page', }; +export const complexSlowPage = (): TemplateResult => html` +
+ +

+ This is a complex slow page. It has a lot of content. Even with a lot of content on the page, + the overlay should still be able to open and close without extreme delay. +

+ +
+ + open modal + + + +

I am a modal type overlay.

+ Enter your email + + + Sign in + + + + + open page + + +

I am a page type overlay.

+ + + + + open manual + + + + Chat Window + + Send + + + + +
+ + + ${Array(30) + .fill(0) + .map( + () => html` +
+ + + + Column Title + + + Column Title + + + Column Title + + + + + + Row Item Alpha + + + Row Item Alpha + + + Row Item Alpha + + + + + Row Item Bravo + + + Row Item Bravo + + + Row Item Bravo + + + + + Row Item Charlie + + + Row Item Charlie + + + Row Item Charlie + + + + + Row Item Delta + + + Row Item Delta + + + Row Item Delta + + + + Row Item Echo + Row Item Echo + Row Item Echo + + + + + + + + + + + + + + Menu Group + Option 1 + Option 2 + + Option 3 + +
+ ` + )} +
+`; + +complexSlowPage.swc_vrt = { + skip: true, +}; + +complexSlowPage.parameters = { + chromatic: { disableSnapshot: true }, +}; + export const click = (args: Properties): TemplateResult => Template(args); click.args = { interaction: 'click', @@ -742,3 +931,52 @@ lazyElements.parameters = { // Disables Chromatic's snapshotting on a global level chromatic: { disableSnapshot: true }, }; + +export const nestedModalOverlays = (): TemplateResult => html` +
+ + Open Outer Modal + + + + + +

+ This is the outer modal content. Press ESC to close it. +

+ + Open Inner Modal + + + + +

+ This is the inner modal content. Press ESC + to close this first, then the outer modal. +

+ + + + + + +
+`; + +nestedModalOverlays.swc_vrt = { + skip: true, +}; + +nestedModalOverlays.parameters = { + chromatic: { disableSnapshot: true }, +}; diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts index 38065b5d532..11c13b2ece9 100644 --- a/packages/overlay/test/overlay.test.ts +++ b/packages/overlay/test/overlay.test.ts @@ -1,14 +1,14 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import '@spectrum-web-components/button/sp-button.js'; import '@spectrum-web-components/dialog/sp-dialog.js'; import '@spectrum-web-components/overlay/sp-overlay.js'; @@ -52,8 +52,7 @@ import { } from '../../../test/testing-helpers.js'; import { Menu } from '@spectrum-web-components/menu'; import { Button } from '@spectrum-web-components/button'; -import { isWebKit } from '@spectrum-web-components/shared'; -import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/overlay/src/InteractionController.js'; +// import { isWebKit } from '@spectrum-web-components/shared'; async function styledFixture( story: TemplateResult @@ -969,12 +968,8 @@ describe('Overlay - timing', () => { }); }); -describe('maintains focus consistency in webkit', () => { - it('should apply remove-focus-ring class in webkit when focus happens after click', async () => { - if (!isWebKit()) { - return; - } - +describe('maintains focus consistency in all browsers', () => { + it('should not have a focus-visible on trigger when focus happens after click', async () => { const overlayTrigger = await fixture( clickAndHoverTarget() ); @@ -1013,7 +1008,7 @@ describe('maintains focus consistency in webkit', () => { }); await closed; - expect(trigger.classList.contains(SAFARI_FOCUS_RING_CLASS)).to.be.true; + expect(trigger.matches(':focus-visible')).to.be.false; }); }); @@ -1063,3 +1058,140 @@ describe('Overlay - Interactive Content', () => { expect(overlay.open).to.be.true; }); }); + +describe('Overlay should correctly trap focus', () => { + it('should trap focus when the overlay type is modal', async () => { + const el = await fixture(html` +
+ Open Overlay + + +

Overlay content

+ button 1 + button 2 + + +
+ `); + + const trigger = el.querySelector('#trigger') as HTMLElement; + const overlay = el.querySelector('sp-overlay') as Overlay; + + await elementUpdated(overlay); + + const opened = oneEvent(overlay, 'sp-opened'); + // use keyboard to open the overlay + trigger.focus(); + await sendKeys({ + press: 'Enter', + }); + await opened; + + expect(overlay.open).to.be.true; + + const button1 = el.querySelector('#button-1') as HTMLElement; + const button2 = el.querySelector('#button-2') as HTMLElement; + + // expect button1 to be focused + expect(document.activeElement).to.equal(button1); + + // press tab to focus on button2 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button2); + + // press tab to focus on button1 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button1); + + // press tab to focus on button2 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button2); + }); + it('should trap focus when the overlay type is page', async () => { + const el = await fixture(html` +
+ Open Overlay + + +

Overlay content

+ button 1 + button 2 + + +
+ `); + + const trigger = el.querySelector('#trigger') as HTMLElement; + const overlay = el.querySelector('sp-overlay') as Overlay; + + await elementUpdated(overlay); + + const opened = oneEvent(overlay, 'sp-opened'); + trigger.click(); + await opened; + + expect(overlay.open).to.be.true; + + const button1 = el.querySelector('#button-1') as HTMLElement; + const button2 = el.querySelector('#button-2') as HTMLElement; + + // expect button1 to be focused + expect(document.activeElement).to.equal(button1); + + // press tab to focus on button2 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button2); + + // press tab to focus on button1 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button1); + + // press tab to focus on button2 + await sendKeys({ + press: 'Tab', + }); + expect(document.activeElement).to.equal(button2); + }); + it('should not trap focus when the overlay type is auto', async () => { + const el = await fixture(html` +
+ Open Overlay + + +

Overlay content

+ test + + + +
+ `); + + const trigger = el.querySelector('#trigger') as HTMLElement; + const overlay = el.querySelector('sp-overlay') as Overlay; + + await elementUpdated(overlay); + + const opened = oneEvent(overlay, 'sp-opened'); + trigger.click(); + await opened; + + expect(overlay.open).to.be.true; + + await sendKeys({ + press: 'Tab', + }); + + const input = el.querySelector('#input') as HTMLInputElement; + expect(document.activeElement).to.equal(input); + }); +}); diff --git a/packages/picker/stories/picker.stories.ts b/packages/picker/stories/picker.stories.ts index d62a2f23824..1d9eb39391f 100644 --- a/packages/picker/stories/picker.stories.ts +++ b/packages/picker/stories/picker.stories.ts @@ -1,14 +1,14 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * 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'; @@ -22,6 +22,8 @@ import '@spectrum-web-components/link/sp-link.js'; import '@spectrum-web-components/menu/sp-menu-item.js'; import '@spectrum-web-components/picker/sp-picker.js'; import '@spectrum-web-components/tooltip/sp-tooltip.js'; +import '@spectrum-web-components/overlay/sp-overlay.js'; +import '@spectrum-web-components/popover/sp-popover.js'; import { ifDefined } from 'lit-html/directives/if-defined.js'; import { spreadProps } from '../../../test/lit-helpers.js'; import '../../overlay/stories/index.js'; @@ -790,3 +792,26 @@ export const BackgroundClickTest = (): TemplateResult => { BackgroundClickTest.swc_vrt = { skip: true, }; + +export const PickerInModal = (): TemplateResult => { + return html` + Overlay Trigger + + + + Save + Finish + Review + + + + `; +}; +PickerInModal.swc_vrt = { + skip: true, +}; diff --git a/packages/picker/test/index.ts b/packages/picker/test/index.ts index 6e0f1ceb1a3..141d0d60bdd 100644 --- a/packages/picker/test/index.ts +++ b/packages/picker/test/index.ts @@ -1,15 +1,14 @@ -/* eslint-disable import/no-extraneous-dependencies */ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import type { Picker } from '@spectrum-web-components/picker'; @@ -32,7 +31,7 @@ import '@spectrum-web-components/menu/sp-menu-item.js'; import '@spectrum-web-components/menu/sp-menu.js'; import '@spectrum-web-components/picker/sp-picker.js'; import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/picker/src/InteractionController.js'; -import { isWebKit } from '@spectrum-web-components/shared'; +import { isFirefox, isWebKit } from '@spectrum-web-components/shared'; import '@spectrum-web-components/shared/src/focus-visible.js'; import '@spectrum-web-components/theme/src/themes.js'; import { Tooltip } from '@spectrum-web-components/tooltip'; @@ -1182,22 +1181,42 @@ export function runPickerTests(): void { input1.remove(); input2.remove(); }); - it('tabs forward through the element', async () => { + it('tabs forward through the element', async function () { + // Increase timeout for this test to avoid timeout failures in webkit + this.timeout(10000); + let focused: Promise>; - if (!isSafari) { - // start at input1 - input1.focus(); - await nextFrame(); - expect(document.activeElement === input1, 'focuses input 1') - .to.true; - // tab to the picker - focused = oneEvent(el, 'focus'); - await sendKeys({ press: 'Tab' }); - } else { - focused = oneEvent(el, 'focus'); + + // start at input1 + input1.focus(); + await nextFrame(); + expect(document.activeElement === input1, 'focuses input 1').to + .true; + // tab to the picker + focused = oneEvent(el, 'focus'); + await sendKeys({ press: 'Tab' }); + + // Increase timeout for focus event to prevent flakiness + try { + await Promise.race([ + focused, + new Promise((_, reject) => + setTimeout( + () => + reject(new Error('Focus event timed out')), + 5000 + ) + ), + ]); + } catch (error) { + console.error('Focus event timed out:', error); el.focus(); + await nextFrame(); + expect( + document.activeElement === el, + 'element focused manually after timeout' + ).to.be.true; } - await focused; expect(el.focused, 'focused').to.be.true; expect(el.open, 'closed').to.be.false; @@ -2192,7 +2211,11 @@ export function runPickerTests(): void { this.el = test.querySelector('sp-picker') as Picker; await elementUpdated(this.el); }); - it('displays the same icon as the selected menu item', async function () { + it.skip('displays the same icon as the selected menu item', async function () { + // TODO: skipping this test because it's flaky in Firefox in CI. Will review in the migration to Spectrum 2. + if (isFirefox()) { + return; + } // Delay long enough for the picker to display the selected item. // Chromium and Webkit require 2 frames, Firefox requires 3 frames. await nextFrame(); diff --git a/packages/picker/test/picker-responsive.test.ts b/packages/picker/test/picker-responsive.test.ts index 152e94ac663..4367044d5a7 100644 --- a/packages/picker/test/picker-responsive.test.ts +++ b/packages/picker/test/picker-responsive.test.ts @@ -1,14 +1,14 @@ -/* -Copyright 2025 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { elementUpdated, @@ -120,6 +120,7 @@ describe('Picker, responsive', () => { */ el.isMobile.matches = true; el.bindEvents(); + await elementUpdated(el); // Wait until the element is fully updated after viewport change await waitUntil( From d46c9e5a33780686c7988e5a3a6268fa1167b696 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 12:13:05 -0700 Subject: [PATCH 21/27] fix(overlay): delete some extra patches --- .../test/action-menu-responsive.test.ts | 23 +++++++------- packages/overlay/src/Overlay.ts | 1 + .../stories/overlay-element.stories.ts | 23 +++++++------- packages/overlay/test/overlay.test.ts | 21 +++++++------ packages/picker/stories/picker.stories.ts | 21 +++++++------ packages/picker/test/index.ts | 30 ++++++++----------- .../picker/test/picker-responsive.test.ts | 21 +++++++------ 7 files changed, 65 insertions(+), 75 deletions(-) diff --git a/packages/action-menu/test/action-menu-responsive.test.ts b/packages/action-menu/test/action-menu-responsive.test.ts index a19fa3bbaee..dfc16309536 100644 --- a/packages/action-menu/test/action-menu-responsive.test.ts +++ b/packages/action-menu/test/action-menu-responsive.test.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import { elementUpdated, @@ -83,7 +82,6 @@ describe('ActionMenu, responsive', () => { it('is a Popover in desktop', async () => { el.open = true; - // in this test we only need to wait to see if a popover opens let popover: Popover | null = null; await waitUntil( @@ -140,7 +138,6 @@ describe('ActionMenu, responsive', () => { it('is a Popover in desktop', async () => { el.open = true; - // in this test we only need to wait to see if a popover opens let popover: Popover | null = null; await waitUntil( diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index 0a4dadc5b41..8613cef5246 100644 --- a/packages/overlay/src/Overlay.ts +++ b/packages/overlay/src/Overlay.ts @@ -3,6 +3,7 @@ Copyright 2023 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language diff --git a/packages/overlay/stories/overlay-element.stories.ts b/packages/overlay/stories/overlay-element.stories.ts index d94690c76d7..3c8e4252400 100644 --- a/packages/overlay/stories/overlay-element.stories.ts +++ b/packages/overlay/stories/overlay-element.stories.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2022 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import { html, render, TemplateResult } from '@spectrum-web-components/base'; import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; @@ -209,7 +208,7 @@ export const complexSlowPage = (): TemplateResult => html` > Sign in - + open page diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts index 11c13b2ece9..4db140fb399 100644 --- a/packages/overlay/test/overlay.test.ts +++ b/packages/overlay/test/overlay.test.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import '@spectrum-web-components/button/sp-button.js'; import '@spectrum-web-components/dialog/sp-dialog.js'; import '@spectrum-web-components/overlay/sp-overlay.js'; diff --git a/packages/picker/stories/picker.stories.ts b/packages/picker/stories/picker.stories.ts index 1d9eb39391f..abe560a1623 100644 --- a/packages/picker/stories/picker.stories.ts +++ b/packages/picker/stories/picker.stories.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +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'; diff --git a/packages/picker/test/index.ts b/packages/picker/test/index.ts index 141d0d60bdd..75befe25f3f 100644 --- a/packages/picker/test/index.ts +++ b/packages/picker/test/index.ts @@ -1,14 +1,14 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* eslint-disable import/no-extraneous-dependencies */ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import type { Picker } from '@spectrum-web-components/picker'; @@ -31,7 +31,7 @@ import '@spectrum-web-components/menu/sp-menu-item.js'; import '@spectrum-web-components/menu/sp-menu.js'; import '@spectrum-web-components/picker/sp-picker.js'; import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/picker/src/InteractionController.js'; -import { isFirefox, isWebKit } from '@spectrum-web-components/shared'; +import { isWebKit } from '@spectrum-web-components/shared'; import '@spectrum-web-components/shared/src/focus-visible.js'; import '@spectrum-web-components/theme/src/themes.js'; import { Tooltip } from '@spectrum-web-components/tooltip'; @@ -2211,11 +2211,7 @@ export function runPickerTests(): void { this.el = test.querySelector('sp-picker') as Picker; await elementUpdated(this.el); }); - it.skip('displays the same icon as the selected menu item', async function () { - // TODO: skipping this test because it's flaky in Firefox in CI. Will review in the migration to Spectrum 2. - if (isFirefox()) { - return; - } + it('displays the same icon as the selected menu item', async function () { // Delay long enough for the picker to display the selected item. // Chromium and Webkit require 2 frames, Firefox requires 3 frames. await nextFrame(); diff --git a/packages/picker/test/picker-responsive.test.ts b/packages/picker/test/picker-responsive.test.ts index 4367044d5a7..9546a3aecf9 100644 --- a/packages/picker/test/picker-responsive.test.ts +++ b/packages/picker/test/picker-responsive.test.ts @@ -1,14 +1,13 @@ -/** - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ +/* +Copyright 2025 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ import { elementUpdated, From 786543baf7d98f0ad22d7c26993a8a7e33ef85b3 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 12:19:24 -0700 Subject: [PATCH 22/27] fix(overlay): ignore more docs changed --- packages/action-menu/test/action-menu-responsive.test.ts | 5 +++++ packages/overlay/test/overlay.test.ts | 1 + packages/picker/stories/picker.stories.ts | 1 + packages/picker/test/index.ts | 1 + packages/picker/test/picker-responsive.test.ts | 1 + 5 files changed, 9 insertions(+) diff --git a/packages/action-menu/test/action-menu-responsive.test.ts b/packages/action-menu/test/action-menu-responsive.test.ts index dfc16309536..169e65c6e79 100644 --- a/packages/action-menu/test/action-menu-responsive.test.ts +++ b/packages/action-menu/test/action-menu-responsive.test.ts @@ -3,6 +3,7 @@ Copyright 2020 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language @@ -81,7 +82,9 @@ describe('ActionMenu, responsive', () => { }); it('is a Popover in desktop', async () => { + el.open = true; + // in this test we only need to wait to see if a popover opens let popover: Popover | null = null; await waitUntil( @@ -137,7 +140,9 @@ describe('ActionMenu, responsive', () => { }); it('is a Popover in desktop', async () => { + el.open = true; + // in this test we only need to wait to see if a popover opens let popover: Popover | null = null; await waitUntil( diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts index 4db140fb399..32d9d495bc4 100644 --- a/packages/overlay/test/overlay.test.ts +++ b/packages/overlay/test/overlay.test.ts @@ -3,6 +3,7 @@ Copyright 2020 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language diff --git a/packages/picker/stories/picker.stories.ts b/packages/picker/stories/picker.stories.ts index abe560a1623..d42592d181a 100644 --- a/packages/picker/stories/picker.stories.ts +++ b/packages/picker/stories/picker.stories.ts @@ -3,6 +3,7 @@ Copyright 2020 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language diff --git a/packages/picker/test/index.ts b/packages/picker/test/index.ts index 75befe25f3f..4d58e553b32 100644 --- a/packages/picker/test/index.ts +++ b/packages/picker/test/index.ts @@ -4,6 +4,7 @@ Copyright 2020 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language diff --git a/packages/picker/test/picker-responsive.test.ts b/packages/picker/test/picker-responsive.test.ts index 9546a3aecf9..88d25c303ee 100644 --- a/packages/picker/test/picker-responsive.test.ts +++ b/packages/picker/test/picker-responsive.test.ts @@ -3,6 +3,7 @@ Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language From ed01e7c0455600c716807aa0c03358d4d2860470 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Mon, 28 Jul 2025 12:22:26 -0700 Subject: [PATCH 23/27] fix(picker): repatch --- packages/picker/src/InteractionController.ts | 27 +++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/picker/src/InteractionController.ts b/packages/picker/src/InteractionController.ts index e1f66439e09..2fb506fffb5 100644 --- a/packages/picker/src/InteractionController.ts +++ b/packages/picker/src/InteractionController.ts @@ -1,14 +1,14 @@ -/* -Copyright 2024 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ +/** + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ import { ReactiveController, @@ -131,7 +131,10 @@ export class InteractionController implements ReactiveController { } ); }); - this.overlay.type = this.host.isMobile.matches ? 'modal' : 'auto'; + this.overlay.type = + this.host.isMobile.matches && !this.host.forcePopover + ? 'modal' + : 'auto'; this.overlay.triggerElement = this.host as HTMLElement; this.overlay.placement = this.host.isMobile.matches && !this.host.forcePopover From 452afded37e4319aa096d4694c301d34610ed52d Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 30 Jul 2025 09:56:02 -0700 Subject: [PATCH 24/27] fix(menu): blur menu item on mouseleave --- packages/menu/src/MenuItem.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/menu/src/MenuItem.ts b/packages/menu/src/MenuItem.ts index 6c6371d4bb7..76a277cbf16 100644 --- a/packages/menu/src/MenuItem.ts +++ b/packages/menu/src/MenuItem.ts @@ -472,20 +472,27 @@ export class MenuItem extends LikeAnchor( super.firstUpdated(changes); this.setAttribute('tabindex', '-1'); this.addEventListener('keydown', this.handleKeydown); - this.addEventListener('mouseover', this.handleMouseover); + this.addEventListener('mouseenter', this.handleMouseenter); + this.addEventListener('mouseleave', this.handleMouseleave); this.addEventListener('pointerdown', this.handlePointerdown); this.addEventListener('pointerenter', this.closeOverlaysForRoot); if (!this.hasAttribute('id')) { this.id = `sp-menu-item-${randomID()}`; } } - handleMouseover(event: MouseEvent): void { + handleMouseenter(event: MouseEvent): void { const target = event.target as HTMLElement; if (target === this) { this.focus(); this.focused = false; } } + handleMouseleave(event: MouseEvent): void { + const target = event.target as HTMLElement; + if (target === this) { + this.blur(); + } + } /** * forward key info from keydown event to parent menu */ From b6394861734d5561adf506a7def62a889940aae2 Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 30 Jul 2025 12:53:41 -0700 Subject: [PATCH 25/27] fix(button): update aria-label update --- packages/button/src/ButtonBase.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/button/src/ButtonBase.ts b/packages/button/src/ButtonBase.ts index 30830f531cf..8d533fd8008 100644 --- a/packages/button/src/ButtonBase.ts +++ b/packages/button/src/ButtonBase.ts @@ -225,13 +225,6 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [ if (!this.hasAttribute('tabindex')) { this.setAttribute('tabindex', '0'); } - if (changed.has('label')) { - if (this.label) { - this.setAttribute('aria-label', this.label); - } else { - this.removeAttribute('aria-label'); - } - } this.manageAnchor(); this.addEventListener('keydown', this.handleKeydown); this.addEventListener('keypress', this.handleKeypress); @@ -254,6 +247,14 @@ export class ButtonBase extends ObserveSlotText(LikeAnchor(Focusable), '', [ // Set up focus delegation this.anchorElement.addEventListener('focus', this.proxyFocus); + + if (changed.has('label')) { + if (this.label) { + this.setAttribute('aria-label', this.label); + } else { + this.removeAttribute('aria-label'); + } + } } } protected override update(changes: PropertyValues): void { From 1cadc7b63f7d53176f58c06e04872fd609c0e5ab Mon Sep 17 00:00:00 2001 From: Helen Le Date: Tue, 5 Aug 2025 09:42:41 -0700 Subject: [PATCH 26/27] fix(overlay): delete nextframe --- packages/overlay/src/OverlayDialog.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/overlay/src/OverlayDialog.ts b/packages/overlay/src/OverlayDialog.ts index 441a4d008d7..09e2d0d510f 100644 --- a/packages/overlay/src/OverlayDialog.ts +++ b/packages/overlay/src/OverlayDialog.ts @@ -31,7 +31,6 @@ export function OverlayDialog>( class OverlayWithDialog extends constructor { protected override async manageDialogOpen(): Promise { const targetOpenState = this.open; - await nextFrame(); await this.managePosition(); if (this.open !== targetOpenState) { return; From 3d46cc32061a02a8a751b5f7900108b08be0f8cf Mon Sep 17 00:00:00 2001 From: Helen Le Date: Wed, 3 Sep 2025 13:37:52 -0700 Subject: [PATCH 27/27] fix(menu): patch scroll fix --- packages/menu/src/Menu.ts | 64 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/menu/src/Menu.ts b/packages/menu/src/Menu.ts index 1378a58402b..fbcd4d95733 100644 --- a/packages/menu/src/Menu.ts +++ b/packages/menu/src/Menu.ts @@ -72,6 +72,22 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { protected rovingTabindexController?: RovingTabindexController; + private touchStartY: number | undefined = undefined; + private touchStartTime: number | undefined = undefined; + private isCurrentlyScrolling = false; + + private scrollThreshold = 10; // pixels + private scrollTimeThreshold = 300; // milliseconds + + public get isScrolling(): boolean { + return this.isCurrentlyScrolling; + } + + public set isScrolling(value: boolean) { + // For testing purposes, allow setting the scrolling state + this.isCurrentlyScrolling = value; + } + /** * label of the menu */ @@ -400,6 +416,13 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { this.addEventListener('pointerup', this.handlePointerup); this.addEventListener('sp-opened', this.handleSubmenuOpened); this.addEventListener('sp-closed', this.handleSubmenuClosed); + + this.addEventListener('touchstart', this.handleTouchStart, { + passive: true, + }); + this.addEventListener('touchmove', this.handleTouchMove, { + passive: true, + }); } /** @@ -443,6 +466,42 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { } } + private handleTouchStart(event: TouchEvent): void { + if (event.touches.length === 1) { + this.touchStartY = event.touches[0].clientY; + this.touchStartTime = Date.now(); + this.isCurrentlyScrolling = false; + } + } + + private handleTouchMove(event: TouchEvent): void { + if ( + event.touches.length === 1 && + this.touchStartY !== undefined && + this.touchStartTime !== undefined + ) { + const currentY = event.touches[0].clientY; + const deltaY = Math.abs(currentY - this.touchStartY); + const deltaTime = Date.now() - this.touchStartTime; + + if ( + deltaY > this.scrollThreshold && + deltaTime < this.scrollTimeThreshold + ) { + this.isCurrentlyScrolling = true; + } + } + } + + private handleTouchEnd(): void { + // Reset scrolling state after a short delay + setTimeout(() => { + this.isCurrentlyScrolling = false; + this.touchStartY = undefined; + this.touchStartTime = undefined; + }, 100); + } + // if the click and pointerup events are on the same target, we should not // handle the click event. private pointerUpTarget = null as EventTarget | null; @@ -461,6 +520,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { } private handlePointerup(event: Event): void { + this.handleTouchEnd(); /* * early return if drag and select is not supported * in this case, selection will be handled by the click event @@ -478,6 +538,10 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { return; } + if (this.isScrolling) { + return; + } + const path = event.composedPath(); const target = path.find((el) => { /* c8 ignore next 3 */