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