-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(Button): update handling of floated prop #1489
Conversation
const rest = getUnhandledProps(Button, this.props) | ||
const ElementType = getElementType(Button, this.props, () => { | ||
if (!_.isNil(label) || !_.isNil(attached)) return 'div' | ||
}) |
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.
I've moved this inline function to method, so it will not be created on each render.
Codecov Report
@@ Coverage Diff @@
## master #1489 +/- ##
==========================================
- Coverage 99.74% 99.74% -0.01%
==========================================
Files 141 141
Lines 2371 2364 -7
==========================================
- Hits 2365 2358 -7
Misses 6 6
Continue to review full report at Codecov.
|
@@ -109,6 +109,11 @@ describe('Button', () => { | |||
wrapper.find('Label').should.not.have.className('disabled') | |||
wrapper.find('button').should.not.have.className('disabled') | |||
}) | |||
it('contains children without floated class when floated attribute is set', () => { |
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.
Would prefer we assert the expected location of the floated class in this test as well. It currently asserts where the className should not be, but doesn't assert where it should be.
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.
I've updated tests as you requested 👍
Released in |
Fixes #1488.
floated
class must be on wrapper if we haveLabel
element, this PR update handling of this.