Skip to content
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.

Sign up for GitHub

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(Checkbox): onFocus and onBlur events #1361

Merged
merged 9 commits into from
Mar 24, 2017
Merged

fix(Checkbox): onFocus and onBlur events #1361

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e1e4544
fix(Checkbox): added onFocus prop
tarang9211 Feb 21, 2017
bee7c99
fix(Checkbox): added handleFocus callback to be triggered
tarang9211 Feb 21, 2017
17a9f4c
fix(Checkbox): added onMouseDown event to invoke focus event
tarang9211 Feb 21, 2017
5a41626
fix: call user onMouseDown prop when onMouseDown event is trigger
tarang9211 Mar 15, 2017
d83b46a
style: added onMouseDown to component propTypes
tarang9211 Mar 15, 2017
7a89fa5
Update Checkbox.js
layershifter Mar 15, 2017
c81b2a8
test: potential fix for checkbox test
tarang9211 Mar 15, 2017
f8672e9
Merge branch 'master' of https://github.com/Semantic-Org/Semantic-UI-…
Mar 21, 2017
1b69037
fix(Checkbox): onFocus and onBlur events
Mar 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/modules/Checkbox/Checkbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ export default class Checkbox extends Component {
}
}

handleMouseDown = (e) => {
_.invoke('focus', this.checkboxRef)
Copy link
Member

@levithomason levithomason Mar 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct, however, we also need to call the user's onMouseDown prop here if they provide one. That is because we handling the prop, so it isn't auto spread on the rendered component. Even if it were, we are also setting the onMouseDown event ourselves, so our handler is the only one that will ever be called.

So, if they pass:

<Checkbox onMouseDown={() => console.log('hi')} />

Then nothing will happen onMouseDown. To ensure they can also pass a onMouseDown handler, we need to also call it in our handler for them. Since mouse down is called before the focus event, let's call it in the right order as well:

handleMouseDown = (e) => {
  _.invoke('onMouseDown', this.props, [e])
  _.invoke('focus', this.checkboxRef)
}

}

// Note: You can't directly set the indeterminate prop on the input, so we
// need to maintain a ref to the input and set it manually whenever the
// component updates.
Expand Down Expand Up @@ -196,7 +200,10 @@ export default class Checkbox extends Component {
else computedTabIndex = disabled ? -1 : 0

return (
<ElementType {...rest} className={classes} onClick={this.handleClick} onChange={this.handleClick}>
<ElementType {...rest} className={classes}
onClick={this.handleClick} onChange={this.handleClick}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a line break between the props here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need the line break here 😉

-        onClick={this.handleClick} onChange={this.handleClick}
+        onClick={this.handleClick}
+        onChange={this.handleClick}

onMouseDown={this.handleMouseDown}
>
<input
checked={checked}
className='hidden'
Expand Down