Open olivercoad opened 5 months ago
I've made some unit tests for dom-expressions and have an idea about how to fix it. I'm thinking perhaps in eventHandler where it walks up the tree, every time it uses host
to walk up a shadowRoot, it can swap out target to that node. I'm just not sure if slots would complicate things or if they would work seamlessly, so would have to test how they work.
Should I go ahead with attempting the fix and make a PR?
Yeah I'm no longer deep in webcomponents so I haven't played with this for a while. I did retargetting due to yes having SolidJS in components. I built apps that had nested levels of Solid web components. So when I extracted a component out like say a wrapper around input I found it odd that I didn't have the input element anymore. I found a React package that did similar retargetting and used that as my solution.
Resetting on host seems reasonable. Basically it is only reversing the event target for the deepest level where the event orginates. I would need a second opinion from some of the web component people in our community like @trusktr. That being said I'm not sure any of them use event delegation anyway because of other issues.
I see now a lot of my webcomponent considerations when building Solid were fighting against them. I didn't really want web components but I wanted to use the platform. I should have just left them to do their own thing earlier and built what I ultimately wanted to.
That being said I'm not sure any of them use event delegation anyway because of other issues
I'm not deep in web components either, just consuming a few from external libraries. What kind of other issues are there? Searching old issues on github, the only things I can see are for non-composed events (but it looks like delegation is whitelisted now anyway) and shadow with mode closed.
I don't want to disable event delegation completely because for this particular project it's actually a massive performance win.
Retargetting on host works well and makes the target correct for things inside as well as outside the component, as well as being correct for non-delegated handlers that run afterwards. However, the unit tests revealed another issue which is that events originating inside a slot don't get passed to delegated handlers inside the shadow DOM. While I'm in the headspace I'll try to find an easy fix for that too, otherwise will make a PR with just the original fix.
Describe the bug
I started investigating this issue when I noticed that adding a SUID button to a page caused all ninja-keys hotkeys (including up/down arrows to select commands) to stop working while the ninja-keys menu is open. After stepping through the debugger, the reason is that ninja-keys is a web component and relies on keyboard events from within the menu having a target of the
ninja-keys
component instead of the actual textinput
element inside the menu. However, the SUID button uses solidonKeyDown
andonKeyUp
, causing these events to use solid's event delegation from the document, which overrides the eventtarget
.Ryan explains in this video about "reverse shadow DOM retargetting". There's not really an explaination for why solid deviates from the web standards here, but I'd assume it's so that events have the expected target when using solid inside web components, such as with solid-element. When listening to events from outside a web component though, I think it should not do reverse shadow DOM retargetting, especially when using an event listener that's not using solid's event delegation (whether it's an external library like ninja-keys using hotkeys-js, or with
on:
directives in solid).It's worth noting that the order the event listeners on document are added matters; if it was only modifying the event "once everyone else is done with it" then that would be fine, but when event listeners are added after solid has already initialised it's the event delegation, then it gets the mutated event.
A simple fix could be to just keep hold of the original target and put it back after solid's event delegation is done with it. That wouldn't make event handlers that do use event delegation get the "correct" target, but it would at least fix it for event handlers that aren't using event delegation.
Your Example Website or App
https://stackblitz.com/edit/solidjs-templates-zz63ns?file=src%2FApp.tsx
Steps to Reproduce the Bug or Issue
keyDown target B
showsINPUT
Expected behavior
All event listeners should show the same thing,
MY-INPUT-COMPONENT
, because the events originate from inside the web component and the listeners are on document, outside the web component.Screenshots or Videos
No response
Platform
Additional context
No response