diff --git a/packages/action-group/test/action-group.test.ts b/packages/action-group/test/action-group.test.ts index 6524e6aca95..ec01e0a373c 100644 --- a/packages/action-group/test/action-group.test.ts +++ b/packages/action-group/test/action-group.test.ts @@ -260,7 +260,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/contextual-help/package.json b/packages/contextual-help/package.json index 36213df84e2..1f4c5579e43 100644 --- a/packages/contextual-help/package.json +++ b/packages/contextual-help/package.json @@ -16,23 +16,23 @@ "type": "module", "exports": { ".": { - "default": "./src/index.js", - "development": "./src/index.dev.js" + "development": "./src/index.dev.js", + "default": "./src/index.js" }, "./package.json": "./package.json", "./sp-contextual-help.js": { - "default": "./sp-contextual-help.js", - "development": "./sp-contextual-help.dev.js" + "development": "./sp-contextual-help.dev.js", + "default": "./sp-contextual-help.js" }, "./src/ContextualHelp.js": { - "default": "./src/ContextualHelp.js", - "development": "./src/ContextualHelp.dev.js" + "development": "./src/ContextualHelp.dev.js", + "default": "./src/ContextualHelp.js" }, "./src/contextual-help-overrides.css.js": "./src/contextual-help-overrides.css.js", "./src/contextual-help.css.js": "./src/contextual-help.css.js", "./src/index.js": { - "default": "./src/index.js", - "development": "./src/index.dev.js" + "development": "./src/index.dev.js", + "default": "./src/index.js" } }, "main": "./src/index.js", diff --git a/packages/dialog/src/DialogBase.ts b/packages/dialog/src/DialogBase.ts index 2973c20ac69..eafbf08fec7 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 +// eslint-disable-next-line import/no-extraneous-dependencies import '@spectrum-web-components/dialog/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'; @@ -155,41 +156,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); } diff --git a/packages/menu/src/MenuItem.ts b/packages/menu/src/MenuItem.ts index 75de1e0de3d..0195abdbf79 100644 --- a/packages/menu/src/MenuItem.ts +++ b/packages/menu/src/MenuItem.ts @@ -195,6 +195,8 @@ export class MenuItem extends LikeAnchor( private _value = ''; + private _lastPointerType?: string; + /** * @private * text content of the menu item minus whitespace @@ -457,25 +459,15 @@ export class MenuItem extends LikeAnchor( } } - private handlePointerdown(event: PointerEvent): void { - if (event.target === this && this.hasSubmenu && this.open) { - this.addEventListener('focus', this.handleSubmenuFocus, { - once: true, - }); - this.overlayElement.addEventListener( - 'beforetoggle', - this.handleBeforetoggle - ); - } - } - protected override firstUpdated(changes: PropertyValues): void { super.firstUpdated(changes); this.setAttribute('tabindex', '-1'); this.addEventListener('keydown', this.handleKeydown); this.addEventListener('mouseover', this.handleMouseover); - this.addEventListener('pointerdown', this.handlePointerdown); - this.addEventListener('pointerenter', this.closeOverlaysForRoot); + // Register pointerenter/leave for ALL menu items (not just those with submenus) + // so items without submenus can close sibling submenus when hovered + this.addEventListener('pointerenter', this.handlePointerenter); + this.addEventListener('pointerleave', this.handlePointerleave); if (!this.hasAttribute('id')) { this.id = `sp-menu-item-${randomID()}`; } @@ -594,11 +586,6 @@ export class MenuItem extends LikeAnchor( } }; - protected closeOverlaysForRoot(): void { - if (this.open) return; - this.menuData.parentMenu?.closeDescendentOverlays(); - } - protected handleFocus(event: FocusEvent): void { const { target } = event; if (target === this) { @@ -613,48 +600,64 @@ export class MenuItem extends LikeAnchor( } } - protected handleSubmenuClick(event: Event): void { + protected handleSubmenuTriggerClick(event: Event): void { if (event.composedPath().includes(this.overlayElement)) { return; } - this.openOverlay(true); - } - protected handleSubmenuFocus(): void { - requestAnimationFrame(() => { - // Wait till after `closeDescendentOverlays` has happened in Menu - // to reopen (keep open) the direct descendent of this Menu Item - this.overlayElement.open = this.open; - this.focused = false; - }); + // If submenu is already open, toggle it closed + if (this.open && this._lastPointerType === 'touch') { + event.preventDefault(); + event.stopPropagation(); // Don't let parent menu handle this + this.open = false; + return; + } + + // All: open if closed + if (!this.open) { + event.preventDefault(); + event.stopImmediatePropagation(); + this.openOverlay(true); + } } - protected handleBeforetoggle = (event: Event): void => { - if ((event as Event & { newState: string }).newState === 'closed') { - this.open = true; - this.overlayElement.manuallyKeepOpen(); - this.overlayElement.removeEventListener( - 'beforetoggle', - this.handleBeforetoggle - ); + protected handlePointerenter(event: PointerEvent): void { + this._lastPointerType = event.pointerType; // Track pointer type + + // For touch: don't handle pointerenter, let click handle it + if (event.pointerType === 'touch') { + return; } - }; - protected handlePointerenter(): void { + // Close sibling submenus before opening this one + this.menuData.parentMenu?.closeDescendentOverlays(); + if (this.leaveTimeout) { clearTimeout(this.leaveTimeout); delete this.leaveTimeout; this.recentlyLeftChild = false; return; } - this.focus(); + + // Only focus items with submenus on hover (to show they're interactive) + // Regular items should not show focus styling on hover, only on keyboard navigation + if (this.hasSubmenu) { + this.focus(); + } this.openOverlay(); } protected leaveTimeout?: ReturnType; protected recentlyLeftChild = false; - protected handlePointerleave(): void { + protected handlePointerleave(event: PointerEvent): void { + this._lastPointerType = event.pointerType; // Update on leave too + + // For touch: don't handle pointerleave, let click handle it + if (event.pointerType === 'touch') { + return; + } + this._closedViaPointer = true; if (this.open && !this.recentlyLeftChild) { this.leaveTimeout = setTimeout(() => { @@ -782,17 +785,7 @@ export class MenuItem extends LikeAnchor( const options = { signal: this.abortControllerSubmenu.signal }; this.addEventListener( 'click', - this.handleSubmenuClick, - options - ); - this.addEventListener( - 'pointerenter', - this.handlePointerenter, - options - ); - this.addEventListener( - 'pointerleave', - this.handlePointerleave, + this.handleSubmenuTriggerClick, options ); this.addEventListener( diff --git a/packages/number-field/src/NumberField.ts b/packages/number-field/src/NumberField.ts index ff82ffb7b19..84a30a6379c 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/overlay/README.md b/packages/overlay/README.md index 5da10213991..8dee65bae39 100644 --- a/packages/overlay/README.md +++ b/packages/overlay/README.md @@ -532,7 +532,6 @@ This means that in both cases, if the transition is meant to be a part of the op .triggerElement=${HTMLElement} .triggerInteraction=${'click' | 'longpress' | 'hover'} type=${'auto' | 'hint' | 'manual' | 'modal' | 'page'} - ?allow-outside-click=${boolean} > ``` @@ -574,24 +573,136 @@ Common in `modal`/`page` overlays for full-screen content -##### Deprecated Properties +The `type` of an Overlay outlines a number of things about the interaction model within which is works. -> **⚠️ Deprecation Notice**: The `allow-outside-click` property is deprecated and will be removed in a future version. +### Modal -The `allow-outside-click` property allows clicks outside the overlay to close it. **We do not recommend using this property for accessibility reasons** as it can cause unexpected behavior and accessibility issues. When set to `true`, it configures the focus trap to allow outside clicks, which may interfere with proper focus management and user expectations. +`'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 - - - -

