sadpandajoe commented on code in PR #42533:
URL: https://github.com/apache/superset/pull/42533#discussion_r3687092529


##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -245,6 +246,11 @@ const config: ControlPanelConfig = {
       multi: false,
     },
   },
+  formDataOverrides: formData => ({
+    ...formData,
+    metric: getStandardizedControls().shiftMetric(),

Review Comment:
   If a Line chart has metrics A and B, switching to Waterfall leaves B queued; 
choosing B in Waterfall and switching back then collects the active B plus the 
queued B, so Line still receives duplicate metrics. Could this reconcile the 
active Waterfall metric with the queued metrics and cover that edit path with a 
real Line → Waterfall → Line regression test?



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -245,6 +246,11 @@ const config: ControlPanelConfig = {
       multi: false,
     },
   },
+  formDataOverrides: formData => ({
+    ...formData,
+    metric: getStandardizedControls().shiftMetric(),
+    groupby: getStandardizedControls().popAllColumns(),

Review Comment:
   Waterfall declares `groupby` as `multi: false`, but this can overwrite a 
memorized single breakdown with every dimension accumulated in another chart. 
`buildQuery` groups by all of them while `transformProps` reads only 
`groupby[0]`, so returning from a Table/Pivot with multiple dimensions silently 
queries finer granularity than it renders. Should this transfer only one column 
and leave the rest queued?



-- 
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