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

[Select] No longer listens to click events #18655

Closed
2 tasks done
aceluby opened this issue Dec 2, 2019 · 4 comments
Closed
2 tasks done

[Select] No longer listens to click events #18655

aceluby opened this issue Dec 2, 2019 · 4 comments
Labels
component: select This is the name of the generic UI component, not the React module! test waiting for 👍 Waiting for upvotes

Comments

@aceluby
Copy link

aceluby commented Dec 2, 2019

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

All my tests fail in version 4.7.1 when I fire click events to start the Select process. I instead have to use 'mouseDown' events in order for tests to work properly. The PR that impacted this is here: #17978

Expected Behavior 🤔

I'd expect click events to bring up the dropdown menu as it was in previous versions

Steps to Reproduce 🕹

https://codesandbox.io/s/create-react-app-2pyv7?fontsize=14

Steps:

  1. Create a dropdown menu as per the docs
  2. Test it using react-testing-library fireEvent.click
  3. Clicks no longer register (where they did before)
  4. Change the click event to mouseDown and it will work

Context 🔦

I have multiple tests where I click select elements as part of the test. Every one of those tests now break as part of the newest release. To me, this feels like a breaking change without any notification the change even occurred, I had to scour through the PR to see why it had happened.

Your Environment 🌎

Tech Version
Material-UI v4.7.1
React 16.12.0
@testing-library/react 9.3.2
jest 24.0.22
@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 2, 2019

@aceluby It sounds like you will need to update your tests. I suspect it will impact a couple of more users. This makes me think of a recent tweet @diegohaz did, which leads to: https://github.com/testing-library/user-event.

@eps1lon What would you recommend?

@oliviertassinari oliviertassinari added component: select This is the name of the generic UI component, not the React module! test labels Dec 2, 2019
@oliviertassinari oliviertassinari changed the title Select no longer listens to click events [Select] No longer listens to click events Dec 3, 2019
@pheeria
Copy link

pheeria commented Dec 10, 2019

Isn't that a major (or even breaking) change?
One expects to casually run npm update for patch version increase, assuming only Backward compatible bugfixes will happen. And then there are (potentially) a bunch of tests failing.

@oliviertassinari
Copy link
Member

We have seen the same feedback from people using test snapshot. I agree, it would have been better to release it under a minor.

@eps1lon
Copy link
Member

eps1lon commented Jan 15, 2020

Isn't that a major (or even breaking) change?

In SemVer this is equivalent terminology.

However, it was a bug fix. If you're relying on buggy behavior than you might consider this a breaking change. We prioritize bug free software over software that doesn't change.

To me, this feels like a breaking change without any notification the change even occurred

I would agree that the changelog entry was not very obvious:

Improve response, react to mouse down

Instead we should've written

Fixed a bug where the Select would only open afer the click instead of mousedown event

or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! test waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

4 participants