michael-s-molina commented on code in PR #33146:
URL: https://github.com/apache/superset/pull/33146#discussion_r2058679991


##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -39,6 +40,48 @@ const config: ControlPanelConfig = {
         ['metric'],
         ['adhoc_filters'],
         ['row_limit'],
+        [
+          {
+            name: 'seriesOrderByColumn',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Series By Column'),

Review Comment:
   ```suggestion
                 label: t('Order series by column'),
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -39,6 +40,48 @@ const config: ControlPanelConfig = {
         ['metric'],
         ['adhoc_filters'],
         ['row_limit'],
+        [
+          {
+            name: 'seriesOrderByColumn',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Series By Column'),
+              description: t(
+                'Column to use for ordering the waterfall series with columns 
not in the chart',
+              ),
+              mapStateToProps: state => ({
+                choices: [
+                  [null, t('None')],
+                  ...(state.datasource?.columns || []).map(col => [
+                    col.column_name,
+                    col.column_name,
+                  ]),
+                ],
+              }),
+              default: null,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'seriesOrderDirection',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Direction'),
+              choices: [
+                [null, t('None')],
+                ['ASC', t('Ascending')],
+                ['DESC', t('Descending')],
+              ],
+              default: null,
+              renderTrigger: true,
+              description: t(
+                'Ordering direction for the series, to be used with "Order 
Series By Column"',

Review Comment:
   ```suggestion
                   'Ordering direction for the series, to be used with "Order 
series by column"',
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -39,6 +40,48 @@ const config: ControlPanelConfig = {
         ['metric'],
         ['adhoc_filters'],
         ['row_limit'],
+        [
+          {
+            name: 'seriesOrderByColumn',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Series By Column'),
+              description: t(
+                'Column to use for ordering the waterfall series with columns 
not in the chart',
+              ),
+              mapStateToProps: state => ({
+                choices: [
+                  [null, t('None')],
+                  ...(state.datasource?.columns || []).map(col => [
+                    col.column_name,
+                    col.column_name,
+                  ]),
+                ],
+              }),
+              default: null,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'seriesOrderDirection',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Direction'),

Review Comment:
   ```suggestion
                 label: t('Order direction'),
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -104,6 +199,20 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'x_axis_label_distance',
+            config: {
+              type: 'TextControl',
+              label: t('X Axis Label Distance'),
+              description: t('Distance of the label from X axis (in pixels)'),

Review Comment:
   Why do we need this? ECharts automatically calculates the distances.



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -39,6 +40,48 @@ const config: ControlPanelConfig = {
         ['metric'],
         ['adhoc_filters'],
         ['row_limit'],
+        [
+          {
+            name: 'seriesOrderByColumn',
+            config: {
+              type: 'SelectControl',
+              label: t('Order Series By Column'),
+              description: t(
+                'Column to use for ordering the waterfall series with columns 
not in the chart',
+              ),
+              mapStateToProps: state => ({
+                choices: [
+                  [null, t('None')],

Review Comment:
   It's important that this select does not accept null values and cannot be 
cleared. It should be set with the x-axis column by default.



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -58,6 +101,58 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'show_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Show Total'),

Review Comment:
   ```suggestion
                 label: t('Show total'),
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -58,6 +101,58 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'show_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Show Total'),
+              default: true,
+              renderTrigger: true,
+              description: t('Show the total value in the waterfall chart'),
+            },
+          },
+        ],
+        [
+          {
+            name: 'bold_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Bold Total'),
+              default: true,
+              renderTrigger: true,
+              description: t(
+                'Bold the total axis label in the waterfall chart',
+              ),
+            },
+          },
+        ],
+        [
+          {
+            name: 'useFirstValueAsSubtotal',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Use first value as subtotal'),
+              default: false,
+              renderTrigger: true,
+              description: t('Render the first bar in the chart as a 
subtotal'),
+            },
+          },
+        ],
+        [
+          {
+            name: 'bold_sub_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Bold first value as subtotal'),

Review Comment:
   Another option would be to have a control like "Bold totals and subtotals".



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -104,6 +199,20 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'x_axis_label_distance',
+            config: {
+              type: 'TextControl',
+              label: t('X Axis Label Distance'),
+              description: t('Distance of the label from X axis (in pixels)'),
+              renderTrigger: true,
+              default: '25',
+              isInt: true,
+              // validators: [v => !Number.isNaN(v) && v >= 0],

Review Comment:
   ```suggestion
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -133,6 +242,53 @@ const config: ControlPanelConfig = {
               description: t('The way the ticks are laid out on the X-axis'),
             },
           },
+          {
+            name: 'x_ticks_wrap_length',

Review Comment:
   Can't this be automatically calculated?



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -58,6 +101,58 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'show_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Show Total'),
+              default: true,
+              renderTrigger: true,
+              description: t('Show the total value in the waterfall chart'),
+            },
+          },
+        ],
+        [
+          {
+            name: 'bold_total',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Bold Total'),

Review Comment:
   Isn't bold by default? If not, we could always make it bold and remove this 
control.



##########
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx:
##########
@@ -146,6 +302,20 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [
+          {
+            name: 'y_axis_label_distance',
+            config: {
+              type: 'TextControl',
+              label: t('Y Axis Label Distance'),
+              description: t('Distance of the label from Y axis (in pixels)'),
+              renderTrigger: true,
+              default: '25',
+              isInt: true,
+              // validators: [v => !Number.isNaN(v) && v >= 0],

Review Comment:
   ```suggestion
   ```



-- 
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: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org

Reply via email to