feat: Adds drill to detail context menu for ECharts visualizations #20891

michael-s-molina · 2022-07-27T18:16:56Z

SUMMARY

Adds drill to detail (drill-through) context menu for ECharts visualizations. The context menu is triggered when right-clicking an EChart series. Upon selecting a drill to detail option, an alert is displayed with the selected values.

The linking between the context menu and the actual feature will be handled in a future PR.

The context menu was added to all ECharts with the exception of the Tree Chart.

The feature is behind the DRILL_TO_DETAIL feature flag.

This first iteration focuses on providing a minimal set of interaction possibilities for the ECharts. More advanced interactions like clicking on legends, subheaders, etc will be handled in follow-up PRs. Drilling to detail using metrics is not supported as well.

@kasiazjc @lauderbaugh @rusackas @codyml

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-07-27.at.2.40.50.PM.mov

TESTING INSTRUCTIONS

1 - Make sure the feature does not introduce any regression when disabled. This is the default value for the feature flag.

2 - Enable the DRILL_TO_DETAIL feature flag and check:

Check if there are no regressions when right-clicking the series for different charts
Check if the right-click works with different chart control values (dimensions, time grain, customizations, etc)
Check if the correct values are displayed in the context menu
Check if the correct values are displayed when selecting a context menu option
Check if other dashboard interactions are not affected by the new feature such as cross-filtering, drag-and-drop, tooltips, etc

Below is the import file for the dashboard shown in the video:
dashboard_export_20220727T144027.zip

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags: DRILL_TO_DETAIL
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2022-07-27T22:03:10Z

Codecov Report

Merging #20891 (e68262b) into master (9114175) will decrease coverage by 0.08%.
The diff coverage is 64.50%.

❗ Current head e68262b differs from pull request most recent head 5adb031. Consider uploading reports for the commit 5adb031 to get more accurate results

@@            Coverage Diff             @@
##           master   #20891      +/-   ##
==========================================
- Coverage   66.35%   66.26%   -0.09%     
==========================================
  Files        1767     1769       +2     
  Lines       67353    67464     +111     
  Branches     7145     7168      +23     
==========================================
+ Hits        44691    44708      +17     
- Misses      20834    20926      +92     
- Partials     1828     1830       +2

Flag	Coverage Δ
javascript	`52.01% <64.48%> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...packages/superset-ui-core/src/query/types/Query.ts	`100.00% <ø> (ø)`
...ackages/superset-ui-core/src/utils/featureFlags.ts	`100.00% <ø> (ø)`
...rts/src/BigNumber/BigNumberTotal/transformProps.ts	`0.00% <0.00%> (ø)`
...lugin-chart-echarts/src/BigNumber/BigNumberViz.tsx	`0.00% <0.00%> (ø)`
...lugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx	`0.00% <0.00%> (ø)`
.../plugins/plugin-chart-echarts/src/BoxPlot/types.ts	`0.00% <ø> (ø)`
.../plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx	`0.00% <0.00%> (ø)`
...d/plugins/plugin-chart-echarts/src/Funnel/types.ts	`100.00% <ø> (ø)`
...ns/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx	`0.00% <0.00%> (ø)`
...ns/plugin-chart-echarts/src/Graph/EchartsGraph.tsx	`0.00% <0.00%> (ø)`
... and 36 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

zhaoyongjie · 2022-07-28T02:12:00Z

@michael-s-molina Looking at the screenshot, there is one thing that needs to be added. When a time dimension is clicked with time grain applied, the time grain needs to be passed at the same time.

BTW, why needs a formattedVal in the filter?

michael-s-molina · 2022-07-28T11:10:43Z

@michael-s-molina Looking at the screenshot, there is one thing that needs to be added. When a time dimension is clicked with time grain applied, the time grain needs to be passed at the same time.

Good point! I'll work on it.

BTW, why needs a formattedVal in the filter?

Currently, the logic for formatting the values is contained in each plugin. Depending on the plugin type the values can be formatted differently. When exhibiting these values in the context menu and also later in the modal, we need to present the values with the exact formatting used by the plugin. At the same time, we need the original value for the endpoint. Some examples are dates, currencies, decimals, etc.

michael-s-molina · 2022-07-28T13:04:38Z

@zhaoyongjie Added the time grain for Timeseries and MixedTimeseries.

jinghua-qa · 2022-07-29T08:09:56Z

/testenv up

github-actions · 2022-07-29T08:59:29Z

@jinghua-qa Ephemeral environment spinning up at http://54.68.14.156:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

jinghua-qa · 2022-07-31T07:43:59Z

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

github-actions · 2022-07-31T07:48:01Z

@jinghua-qa Ephemeral environment spinning up at http://34.219.213.123:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

michael-s-molina · 2022-08-01T12:44:38Z

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

github-actions · 2022-08-01T12:48:39Z

@michael-s-molina Ephemeral environment spinning up at http://54.148.127.0:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

jinghua-qa · 2022-08-02T16:31:46Z

I found that in treemap, if i move my mouse to the white edge and it will show drill to details too, is that expected?

Screen.Recording.2022-08-02.at.9.11.42.AM.mov

michael-s-molina · 2022-08-02T16:51:27Z

I found that in treemap, if i move my mouse to the white edge and it will show drill to details too, is that expected?

No, it's not 😆. I'll fix it.

michael-s-molina · 2022-08-04T13:21:41Z

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

michael-s-molina · 2022-08-04T13:22:16Z

I found that in treemap, if i move my mouse to the white edge and it will show drill to details too, is that expected?

Fixed

github-actions · 2022-08-04T13:26:01Z

@michael-s-molina Ephemeral environment spinning up at http://35.89.204.156:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

superset-frontend/src/components/Chart/ChartRenderer.jsx

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx

jinghua-qa

tested regression and feature flag on, LGTM

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx

@@ -62,6 +64,13 @@ type BigNumberVisProps = {
  trendLineData?: TimeSeriesDatum[];
  mainColor: string;
  echartOptions: EChartsCoreOption;
+  onContextMenu?: (
+    filters: QueryObjectFilterClause[],
+    offsetX: number,


zhaoyongjie

thanks for the new feature, quick first pass review.

zhaoyongjie · 2022-08-09T15:37:04Z

superset-frontend/src/components/Chart/ChartRenderer.jsx

+      if (!isEqual(this.state, nextState)) {
+        return true;
+      }


Suggested change

if (!isEqual(this.state, nextState)) {

return true;

}

return !isEqual(this.state, nextState)

I don't want to return unless the state changes. If the state is the same, it should execute the code below these lines.

if (!isEqual(this.state, nextState)) { return true; } this.hasQueryResponseChange = nextProps.queriesResponse !== this.props.queriesResponse; ...

superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx

superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx

@@ -84,6 +86,25 @@ export default function EchartsTreemap({
        handleChange([name]);
      }
    },
+    contextmenu: eventParams => {


codyml · 2022-08-09T16:12:06Z

Tested each viz type and left 2 notes for issues I found, otherwise looks great!

geido · 2022-08-09T16:20:39Z

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

github-actions · 2022-08-09T16:39:20Z

@geido Ephemeral environment spinning up at http://34.216.213.189:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido · 2022-08-09T17:21:15Z

First of all, this looks great!

A few minor issues that I found:

Not a big deal, but you can click way after the number in the Big Number

Another small one. It would be great if the dropdown could be shown in the upper side when it does not have enough visible space to save some of the scrolling

In a graph chart we can only click on the line but not on the node, is that normal?
One more thing about the graph is that when right-clicking the animation is triggered and it creates confusion as the nodes start moving. Not sure if there is a way to stop the chart animation when right-clicking
For some reasons, when trying to close the dropdown by clicking 2 or 3 px from its left side, it won't close. I found it annoying for the user as I ended up clicking a few times to try closing it up. For some reasons, it does not happen if you click on any other side, even 1 px away from its side. This is an Antdesign issue though. Just pointing out if there is a quick fix.

I believe the above are all minor issues that we can handle in separate PRs.

michael-s-molina · 2022-08-09T18:21:48Z

@codyml @geido Great comments! Thanks again for testing the feature.

My last commit fixed:

If you right-click on the dimension label for Treemap v2, the menu looks like it has a broken option:

Not a big deal, but you can click way after the number in the Big Number

I decided to follow @geido's suggestion and tackle the remaining comments in follow-up PRs so that @codyml can rebase his PR on top of this one. I'll create tickets for them.

codyml

LGTM!

github-actions · 2022-08-09T20:02:53Z

Ephemeral environment shutdown and build artifacts deleted.

michael-s-molina requested review from zhaoyongjie, stephenLYZ, kgabryje, geido and jinghua-qa July 27, 2022 18:16

pull-request-size bot added the size/XL label Jul 27, 2022

michael-s-molina force-pushed the echarts-drill-context-menu branch from 5fc9353 to 7c0184a Compare July 27, 2022 19:40

michael-s-molina force-pushed the echarts-drill-context-menu branch from 7c0184a to a6eca35 Compare July 28, 2022 13:03

michael-s-molina added the need:qa-review Requires QA review label Jul 28, 2022

michael-s-molina force-pushed the echarts-drill-context-menu branch from 5b97dca to 076a905 Compare August 4, 2022 11:57

codyml reviewed Aug 8, 2022

View reviewed changes

superset-frontend/src/components/Chart/ChartRenderer.jsx Outdated Show resolved Hide resolved

codyml reviewed Aug 8, 2022

View reviewed changes

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx Show resolved Hide resolved

jinghua-qa approved these changes Aug 8, 2022

View reviewed changes

michael-s-molina added 2 commits August 9, 2022 08:58

feat: Adds drill to detail context menu for ECharts visualizations

545cfda

Rebases and adds time grain

78927bb

michael-s-molina added 3 commits August 9, 2022 08:58

Fixes selected gauge values

d275ec2

Fixes Treemap edge click

db3f21b

Adds right click to big number trendline

5eccb92

michael-s-molina force-pushed the echarts-drill-context-menu branch from 076a905 to 5eccb92 Compare August 9, 2022 12:41

michael-s-molina mentioned this pull request Aug 9, 2022

feat: POC to support right-click on plugins #20311

Closed

9 tasks

codyml reviewed Aug 9, 2022

View reviewed changes

zhaoyongjie reviewed Aug 9, 2022

View reviewed changes

codyml reviewed Aug 9, 2022

View reviewed changes

superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx

@@ -84,6 +86,25 @@ export default function EchartsTreemap({

handleChange([name]);

}

},

contextmenu: eventParams => {

This comment was marked as resolved.

Sign in to view

Address some comments

5adb031

codyml approved these changes Aug 9, 2022

View reviewed changes

michael-s-molina merged commit 3df8335 into apache:master Aug 9, 2022

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds drill to detail context menu for ECharts visualizations #20891

feat: Adds drill to detail context menu for ECharts visualizations #20891

michael-s-molina commented Jul 27, 2022

codecov bot commented Jul 27, 2022 •

edited

Loading

zhaoyongjie commented Jul 28, 2022

michael-s-molina commented Jul 28, 2022 •

edited

Loading

michael-s-molina commented Jul 28, 2022

jinghua-qa commented Jul 29, 2022

github-actions bot commented Jul 29, 2022

jinghua-qa commented Jul 31, 2022

github-actions bot commented Jul 31, 2022

michael-s-molina commented Aug 1, 2022

github-actions bot commented Aug 1, 2022

jinghua-qa commented Aug 2, 2022

michael-s-molina commented Aug 2, 2022

michael-s-molina commented Aug 4, 2022

michael-s-molina commented Aug 4, 2022 •

edited

Loading

github-actions bot commented Aug 4, 2022

jinghua-qa left a comment

This comment was marked as resolved.

zhaoyongjie left a comment

zhaoyongjie Aug 9, 2022

michael-s-molina Aug 9, 2022

This comment was marked as resolved.

codyml commented Aug 9, 2022

geido commented Aug 9, 2022

github-actions bot commented Aug 9, 2022

geido commented Aug 9, 2022

michael-s-molina commented Aug 9, 2022

codyml left a comment

github-actions bot commented Aug 9, 2022

feat: Adds drill to detail context menu for ECharts visualizations #20891

feat: Adds drill to detail context menu for ECharts visualizations #20891

Conversation

michael-s-molina commented Jul 27, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jul 27, 2022 • edited Loading

Codecov Report

zhaoyongjie commented Jul 28, 2022

michael-s-molina commented Jul 28, 2022 • edited Loading

michael-s-molina commented Jul 28, 2022

jinghua-qa commented Jul 29, 2022

github-actions bot commented Jul 29, 2022

jinghua-qa commented Jul 31, 2022

github-actions bot commented Jul 31, 2022

michael-s-molina commented Aug 1, 2022

github-actions bot commented Aug 1, 2022

jinghua-qa commented Aug 2, 2022

michael-s-molina commented Aug 2, 2022

michael-s-molina commented Aug 4, 2022

michael-s-molina commented Aug 4, 2022 • edited Loading

github-actions bot commented Aug 4, 2022

jinghua-qa left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

zhaoyongjie left a comment

Choose a reason for hiding this comment

zhaoyongjie Aug 9, 2022

Choose a reason for hiding this comment

michael-s-molina Aug 9, 2022

Choose a reason for hiding this comment

This comment was marked as resolved.

codyml commented Aug 9, 2022

geido commented Aug 9, 2022

github-actions bot commented Aug 9, 2022

geido commented Aug 9, 2022

michael-s-molina commented Aug 9, 2022

codyml left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 9, 2022

codecov bot commented Jul 27, 2022 •

edited

Loading

michael-s-molina commented Jul 28, 2022 •

edited

Loading

michael-s-molina commented Aug 4, 2022 •

edited

Loading