-
Notifications
You must be signed in to change notification settings - Fork 49.9k
Improve test harness of submit events #13463
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
Conversation
I found a test that was written more than 5 years ago and probably never run until now. The behavior still works, although the API changed quite a bit over the years. Seems like this was part of the initial public release already: facebook@75897c2#diff-1bf5126edab96f3b7fea034cd3b0c742R31
For more information, see facebook#13462
| // This is a special case for submit and reset events as they are listened on | ||
| // at the element level and not the document. | ||
| // @see https://github.com/facebook/react/pull/13462 | ||
| it('should not receive submit events if native, interim DOM handler prevents it', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test indeed fails if I re-apply my changes from #13358. If the onSubmit handler added a preventDefault(), this would never be called.
gaearon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
|
||
| formRef.current.dispatchEvent( | ||
| new Event('reset', { | ||
| // https://developer.mozilla.org/en-US/docs/Web/Events/reset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This PR includes a test that I've enabled in #13358 and another test that we've discussed in #13462 as well as some random cleanup while I'm at it.