-
Notifications
You must be signed in to change notification settings - Fork 77
fix: Making viz components respect D3 Format from metric #280
fix: Making viz components respect D3 Format from metric #280
Conversation
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
Deploy preview for superset-ui-plugins ready! Built with commit 96e9d75 |
67544e3
to
fd67f96
Compare
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
f313206
to
4685276
Compare
Codecov Report
@@ Coverage Diff @@
## master #280 +/- ##
======================================
Coverage 36.9% 36.9%
======================================
Files 12 12
Lines 233 233
Branches 24 24
======================================
Hits 86 86
Misses 135 135
Partials 12 12 Continue to review full report at Codecov.
|
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.
some questions
packages/superset-ui-legacy-preset-chart-big-number/src/BigNumber/transformProps.js
Show resolved
Hide resolved
packages/superset-ui-legacy-preset-chart-nvd3/src/transformProps.js
Outdated
Show resolved
Hide resolved
packages/superset-ui-legacy-preset-chart-nvd3/src/transformProps.js
Outdated
Show resolved
Hide resolved
@etr2460 I think everything here was addressed and/or improved upon (thanks for looking at it!) - let me know if there's anything else that looks like it needs attention. |
@etr2460 / @mistercrunch wondering if you wouldn't mind a (re)review on this. |
…rset#280) * fix: BigNumber uses metric's d3format value now, when populated. * fix: respecting D3 Format column for a handful of NVD3 charts * fix: treemap respects metric's D3 Format setting * style: Simpler loop syntax using forEach, Prettier cleanup. * style: prettier * style: re-installed modules, re-ran prettier * fix: eslint nits * style: moving grabD3Format outside the transformProps block * fix: allow formData to override metric's D3 Format * style: overwriting yAxisFormat rather than declaring a new var
🐛 Bug Fix
Addresses this bug:
apache/superset#7650
Riffs on:
https://github.com/apache-superset/superset-ui-plugins/pull/171/files
Screenshots with a goofy number format to prove the point:
Bar (before):

Bar (after):

Line (before):

Line (after):

Pie (before):

Pie (after):

Dual line (before):

Dual line (after):

Area (before):

Area (after):

Treemap (before):

Treemap (after):

Reviewers:
@kristw @etr2460 @mistercrunch