feat(chart): let Drill By scope to the clicked x-axis value - #42296
feat(chart): let Drill By scope to the clicked x-axis value#42296rusackas wants to merge 3 commits into
Conversation
Adopts the feature from apache#36901: right-clicking a bar/point on a Timeseries-family chart and drilling by now offers the clicked x-axis value as a drill filter, so the drilled chart can be scoped to the clicked bar. When both an x value and a series value are available, the Drill By popover lets the user pick which to apply (x, series, or both), mirroring the Drill to detail by pattern. Temporal x-axis values become TEMPORAL_RANGE filters covering the clicked time bucket, computed with calendar-aware UTC arithmetic. Co-authored-by: Jacob Hartmann (BUVM-STIL) <jacob.hartmann@stil.dk> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The flagged issue is correct. In To resolve this, update the lookup to use // superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx
const xAxisFilter =
coltypeMapping?.[xAxisCol] === GenericDataType.Temporal
? getTemporalXAxisDrillByFilter(
xAxisCol,
categoryAxisValue,
formData.timeGrainSqla,
String(eventParams.name ?? categoryAxisValue),
)
: {
col: xAxisCol,
op: '==' as const,
val: categoryAxisValue,
formattedVal: String(categoryAxisValue),
};I have checked the PR comments and there are no other pending review comments to address. Would you like me to implement this fix for you? superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx |
There was a problem hiding this comment.
Code Review Agent Run #78db08
Actionable Suggestions - 1
-
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx - 1
- Missing test coverage for new state · Line 159-162
Additional Suggestions - 1
-
superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx - 1
-
Missing prop: explicit value · Line 328-328The Radio.Group is missing an explicit `value` prop while individual Radio children have `value` attributes. This creates ambiguity between controlled and uncontrolled behavior. Although the default initial state is `filterScope = DrillByFilterScope.All`, explicitly passing `value={filterScope}` makes the controlled state intent clear and prevents potential TypeScript warnings about missing value in controlled Radio.Group.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx - 1
- Missing event type annotation · Line 301-301
Review Details
-
Files reviewed - 7 · Commit Range:
a8948ae..a8948ae- superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts
- superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx
- superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts
- superset-frontend/plugins/plugin-chart-echarts/test/utils/xAxisDrillByFilter.test.ts
- superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
- superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
- superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #42296 +/- ##
==========================================
+ Coverage 65.15% 65.16% +0.01%
==========================================
Files 2789 2790 +1
Lines 157543 157659 +116
Branches 35864 35912 +48
==========================================
+ Hits 102644 102745 +101
- Misses 52931 52946 +15
Partials 1968 1968
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:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the Drill By experience for ECharts Timeseries charts by allowing drill filters to scope to the clicked x-axis value (time bucket or category), the clicked series value, or both—addressing cases where drilling previously wasn’t constrained to the clicked bar/point.
Changes:
- Extend
ContextMenuFilters['drillBy']with optionalxAxisFiltersto represent x-axis-scoped filters separately from series-scoped filters. - Add a Drill By “Filter by” scope selector UI (x value / series value / both) and plumb the selected effective filter set into Drill By actions/modal.
- Add calendar-aware UTC time-bucket range computation for temporal x-axis drill filters, with unit tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx | Adds scope selector UI and computes an effective drill-by config based on selected scope. |
| superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx | Adds coverage for scope selector rendering and scope-dependent filter application. |
| superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx | Ensures the Drill By modal uses the effective drill-by config selected in the submenu. |
| superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts | Implements temporal x-axis drill filter generation with calendar-aware bucket ranges. |
| superset-frontend/plugins/plugin-chart-echarts/test/utils/xAxisDrillByFilter.test.ts | Adds unit tests for time bucket range math and temporal filter fallbacks. |
| superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx | Emits xAxisFilters in context menu payloads for both temporal and categorical x-axes. |
| superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts | Extends shared context menu filter types with optional xAxisFilters. |
The exact-match fallback in getTemporalXAxisDrillByFilter truncated timestamps to whole seconds, so temporal x-axis values with millisecond precision (raw timestamps with no time grain) could fail to match the clicked row. formatNaiveDateTime now preserves milliseconds when present. Also adds test coverage asserting that a Drill By scope selected in the submenu takes precedence over the raw context-menu filters when opening the DrillByModal, per code review feedback on PR apache#42296. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #c3945fActionable 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 |
… test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review Agent Run #dfce1bActionable 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 the feature from #36901 by @JayHealth (thanks Jay!), reworked around the approach agreed there: instead of special-casing
echarts_timeseries_bar, Drill by now mirrors theDrill to detail bypattern from @michael-s-molina, letting the user pick what should be considered when drilling: the clicked x-axis value, the clicked series, or both.Fixes the confusion reported in #36766: right-clicking the "Germany" bar on a country bar chart and drilling by
citypreviously showed cities for all countries, because the clicked x-axis value never made it into the drill-by filters. Only the series (dimension) values did.What changed:
ContextMenuFilters['drillBy']gains an optionalxAxisFiltersfield alongside the existing series-scopedfilters.TEMPORAL_RANGEfilter covering the clicked time bucket, computed with calendar-aware UTC arithmetic (correct month/quarter/year lengths, week-ending grains spanning backwards from their label) rather than fixed day counts. Falls back to an exact match when no time grain is set.==filter on the clicked category. If the column under a force-categorical axis is temporal, it still gets the time-bucket treatment.Review note: defaulting the selector to "both" is a deliberate behavior change for timeseries charts with dimensions (previously series-only). Happy to flip the default to preserve the old behavior if folks prefer.
Follow-up candidates: MixedTimeseries has its own contextmenu handler and can adopt
xAxisFiltersthe same way; other x/y-style plugins can join incrementally.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: right-clicking a bar and drilling by only carried the series values into the drilled chart (or nothing at all for charts without a dimension), so the drilled chart wasn't scoped to the clicked bar. After: the Drill by popover offers a "Filter by" choice between the clicked x value, the series value, or both (shown only when both scopes exist), and the drilled chart is filtered accordingly. #36901 has screenshots of the original problem.
TESTING INSTRUCTIONS
echarts_timeseries_bar) with a categorical x-axis (e.g. country) and a metric; optionally add a dimension.New unit tests cover the time-bucket range math (
xAxisDrillByFilter.test.ts) and the scope selector behavior (DrillBySubmenu.test.tsx).ADDITIONAL INFORMATION
🤖 Generated with Claude Code