Skip to content

Conversation

@delesseps
Copy link
Contributor

@delesseps delesseps commented Jun 9, 2017

Also...

  • Add space and enter key support for toggling checked state as per a regular checkbox
  • Fix demo page not showing component
  • Added tests

Add space and enter key support for toggling checked state
Disable key behavior when component in disabled state
@delesseps delesseps changed the title Disable key behavior when component in disabled state Disable key behavior when component disabled Jun 9, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.0%) to 83.333% when pulling be84fed on delesseps:master_fix into 7b33f03 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 90.0% when pulling 10c0439 on delesseps:master_fix into 7b33f03 on react-component:master.

@delesseps
Copy link
Contributor Author

@afc163 Is this suitable for merge? Thanks!

disabled: false,
};
toggle = () => {
const Test = React.createClass({
Copy link
Member

Choose a reason for hiding this comment

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

Why??? We should not use React.createClass after React@15.5, just use ES6 class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Np, updating was just following conventions used in other components

@benjycui
Copy link
Member

👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 90.0% when pulling ccfab8e on delesseps:master_fix into 7b33f03 on react-component:master.

src/Switch.jsx Outdated
}
if (e.keyCode === 39) {
this.setChecked(true);
if (!this.props.disabled) {
Copy link
Member

Choose a reason for hiding this comment

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

It is better to put disabled logic into setChecked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok done

@coveralls
Copy link

Coverage Status

Coverage increased (+4.5%) to 93.75% when pulling 2b554fe on delesseps:master_fix into 7b33f03 on react-component:master.

@afc163 afc163 merged commit b63ec27 into react-component:master Jun 17, 2017
@afc163
Copy link
Member

afc163 commented Jun 17, 2017

Thx @delesseps ! 1.5.2 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants