Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/modal-click-blocking-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@spectrum-web-components/overlay': minor
---

**Fixed**: Modal and page overlays now properly block external clicks, restoring the expected modal interaction pattern while maintaining the performance benefits of `showPopover()`.

After migrating from `dialog.showModal()` to `dialog.showPopover()` in v1.7.0, modal overlays no longer prevented clicks on elements outside the overlay. This fix manually implements the click-blocking functionality by intercepting pointer and click events in the capture phase and blocking external clicks using `event.composedPath()` to detect if clicks originate inside modal dialogs.
168 changes: 165 additions & 3 deletions 1st-gen/packages/overlay/src/OverlayStack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,20 @@ class OverlayStack {

private bodyScrollBlocked = false;

private modalBackdrop: HTMLElement | null = null;

constructor() {
this.bindEvents();
}

bindEvents(): void {
this.document.addEventListener('pointerdown', this.handlePointerdown);
this.document.addEventListener('pointerdown', this.handlePointerdown, {
capture: true,
});
this.document.addEventListener('pointerup', this.handlePointerup);
this.document.addEventListener('click', this.handleClick, {
capture: true,
});
this.document.addEventListener('keydown', this.handleKeydown);
this.document.addEventListener('scroll', this.handleScroll, {
capture: true,
Expand Down Expand Up @@ -85,6 +92,7 @@ class OverlayStack {
overlay.open = false;

this.manageBodyScroll();
this.manageModalBackdrop();
}

/**
Expand All @@ -105,13 +113,166 @@ class OverlayStack {
}

/**
* Cach the `pointerdownTarget` for later testing
* Get all open modal/page overlays from the stack.
* Cached to avoid repeated filtering.
*/
private getModalOverlays(): Overlay[] {
return this.stack.filter(
(overlay) =>
overlay.open &&
(overlay.type === 'modal' || overlay.type === 'page')
);
}

/**
* Check if an event path intersects with any modal overlay dialog.
* This is the core logic for determining if a click/pointer event is inside a modal.
*
* @param event {ClickEvent}
* @param eventPath {EventTarget[]} The composed path from the event
* @param modalOverlays {Overlay[]} The modal overlays to check against
* @returns {boolean} True if the event is inside any modal overlay
*/
private isEventInsideModal(
eventPath: EventTarget[],
modalOverlays: Overlay[]
): boolean {
for (const overlay of modalOverlays) {
// Check if overlay element itself is in the path
if (eventPath.includes(overlay)) {
return true;
}

try {
let dialogEl: HTMLElement | null = overlay.dialogEl;
if (!dialogEl) {
// Try to get it from the shadow root if not available via query
const shadowRoot = overlay.shadowRoot;
if (shadowRoot) {
dialogEl = shadowRoot.querySelector('.dialog');
if (!dialogEl) {
continue;
}
} else {
continue;
}
}

// When clicking inside a popover dialog, the dialog element
// should be in the composedPath even though it's in the top layer
if (eventPath.includes(dialogEl)) {
return true;
}

// Check if any element in the path is contained by dialog
for (const element of eventPath) {
if (element instanceof Node && dialogEl.contains(element)) {
return true;
}
}
} catch {
// dialogEl might not be accessible yet, ignore
continue;
}
}
return false;
}

/**
* Manage backdrop element for modal/page overlays to block clicks outside.
* The backdrop catches all pointer events, and we check if they're inside
* the modal dialog before allowing them through.
*/
private manageModalBackdrop(): void {
const hasModalOverlay = this.stack.some(
(overlay) =>
overlay.open &&
(overlay.type === 'modal' || overlay.type === 'page')
);

if (hasModalOverlay && !this.modalBackdrop) {
// Create backdrop element that covers the entire screen
// It will be below the dialog (which is in top layer) but above everything else
this.modalBackdrop = document.createElement('div');
this.modalBackdrop.style.cssText = `
position: fixed;
inset: 0;
z-index: 999998;
background: transparent;
pointer-events: auto;
`;
// The backdrop will catch pointer events, and our handlers will check
// if the click is inside a modal before allowing it through
document.body.appendChild(this.modalBackdrop);
} else if (!hasModalOverlay && this.modalBackdrop) {
// Remove backdrop when no modal overlays are open
this.modalBackdrop.remove();
this.modalBackdrop = null;
}
}

/**
* Cache the `pointerdownTarget` for later testing and prevent clicks outside modal overlays
*
* @param event {PointerEvent}
*/
handlePointerdown = (event: Event): void => {
this.pointerdownPath = event.composedPath();
this.lastOverlay = this.stack[this.stack.length - 1];

// Check for modal overlays and prevent pointerdown outside them
// This ensures we block the interaction before click handlers can run
if (!this.stack.length) return;

const modalOverlays = this.getModalOverlays();
if (!modalOverlays.length) return;

const pointerPath = event.composedPath();
if (!this.isEventInsideModal(pointerPath, modalOverlays)) {
// Block pointerdown outside modal overlays
event.preventDefault();
event.stopPropagation();
event.stopImmediatePropagation();
// Also cancel the pointer event to prevent click from firing
if (event instanceof PointerEvent) {
(event.target as HTMLElement)?.releasePointerCapture?.(
event.pointerId
);
}
}
};

/**
* Prevent clicks outside modal overlays from reaching external elements.
* This replicates the behavior of dialog.showModal() which was removed
* in favor of showPopover() for performance reasons.
*
* @param event {MouseEvent}
*/
handleClick = (event: MouseEvent): void => {
if (!this.stack.length) return;

const modalOverlays = this.getModalOverlays();
if (!modalOverlays.length) return;

// If click is on the backdrop, it's outside - block it
if (event.target === this.modalBackdrop) {
event.preventDefault();
event.stopPropagation();
event.stopImmediatePropagation();
return;
}

// Check if the click is inside any modal dialog
// When a popover dialog is open, clicking inside it will have the dialog
// element in the composedPath. Clicking outside won't.
const clickPath = event.composedPath();
if (!this.isEventInsideModal(clickPath, modalOverlays)) {
// If click is outside all modal overlays, prevent it from reaching the target
// This replicates the behavior that showModal() provided automatically
event.stopImmediatePropagation();
event.stopPropagation();
event.preventDefault();
}
};

/**
Expand Down Expand Up @@ -279,6 +440,7 @@ class OverlayStack {
once: true,
});
this.manageBodyScroll();
this.manageModalBackdrop();
});
}

Expand Down
51 changes: 51 additions & 0 deletions 1st-gen/packages/overlay/stories/overlay-element.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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';
import '@spectrum-web-components/action-group/sp-action-group.js';
import '@spectrum-web-components/button/sp-button.js';
import '@spectrum-web-components/popover/sp-popover.js';
import '@spectrum-web-components/menu/sp-menu-group.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
Expand Down Expand Up @@ -980,3 +981,53 @@ nestedModalOverlays.swc_vrt = {
nestedModalOverlays.parameters = {
chromatic: { disableSnapshot: true },
};

export const modalClickBlocking = (): TemplateResult => html`
<div style="padding: 20px;">
<h2>Modal Overlay Click Blocking Test</h2>
<p>
Click "Open overlay" to open a modal overlay. Then try clicking the
"External" button. The external button should NOT be clickable when
the modal is open.
</p>
<sp-button id="trigger">Open overlay</sp-button>

<sp-overlay trigger="trigger@click" type="modal" placement="bottom">
<sp-popover style="padding: 10px">
<sp-button
onclick="alert('Internal button clicked! This should work.')"
>
Add a div
</sp-button>
<p style="margin-top: 10px;">
This is inside the modal overlay. Clicking the button above
should work.
</p>
</sp-popover>
</sp-overlay>

<sp-button
id="externalButton"
onclick="alert('External button clicked! This should NOT work when modal is open.')"
style="margin-top: 20px;"
>
External
</sp-button>
<p style="margin-top: 10px; color: red;">
⚠️ When the modal is open, clicking "External" should be blocked. If
you can click it and see the alert, the fix is not working.
</p>
</div>
`;

modalClickBlocking.swc_vrt = {
skip: true,
};

modalClickBlocking.parameters = {
tags: ['!dev'],
};

modalClickBlocking.parameters = {
chromatic: { disableSnapshot: true },
};
72 changes: 72 additions & 0 deletions 1st-gen/packages/overlay/test/overlay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,78 @@ describe('Overlay - type="modal"', () => {
expect(document.activeElement === trigger, 'trigger focused').to.be
.true;
});

it('should prevent clicks on external elements when modal overlay is open', async () => {
const externalButtonClickSpy = spy();
const internalButtonClickSpy = spy();

const el = await fixture<HTMLDivElement>(html`
<div>
<sp-button id="trigger">Open Overlay</sp-button>
<sp-overlay trigger="trigger@click" type="modal">
<sp-popover>
<sp-button
id="internal-button"
@click=${internalButtonClickSpy}
>
Internal Button
</sp-button>
<p>Modal content</p>
</sp-popover>
</sp-overlay>
<sp-button
id="external-button"
@click=${externalButtonClickSpy}
>
External Button
</sp-button>
</div>
`);

const trigger = el.querySelector('#trigger') as HTMLElement;
const overlay = el.querySelector('sp-overlay') as Overlay;
const externalButton = el.querySelector(
'#external-button'
) as HTMLElement;
const internalButton = el.querySelector(
'#internal-button'
) as HTMLElement;

await elementUpdated(overlay);

// Open modal overlay
const opened = oneEvent(overlay, 'sp-opened');
trigger.click();
await opened;

expect(overlay.open).to.be.true;

// Try to click external button - should be blocked
externalButton.click();
await nextFrame();

// External button click should not have fired
expect(externalButtonClickSpy.called).to.be.false;

// Internal button click should work
internalButton.click();
await nextFrame();

// Internal button click should have fired
expect(internalButtonClickSpy.called).to.be.true;

// Close modal overlay
const closed = oneEvent(overlay, 'sp-closed');
overlay.open = false;
await closed;

// After closing, external button should be clickable
externalButton.click();
await nextFrame();

// External button click should now fire
expect(externalButtonClickSpy.called).to.be.true;
});
});
describe('Overlay - timing', () => {
it('manages multiple modals in a row without preventing them from closing', async () => {
Expand Down
4 changes: 2 additions & 2 deletions 1st-gen/packages/picker/stories/picker.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ BackgroundClickTest.swc_vrt = {
skip: true,
};

export const PickerInModalOverlay = (): TemplateResult => {
export const PickerInOverlay = (): TemplateResult => {
return html`
<div>
<div>
Expand Down Expand Up @@ -881,7 +881,7 @@ export const PickerInModalOverlay = (): TemplateResult => {
</div>
`;
};
PickerInModalOverlay.swc_vrt = {
PickerInOverlay.swc_vrt = {
skip: true,
};

Expand Down
Loading