buttonType and default of type button #54
Conversation
src/components/button/button.js
Outdated
| @@ -41,6 +41,7 @@ export const EuiButton = ({ | |||
| size, | |||
| fill, | |||
| isDisabled, | |||
| buttonType = 'button', | |||
There was a problem hiding this comment.
I was thinking we should rename the existing type prop to buttonType and use type for things like submit and button. This way it will just get passed through via the ...rest argument. What do you think?
Can we also use defaultProps to specify the default button value?
There was a problem hiding this comment.
The reason I think we should use buttonType this way is because a lot of our components could have "types", e.g. tables, modals, pretty much anything. We can use this naming pattern to name these props tableType, modalType, etc. This will let us avoid collisions with the native type attribute throughout our code.
bevacqua
force-pushed
the
button-type-button
branch
from
f8e93e9 to
a38c216
Compare
| @@ -167,7 +167,7 @@ export default () => ( | |||
| | |||
|
|
|||
| <EuiButtonEmpty | |||
| type="text" | |||
| color="text" | |||
There was a problem hiding this comment.
Hmm this is a little weird. I don't think most people will naturally associate a particular color or appearance with "text". Can we find a better name... "default" maybe? @snide Thoughts?
There was a problem hiding this comment.
I think this is fine. "primary" isn't a color either.
src/components/call_out/call_out.js
Outdated
| }; | ||
|
|
||
| EuiCallOut.defaultProps = { | ||
| type: 'info', | ||
| color: 'info', |
There was a problem hiding this comment.
Should we rename "info" to "primary"?
| }; | ||
|
|
||
| EuiBadge.defaultProps = { | ||
| type: 'default', | ||
| color: 'default', |
There was a problem hiding this comment.
Here's precedent for "default".
There was a problem hiding this comment.
Default in this case is the actual default though.
src/components/toast/toast.js
Outdated
| @@ -11,17 +11,17 @@ import { | |||
| EuiText, | |||
| } from '..'; | |||
|
|
|||
| const typeToClassNameMap = { | |||
| const colorToClassNameMap = { | |||
| info: 'euiToast--info', | |||
Fixes #3 and #53