Skip to content

Conversation

priyanshu92
Copy link
Contributor

Fixes #134

Prevents the dropdown from toggling again and again when the selected tags are removed.

@coveralls
Copy link

coveralls commented Aug 1, 2018

Coverage Status

Coverage increased (+0.04%) to 91.247% when pulling a2ccfc4 on priyanshu92:fix-dropdown-toggle into c0ee452 on dowjones:master.

src/tag/index.js Outdated
<span className={cx('tag')}>
{label}
<button onClick={this.handleClick} className={cx('tag-remove')} type="button">
<button onClick={e => this.handleClick(e)} className={cx('tag-remove')} type="button">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be simply onClick={this.handleClick} (avoids creating a function on every render). I wonder why linting didn't flag this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll update it.

handleClick = e => {
const { id, onDelete } = this.props

e.stopPropagation()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't remember why but I think we also need to add e.stopImmediatePropagation() too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. We can use stopImmediatePropagation() because it implicitly calls stopPropagation(). I'll update the code.

Copy link
Collaborator

@mrchief mrchief left a comment

Choose a reason for hiding this comment

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

LGTM!

@mrchief mrchief changed the title fix: Fix dropdown toggle when any selected pill fix: Do not toggle dropwdown on pill removal Aug 2, 2018
@mrchief mrchief merged commit 18ab0d9 into dowjones:master Aug 2, 2018
priyanshu92 added a commit to priyanshu92/react-dropdown-tree-select that referenced this pull request Aug 3, 2018
fix: Do not toggle dropwdown on pill removal (dowjones#141) 🐛
mrchief added a commit that referenced this pull request Aug 14, 2018
* fix: Do not toggle dropwdown on pill removal (#141) 🐛

* fix: Show partial select with empty children (#139) 🐛

* fix: Fix outside click in case of multiple dropdowns

* docs: Add showDropdown prop description to readme (#152)

Closes #144
mrchief added a commit that referenced this pull request Aug 14, 2018
* fix: Do not toggle dropwdown on pill removal (#141) 🐛

* fix: Show partial select with empty children (#139) 🐛

* fix: Fix outside click in case of multiple dropdowns

* docs: Add showDropdown prop description to readme (#152)

Closes #144
mrchief added a commit that referenced this pull request Aug 14, 2018
* fix: Do not toggle dropwdown on pill removal (#141) 🐛

* fix: Show partial select with empty children (#139) 🐛

* fix: Fix outside click in case of multiple dropdowns

* docs: Add showDropdown prop description to readme (#152)

Closes #144
mrchief pushed a commit that referenced this pull request Sep 16, 2018
fix: Do not toggle dropwdown on pill removal
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