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

Blocks: Update ToggleControl to pass boolean onChange #4720

Merged
merged 2 commits into from
Jan 30, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 28, 2018

Closes #4613

This pull request seeks to change the argument signature of the onChange callback of InspectorControls.ToggleControl, passing a boolean value instead of the event object, for consistency with other inspector controls.

Existing core blocks tend to ignore this argument altogether, instead opting to infer the change as toggle of the current value (i.e. setAttributes( { checked: ! checked } ); ).

Testing instructions:

Verify there are no regressions in behavior of any block using the InspectorControls.ToggleControl block helper component:

https://github.com/WordPress/gutenberg/search?utf8=%E2%9C%93&q=togglecontrol&type=

@aduth aduth added [Feature] Blocks Overall functionality of blocks [Feature] UI Components Impacts or related to the UI component system labels Jan 28, 2018
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Looks great. Works great. Thanks for adding tests!

id={ id }
checked={ checked }
onChange={ this.onChange }
aria-describedby={ !! help ? id + '__help' : undefined }
Copy link
Member

Choose a reason for hiding this comment

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

Nit: this !! isn't necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call! Simplified in a721c08.

@aduth aduth added the [Type] Breaking Change For PRs that introduce a change that will break existing functionality label Jan 30, 2018
@aduth aduth merged commit 0d974c8 into master Jan 30, 2018
@aduth aduth deleted the fix/4613-toggle-control-on-change branch January 30, 2018 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Feature] UI Components Impacts or related to the UI component system [Type] Breaking Change For PRs that introduce a change that will break existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants