Skip to content

Conversation

apalaniuk
Copy link
Contributor

@apalaniuk apalaniuk commented Mar 23, 2020

What:

When using the fireEvent interface to generate synthetic events, if events which are normally composed are dispatched on elements that are within the shadow DOM of a web component, and the listeners expected to handle that event type are registered on the custom element itself (a common configuration), the event listeners will not be triggered as expected, as the event will stop bubbling at the shadow DOM boundary. This deviates from the behaviour of the matching native browser events which are composed, which will propagate outside of the shadow DOM.

Why:

This change updates synthetic event init definitions to more closely match their related browser events by setting their composed property to true if that event type is composed. Without this change, workarounds which couple tests to implementation details are often required in order to find the parent element against which events may be dispatched and handled, rather than dispatching the event against the found element directly and allowing for standard event propagation.

How:

A composed property is added to existing event initialization definitions where relevant. This allows for events to be composed in any environment that has, at minimum, the window.Event constructor.

Checklist:

  • Documentation added to the
    docs site N/A
  • I've prepared a PR for types targeting
    DefinitelyTyped N/A
  • Tests
  • Ready to be merged

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 23, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@codecov
Copy link

codecov bot commented Mar 23, 2020

Codecov Report

Merging #496 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #496   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        23    +1     
  Lines          425       425           
  Branches       101       101           
=========================================
  Hits           425       425           
Impacted Files Coverage Δ
src/events.js 100.00% <ø> (ø)
src/event-map.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f8f029...13e3d5e. Read the comment docs.

@apalaniuk apalaniuk force-pushed the update-composed-events branch from 2af860b to 632cf76 Compare March 23, 2020 22:09
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this. Thank you! Just a few notes. Thank you!

@apalaniuk apalaniuk force-pushed the update-composed-events branch from c698d97 to 632cf76 Compare March 26, 2020 00:34
},
pointerEnter: {
EventType: 'PointerEvent',
defaultInit: {bubbles: false, cancelable: false},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm deferring to https://w3c.github.io/pointerevents/index.html#attributes-and-default-actions, but I noticed that Chromium-based browsers are non-conformant and set these as composed, while at least Firefox seems conformant. Calling it out in case you have a different preference.

@eps1lon eps1lon changed the title Set composed property on relevant synthetic events generated by fireEvent fix(fireEvent): Set composed property on relevant synthetic events Mar 27, 2020
@eps1lon eps1lon added the bug Something isn't working label Mar 27, 2020
eps1lon
eps1lon previously approved these changes Mar 27, 2020
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Kent should still have final approval since I have little experience with shadow dowm

@eps1lon eps1lon requested a review from kentcdodds March 27, 2020 09:36
@apalaniuk apalaniuk force-pushed the update-composed-events branch from e7f5acd to 13e3d5e Compare March 27, 2020 16:58
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super. Thank you!

@kentcdodds kentcdodds merged commit 39dc929 into testing-library:master Mar 27, 2020
@kentcdodds
Copy link
Member

@all-contributors please add @apalaniuk for code and tests

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @apalaniuk! 🎉

@apalaniuk apalaniuk deleted the update-composed-events branch March 27, 2020 18:50
@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants