Repository navigation
feat(echarts): respect time grain in time-series tooltips - #41350
Conversation
Code Review Agent Run #8ab929Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41350 +/- ##
=======================================
Coverage 64.61% 64.62%
=======================================
Files 2684 2684
Lines 148514 148524 +10
Branches 34263 34272 +9
=======================================
+ Hits 95969 95981 +12
+ Misses 50786 50784 -2
Partials 1759 1759
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0ef74b6 to
4223c94
Compare
Code Review Agent Run #3977afActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
bc298b4 to
d53b7bf
Compare
Code Review Agent Run #79a181Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
48e91a7 to
453095b
Compare
Code Review Agent Run #1454b5Actionable Suggestions - 0Additional Suggestions - 3
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Adopts #31765 (original by @gerbermichi), splitting the time-grain formatting fix out from the i18n change and breaking-change concerns raised on that PR. Time-series and mixed time-series tooltips now honor the chart's time grain (and any dashboard-level override delivered via extra_form_data) when no explicit tooltip time format is set, matching the behavior the x-axis already applies via getSmartDateFormatter. Tooltips read grain-appropriate labels such as 'Jan 2021' (month), '2021 Q1' (quarter), '2021' (year), and weekly date ranges. An explicit custom tooltip format always wins. The original PR threaded the grain through getTimeFormatter using the SMART_DATE_VERBOSE_ID format id, which produced the literal string 'smart_date_verbose' because getTimeFormatter treats the id as a d3 format string when a granularity is supplied. This reworks the tooltip path to use the granularity formatter (getTimeFormatter(undefined, grain)) instead, with unit-test coverage proving the rendered output for each grain. Adds an UPDATING.md note per villebro's request. Co-Authored-By: Michael Gerber <michael.gerber@nxt.engineering> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Align minInterval/maxInterval guards with the resolved grain so dashboard-level time_grain_sqla overrides drive tick spacing the same way they already drive tooltip and x-axis label formatting, and fall back safely when a grain is missing from TIMEGRAIN_TO_TIMESTAMP. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The showMaxLabel/dedup branch still keyed off chart-level timeGrainSqla while formatter and interval logic use resolvedTimeGrain. Align it so a dashboard grain override drives axis-label overlap handling too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
453095b to
fd176a1
Compare
Code Review Agent Run #36849cActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
Two things worth addressing before merge:
|
…UPDATING.md wording Adds transformProps-level tests for Timeseries and MixedTimeseries asserting that an extra_form_data time-grain override reaches the rendered tooltip, and softens the UPDATING.md note per review feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @kgabryje, both fixed. Added transformProps-level tests for Timeseries and MixedTimeseries confirming the extra_form_data grain override reaches the rendered tooltip, and softened the UPDATING.md wording, Adaptive formatting is grain-aware too, only a custom d3 format string is unaffected. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #4fb616Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…asts Addresses bot review feedback: use standalone test() cases instead of a new describe() wrapper, and type the tooltip formatter access instead of casting to any. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review Agent Run #3b83c9Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
Adopts #31765 (original by @gerbermichi), carrying forward the time-grain formatting fix while splitting it off from the i18n change and the breaking-change concerns raised on that PR by @villebro. @gerbermichi gave the go-ahead to pick this up on a fresh PR.
Time-series and mixed time-series tooltips now honor the chart's selected time grain — and any dashboard-level grain override delivered via
extra_form_data— when no explicit tooltip time format is set. This matches the behavior the x-axis already applies throughgetSmartDateFormatter. Tooltips read grain-appropriate labels:2021-01-072021-01-07 — 2021-01-13Jan 20212021 Q12021An explicit custom tooltip time format always wins, so users who pinned a format see no change.
How this was ported
Most of the original PR's intent had already landed on master since it was opened:
getSmartDateFormatter(timeGrain)).The remaining genuine gap was the tooltip not receiving the time grain, so this PR focuses there. The original PR threaded the grain through
getTimeFormatterusing theSMART_DATE_VERBOSE_IDformat id, but that path produced the literal stringsmart_date_verbose—getTimeFormattertreats the id as a d3 format string once a granularity is supplied. This reworksgetTooltipTimeFormatterto use the granularity formatter (getTimeFormatter(undefined, grain)) instead, which renders the correct labels above. The dashboard-level override (extra_form_data.time_grain_sqla) is read with optional chaining to avoid the runtime-crash risk the review bots flagged on the original PR.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
See the before/after screenshots on the original PR #31765. Behavior here is the tooltip half of that change.
TESTING INSTRUCTIONS
2021 Q1,Jan 2021,2021, or a weekly range rather than a full timestamp.Automated coverage:
superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.tsadds unit tests asserting the rendered output ofgetTooltipTimeFormatterfor each grain, the no-explicit-format case, and the explicit-format-wins case.ADDITIONAL INFORMATION
Adopts #31765, original by @gerbermichi. Includes an
UPDATING.mdnote per @villebro's request on the original PR.🤖 Generated with Claude Code