fix(Dropdown): open/close on focus/blur #500
Conversation
| @@ -849,9 +863,11 @@ export default class Dropdown extends Component { | |||
| active={isActive(opt.value)} | |||
| onClick={this.handleItemClick} | |||
| selected={selectedIndex === i} | |||
| onMouseDown={e => e.preventDefault()} // prevent default to allow item select without closing on blur | |||
| // prevent default to allow item select without closing on blur | |||
| onMouseDown={e => e.preventDefault()} | |||
There was a problem hiding this comment.
I think you can get rid of this now since preventDefault here is meant to prevent closing on blur but you're preventing that more completely via this PR.
There was a problem hiding this comment.
I would have thought so, but it didn't seem to be the case. I'll look at it again and see.
|
I pulled this branch down to test it locally and it seems to have fixed the problems referenced in that issue, including interacting with focusable elements within the dropdown or items. |
|
Awesome, thanks much. Just need to get tests updated now. |
Current coverage is 98.68% (diff: 100%)@@ master #500 diff @@
==========================================
Files 102 102
Lines 1503 1520 +17
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 1483 1500 +17
Misses 20 20
Partials 0 0
|
267ba5a to
2f4ddf1
Compare
| }, { | ||
| dropdownClasses: 'active visible', | ||
| menuClasses: 'visible', | ||
| }) |
There was a problem hiding this comment.
We should only be setting the visibility classes if the open state was successful. I moved these to their respective render functions.
|
I've updated per the comments, thanks. I also fixed another bug. The visibility classes were being assigned every time open/close was called. However, open/close only tries to set state, but defers to the |
|
Released in |
|
I am noticing an issue where I have a drop down with an OnChange event and after clicking on the drop down and selecting the value the OnChange fires off as expected, but if after doing this action and i click anywhere else on the screen it triggers the OnChange event again. I was just wondering does this fix address this issue or is what im experiencing completely seperate from this fix? |
|
This sounds like an unrelated issue. Suggest testing against the latest version. If it still occurs, please open a separate issue with explicit steps to reproduce. Thanks for the report. |
Fixes #493
The Dropdown opens/closes on focus/blur. Focus and blur are triggered by either keyboard (tab) or mouse events (click). Focus and blur fire before on mouse down while click fires on mouse up.
This meant focus and blur would cause the menu to open/close before the click handler would also toggle the menu close. Further, the blur event would close the menu before the click event could fire on a menu item.
This PR adds a
isMouseDownflag. Focus/blur functionality for open/close is only called if the mouse is not down. This allows the user to tab to open/close, but the focus/blur fired from a click is ignored. Allowing the user to select items without the Dropdown closing and click on the Dropdown to toggle its open state without double toggling the open state.