-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(Search): prevent blur event when SearchResult is clicked #3456
fix(Search): prevent blur event when SearchResult is clicked #3456
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3456 +/- ##
=========================================
Coverage ? 99.89%
=========================================
Files ? 172
Lines ? 2796
Branches ? 0
=========================================
Hits ? 2793
Misses ? 3
Partials ? 0
Continue to review full report at Codecov.
|
src/modules/Search/Search.js
Outdated
@@ -552,6 +552,7 @@ export default class Search extends Component { | |||
key={childKey || result.title} | |||
active={selectedIndex === offsetIndex} | |||
onClick={this.handleItemClick} | |||
onMouseDown={e => e.preventDefault()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please move this code to a separate class method and place it under
handleItemClick
:
handleItemMouseDown = (e) => {
debug('handleItemMouseDown()')
// We should prevent default to prevent blur events.
// https://github.com/Semantic-Org/Semantic-UI-React/issues/3298
e.preventDefault()
}
- As we there, let's use
_.invoke()
:
handleFocus = (e) => {
debug('handleFocus()')
_.invoke(this.props, 'onFocus', e, this.props)
this.setState({ focus: true })
}
handleBlur = (e) => {
debug('handleBlur()')
_.invoke(this.props, 'onBlur', e, this.props)
this.setState({ focus: false })
}
- onMouseDown={e => e.preventDefault()}
+ onMouseDown={this.handleItemMouseDown}
- About a new unit test, we want to assert that
onBlur
will not happen on the item selection, we can test this:
it('is not called on an item click', () => {
const onBlur = sandbox.spy()
wrapperMount(<Search results={options} onBlur={onBlur} />)
openSearchResults()
wrapper.find('SearchResult').at('0').simulate('click', nativeEvent)
onBlur.should.have.not.been.called()
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs some changes, thanks for taking this 👍
…React into fix-search-prevent-blur-event
Applied changes to get this fix 👍 |
Released in |
fixes #3298
I couldn't write the test code.
If you know how to do it, please let me know
thanks in advance