fix(Dropdown): retains focus after selection

[jongsue]

Fixes #3349.

@layershifter Could you please review this one?

[codecov-io]

Codecov Report

Merging #3452 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3452      +/-   ##
==========================================
+ Coverage   99.83%   99.83%   +<.01%     
==========================================
  Files         175      175              
  Lines        3120     3121       +1     
==========================================
+ Hits         3115     3116       +1     
  Misses          5        5

Impacted Files	Coverage Δ
src/modules/Dropdown/Dropdown.js	100% <100%> (ø)	⬆️

---

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 0d66fa7...0beff78. Read the comment docs.

[levithomason]

Thanks for this, I'd been wanting to update this to match React's API. There are only a few cases where we use the state argument as well. I'm thinking we should also remove it to match the React API 1:1. Otherwise, to use the callback you need to pass some null state:

this.trySetState(
  { count: 1 },             // maybeState
  null,                     // state, I don't need...
  () => console.log('done') // complete callback
)

The current trySetState implementation throws warnings about including state keys that are not listed in autoControlledProps, however, it could instead ignore them and include them as-is in the actual setState call. This way we can just combine the maybe state and the actual state.

[jongsue]

I tried to remove the state arguments out of trySetState.
I'm not sure I did understand what you said.
please review this code again

[levithomason]

cruft?

[jongsue]

This is on purpose.
callback the 2nd argument in close method has default value.
But not using default value in this case.

close = (e, callback = this.handleClose) => {
    const { open } = this.state
    debug('close()', { open })

    if (open) {
      _.invoke(this.props, 'onClose', e, this.props)
      this.trySetState({ open: false }, callback)
    }
  }

[levithomason]

Case-in-point 😉

[levithomason]

After making a selection in a search selection, the input DOM node retains focus but the Dropdown loses its blue border. I'm pretty sure this means this.state.focus is not true.

After making a selection:

Note the missing blue border, as there is when first focusing the Dropdown:

[levithomason]

⚠️ Heads Up!

I also merged master and fixed conflicts here. You'll need to pull before resuming work.

[jongsue]

After making a selection in a search selection, the input DOM node retains focus but the Dropdown loses its blue border. I'm pretty sure this means this.state.focus is not true.

After making a selection:

Note the missing blue border, as there is when first focusing the Dropdown:

semantic.css .ui.selection.dropdown:focus seems to make the blue border.

As you mentioned After making a selection in a search selection, the input DOM node retains focus not Dropdown.
That's why there is no blue border.
I don't know how to solve this problem. do you have any idea?

[jongsue]

I tried adding ‘active’ class on Dropdown when ‘focus’ state is true.
Then we could get the blue border but there is ‘border-bottom-left-radius’ to make it look like not right

[amrita-syn]

Any update on this? Would save a lot of effort...

[jongsue]

@levithomason could you review this again please ?
The blue border is css issues.
If Semantic UI would fix the problem then It'll fix ours too.

[layershifter]

Yep, that blue border is a CSS issue, want to check this additionally. For the late feedback 🐑

[layershifter]

Tested it manually, works perfectly 👍