-
Notifications
You must be signed in to change notification settings - Fork 13.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
feat: format timestamps in drill by breadcrumbs #23698
feat: format timestamps in drill by breadcrumbs #23698
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23698 +/- ##
==========================================
+ Coverage 67.87% 68.02% +0.14%
==========================================
Files 1925 1936 +11
Lines 74389 74935 +546
Branches 8108 8141 +33
==========================================
+ Hits 50494 50974 +480
- Misses 21818 21873 +55
- Partials 2077 2088 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
A comment to ensure charts a future proofed for GENERIC_CHART_AXES
, which is nowadays set to True
by default
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js
Outdated
Show resolved
Hide resolved
6855dc1
to
49302fa
Compare
op: '==', | ||
val: node.name, | ||
formattedVal: formatSeriesName(node.name, { | ||
timeFormatter: getTimeFormatter(node.name), |
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.
The argument of getTimeFormatter/getNumberFormatter
should be the time/number format selected by the user, available in formData.dateFormat/formData.numberFormat
(or sth similar, not sure if the field name is the same for all charts)
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
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION