Skip to content

Conversation

@Rajdeepc
Copy link
Contributor

@Rajdeepc Rajdeepc commented Nov 24, 2025

Description

This PR restores the click-blocking behavior for modal and page overlays that was lost when migrating from dialog.showModal() to dialog.showPopover() for performance reasons. The fix manually implements the click-blocking functionality that showModal() provided automatically, ensuring that users cannot interact with elements outside of an open modal overlay.

Key changes:

  • Added event handlers (handlePointerdown and handleClick) that intercept pointer and click events in the capture phase
  • Implemented logic to detect if clicks/pointer events originate inside modal overlay dialogs using event.composedPath()
  • Block external clicks by calling preventDefault(), stopPropagation(), and stopImmediatePropagation() when clicks occur outside modal overlays
  • Added a transparent backdrop element that helps catch clicks outside modal dialogs
  • Refactored duplicate logic into reusable helper methods (getModalOverlays() and isEventInsideModal()) for better maintainability

Motivation and context

In version 1.7.0, the overlay implementation migrated from using dialog.showModal() to dialog.showPopover() for performance improvements. However, showModal() provided native click-blocking behavior that prevented users from clicking elements outside the modal overlay, while showPopover() does not provide this behavior.

This created a regression where modal overlays no longer blocked external clicks, allowing users to interact with elements behind the modal overlay. This violates accessibility best practices and user expectations for modal dialogs.

Expected behavior: When a modal overlay is open, users should not be able to click on elements outside of the overlay without first closing the overlay.

Previous behavior (1.4.0): Modal overlays correctly blocked external clicks using showModal().

Current behavior (1.7.0+): Modal overlays allowed external clicks, breaking the modal interaction pattern.

This fix restores the expected behavior while maintaining the performance benefits of using showPopover().

Related issue(s)

Screenshots (if appropriate)

Before (broken behavior):

  • External button remains clickable when modal overlay is open
  • Cursor changes to pointer when hovering over external elements
  • Click events reach external elements despite modal being open

DEMO: https://stackblitz.com/edit/vitejs-vite-nvgyzymb?file=package.json,src%2Fmy-element.ts

After (fixed behavior):

  • External button is not clickable when modal overlay is open
  • Cursor does not change when hovering over external elements
  • Click events are blocked from reaching external elements

DEMO: https://swcpreviews.z13.web.core.windows.net/pr-5907/docs/first-gen-storybook/?path=/story/overlay-element--modal-click-blocking

Technical implementation details

Event handling strategy

The fix uses a two-pronged approach:

  1. Capture phase interception: Event listeners are attached with capture: true to intercept events before they reach their target elements
  2. Path-based detection: Uses event.composedPath() to determine if the event originated inside a modal overlay dialog

Code structure

  • getModalOverlays(): Helper method that filters the overlay stack to get only open modal/page overlays
  • isEventInsideModal(): Core logic that checks if an event path intersects with any modal overlay dialog
  • handlePointerdown(): Intercepts pointerdown events to block interactions early in the event chain
  • handleClick(): Intercepts click events as a secondary layer of protection
  • manageModalBackdrop(): Creates/removes a transparent backdrop element to help catch external clicks

Browser compatibility

The implementation uses standard DOM APIs (composedPath(), contains(), preventDefault(), etc.) that are supported across all modern browsers. The fix has been tested and verified to work in:

  • Chromium-based browsers (Chrome, Edge)
  • Firefox
  • WebKit-based browsers (Safari)

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Modal overlay click blocking

    1. Navigate to the Storybook story overlay-element.stories.tsmodalClickBlocking
    2. Click the "Open overlay" button to open a modal overlay
    3. Try to click the "External" button below the modal
    4. Expected: The external button should NOT be clickable. No alert should appear. The cursor should not change to a pointer when hovering over the external button.
    5. Click the button inside the modal overlay
    6. Expected: The internal button should work correctly and show an alert
    7. Close the modal overlay (press ESC or click outside if allow-outside-click is enabled)
    8. Try clicking the "External" button again
    9. Expected: The external button should now be clickable and show an alert
  • Nested modal overlays

    1. Navigate to the Storybook story overlay-element.stories.tsnestedModalOverlays
    2. Open the outer modal overlay
    3. Open the inner modal overlay
    4. Try clicking elements outside both modals
    5. Expected: External clicks should be blocked
    6. Close the inner modal, then try clicking outside
    7. Expected: External clicks should still be blocked (outer modal is still open)
    8. Close the outer modal
    9. Expected: External elements should now be clickable
  • Page overlays

    1. Test with type="page" overlays
    2. Expected: Page overlays should also block external clicks (same behavior as modal overlays)
  • Non-modal overlays

    1. Test with type="auto", type="hint", and type="manual" overlays
    2. Expected: These overlay types should NOT block external clicks (only modal and page types should block)
  • Keyboard navigation

    1. Open a modal overlay
    2. Use Tab key to navigate focus
    3. Expected: Focus should be trapped within the modal (existing behavior, not changed by this PR)
    4. Press ESC to close
    5. Expected: Modal should close and focus should return appropriately

Device review

  • Did it pass in Desktop?
  • Did it pass in (emulated) Mobile?
  • Did it pass in (emulated) iPad?

Testing

Automated tests

All existing overlay tests pass (191 tests across Firefox, Chromium, and Webkit):

  • yarn test:focus overlay - All tests passing ✅

