rusackas opened a new pull request, #42296:
URL: https://github.com/apache/superset/pull/42296

   ### 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 the `Drill to detail by` pattern 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 
https://github.com/apache/superset/discussions/36766: right-clicking the 
"Germany" bar on a country bar chart and drilling by `city` previously 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 optional `xAxisFilters` field 
alongside the existing series-scoped `filters`.
   - The Timeseries plugin (bar/line/area/scatter/step, vertical + horizontal) 
now emits a filter for the clicked x-axis value:
     - Temporal x-axis: a `TEMPORAL_RANGE` filter 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.
     - Categorical x-axis: an `==` filter on the clicked category. If the 
column under a force-categorical axis is temporal, it still gets the 
time-bucket treatment.
   - The Drill by popover shows a "Filter by" selector (x value / series value 
/ both) when both scopes are available, defaulting to **both**, which matches 
how the Pie chart has always drilled and what users in the discussion expected. 
The previous series-only behavior is one click away. When only one scope exists 
there's no selector and behavior is unchanged (except charts with no dimensions 
at all, where drill-by previously applied no filter and now correctly filters 
by the clicked bar).
   - The selected scope flows through both the initial modal open and further 
drills inside the modal, and shows up in the modal's breadcrumbs.
   
   **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 `xAxisFilters` the 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
   1. Create a bar chart (`echarts_timeseries_bar`) with a categorical x-axis 
(e.g. country) and a metric; optionally add a dimension.
   2. Right-click a bar, hover "Drill by".
   3. With a dimension set, a "Filter by" selector appears at the top of the 
column list: pick the x value, the series value, or both, then pick a column 
(e.g. city).
   4. The drilled chart is now filtered to the selection; e.g. drilling the 
Germany bar by city with the x value selected shows only German cities.
   5. Repeat with a time-based x-axis and a time grain (month/quarter/year): 
the drilled data is filtered to the clicked time bucket.
   6. Drill further inside the modal: the same selector appears for the nested 
chart, and breadcrumbs reflect the applied values.
   
   New unit tests cover the time-bucket range math 
(`xAxisDrillByFilter.test.ts`) and the scope selector behavior 
(`DrillBySubmenu.test.tsx`).
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: 
https://github.com/apache/superset/discussions/36766 (supersedes #36901)
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to