-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(tile): update props to remove errors, update tile maxheight #4630
Conversation
Deploy preview for the-carbon-components ready! Built with commit 3c0361b https://deploy-preview-4630--the-carbon-components.netlify.com |
Deploy preview for carbon-elements ready! Built with commit 3c0361b |
Deploy preview for carbon-components-react ready! Built with commit 3c0361b https://deploy-preview-4630--carbon-components-react.netlify.com |
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.
LGTM just a small suggestion - Thanks @jendowns!
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.
looks good to me
Co-Authored-By: Josh Black <josh@josh.black>
Thanks @joshblack! Just applied your suggestion 👍 |
Closes #4609
Changelog
Changed
expanded
andonBeforeClick
explicitly so they aren't included in misc props that get passed to the component & cause 2 errors identified in React warnings in console with ExpandableTile #4609tileMaxHeight
andtilePadding
to prevent theNaN
error identified in React warnings in console with ExpandableTile #4609 (comment)Also, optional but recommended:
expanded
state shares a name withexpanded
prop. So to prevent confusion/collision, I suggestremovingusingconst { expanded } = this.state
and just referencingthis.state.expanded
instead.const { expanded: isExpanded
} = this.state`