fix(Dropdown): respect closeOnBlur={false}
#1148
Conversation
| @@ -420,7 +420,11 @@ export default class Dropdown extends Component { | |||
| if (!this.isMouseDown) { | |||
| const { closeOnBlur } = this.props | |||
| debug('mouse is not down, closing') | |||
| if (closeOnBlur) this.close() | |||
| if (closeOnBlur) { | |||
There was a problem hiding this comment.
@levithomason this is what I meant. With this piece of code, when I tab away from this dropdown, it stays open until a user clicks somewhere else on the screen. Let me know your thoughts.
There was a problem hiding this comment.
Hm, I'll mark this for review to investigate. I can't think of a reason we'd need to set open again when it is already open. Thanks for the note here.
There was a problem hiding this comment.
@levithomason sounds good. let me know how we can fix this.
Current coverage is 95.88% (diff: 100%)@@ master #1148 diff @@
==========================================
Files 879 879
Lines 4889 4883 -6
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 4688 4682 -6
Misses 201 201
Partials 0 0
|
|
@levithomason any idea what's going with the codecov failing? |
|
Codecov is failing because there were new branches of logic added that do not have test cases. https://github.com/Semantic-Org/Semantic-UI-React/blob/master/.github/CONTRIBUTING.md#coverage |
|
yea it complained about the if/else block not having code coverage. |
|
@levithomason Do we have any feedback or review on this? |
|
I'll get a review on the functionality as is for now. However, this PR needs more coverage before it can be merged. The added branch logic needs a test, this is why Codecov is failing the PR. |
|
OK, so the issue it turns out is that blurring calls This logic flow worked fine before adding With this fix, we're able to avoid having to re-open the Dropdown on blur as was originally the case in this PR. |
| @@ -547,7 +547,6 @@ export default class Dropdown extends Component { | |||
| } else { | |||
| this.setValue(value) | |||
| this.handleChange(e, value) | |||
| this.close() | |||
There was a problem hiding this comment.
This was the bug, we were closing every time an item was activated. Even when closeOnBlur was false. Since, this method was called on blur due to blur activating the selected item.
levithomason
force-pushed
the
feat/dropdown-fixes-tests
branch
from
f08fdb9
to
694121e
Compare
| @@ -902,9 +906,7 @@ export default class Dropdown extends Component { | |||
|
|
|||
| scrollSelectedItemIntoView = () => { | |||
| debug('scrollSelectedItemIntoView()') | |||
| // Do not access document when server side rendering | |||
| if (!isBrowser) return | |||
| const menu = document.querySelector('.ui.dropdown.active.visible .menu.visible') | |||
There was a problem hiding this comment.
This fixes another bug. If multiple Dropdowns were open, this would select the first one on the page. This meant the Menu for a Dropdown not belonging to this one was being operated on.
I've fixed that by selecting this Dropdown's Menu.
levithomason
changed the title
closeOnBlur={false}
levithomason
force-pushed
the
feat/dropdown-fixes-tests
branch
from
694121e
to
c09ab6f
Compare
|
@levithomason Ah ok that makes sense. I did notice that the dropdown was closing after making a selection on an item. Sounds good, thank you for the review and guidance! |
|
Released in |
* Semantic-Org/master: (111 commits) fix(docs): fix usage of arrow function (Semantic-Org#1228) fix(Icon): Incorrect definition in typings (Semantic-Org#1225) fix(Button): fix `tabIndex` in typings (Semantic-Org#1224) fix(typings): add item prop to the DropdownProps (Semantic-Org#1223) docs(examples): add missing `key` props (Semantic-Org#1220) docs(changelog): update changelog [ci skip] 0.64.4 feat(Form): add `inverted` prop (Semantic-Org#1218) fix(ComponentExample): use explicit babel presets (Semantic-Org#1219) style(Button): update typings and propTypes usage (Semantic-Org#1216) style(Embed): update typings and propTypes usage (Semantic-Org#1217) chore(package): support for jsnext:main (Semantic-Org#1210) style(Step): update typings, tests and propTypes usage (Semantic-Org#1203) fix(Portal): do not take focus after first render (Semantic-Org#1215) style(Table|mixed): update typings, tests and propTypes usage (Semantic-Org#1200) fix(Dropdown): use `item` instead of `as={Menu.Item}` (Semantic-Org#659) fix(Dropdown): respect `closeOnBlur={false}` (Semantic-Org#1148) style(Breadcrumb): update typings and propTypes usage (Semantic-Org#1209) style(Dimmer): update typings (Semantic-Org#1208) fix(Divider|FormInput): fix the broken typings (Semantic-Org#1179) ...
* fix(Dropdown): possible solution for closeOnBlur prop bug
* fix(Dropdown): respect closeOnBlur={false}
No description provided.