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

stopPropagation on the parent element affects event handling on the child #204

Open
w1ndy opened this issue Oct 16, 2022 · 7 comments
Open
Labels
documentation Improvements or additions to documentation

Comments

@w1ndy
Copy link

w1ndy commented Oct 16, 2022

Describe the bug

I'm trying to implement a 'onClickOutside' directive. Somehow when I stop the click event propagation on the parent element in the directive, the onClick handler on the child element no longer works. A bug reproduction is provided below.

Your Example Website or App

https://playground.solidjs.com/?hash=1327590733&version=1.4.1

Steps to Reproduce the Bug or Issue

Click on the click me button.

Expected behavior

Expected console output: works followed by stopped, indicating that the click event is received by the button first, and then bubbled up to the onClickOutside directive, which stops the event propagation to the document.

Current console output: stopped. Only the directive is getting the click event.

Screenshots or Videos

No response

Platform

  • OS: Windows
  • Browser: Firefox
  • Version: 105.0.3

Additional context

No response

@ryansolid
Copy link
Member

Yeah this is because of the timing as on element events will run before the event delegation. This is similar to React. Trying to decide what the best answer to tell you is. You can use Solid's event listener helper directly. Like this: https://playground.solidjs.com/?hash=1027922061&version=1.4.1.

But part of me would like to point to something that feels more idiomatic. Probably some ability to extend element with native props.. like basically using Solid's internal spread helper, but then I'm reminded this is just more efficient even if not documented.

@LiQuidProQuo
Copy link

for click outside, you can prioritize the detection handler by "capturing" the event.

  document.addEventListener("click", handleClick, { capture: true });

https://playground.solidjs.com/?hash=1020624715&version=1.4.1

@w1ndy
Copy link
Author

w1ndy commented Oct 17, 2022

@ryansolid @LiQuidProQuo Thanks for the tips! I tried both examples, and they worked correctly (although I had to use stopImmediatePropagation with Ryan's example).

However, I'm still struggling to understand why stopPropagation during the event bubbling phase on the parent element will prevent the onClick handler on the target element from being executed... The order of handler execution should be capturing -> target -> bubbling, right? So, why the event handler on the target element is not executed at all?

@ryansolid
Copy link
Member

We delegate to the document. It's a performance trick, works well with Portals, and helps with some fun hydration stuff. Many of Solid's events, including mouse, pointer, input events are all attached to the document. So adding a direct event handler actually triggers before any added in the JSX when bubbling. This implicit delegation is common in a number of frameworks including React.

@w1ndy
Copy link
Author

w1ndy commented Oct 17, 2022

@ryansolid Good to know, thanks! Shall we add some text to docs (maybe this page since the example provided is to add event listeners?) to remind people about the order of event handler execution?

@ryansolid
Copy link
Member

Yeah delegation is mentioned but not explained at all. It belongs under on__ definitely.

@ryansolid ryansolid added the documentation Improvements or additions to documentation label Oct 17, 2022
@ryansolid ryansolid transferred this issue from solidjs/solid Oct 18, 2022
@ryansolid
Copy link
Member

This is completely a docs issue. I'm going to move it there so we can be reminded to put in the right mention.

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

No branches or pull requests

3 participants