Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(interact outside): enable interact outside interception #1114

Merged

Conversation

anatolzak
Copy link
Contributor

@anatolzak anatolzak commented Mar 22, 2024

Closes #917, #1128

This PR adds the ability for a DOM element to intercept an outside interaction by calling e.stopPropagation().

Let's use an example that demonstrates how the DOM behaves and the challenges it imposes for this PR:

On a mouse device, clicking an element will trigger all of the following events:

  1. pointerdown
  2. pointerup
  3. mousedown
  4. mouseup
  5. click

If the user intercepts the pointerdown event, pointerup and click will still trigger while the mouse events will not.
If the user intercepts mousedown or mouseup, all the other events will still be triggered.

Let's use another example: on a touch device, clicking an element will trigger all of the following events:

  1. pointerdown
  2. touchstart
  3. pointerup
  4. touchend
  5. mousedown
  6. mouseup
  7. click

If the user intercepts touchstart or touchend, the mouse and click events will NOT be triggered but the pointer events WILL.

Ok, so ideally for our sake, the user should be able to intercept any of the events and prevent an outside interaction.

Currently, we default to only using pointer events and fallback to mouse and touch events when pointer events are not available.

But in the case of issue #917, clicking on the 1Password button inside an input that is inside a dialog closes the dialog. Despite 1Password intercepting the mouse events, our pointerup handler is still triggered, leading to the unintended closure of the dialog.

So we should listen to all the appropriate events and trigger an outside interaction only if none of the events were intercepted by the user.

Similar to PR #845 that enabled intercepting an escape key down by not using capture listeners, in order to allow intercepting outside interactions, we should not trigger an outside interaction using capture listeners.

However the challenge is--how do you determine if any of the events were intercepted when the browser will trigger a bunch of other events nonetheless.

To identify intercepted events during an interaction, we implement a two-step approach for the following events: pointerdown, touchstart, pointerup, touchend, mousedown, mouseup, and click. First, we attach an event listener in the capture phase (capture: true) to flag the event as intercepted. Subsequently, we add another event listener for the bubbling phase, designed to tag the event as NOT intercepted if it passes through. This setup relies on the principle that the bubbling phase listener is called only if the event was not intercepted by the user. Conversely, if an event is intercepted, only the capture phase listener will trigger, which marks the event as intercepted.

So now we can easily check if any events were intercepted and not trigger an outside interaction in that case.

Since we attach event listeners for multiple events, the browser may trigger multiple events at the same time (i.e. pointerdown touchstart, pointerup touchend, etc.). In order to make sure that we don't run into race conditions and prematurely check whether any events were intercepted before all other bubbling handlers have potentially marked the events as not intercepted, we debounce the event handlers, which also prevents them from being executed more than once.

After an interaction has finished, we also need to correctly reset the interceptedEvents object so that future interactions that are not intercepted will behave as expected. We should only reset interceptedEvents at the end of an interaction (touchend, pointerup, mouseup, click) in case the user intercepted the beginning of an interaction while still expecting the whole interaction to be intercepted. We debounce resetting interceptedEvents to avoid race conditions. We should also call resetInterceptedEventsDebounced in the capture phase handlers since the bubbling phase handlers may not be called due to user interception.

I modified the dialog example in the docs and added buttons that intercept different events for demonstration purposes. The modified docs should not be included in the final PR.

Also, I added a lot of tests that make sure all this works correctly so don't be intimidated by the number of lines added by this PR :)

Thanks for reading all this :)

Before

Screen.Recording.2024-03-22.at.5.22.06.PM.mov

After

Screen.Recording.2024-03-22.at.5.22.34.PM.mov

Mouse Device

Screen.Recording.2024-03-22.at.5.23.48.PM.mov

Touch Device

Screen.Recording.2024-03-23.at.7.25.44.PM.mov

@anatolzak anatolzak marked this pull request as draft March 22, 2024 15:27
Copy link

changeset-bot bot commented Mar 22, 2024

🦋 Changeset detected

Latest commit: bcf7236

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

This PR includes changesets to release 1 package
Name Type
@melt-ui/svelte Patch

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

@anatolzak anatolzak marked this pull request as ready for review March 23, 2024 17:25
@anatolzak anatolzak marked this pull request as ready for review March 27, 2024 13:02
@huntabyte
Copy link
Member

Hey @anatolzak this looks great, incredible investigative work to tackle this problem! Whenever you're ready (after cleaning up the demo) let me know and we'll get this merged!

@anatolzak
Copy link
Contributor Author

anatolzak commented Mar 28, 2024

@huntabyte I am glad all this makes sense lol :)
I cleaned up the demo so this PR is ready! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dialog: closes when 1password icon is clicked
2 participants