From d5ebf315d0f521368f20c6bd60de66751855d233 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Thu, 25 Aug 2022 10:07:06 -0300 Subject: [PATCH 1/2] feat: Adds drill to detail context menu to Pivot Table --- .../src/WorldMap.js | 2 +- .../src/BigNumber/BigNumberViz.tsx | 16 +++---- .../src/Graph/EchartsGraph.tsx | 2 +- .../plugin-chart-echarts/src/Graph/types.ts | 4 +- .../EchartsMixedTimeseries.tsx | 2 +- .../src/Timeseries/EchartsTimeseries.tsx | 2 +- .../src/Treemap/EchartsTreemap.tsx | 6 +-- .../plugins/plugin-chart-echarts/src/types.ts | 4 +- .../src/utils/eventHandlers.ts | 2 +- .../src/PivotTableChart.tsx | 43 +++++++++++++++++++ .../src/plugin/transformProps.ts | 3 +- .../src/react-pivottable/PivotTable.jsx | 5 +-- .../src/react-pivottable/TableRenderers.jsx | 5 +++ .../plugin-chart-pivot-table/src/types.ts | 6 +++ .../src/components/Chart/ChartContextMenu.tsx | 22 +++++----- 15 files changed, 90 insertions(+), 34 deletions(-) diff --git a/superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js b/superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js index 8b8a0abf4361a..195255687ee4c 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js +++ b/superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js @@ -118,7 +118,7 @@ function WorldMap(element, props) { formattedVal, }, ]; - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); }; const map = new Datamap({ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx index 7d7658ff29316..9a27bc000c072 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx @@ -66,8 +66,8 @@ type BigNumberVisProps = { echartOptions: EChartsCoreOption; onContextMenu?: ( filters: QueryObjectFilterClause[], - offsetX: number, - offsetY: number, + clientX: number, + clientY: number, ) => void; xValueFormatter?: TimeFormatter; formData?: BigNumberWithTrendlineFormData; @@ -173,8 +173,8 @@ class BigNumberVis extends React.PureComponent { e.preventDefault(); this.props.onContextMenu( [], - e.nativeEvent.offsetX, - e.nativeEvent.offsetY, + e.nativeEvent.clientX, + e.nativeEvent.clientY, ); } }; @@ -234,7 +234,7 @@ class BigNumberVis extends React.PureComponent { return null; } - renderTrendline(maxHeight: number, chartHeight: number) { + renderTrendline(maxHeight: number) { const { width, trendLineData, echartOptions } = this.props; // if can't find any non-null values, no point rendering the trendline @@ -259,8 +259,8 @@ class BigNumberVis extends React.PureComponent { }); this.props.onContextMenu( filters, - pointerEvent.offsetX, - chartHeight - 100, + pointerEvent.clientX, + pointerEvent.clientY, ); } } @@ -307,7 +307,7 @@ class BigNumberVis extends React.PureComponent { ), )} - {this.renderTrendline(chartHeight, height)} + {this.renderTrendline(chartHeight)} ); } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx index 82e3dcd86e0af..cefd4f9f6af59 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx @@ -61,7 +61,7 @@ export default function EchartsGraph({ formattedVal: targetValue, }, ]; - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); } } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts index ffad9b9cd65cc..63b55d51bd366 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts @@ -89,7 +89,7 @@ export type GraphChartTransformedProps = EchartsProps & { formData: PlainObject; onContextMenu?: ( filters: QueryObjectFilterClause[], - offsetX: number, - offsetY: number, + clientX: number, + clientY: number, ) => void; }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx index 0a3834681c2ae..355e61127e17b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx @@ -135,7 +135,7 @@ export default function EchartsMixedTimeseries({ formattedVal: String(values[i]), }), ); - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); } } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx index 5779ef697dad8..9ae39bead5e3a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx @@ -199,7 +199,7 @@ export default function EchartsTimeseries({ formattedVal: String(values[i]), }), ); - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); } } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx index c19439d91c533..47620eb5dd9f4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { QueryObjectFilterClause } from '@superset-ui/core'; +import { DataRecordValue, QueryObjectFilterClause } from '@superset-ui/core'; import React, { useCallback } from 'react'; import Echart from '../components/Echart'; import { EventHandlers } from '../types'; @@ -48,7 +48,7 @@ export default function EchartsTreemap({ values.length === 0 ? [] : groupby.map((col, idx) => { - const val = groupbyValues.map(v => v[idx]); + const val: DataRecordValue[] = groupbyValues.map(v => v[idx]); if (val === null || val === undefined) return { col, @@ -101,7 +101,7 @@ export default function EchartsTreemap({ formattedVal: path, }), ); - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); } } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts index 693a889ac957f..771c416f417e9 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts @@ -118,8 +118,8 @@ export interface EChartTransformedProps { legendData?: OptionName[]; onContextMenu?: ( filters: QueryObjectFilterClause[], - offsetX: number, - offsetY: number, + clientX: number, + clientY: number, ) => void; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts index e40e77b4b8f31..2cd3e11a55bc3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts @@ -60,7 +60,7 @@ export const contextMenuEventHandler = }), ); } - onContextMenu(filters, pointerEvent.offsetX, pointerEvent.offsetY); + onContextMenu(filters, pointerEvent.clientX, pointerEvent.clientY); } }; diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx index 0ce5c66df843b..3a57b7cc3c254 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx @@ -28,6 +28,7 @@ import { styled, useTheme, isAdhocColumn, + QueryObjectFilterClause, } from '@superset-ui/core'; import { PivotTable, sortAs, aggregatorTemplates } from './react-pivottable'; import { @@ -142,6 +143,7 @@ export default function PivotTableChart(props: PivotTableProps) { metricsLayout, metricColorFormatters, dateFormatters, + onContextMenu, } = props; const theme = useTheme(); @@ -360,6 +362,46 @@ export default function PivotTableChart(props: PivotTableProps) { [colSubtotalPosition, rowSubtotalPosition], ); + const handleContextMenu = ( + e: MouseEvent, + colKey: DataRecordValue[] | undefined, + rowKey: DataRecordValue[] | undefined, + ) => { + if (onContextMenu) { + e.preventDefault(); + const filters: QueryObjectFilterClause[] = []; + if (colKey && colKey.length > 1) { + colKey.forEach((val, i) => { + const col = cols[i]; + const formattedVal = + dateFormatters[col]?.(val as number) || String(val); + if (i > 0) { + filters.push({ + col, + op: '==', + val, + formattedVal, + }); + } + }); + } + if (rowKey) { + rowKey.forEach((val, i) => { + const col = rows[i]; + const formattedVal = + dateFormatters[col]?.(val as number) || String(val); + filters.push({ + col, + op: '==', + val, + formattedVal, + }); + }); + } + onContextMenu(filters, e.clientX, e.clientY); + } + }; + return ( @@ -378,6 +420,7 @@ export default function PivotTableChart(props: PivotTableProps) { tableOptions={tableOptions} subtotalOptions={subtotalOptions} namesMapping={verboseMap} + onContextMenu={handleContextMenu} /> diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts index 8666705b7a6a1..dce9f037e25f5 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts @@ -77,7 +77,7 @@ export default function transformProps(chartProps: ChartProps) { queriesData, formData, rawFormData, - hooks: { setDataMask = () => {} }, + hooks: { setDataMask = () => {}, onContextMenu }, filterState, datasource: { verboseMap = {}, columnFormats = {} }, } = chartProps; @@ -164,5 +164,6 @@ export default function transformProps(chartProps: ChartProps) { metricsLayout, metricColorFormatters, dateFormatters, + onContextMenu, }; } diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/PivotTable.jsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/PivotTable.jsx index bb79069d90192..610e80c0c3fe0 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/PivotTable.jsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/PivotTable.jsx @@ -18,7 +18,6 @@ */ import React from 'react'; -import { PivotData } from './utilities'; import { TableRenderer } from './TableRenderers'; class PivotTable extends React.PureComponent { @@ -27,7 +26,7 @@ class PivotTable extends React.PureComponent { } } -PivotTable.propTypes = PivotData.propTypes; -PivotTable.defaultProps = PivotData.defaultProps; +PivotTable.propTypes = TableRenderer.propTypes; +PivotTable.defaultProps = TableRenderer.defaultProps; export default PivotTable; diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx index fe0c2fb0d522c..54b381be28a1d 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx @@ -700,6 +700,7 @@ export class TableRenderer extends React.Component { className="pvtVal" key={`pvtVal-${flatColKey}`} onClick={rowClickHandlers[flatColKey]} + onContextMenu={e => this.props.onContextMenu(e, colKey, rowKey)} style={style} > {agg.format(aggValue)} @@ -717,6 +718,7 @@ export class TableRenderer extends React.Component { key="total" className="pvtTotal" onClick={rowTotalCallbacks[flatRowKey]} + onContextMenu={e => this.props.onContextMenu(e, undefined, rowKey)} > {agg.format(aggValue)} @@ -776,6 +778,7 @@ export class TableRenderer extends React.Component { className="pvtTotal pvtRowTotal" key={`total-${flatColKey}`} onClick={colTotalCallbacks[flatColKey]} + onContextMenu={e => this.props.onContextMenu(e, colKey, undefined)} style={{ padding: '5px' }} > {agg.format(aggValue)} @@ -793,6 +796,7 @@ export class TableRenderer extends React.Component { key="total" className="pvtGrandTotal pvtRowTotal" onClick={grandTotalCallback} + onContextMenu={e => this.props.onContextMenu(e, undefined, undefined)} > {agg.format(aggValue)} @@ -886,5 +890,6 @@ export class TableRenderer extends React.Component { TableRenderer.propTypes = { ...PivotData.propTypes, tableOptions: PropTypes.object, + onContextMenu: PropTypes.func, }; TableRenderer.defaultProps = { ...PivotData.defaultProps, tableOptions: {} }; diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts index 38d725e922b3a..ded6c48b2e69e 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts @@ -26,6 +26,7 @@ import { NumberFormatter, QueryFormMetric, QueryFormColumn, + QueryObjectFilterClause, } from '@superset-ui/core'; import { ColorFormatters } from '@superset-ui/chart-controls'; @@ -72,6 +73,11 @@ interface PivotTableCustomizeProps { dateFormatters: Record; legacy_order_by: QueryFormMetric[] | QueryFormMetric | null; order_desc: boolean; + onContextMenu?: ( + filters: QueryObjectFilterClause[], + clientX: number, + clientY: number, + ) => void; } export type PivotTableQueryFormData = QueryFormData & diff --git a/superset-frontend/src/components/Chart/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu.tsx index 49e084bb868c2..b2bc211cb2723 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu.tsx @@ -36,8 +36,8 @@ export interface ChartContextMenuProps { export interface Ref { open: ( filters: QueryObjectFilterClause[], - offsetX: number, - offsetY: number, + clientX: number, + clientY: number, ) => void; } @@ -54,9 +54,9 @@ const ChartContextMenu = ( ) => { const [state, setState] = useState<{ filters: QueryObjectFilterClause[]; - offsetX: number; - offsetY: number; - }>({ filters: [], offsetX: 0, offsetY: 0 }); + clientX: number; + clientY: number; + }>({ filters: [], clientX: 0, clientY: 0 }); const menu = ( @@ -81,8 +81,8 @@ const ChartContextMenu = ( ); const open = useCallback( - (filters: QueryObjectFilterClause[], offsetX: number, offsetY: number) => { - setState({ filters, offsetX, offsetY }); + (filters: QueryObjectFilterClause[], clientX: number, clientY: number) => { + setState({ filters, clientX, clientY }); // Since Ant Design's Dropdown does not offer an imperative API // and we can't attach event triggers to charts SVG elements, we @@ -111,9 +111,11 @@ const ChartContextMenu = ( id={`hidden-span-${id}`} css={{ visibility: 'hidden', - position: 'absolute', - top: state.offsetY, - left: state.offsetX, + position: 'fixed', + top: state.clientY, + left: state.clientX, + width: 1, + height: 1, }} /> From 66335ddc2a86607ce6a365e0129063fcc99d2728 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Thu, 25 Aug 2022 16:07:26 -0300 Subject: [PATCH 2/2] Adds useCallback --- .../src/PivotTableChart.tsx | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx index 3a57b7cc3c254..924470559fb8d 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx @@ -362,45 +362,48 @@ export default function PivotTableChart(props: PivotTableProps) { [colSubtotalPosition, rowSubtotalPosition], ); - const handleContextMenu = ( - e: MouseEvent, - colKey: DataRecordValue[] | undefined, - rowKey: DataRecordValue[] | undefined, - ) => { - if (onContextMenu) { - e.preventDefault(); - const filters: QueryObjectFilterClause[] = []; - if (colKey && colKey.length > 1) { - colKey.forEach((val, i) => { - const col = cols[i]; - const formattedVal = - dateFormatters[col]?.(val as number) || String(val); - if (i > 0) { + const handleContextMenu = useCallback( + ( + e: MouseEvent, + colKey: DataRecordValue[] | undefined, + rowKey: DataRecordValue[] | undefined, + ) => { + if (onContextMenu) { + e.preventDefault(); + const filters: QueryObjectFilterClause[] = []; + if (colKey && colKey.length > 1) { + colKey.forEach((val, i) => { + const col = cols[i]; + const formattedVal = + dateFormatters[col]?.(val as number) || String(val); + if (i > 0) { + filters.push({ + col, + op: '==', + val, + formattedVal, + }); + } + }); + } + if (rowKey) { + rowKey.forEach((val, i) => { + const col = rows[i]; + const formattedVal = + dateFormatters[col]?.(val as number) || String(val); filters.push({ col, op: '==', val, formattedVal, }); - } - }); - } - if (rowKey) { - rowKey.forEach((val, i) => { - const col = rows[i]; - const formattedVal = - dateFormatters[col]?.(val as number) || String(val); - filters.push({ - col, - op: '==', - val, - formattedVal, }); - }); + } + onContextMenu(filters, e.clientX, e.clientY); } - onContextMenu(filters, e.clientX, e.clientY); - } - }; + }, + [cols, dateFormatters, onContextMenu, rows], + ); return (