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

Dropdown: When clicking to close a dropdown, it fires "click" and "blur" events #2641

Open
hugw opened this issue Mar 13, 2018 · 8 comments
Open

Comments

@hugw
Copy link

hugw commented Mar 13, 2018

This issue happens when controlling manually (via states) the status of a dropdown.

Steps

If you click the "Not ok button" to close the drop-down you will notice that it triggers the click and blur events (It shouldn't fire blur cause I'm still inside the component).

Now, if you click the "ok button" repeatedly, you will notice that only the click event is fired, regardless if the dropdown is opening or closing, and the blur happens only when you actually click outside of the related button, as expected.

Expected Result

When clicking the "not ok button" to close a drop-down, it should fire only a click event.

Actual Result

When clicking the "not ok button" to close a drop-down, it fires a click and blur event.

Version

0.78.3
OS: High Sierra

Testcase

https://codesandbox.io/s/3q781mpvw1

@welcome
Copy link

welcome bot commented Mar 13, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@levithomason
Copy link
Member

levithomason commented Apr 13, 2018

Thanks for the report and test case. Here's our problem in Dropdown.js, the close handler is explicitly calling blur

handleClose = () => {
  debug('handleClose()')
  const hasSearchFocus = document.activeElement === this.searchRef
  const hasDropdownFocus = document.activeElement === this.ref
  const hasFocus = hasSearchFocus || hasDropdownFocus
  // https://github.com/Semantic-Org/Semantic-UI-React/issues/627
  // Blur the Dropdown on close so it is blurred after selecting an item.
  // This is to prevent it from re-opening when switching tabs after selecting an item.
  if (!hasSearchFocus) {
    this.ref.blur()
  }

  // We need to keep the virtual model in sync with the browser focus change
  // https://github.com/Semantic-Org/Semantic-UI-React/issues/692
  this.setState({ focus: hasFocus })
}

I believe the culprit is #627 (see inline comments in code above). Rather than blurring the Dropdown to prevent the issue noted, it seems we should be detecting when the browser tab is switched back and not reopen.

I'd investigate which methods are being called when the browser tab is switched away and back. That process needs to not re-open the Dropdown AND we should retain its focus.

@willb335
Copy link
Contributor

willb335 commented May 7, 2018

I deleted

if (!hasSearchFocus) {
    this.ref.blur()
  }

from Dropdown.js and it fires only the click event. Also, #627 seems to be unaffected. I'll write some tests and issue a pull request

@willb335
Copy link
Contributor

willb335 commented May 7, 2018

If I am missing something please let me know! Thanks

@willb335
Copy link
Contributor

willb335 commented May 9, 2018

I have a pull request at #2776

@stale
Copy link

stale bot commented Aug 9, 2018

There has been no activity in this thread for 90 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 90 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Aug 9, 2018
@stale
Copy link

stale bot commented Feb 6, 2019

This issue will be closed due to lack of activity for 12 months. If you’d like this to be reopened, just leave a comment; we do monitor them!

@stale stale bot closed this as completed Feb 6, 2019
@layershifter layershifter reopened this Feb 6, 2019
@stale
Copy link

stale bot commented Aug 5, 2019

This issue will be closed due to lack of activity for 12 months. If you’d like this to be reopened, just leave a comment; we do monitor them!

@stale stale bot closed this as completed Aug 5, 2019
@layershifter layershifter reopened this Aug 5, 2019
@stale stale bot removed the stale label Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants