-
Notifications
You must be signed in to change notification settings - Fork 77
fix: pass all props to transformProps in LineMulti chart #247
Conversation
hooks: PropTypes.shape({ | ||
onAddFilter: PropTypes.func, | ||
onError: PropTypes.func, | ||
}), | ||
}; | ||
const defaultProps = { | ||
onAddFilter() {}, |
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.
You don't need this.now.
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.
oh, didn't see that there. Wonder why is was failing on master if it has that fallback. 🤷♂
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 is an ongoing issue with build tools, probably not related to this.
3e11ccf
to
051b925
Compare
Looks like the build is complaining with a babel error: I was getting the same in dev when trying to build. |
Please rebase |
051b925
to
193e564
Compare
Deploy preview for superset-ui-plugins ready! Built with commit 193e564 |
Codecov Report
@@ Coverage Diff @@
## master #247 +/- ##
=======================================
Coverage 37.99% 37.99%
=======================================
Files 12 12
Lines 229 229
Branches 21 21
=======================================
Hits 87 87
Misses 132 132
Partials 10 10 Continue to review full report at Codecov.
|
🔨 🐛 🔥 |
…superset#247) * feat: add axis encoder * test: add unit test * fix: params * refactor: rename * fix: address comments * fix: update import * fix: error * fix: lint
💔 Breaking Changes
🏆 Enhancements
📜 Documentation
🐛 Bug Fix
This pr fixes an issue where LineMulti chart doesn't render with the current superset/master code.
@mistercrunch
🏠 Internal