Skip to content

Conversation

@amdrew
Copy link
Contributor

@amdrew amdrew commented Sep 11, 2018

Fixes #9785

Description

Removed the isButton prop

How has this been tested?

Removed prop, recompiled, opened color settings panel and there was no error.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@amdrew amdrew changed the title Remove isButton prop Issue/9785 - Remove isButton prop Sep 11, 2018
@Soean
Copy link
Member

Soean commented Sep 11, 2018

@amdrew You also have to change the snapshots in the tests.

The prop was introduced in #8875

@amdrew
Copy link
Contributor Author

amdrew commented Sep 11, 2018

@Soean Cool, wasn't sure if I should remove those, fixed 👍

Copy link
Member

@Soean Soean left a comment

Choose a reason for hiding this comment

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

The tests are fine now.

There is no isButton prop, see https://github.com/WordPress/gutenberg/blob/master/packages/components/src/button/index.js
The is-button class is applied, if you use isDefault. So I think we can merge it. @tofumatt do you agree?

@Soean Soean requested a review from tofumatt September 11, 2018 11:06
@Soean Soean added [Type] Bug An existing feature does not function as intended [Component] Button labels Sep 11, 2018
@Soean Soean added this to the 3.9 milestone Sep 11, 2018
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Makes sense to me. isButton is a weird prop for a button to have, happy to nix it 😄

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

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants