rusackas commented on code in PR #42296:
URL: https://github.com/apache/superset/pull/42296#discussion_r3627525268
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -343,6 +345,55 @@ export default function EchartsTimeseries({
});
});
+ // Filters for the clicked x-axis value, so Drill By can subset the
+ // drilled data to the clicked bar/point rather than only the series
+ const xAxisFilters: BinaryQueryObjectFilterClause[] = [];
+ const xAxisCol =
+ // if the xAxis is '__timestamp', granularity_sqla will be the
column of filter
+ xAxis.label === DTTM_ALIAS ? formData.granularitySqla : xAxis.label;
+ if (data && xAxis.type === AxisType.Time && xAxisCol) {
+ // For horizontal orientation the [x, value] pair is swapped
+ const xValue = Array.isArray(data)
+ ? data[categoryAxisValueIndex]
+ : data;
+ const xAxisFilter = getTemporalXAxisDrillByFilter(
+ xAxisCol,
+ xValue,
+ formData.timeGrainSqla,
+ String(xValueFormatter(xValue as number)),
+ );
+ if (xAxisFilter) {
+ xAxisFilters.push(xAxisFilter);
+ }
+ } else if (xAxis.type === AxisType.Category && xAxisCol) {
+ const categoryAxisValue = getCategoryAxisValue(
+ data,
+ eventParams.name,
+ );
+ if (categoryAxisValue !== undefined) {
+ // A category axis can still sit on a temporal column when the
+ // axis is forced categorical; filter by time bucket in that case
+ const xAxisFilter =
+ coltypeMapping?.[getColumnLabel(xAxis.label)] ===
+ GenericDataType.Temporal
Review Comment:
I think Codeant has this backwards. `coltypeMapping` is keyed by the result
column names, so when the axis is `__timestamp` that is the key holding the
temporal type. `xAxisCol` is the source `granularitySqla`, which is not in the
map. Same lookup `transformProps` uses for `xAxisDataType`.
##########
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx:
##########
@@ -155,17 +156,29 @@ const ChartContextMenu = (
const [drillModalIsOpen, setDrillModalIsOpen] = useState(false);
const [drillByColumn, setDrillByColumn] = useState<Column>();
+ // Drill by config as selected in the submenu (e.g. with the chosen
+ // x-axis/series filter scope applied), used over the raw context filters
+ const [selectedDrillByConfig, setSelectedDrillByConfig] =
+ useState<ContextMenuFilters['drillBy']>();
Review Comment:
The submenu-selected config path is already covered in
`DrillBySubmenu.test.tsx`, where the scope selector drives the `drillByConfig`
handed to `onDrillBy`. The `??` fallback here is trivial glue over that, so I
do not think a separate test earns its keep.
--
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]
