-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Update cancel check to be shadowRoot compatible #2086
base: main
Are you sure you want to change the base?
Conversation
This proposal is made to correctly check if the event should be canceled if the first element in the composedPath (or the event.target) matches with the options.cancel selector
|
@mgol What do you think about this? |
@fnagel sure thing 👍 |
Fixed @fnagel. Surely this is a specific case scenario, where I'm using Sortable with Web Components, but this may come in hand when Web Components get more and more popular As answered in the question below, the purpose of using event.composedPath() is to get specifically which element triggered the event within the Web-Component (container). |
92bb322
to
e272483
Compare
@malarahfelipe Tests still fail :-( |
Sorry, missed the grunt test workflow |
Still failing. |
When using web-components with shadowRoot, the mouseDown event is triggered with the event.target pointing to the entire web component, not inner nodes that were clicked, thus to cancel the mouseDown based on inner-elements we have to use composedPath instead of event.target
It'd be good to see what case currently doesn't work on a live test case. This PR needs tests anyway, preparing an isolated test case may be a good starting point in creating one. As it stands, I don't have a specific opinion as I don't understand what exactly this PR is trying to fix. |
@mgol Thanks for the feedback! I must confess I'm not really familiar with Web Components so I hoped for your input :-) Maybe @dmethvin can help us out here? @malarahfelipe Thanks for fixing the tests! Can you give some some more insights? |
Thanks for the feedback on this guys Sure @fnagel |
Okay, after long time 😅
Also I've record a <1min loom video just dragging things around so you can see the output in the html / console https://www.loom.com/share/a2c4f95d5eae4589967a00041bcb7307 Thank you |
This proposal is made to ensure if the event should be canceled if the first element in the composedPath (or the event.target as fallback) matches with the options.cancel selector
This behaviour happens when we're using shadowRoot and we're trying to cancel the mousedown event when it starts from an element within the shadowRoot.