This overlay can be closed by clicking outside

+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 + ``` -**Alternative approaches**: Instead of using `allow-outside-click`, consider implementing explicit close buttons or using the `type="modal"` or `type="page"` overlay types which provide better accessibility and user experience. +### 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/package.json b/packages/overlay/package.json index fe4afbc8d1a..856b88a73b6 100644 --- a/packages/overlay/package.json +++ b/packages/overlay/package.json @@ -170,8 +170,7 @@ "@spectrum-web-components/base": "1.9.0", "@spectrum-web-components/reactive-controllers": "1.9.0", "@spectrum-web-components/shared": "1.9.0", - "@spectrum-web-components/theme": "1.9.0", - "focus-trap": "7.6.5" + "@spectrum-web-components/theme": "1.9.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 e02e80f8135..3c250c3e258 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; } diff --git a/packages/overlay/src/HoverController.ts b/packages/overlay/src/HoverController.ts index bd77ae8fdc9..51bef438c68 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 c9f9278409e..fb762e85c0a 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-next-line import/no-extraneous-dependencies import('@spectrum-web-components/overlay/sp-overlay.js'); } diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts index f1ec3e07f29..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); } /** @@ -79,7 +82,6 @@ if (!browserSupportsPopover) { * @attr {string} receives-focus - How focus should be handled ('true'|'false'|'auto') * @attr {boolean} delayed - Whether the overlay should wait for a warm-up period before opening * @attr {boolean} open - Whether the overlay is currently open - * @attr {boolean} allow-outside-click - @deprecated Whether clicks outside the overlay should close it (not recommended for accessibility) */ export class Overlay extends ComputedOverlayBase { static override styles = [styles]; @@ -290,18 +292,6 @@ export class Overlay extends ComputedOverlayBase { @property({ attribute: 'receives-focus' }) override receivesFocus: 'true' | 'false' | 'auto' = 'auto'; - /** - * @deprecated This property will be removed in a future version. - * We do not recommend using this property for accessibility reasons. - * It allows clicks outside the overlay to close it, which can cause - * unexpected behavior and accessibility issues. - * - * @type {boolean} - * @default false - */ - @property({ type: Boolean, attribute: 'allow-outside-click' }) - allowOutsideClick = false; - /** * A reference to the slot element within the overlay. * @@ -407,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, @@ -429,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. * @@ -444,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: @@ -508,6 +502,7 @@ export class Overlay extends ComputedOverlayBase { * * This method handles the necessary steps to open the popover, including managing delays, * ensuring the popover is in the DOM, making transitions, and applying focus. + * * @protected * @override * @returns {Promise} A promise that resolves when the popover has been fully opened. @@ -550,27 +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, - allowOutsideClick: this.allowOutsideClick, - }); - - 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); } @@ -713,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. * @@ -729,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. @@ -761,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(); } @@ -778,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', @@ -797,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, - } - ); - } - } } /** @@ -985,23 +938,6 @@ export class Overlay extends ComputedOverlayBase { ); } - // Warn about deprecated allowOutsideClick property - if (changes.has('allowOutsideClick') && this.allowOutsideClick) { - if (window.__swc?.DEBUG) { - window.__swc.warn( - this, - `The "allow-outside-click" attribute on <${this.localName}> has been deprecated and will be removed in a future release. We do not recommend using this attribute for accessibility reasons. It allows clicks outside the overlay to close it, which can cause unexpected behavior and accessibility issues.`, - 'https://opensource.adobe.com/spectrum-web-components/components/overlay/', - { level: 'deprecation' } - ); - } else { - // Fallback for testing environments or when SWC is not available - console.warn( - `[${this.localName}] The "allow-outside-click" attribute has been deprecated and will be removed in a future release. We do not recommend using this attribute for accessibility reasons. It allows clicks outside the overlay to close it, which can cause unexpected behavior and accessibility issues.` - ); - } - } - // Manage the open state if the 'open' property has changed. if (changes.has('open') && (this.hasUpdated || this.open)) { this.manageOpen(changes.get('open')); @@ -1111,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. * @@ -1134,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 5a1aaead2e1..e5f9ca35517 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/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts index cc5c8ffd99b..4c174840f20 100644 --- a/packages/overlay/src/OverlayStack.ts +++ b/packages/overlay/src/OverlayStack.ts @@ -27,10 +27,6 @@ class OverlayStack { stack: Overlay[] = []; - private originalBodyOverflow = ''; - - private bodyScrollBlocked = false; - constructor() { this.bindEvents(); } @@ -83,25 +79,6 @@ 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; - } } /** @@ -271,7 +248,6 @@ class OverlayStack { overlay.addEventListener('beforetoggle', this.handleBeforetoggle, { once: true, }); - this.manageBodyScroll(); }); } diff --git a/packages/overlay/src/overlay.css b/packages/overlay/src/overlay.css index 74202e5f601..5912a2c7ca4 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 d94690c76d7..f6f8d242efc 100644 --- a/packages/overlay/stories/overlay-element.stories.ts +++ b/packages/overlay/stories/overlay-element.stories.ts @@ -13,7 +13,6 @@ 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'; @@ -27,16 +26,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'; @@ -931,52 +920,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/index.ts b/packages/overlay/test/index.ts index fc70d7572a6..71bf71f4e2c 100644 --- a/packages/overlay/test/index.ts +++ b/packages/overlay/test/index.ts @@ -650,85 +650,6 @@ export const runOverlayTriggerTests = (type: string): void => { 'hover content is not available at point' ).to.be.false; }); - - it('blocks body scroll when modal overlay is opened and restores when closed', async function () { - const originalBodyOverflow = document.body.style.overflow; - - const modalTrigger = this.outerTrigger; - modalTrigger.type = 'modal'; - await elementUpdated(modalTrigger); - - // Open modal overlay - const opened = oneEvent(modalTrigger, 'sp-opened'); - this.outerButton.click(); - await opened; - - // Check that body scroll is blocked - expect(document.body.style.overflow).to.equal('hidden'); - - // Close modal overlay - const closed = oneEvent(modalTrigger, 'sp-closed'); - sendMouse({ - steps: [ - { - type: 'click', - position: [1, 1], - }, - ], - }); - await closed; - - // Check that body scroll is restored - expect(document.body.style.overflow).to.equal( - originalBodyOverflow - ); - }); - - it('actually prevents page scrolling when modal overlay is open and restores scrolling when closed', async function () { - // Create a long enough page to enable scrolling - const longContent = document.createElement('div'); - longContent.style.height = '200vh'; // Make page twice viewport height - longContent.style.width = '100%'; - longContent.style.backgroundColor = 'transparent'; - document.body.appendChild(longContent); - - const modalTrigger = this.outerTrigger; - modalTrigger.type = 'modal'; - await elementUpdated(modalTrigger); - - // Open modal overlay - const opened = oneEvent(modalTrigger, 'sp-opened'); - this.outerButton.click(); - await opened; - - // Attempt to scroll while modal is open - const scrollYBeforeScroll = window.scrollY; - window.scrollTo(0, 100); - await nextFrame(); - - // Verify that scrolling was prevented - expect(window.scrollY).to.equal(scrollYBeforeScroll); - - // Close modal overlay - const closed = oneEvent(modalTrigger, 'sp-closed'); - await sendMouse({ - steps: [ - { - type: 'click', - position: [1, 1], - }, - ], - }); - await closed; - - // Verify scrolling works again after modal is closed - window.scrollTo(0, 100); - await nextFrame(); - expect(window.scrollY).to.equal(100); - - // Clean up - document.body.removeChild(longContent); - }); }); describe('System interactions', () => { afterEach(async () => { diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts index 41c188e0f33..6cb6c8606fe 100644 --- a/packages/overlay/test/overlay.test.ts +++ b/packages/overlay/test/overlay.test.ts @@ -10,20 +10,20 @@ * governing permissions and limitations under the License. */ import '@spectrum-web-components/button/sp-button.js'; -import { Dialog } from '@spectrum-web-components/dialog'; import '@spectrum-web-components/dialog/sp-dialog.js'; +import '@spectrum-web-components/overlay/sp-overlay.js'; +import '@spectrum-web-components/overlay/overlay-trigger.js'; +import '@spectrum-web-components/tooltip/sp-tooltip.js'; +import { Dialog } from '@spectrum-web-components/dialog'; +import '@spectrum-web-components/popover/sp-popover.js'; +import { Popover } from '@spectrum-web-components/popover'; +import { setViewport } from '@web/test-runner-commands'; import { Overlay, OverlayTrigger, Placement, VirtualTrigger, } from '@spectrum-web-components/overlay'; -import '@spectrum-web-components/overlay/overlay-trigger.js'; -import '@spectrum-web-components/overlay/sp-overlay.js'; -import { Popover } from '@spectrum-web-components/popover'; -import '@spectrum-web-components/popover/sp-popover.js'; -import '@spectrum-web-components/tooltip/sp-tooltip.js'; -import { setViewport } from '@web/test-runner-commands'; import { elementUpdated, @@ -32,29 +32,26 @@ import { nextFrame, oneEvent, } from '@open-wc/testing'; -import { render, TemplateResult } from '@spectrum-web-components/base'; -import { Button } from '@spectrum-web-components/button'; -import { Menu } from '@spectrum-web-components/menu'; -import { Theme } from '@spectrum-web-components/theme'; -import '@spectrum-web-components/theme/sp-theme.js'; -import '@spectrum-web-components/theme/src/themes.js'; import { sendKeys } from '@web/test-runner-commands'; -import { spy } from 'sinon'; +import { + clickAndHoverTarget, + definedOverlayElement, + virtualElement, +} from '../stories/overlay.stories'; +import { PopoverContent } from '../stories/overlay-story-components.js'; import { sendMouse } from '../../../test/plugins/browser.js'; -import { isFirefox } from '@spectrum-web-components/shared/src/platform.js'; +import { spy } from 'sinon'; +import '@spectrum-web-components/theme/sp-theme.js'; +import '@spectrum-web-components/theme/src/themes.js'; +import { Theme } from '@spectrum-web-components/theme'; +import { render, TemplateResult } from '@spectrum-web-components/base'; import { fixture, isInteractive, isOnTopLayer, - sendShiftTabKey, - sendTabKey, } from '../../../test/testing-helpers.js'; -import { PopoverContent } from '../stories/overlay-story-components.js'; -import { - clickAndHoverTarget, - definedOverlayElement, - virtualElement, -} from '../stories/overlay.stories'; +import { Menu } from '@spectrum-web-components/menu'; +import { Button } from '@spectrum-web-components/button'; // import { isWebKit } from '@spectrum-web-components/shared'; async function styledFixture( @@ -155,10 +152,7 @@ describe('Overlays', () => { clickSpy(); }); - expect( - await isInteractive(outerPopover), - 'outside popover is not interactive' - ).to.be.false; + expect(await isInteractive(outerPopover)).to.be.false; expect(button).to.exist; const opened = oneEvent(outerPopover, 'sp-opened'); @@ -215,22 +209,32 @@ describe('Overlays', () => { * that triggered the dialog and is outside of the modal. A test that was able to cycle would be better. */ - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement === button).to.be.false; - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement === button).to.be.false; - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); expect(document.activeElement === button).to.be.false; - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); expect(document.activeElement === button).to.be.false; - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); expect(document.activeElement === button).to.be.false; }); @@ -519,12 +523,16 @@ describe('Overlays', () => { expect(document.activeElement).to.equal(input); const closed = oneEvent(content, 'sp-closed'); - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); await closed; expect(document.activeElement).to.equal(trigger); - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(after); expect(await isInteractive(content)).to.be.false; }); @@ -563,11 +571,15 @@ describe('Overlays', () => { expect(document.activeElement).to.equal(input); - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); expect(document.activeElement).to.equal(trigger); - await sendShiftTabKey(); + await sendKeys({ + press: 'Shift+Tab', + }); expect(document.activeElement).to.equal(before); }); @@ -612,7 +624,7 @@ describe('Overlay - type="modal"', () => { before(async () => { render( html` - + ${virtualElement({ ...virtualElement.args, offset: 6, @@ -687,13 +699,15 @@ describe('Overlay - type="modal"', () => { }); it('closes the second "contextmenu" when clicking away', async () => { const closed = oneEvent(document, 'sp-closed'); - await sendMouse({ - type: 'click', - position: [width - width / 8, height - height / 8], + sendMouse({ + steps: [ + { + type: 'click', + position: [width - width / 8, height - height / 8], + }, + ], }); await closed; - await elementUpdated(firstMenu); - await elementUpdated(secondMenu); expect(firstRect.top).to.not.equal(secondRect.top); expect(firstRect.left).to.not.equal(secondRect.left); }); @@ -720,19 +734,21 @@ describe('Overlay - type="modal"', () => { const opened = oneEvent(document, 'sp-opened'); // Right click to open "context menu" overlay. - await sendMouse([ - { - type: 'move', - position: [270, 10], - }, - { - type: 'click', - options: { - button: 'right', + sendMouse({ + steps: [ + { + type: 'move', + position: [270, 10], }, - position: [270, 10], - }, - ]); + { + type: 'click', + options: { + button: 'right', + }, + position: [270, 10], + }, + ], + }); await opened; const firstMenu = document.querySelector('sp-menu') as Menu; @@ -740,7 +756,9 @@ describe('Overlay - type="modal"', () => { expect(await isInteractive(firstMenu)).to.be.true; const closed = oneEvent(document, 'sp-closed'); - sendKeys({ press: 'Escape' }); + sendKeys({ + press: 'Escape', + }); await closed; expect(await isInteractive(firstMenu)).to.be.false; @@ -791,9 +809,13 @@ describe('Overlay - type="modal"', () => { expect(overlayTrigger.open).to.equal('click'); const closed = oneEvent(trigger, 'sp-closed'); - await sendMouse({ - type: 'click', - position: [1, 1], + sendMouse({ + steps: [ + { + type: 'click', + position: [1, 1], + }, + ], }); await closed; @@ -803,8 +825,6 @@ describe('Overlay - type="modal"', () => { }); it('should not open hover overlay right after closing the click overlay using the keyboard', async () => { - // @TODO: skipping on Firefox due to flakiness. Will review in the migration to Spectrum 2. - if (isFirefox()) return; const overlayTrigger = await fixture( clickAndHoverTarget() ); @@ -820,9 +840,10 @@ describe('Overlay - type="modal"', () => { expect(overlayTrigger.open).to.equal('click'); const closed = oneEvent(trigger, 'sp-closed'); - sendKeys({ press: 'Escape' }); + sendKeys({ + press: 'Escape', + }); await closed; - await elementUpdated(overlayTrigger); expect(overlayTrigger.open).to.be.undefined; expect(document.activeElement === trigger, 'trigger focused').to.be @@ -833,21 +854,13 @@ describe('Overlay - timing', () => { it('manages multiple modals in a row without preventing them from closing', async () => { const test = await fixture(html`
- + Trigger 1

Hover contentent for "Trigger 1".

 - + Trigger 2

Click contentent for "Trigger 2".

@@ -885,23 +898,35 @@ describe('Overlay - timing', () => { // Move pointer over "Trigger 1", should _start_ to open "hover" content. await sendMouse({ - type: 'move', - position: trigger1Position, + steps: [ + { + type: 'move', + position: trigger1Position, + }, + ], }); await nextFrame(); await nextFrame(); // Move pointer out of "Trigger 1", should _start_ to close "hover" content. await sendMouse({ - type: 'move', - position: outsideTriggers, + steps: [ + { + type: 'move', + position: outsideTriggers, + }, + ], }); await nextFrame(); await nextFrame(); // Move pointer over "Trigger 2", should _start_ to open "hover" content. await sendMouse({ - type: 'move', - position: trigger2Position, + steps: [ + { + type: 'move', + position: trigger2Position, + }, + ], }); await nextFrame(); await nextFrame(); @@ -909,8 +934,12 @@ describe('Overlay - timing', () => { const opened = oneEvent(trigger2, 'sp-opened'); // Click "Trigger 2", should _start_ to open "click" content and _start_ to close "hover" content. await sendMouse({ - type: 'click', - position: trigger2Position, + steps: [ + { + type: 'click', + position: trigger2Position, + }, + ], }); await opened; await nextFrame(); @@ -923,8 +952,12 @@ describe('Overlay - timing', () => { const closed = oneEvent(overlayTrigger2, 'sp-closed'); await sendMouse({ - type: 'click', - position: outsideTriggers, + steps: [ + { + type: 'click', + position: outsideTriggers, + }, + ], }); await closed; @@ -950,10 +983,14 @@ describe('maintains focus consistency in all browsers', () => { const opened = oneEvent(trigger, 'sp-opened'); await sendMouse({ - type: 'click', - position: [ - boundingRect.left + boundingRect.width / 2, - boundingRect.top + boundingRect.height / 2, + steps: [ + { + type: 'click', + position: [ + boundingRect.left + boundingRect.width / 2, + boundingRect.top + boundingRect.height / 2, + ], + }, ], }); await opened; @@ -962,8 +999,12 @@ describe('maintains focus consistency in all browsers', () => { const closed = oneEvent(trigger, 'sp-closed'); await sendMouse({ - type: 'click', - position: [0, 0], + steps: [ + { + type: 'click', + position: [0, 0], + }, + ], }); await closed; @@ -1041,7 +1082,9 @@ describe('Overlay should correctly trap focus', () => { const opened = oneEvent(overlay, 'sp-opened'); // use keyboard to open the overlay trigger.focus(); - await sendKeys({ press: 'Enter' }); + await sendKeys({ + press: 'Enter', + }); await opened; expect(overlay.open).to.be.true; @@ -1053,15 +1096,21 @@ describe('Overlay should correctly trap focus', () => { expect(document.activeElement).to.equal(button1); // press tab to focus on button2 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button2); // press tab to focus on button1 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button1); // press tab to focus on button2 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button2); }); it('should trap focus when the overlay type is page', async () => { @@ -1096,15 +1145,21 @@ describe('Overlay should correctly trap focus', () => { expect(document.activeElement).to.equal(button1); // press tab to focus on button2 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button2); // press tab to focus on button1 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button1); // press tab to focus on button2 - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); expect(document.activeElement).to.equal(button2); }); it('should not trap focus when the overlay type is auto', async () => { @@ -1132,44 +1187,12 @@ describe('Overlay should correctly trap focus', () => { expect(overlay.open).to.be.true; - await sendTabKey(); + await sendKeys({ + press: 'Tab', + }); const input = el.querySelector('#input') as HTMLInputElement; expect(document.activeElement).to.equal(input); }); }); -describe('Overlay - Deprecated Properties', () => { - it('should support allowOutsideClick property with deprecation warning', async () => { - const consoleSpy = spy(console, 'warn'); - - const el = await fixture(html` -
- Open Overlay - - -

Overlay content

- - -
- `); - - const overlay = el.querySelector('sp-overlay') as Overlay; - await elementUpdated(overlay); - - // Verify the property is set correctly - expect(overlay.allowOutsideClick).to.be.true; - expect(overlay.hasAttribute('allow-outside-click')).to.be.true; - - // Verify the deprecation warning is shown (either via SWC or console.warn fallback) - expect(consoleSpy.calledOnce).to.be.true; - expect(consoleSpy.firstCall.args[0]).to.include('allow-outside-click'); - expect(consoleSpy.firstCall.args[0]).to.include('deprecated'); - - consoleSpy.restore(); - }); -}); diff --git a/packages/picker/src/InteractionController.ts b/packages/picker/src/InteractionController.ts index 3c34907ac0f..eaea85a23a6 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/test/index.ts b/packages/picker/test/index.ts index 3242018f159..4725a6a7455 100644 --- a/packages/picker/test/index.ts +++ b/packages/picker/test/index.ts @@ -1241,6 +1241,15 @@ export function runPickerTests(): void { await sendTabKey(); await focused; + expect(el.focused, 'focused').to.be.true; + expect(!el.open, 'closed').to.be.true; + expect(document.activeElement === el, 'focuses el').to.be.true; + + // tab through the picker to input2 + focused = oneEvent(input2, 'focus'); + await sendTabKey(); + await focused; + expect(document.activeElement === input2, 'focuses input 2').to .be.true; }); diff --git a/packages/picker/test/picker-responsive.test.ts b/packages/picker/test/picker-responsive.test.ts index 4367044d5a7..703d5614655 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/packages/tabs/src/Tabs.ts b/packages/tabs/src/Tabs.ts index 0c66f010a11..430baa70609 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 e5f6fb7a864..8c2a6c58105 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 7983c9b1b6a..ef2fb53ced6 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. */ @@ -67,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; /** @@ -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`