fix(Dropdown): blur dropdown on close()

[Chris-R3]

fixes #627

[levithomason]

Cool, this just needs a test. Codecov will fail the PR otherwise. We keep coverage at 100%, though this line is executed during tests, there are no assertions for it. Suggest adding a test to Dropdown-test.js just after:

  it('closes on blur', () => {

Named something like:

  it('blurs the Dropdown node on close', () => {

[codecov-io]

Current coverage is 100% (diff: 100%)

Merging #628 into master will not change coverage

@@           master   #628   diff @@
====================================
  Files         131    131          
  Lines        2098   2099     +1   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits         2098   2099     +1   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last update f561ce0...42d397a

[Chris-R3]

I added a test but it fails. Could you take a look please and check if my test code is correct? It's the first test I've written so there's probably something I'm missing 😉

[levithomason]

So close, just need to start spying before closing the Dropdown. The spy will track all activity after it is initialized. I just moved the const instance part above the dropdownMenuIs.. part:

wrapperMount(<Dropdown options={options} selection defaultOpen />)

const instance = wrapper.instance()
sandbox.spy(instance._dropdown, 'blur')

dropdownMenuIsOpen()
wrapper.simulate('click')
dropdownMenuIsClosed()

instance._dropdown.blur
  .should.have.been.calledOnce()

[Chris-R3]

Thanks for your help, makes sense now 😄
I changed

instance._dropdown.blur
  .should.have.been.calledOnce()

to

instance._dropdown.blur
  .should.have.been.called()

because blur() was called twice. Is that alright?

[levithomason]

Thanks for the update. I've marked this as needing review. I'll investigate why this is being called twice. It should only be called once. Pretty sure I know where the issue is. Will try to get this soon as I am able.

[levithomason]

Figured it out. The open and close methods attempt to open or close the Dropdown, however, it may or may not respond depending on whether or not it is a controlled component (open prop is set).

The componentDidUpdate method has checks to see if the open state has toggled from false to true (has opened) or true to false (has closed). This is the actual indicator that the Dropdown has opened or closed.

Let's add a handleClose() method just after the close() method and call it from componentDidUpdate. All it will do for now is blur the Dropdown:

in componentDidUpdate()

    // opened / closed
    if (!prevState.open && this.state.open) {
      debug('dropdown opened')
-     this.open()
      document.addEventListener('keydown', this.closeOnEscape)
      document.addEventListener('keydown', this.moveSelectionOnKeyDown)
      document.addEventListener('keydown', this.selectItemOnEnter)
      document.addEventListener('keydown', this.removeItemOnBackspace)
      document.addEventListener('click', this.closeOnDocumentClick)
      document.removeEventListener('keydown', this.openOnArrow)
      document.removeEventListener('keydown', this.openOnSpace)
    } else if (prevState.open && !this.state.open) {
      debug('dropdown closed')
-     this.close()
+     this.handleClose()
      document.removeEventListener('keydown', this.closeOnEscape)
      document.removeEventListener('keydown', this.moveSelectionOnKeyDown)
      document.removeEventListener('keydown', this.selectItemOnEnter)
      document.removeEventListener('keydown', this.removeItemOnBackspace)
      document.removeEventListener('click', this.closeOnDocumentClick)
      if (prevState.focus && this.state.focus) {
        document.addEventListener('keydown', this.openOnArrow)
        document.addEventListener('keydown', this.openOnSpace)
      }
    }

add handleClose()

  close = () => {
    debug('close()')
    this.trySetState({ open: false })
-   this._dropdown.blur()
  }
+ handleClose = () => {
+   debug('handleClose()')
+   this._dropdown.blur()
+ }

Then blur is only called once since it is only called when the open state goes from true to false.

[Chris-R3]

I updated the PR with the changes you suggested, works like a charm.
Thanks again for your help. 👍

[levithomason]

Released in semantic-ui-react@0.55.1, thanks much!

[eXtreme]

@levithomason @Chris-R3 from what I see, this change actually broke dropdown as menu item, reporting Uncaught TypeError: a._dropdown.blur is not a function in console.
You can encounter it here: http://react.semantic-ui.com/collections/menu
Scroll to "Vertical Menu" with dropdown example, and then click "Display Options" menu item.
Same in "Dropdown Item" example and sizes examples on the same doc page.

[levithomason]

Indeed. A ref returns the DOM node for DOM components and a reference to the class/function for composite components. Because the Dropdown is rendering as={MenuItem}, the ref is then a MenuItem, not a div as it is in a standard Dropdown.

I've added #659 to address this issue.