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.

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

breaking(Checkbox): callback with new checked value in onClick #2014

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

areinmeyer
Copy link
Contributor

@areinmeyer areinmeyer commented Aug 28, 2017

Breaking Change

Currently, it represented the previous checked state the checked value in a Checkbox's onClick callback represents the old state. It now represents the new checked state.

Before

You needed to negate the checked value to get the correct new value:

handleClick = (e, data) => this.setState({ checked: !data.checked })

<Checkbox onClick={this.handleClick} />

After

The checked value represents the correct new value.

handleClick = (e, data) => this.setState({ checked: data.checked })

<Checkbox onClick={this.handleClick} />

Note, this is analogous to a controlled <Input />'s value.


change onClick's checked value to be the new representation of the
checked state instead of the representation before the event was
triggered

BREAKING CHANGE: Checkbox.onClick has changed how the checked property
is defined. To migrate update code that references checked property to
the opposite value currently assumed.
Closes #1936

change onClick's checked value to be the new representation of the
checked state instead of the representation before the event was
triggered

BREAKING CHANGE: Checkbox.onClick has changed how the checked property
is defined.  To migrate update code that references checked property to
the opposite value currently assumed.
Closes Semantic-Org#1936
@codecov-io
Copy link

codecov-io commented Aug 28, 2017

Codecov Report

Merging #2014 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2014      +/-   ##
=========================================
+ Coverage   99.76%   99.8%   +0.04%     
=========================================
  Files         148     148              
  Lines        2556    2589      +33     
=========================================
+ Hits         2550    2584      +34     
+ Misses          6       5       -1
Impacted Files Coverage Δ
src/modules/Checkbox/Checkbox.js 100% <100%> (ø) ⬆️
src/modules/Dropdown/Dropdown.js 100% <0%> (ø) ⬆️
src/behaviors/Visibility/Visibility.js 100% <0%> (ø) ⬆️
src/addons/Portal/Portal.js 100% <0%> (ø) ⬆️
src/modules/Sticky/Sticky.js 100% <0%> (ø) ⬆️
src/modules/Tab/Tab.js 100% <0%> (ø) ⬆️
src/modules/Modal/Modal.js 100% <0%> (ø) ⬆️
src/modules/Modal/ModalActions.js 100% <0%> (ø) ⬆️
src/modules/Search/Search.js 100% <0%> (ø) ⬆️
src/modules/Tab/TabPane.js 100% <0%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1ff28a...e5b3d8c. Read the comment docs.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

@areinmeyer Thanks for PR 👍

@levithomason levithomason changed the title fix(Checkbox): change behavior of onClick to match onChange breaking(Checkbox): callback with new checked value in onClick Sep 1, 2017
@levithomason levithomason merged commit f1f7ceb into Semantic-Org:master Sep 1, 2017
@levithomason
Copy link
Member

Thanks again!

@levithomason
Copy link
Member

Released in [email protected]

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

Successfully merging this pull request may close these issues.

5 participants