villebro commented on code in PR #20010:
URL: https://github.com/apache/superset/pull/20010#discussion_r873491796


##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -517,6 +517,11 @@ const config: ControlPanelConfig = {
       ],
     },
   ],
+  denormalizeFormData: formData => ({
+    ...formData,
+    metrics: formData.standardized_form_data.sharedFormData.metrics,

Review Comment:
   I'm wondering if we should consider adding `percent_metrics` to the metrics 
array in `sharedFormData`? Let's say we have a table chart with just a single 
metric in `percentage_metrics` and change to Pie - currently the metric will be 
lost.
   
   As a solution - Maybe we could add a property to the normalized control 
values that states some additional context about the origin of the value. In 
this case it could specify that it originated from the `percent_metrics` 
control rather than a "regular" metric control. So the additional value could, 
for instance, be `sourceControl`.
   
   This would also address the problem identified in the Pivot table - if we 
had the additional context about what type of a control the value originated 
from, we would more easily be able to map them back to their original controls 
(`groupbyRows` or `groupbyColums`). Ping @kgabryje



##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -517,6 +517,11 @@ const config: ControlPanelConfig = {
       ],
     },
   ],
+  denormalizeFormData: formData => ({
+    ...formData,
+    metrics: formData.standardized_form_data.sharedFormData.metrics,

Review Comment:
   I'm wondering if we should consider adding `percent_metrics` to the metrics 
array in `sharedFormData`? Let's say we have a table chart with just a single 
metric in `percentage_metrics` and change to Pie - currently the metric will be 
lost.
   
   As a solution - Maybe we could add a property to the normalized control 
values that states some additional context about the origin of the value. In 
this case it could specify that it originated from the `percent_metrics` 
control rather than a "regular" metric control. So the additional value could, 
for instance, be called `sourceControl`.
   
   This would also address the problem identified in the Pivot table - if we 
had the additional context about what type of a control the value originated 
from, we would more easily be able to map them back to their original controls 
(`groupbyRows` or `groupbyColums`). Ping @kgabryje



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