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

Fixes #3335 - removes the call to onMenuOpen within handleInputChange #3897

Merged
merged 3 commits into from
Aug 27, 2020

Conversation

lorisdev
Copy link
Contributor

@lorisdev lorisdev commented Jan 7, 2020

As discussed in #3335 and #3620 , the expected behavior of onMenuOpen is to be triggered when the Menu opens.
Currently it is called on every input change as well which is quite confusing.

Removing the call of onMenuOpen within handleInputChange gives us the expected behavior.

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2020

🦋 Changeset is good to go

Latest commit: 80e7e17

We got this.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pruik
Copy link

pruik commented Feb 25, 2020

Although I would agree with you that the onMenuOpen should only be called once when the menu opens, your proposed change would break the following scenario:

  • Open dialog
  • Press escape. Dialog will close, but selector will still have focus
  • Type anything.
  • Expected behaviour would be to reopen the menu and perform a search based on the typed input
  • However, with your proposed change the menu won't reopen.

I would suggest wrapping the onMenuOpen in a check:

if (!this.props.menuIsOpen) {
	this.onMenuOpen();
}

pruik pushed a commit to FortesSolutions/react-select that referenced this pull request Feb 25, 2020
…selectors on a single page

- Only perform onMenuOpen / onMenuClose if the menu is actually closed / open.
-- Will invalidate PR JedWatson#3897 / Issues JedWatson#3335 JedWatson#3620
@lorisdev
Copy link
Contributor Author

Thanks for your input !
I've seen that some test were broken but I didn't have time to understand in which scenario.

@codesandbox-ci
Copy link

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.

Latest deployment of this branch, based on commit 80e7e17:

Sandbox Source
react-codesandboxer-example Configuration

@Cmoen11
Copy link

Cmoen11 commented Apr 27, 2020

@pruik suggestion should be implemented. We've faced this issue and needed to implement this solution in our code. I think this should be prioritized..

@bladey bladey added pr/needs-review PRs that need to be reviewed to determine outcome enhancement and removed uncertain labels Jun 4, 2020
@bladey bladey linked an issue Jun 12, 2020 that may be closed by this pull request
@bladey bladey added pr/priority PRs that should be addressed sooner rather than later and removed pr/needs-review PRs that need to be reviewed to determine outcome labels Jun 12, 2020
@bladey bladey requested review from bladey and JedWatson June 12, 2020 02:17
@bladey
Copy link
Contributor

bladey commented Jun 12, 2020

Can verify this is an issue, PR looks good, @JedWatson to confirm the original change wasn't intentional.

@bladey bladey added the pr/in-review PRs currently in review by maintainers for the next release label Jun 12, 2020
@bladey bladey added pr/enhancement PRs that are specifically enhancing the project and removed category/enhancement labels Jun 24, 2020
@vvitto
Copy link

vvitto commented Aug 4, 2020

Any updates regarding this bug?

@bladey bladey added pr/in-review PRs currently in review by maintainers for the next release and removed pr/in-review PRs currently in review by maintainers for the next release labels Aug 24, 2020
@JedWatson
Copy link
Owner

This looks good - thanks @lorisdev and @pruik! apologies for the delay on merge, we'll have it released shortly.

@JedWatson JedWatson merged commit fc7374c into JedWatson:master Aug 27, 2020
@JedWatson JedWatson removed their request for review August 27, 2020 05:29
This was referenced Aug 27, 2020
@andrewmtam
Copy link

Hey all, just wanted to check in on this thread -- any idea when the release for this bug fix will happen? I saw the Version Packages PR was not yet merged:

#3980

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Nov 14, 2020

I've started a fork of react-select. This PR has been released in react-select-oss@3.1.1.

EDIT: 🎉 I've archived the fork now that we've got some momentum in this repo and Jed is involved again. Sorry for the disturbance!

@PSoltes
Copy link

PSoltes commented Jun 4, 2021

Hello, i am using 4.3.1 version of react-select and this is still happening... when input is blurred onMenuClose is called even though menu is closed

@Methuselah96
Copy link
Collaborator

@PSoltes Can you a new issue with a CodeSandbox that clearly demonstrates the incorrect behavior? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/enhancement PRs that are specifically enhancing the project pr/in-review PRs currently in review by maintainers for the next release pr/priority PRs that should be addressed sooner rather than later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

onMenuOpen triggers on each input
9 participants