Skip to content

Conversation

@jimfb
Copy link
Contributor

@jimfb jimfb commented Feb 24, 2015

Register for events in the proper Document.

When components are mounted into a shadow DOM, the default document may not be the correct document for listening for events. This change finds the proper document for the container component and registers for events.

@sophiebits
Copy link
Collaborator

cf. #1877

@sophiebits
Copy link
Collaborator

also #2617

@sebmarkbage
Copy link
Collaborator

The way we register events is already claimed to be one of the slowest parts of React and this makes it worse. I also don't think that registering deep listeners is the right solution. We should use document level listeners and use event.path to figure out which is the shadow target ID and delegate that to the right React components. Also suggested by the Blink/Web Components team. It doesn't solve the case when other frameworks blocks propagation above, but that kind of bubbling interop is fundamentally broken anyway.

@jimfb
Copy link
Contributor Author

jimfb commented Apr 8, 2015

@sebmarkbage Can't use event.path in this case, because the event gets repathed (is that a word? reparented? rerooted?) when it leaves the fragment. You need to attach a listener within the document/fragment in order to get the correct path.

@sebmarkbage
Copy link
Collaborator

@JSFB Why not? This seems to work: http://jsfiddle.net/j6hxede8/

You can't use event.target because it gets a different scope environment but event.path is designed for this particular use case of event delegation into a shadow tree.

@jimfb
Copy link
Contributor Author

jimfb commented Apr 8, 2015

Edit: Actually, I think you're right. We would need to do fancy magic when creating the synthetic event, but a path based approach could probably work.

@jimfb
Copy link
Contributor Author

jimfb commented Jul 11, 2015

Closing in favor of: #4150

@jimfb jimfb closed this Jul 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants