-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add type to buttons #349
Add type to buttons #349
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the percy tests?
src/components/buttons/secondary-button/secondary-button.percy.js
Outdated
Show resolved
Hide resolved
2768064
to
3787d47
Compare
Thanks for fixing things @montezume. Some tests broke due to the added warning (because we were passing |
@@ -343,7 +343,7 @@ describe('interaction', () => { | |||
let wrapper; | |||
describe('when not disabled', () => { | |||
beforeEach(() => { | |||
props = createProps({ linkTo: '/foo/bar' }); | |||
props = createProps({ type: undefined, linkTo: '/foo/bar' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready when you are 🚀
BREAKING CHANGE: renamed type prop to tone
4870c81
to
622ba65
Compare
This PR adds the
type
prop to multiple buttons (FlatButton
,IconButton
,PrimaryButton
,SecondaryButton
,SecondaryIconButton
) to enable using them as submit/reset buttons in forms.However the
FlatButton
already had a prop calledtype
which only supportedprimary
orsecondary
as values. That prop was renamed totone
. The naming is not ideal but good enough until we redo the buttons. ThePrimaryButton
also uses atone
prop at the moment. Note that the usage oftone
is a temporary change and is supposed to be redone once the buttons get reorganised.I further did not add a
type
prop to theLinkButton
as it's not really a button in the first place. It always renders a link. We should have a Link category and this should probably be aButtonLink
(as it looks like a button but is a link).Closes #340
BREAKING CHANGE: The
type
prop ofFlatButton
was renamed totone
. A newtype
prop was added toFlatButton
instead for which possible values aresubmit
,reset
andbutton
.