Skip to content
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

fix: update onChange/input events fired on selected option #370

Merged
merged 4 commits into from Jun 22, 2020

Conversation

marcosvega91
Copy link
Member

fix #358

What:

This PR will fix the issue that events (onChage/input) are not fired on ancestors.
This issue was related to a problem with React because it fires event on bubbling phase.
As we can see from the images below

the change event is an Event of type change
image

the input event is an Event of type input
image

Both events are of a generic event type.

Why:

Because in particular with React the change event is fired but the onChange listener didn't work

How:

By calling

fireEvent(
  select,
  createEvent('input', select, {
    bubbles: true,
    cancelable: false,
    composed: true,
    ...init,
  }),
)
fireEvent.change(select, init)

Checklist:

  • Documentation
  • Tests
  • Typings
  • Ready to be merged

@codecov
Copy link

codecov bot commented Jun 20, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #370   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          385       385           
  Branches       111       111           
=========================================
  Hits           385       385           
Impacted Files Coverage Δ
src/select-options.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 2e55359...d757b47. Read the comment docs.

Copy link
Member

@nickmccurdy nickmccurdy 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, just have a minor code style note.

src/__tests__/select-options.js Outdated Show resolved Hide resolved
@marcosvega91
Copy link
Member Author

marcosvega91 commented Jun 21, 2020

Thanks for your reviews, I appreciate that 😄 . I have fixed what was wrong

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.

Coolio 👍👍

@kentcdodds
Copy link
Member

@nickmccurdy, I'll let you merge this one. You should be able to once the build is successful. Just make sure to use squash and merge and follow the commit message convention so we get the right release for it (I think it should be good without change actually). Actually you'll want to add the text "Closes #358" somewhere in the body of the commit to semantic release will add a release comment to that issue.

Thanks!

@fabb
Copy link

fabb commented Jun 22, 2020

Will this fix this issue where userEvent.selectOptions does not correctly update Formik state?
https://codesandbox.io/s/usereventselectoptions-bug-gn8wb

Or should I create a separate bug?

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.

Cool. Thanks!

@kentcdodds kentcdodds merged commit 6acbdb4 into testing-library:master Jun 22, 2020
@kentcdodds
Copy link
Member

@fabb, I don't know. Can you try when this is released?

@kentcdodds
Copy link
Member

Hey @marcosvega91, you've been added as an official member of the GitHub org :) Thanks for all you do!

@kentcdodds
Copy link
Member

🎉 This PR is included in version 12.0.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

@marcosvega91
Copy link
Member Author

Hey @marcosvega91, you've been added as an official member of the GitHub org :) Thanks for all you do!

Thank you @kentcdodds. I don't have other words, I started to contribute some months ago after one of your tweets and now I'm a member of testing lib. I discovered a world and I'm learning a lot thanks to you.

Thank you for this opportunity and for your trust 🎉

Regards
Marco

@marcosvega91 marcosvega91 deleted the pr/fix_select_options branch June 22, 2020 21:11
@fabb
Copy link

fabb commented Jun 23, 2020

@fabb, I don't know. Can you try when this is released?

@kentcdodds yes, it's fixed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

userEvent.selectOptions does not trigger onChange event
4 participants