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(Dropdown): clear searchQuery on item selection by mouse click #3317

Merged

Conversation

kohakukun
Copy link
Contributor

@kohakukun kohakukun commented Dec 3, 2018

  • Remove check to have consistent behavior with selectItemOnEnter

Fixes #3306.

 - Remove check to have consistent behavior with `selectItemOnEnter`

Relates to Semantic-Org#3306
@codecov-io
Copy link

codecov-io commented Dec 3, 2018

Codecov Report

Merging #3317 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3317      +/-   ##
==========================================
- Coverage   99.89%   99.89%   -0.01%     
==========================================
  Files         170      170              
  Lines        2802     2801       -1     
==========================================
- Hits         2799     2798       -1     
  Misses          3        3
Impacted Files Coverage Δ
src/modules/Dropdown/Dropdown.js 99.76% <100%> (-0.01%) ⬇️

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 1b513d0...39106eb. Read the comment docs.

@@ -729,8 +729,7 @@ export default class Dropdown extends Component {
this.setValue(newValue)
this.setSelectedIndex(value)

const optionSize = _.size(this.getMenuOptions())
if (!multiple || isAdditionItem || optionSize === 1) this.clearSearchQuery()
this.clearSearchQuery()
Copy link
Member

Choose a reason for hiding this comment

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

This change is absolutely valid, we did this in #2162 and even had an comment about this #2162 (comment).

See our decision #2162 (comment). I checked examples manually, LGTM.

@layershifter
Copy link
Member

@kohakukun thanks for the investigation 👍

@layershifter layershifter changed the title fix(Dropdown): Clear search on mouse click fix(Dropdown): clear searchQuery on item selection by mouse click Dec 4, 2018
@layershifter layershifter merged commit 22fe8ed into Semantic-Org:master Dec 4, 2018
@welcome
Copy link

welcome bot commented Dec 4, 2018

Congrats on merging your first pull request! 🎉🎉🎉

robot victory dance

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

Successfully merging this pull request may close these issues.

3 participants