Breaking changes

None. This is a bug fix that restores expected behavior.

Additional notes

  • The implementation maintains backward compatibility with existing overlay usage
  • The fix does not affect non-modal overlay types (auto, hint, manual)
  • Focus trapping behavior (existing feature) remains unchanged
  • Body scroll blocking (existing feature) remains unchanged

@Rajdeepc Rajdeepc self-assigned this Nov 24, 2025
@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2025

🦋 Changeset detected

Latest commit: db515d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 78 packages
Name Type
@spectrum-web-components/overlay Minor
@spectrum-web-components/action-menu Minor
@spectrum-web-components/combobox Minor
@spectrum-web-components/contextual-help Minor
@spectrum-web-components/menu Minor
@spectrum-web-components/picker Minor
@spectrum-web-components/popover Minor
@spectrum-web-components/tooltip Minor
@spectrum-web-components/bundle Minor
@spectrum-web-components/truncated Minor
@spectrum-web-components/breadcrumbs Minor
@spectrum-web-components/action-bar Minor
@spectrum-web-components/card Minor
@spectrum-web-components/coachmark Minor
@spectrum-web-components/accordion Minor
@spectrum-web-components/action-button Minor
@spectrum-web-components/action-group Minor
@spectrum-web-components/alert-banner Minor
@spectrum-web-components/alert-dialog Minor
@spectrum-web-components/asset Minor
@spectrum-web-components/avatar Minor
@spectrum-web-components/badge Minor
@spectrum-web-components/button-group Minor
@spectrum-web-components/button Minor
@spectrum-web-components/checkbox Minor
@spectrum-web-components/clear-button Minor
@spectrum-web-components/close-button Minor
@spectrum-web-components/color-area Minor
@spectrum-web-components/color-field Minor
@spectrum-web-components/color-handle Minor
@spectrum-web-components/color-loupe Minor
@spectrum-web-components/color-slider Minor
@spectrum-web-components/color-wheel Minor
@spectrum-web-components/dialog Minor
@spectrum-web-components/divider Minor
@spectrum-web-components/dropzone Minor
@spectrum-web-components/field-group Minor
@spectrum-web-components/field-label Minor
@spectrum-web-components/help-text Minor
@spectrum-web-components/icon Minor
@spectrum-web-components/icons-ui Minor
@spectrum-web-components/icons-workflow Minor
@spectrum-web-components/icons Minor
@spectrum-web-components/iconset Minor
@spectrum-web-components/illustrated-message Minor
@spectrum-web-components/infield-button Minor
@spectrum-web-components/link Minor
@spectrum-web-components/meter Minor
@spectrum-web-components/modal Minor
@spectrum-web-components/number-field Minor
@spectrum-web-components/picker-button Minor
@spectrum-web-components/progress-bar Minor
@spectrum-web-components/progress-circle Minor
@spectrum-web-components/radio Minor
@spectrum-web-components/search Minor
@spectrum-web-components/sidenav Minor
@spectrum-web-components/slider Minor
@spectrum-web-components/split-view Minor
@spectrum-web-components/status-light Minor
@spectrum-web-components/swatch Minor
@spectrum-web-components/switch Minor
@spectrum-web-components/table Minor
@spectrum-web-components/tabs Minor
@spectrum-web-components/tags Minor
@spectrum-web-components/textfield Minor
@spectrum-web-components/thumbnail Minor
@spectrum-web-components/toast Minor
@spectrum-web-components/top-nav Minor
@spectrum-web-components/tray Minor
@spectrum-web-components/underlay Minor
@spectrum-web-components/base Minor
@spectrum-web-components/grid Minor
@spectrum-web-components/opacity-checkerboard Minor
@spectrum-web-components/reactive-controllers Minor
@spectrum-web-components/shared Minor
@spectrum-web-components/styles Minor
@spectrum-web-components/theme Minor
@spectrum-web-components/eslint-plugin Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rajdeepc Rajdeepc added the Status: WIP PR is a work in progress or draft label Nov 24, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 24, 2025

📚 Branch Preview Links

🔍 First Generation Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-5907

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@Rajdeepc Rajdeepc marked this pull request as ready for review November 24, 2025 11:43
@Rajdeepc Rajdeepc requested a review from a team as a code owner November 24, 2025 11:43
@Rajdeepc Rajdeepc added Status: Ready for review PR ready for review or re-review. SEV 2 significant problem re: design, usability, or functionality of the system/components for some users SEV 3 Issue that cause noticeable errors/minor usability problems which cause confusion or degrade UX and removed Status: WIP PR is a work in progress or draft SEV 2 significant problem re: design, usability, or functionality of the system/components for some users labels Nov 24, 2025
@TarunAdobe
Copy link
Contributor

I have a question for my understanding... If I navigate to overlay elements - nested modal overlays story and open both outer and inner overlay and then click outside in the empty space, both the overlays close. Is that the expected behaviour?

@Rajdeepc Rajdeepc changed the title fix(overlay): click-blocking behavior for modal and page overlays fix(overlay): click-blocking behavior for type="modal" and "page" overlays Nov 25, 2025
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed PR instructions. It made testing super easy.

The small nit with this story being named modal when the type is auto.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

SEV 3 Issue that cause noticeable errors/minor usability problems which cause confusion or degrade UX Status: Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Modal overlays do not prevent clicks on external elements

5 participants