[EuiButtonGroup] pass a type of button when rendering a button for button group button #4368
Conversation
…tton group button
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
💚 CLA has been signed |
src/components/button/button.tsx
Outdated
| /** | ||
| * The type of the underlying HTML button | ||
| */ | ||
| type?: 'button' | 'submit' | 'reset'; |
There was a problem hiding this comment.
I don't think we actually need this here because EuiButton extends the HTML button type when it renders as one.
There was a problem hiding this comment.
Instead, we might need to add it directly to EuiButtonContentProps so that...
|
|
||
| /** | ||
| * The type of the underlying HTML button | ||
| */ | ||
| buttonType?: 'button' | 'submit' | 'reset'; |
There was a problem hiding this comment.
... this doesn't need to be explicitly added but instead inherited from the extension of EuiButtonContentProps on line 30.
| type = 'button', | ||
| buttonType = 'button', |
There was a problem hiding this comment.
That would just mean then that we'd have a conflict of naming with type here, but when looking at the current type prop, it really should probably be called element since it's determining the element to render.
…add type attribute to button content instead of in both button and button group option
|
@cchaos Thanks for the feedback! I've attempted to implement it. Let me know what you think. |
And added a test for changing it
There was a problem hiding this comment.
Ugh sorry, I didn't look to hard at my suggestion. EuiButtonContent doesn't actually do anything with type so I went ahead and moved it back to the EuiButtonGroupOptionProps. I also added it to one of the options in the test suite to check against allowing it to change. 👍
Seems to work well! We'll get this merged once CI passes.
|
Jenkins, test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4368/ |
|
Jenkins, test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4368/ |
Summary
Attempt to Fix #4360
Not having a type of button was making my button group buttons trigger a form submit on click, since the default type is submit. Enable button group button to take a buttonType prop, which defaults to button and will get passed down to the underlying button.
Checklist