-
Notifications
You must be signed in to change notification settings - Fork 14k
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
feat(explore): Denormalize form data in echarts, world map and nvd3 bar and line charts #20313
feat(explore): Denormalize form data in echarts, world map and nvd3 bar and line charts #20313
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20313 +/- ##
==========================================
- Coverage 66.65% 66.58% -0.07%
==========================================
Files 1729 1733 +4
Lines 64910 64978 +68
Branches 6842 6858 +16
==========================================
+ Hits 43267 43268 +1
- Misses 19894 19955 +61
- Partials 1749 1755 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
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.
LGTM with one nit.
@@ -210,6 +210,11 @@ const config: ControlPanelConfig = { | |||
], | |||
}, | |||
], | |||
denormalizeFormData: formData => ({ |
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.
It looks like the denormalizeFormData is consistent for most charts, so maybe we can abstract out a generic function?
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.
Different viz have different control names, for example, the BigNumber
supported 1 metric and 0 dimension, the metrics in SFD should pick only one, and drop other columns.
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 great!
SUMMARY
Applies denormalization introduced in #20010 to following charts:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Listed charts should work like described in #20010
ADDITIONAL INFORMATION