Antonio-RiveroMartnez commented on code in PR #27716:
URL: https://github.com/apache/superset/pull/27716#discussion_r1548463091
##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -520,41 +598,41 @@ const config: ControlPanelConfig = {
],
[
{
- name: 'allow_rearrange_columns',
+ name: 'comparison_color_enabled',
config: {
type: 'CheckboxControl',
- label: t('Allow columns to be rearranged'),
+ label: t('basic conditional formatting'),
renderTrigger: true,
+ visibility: ({ controls }) =>
+ Boolean(controls?.enable_time_comparison?.value) &&
+ isFeatureEnabled(FeatureFlag.ChartPluginsExperimental),
default: false,
description: t(
- "Allow end user to drag-and-drop column headers to rearrange
them. Note their changes won't persist for the next time they open the chart.",
+ 'This will be applied to the whole table. Arrows (↑ and ↓)
will be added to ' +
+ 'main columns for increase and decrease. Basic conditional
formatting can be ' +
+ 'overwritten by conditional formatting below.',
),
- visibility: ({ controls }) =>
- !controls?.enable_time_comparison?.value ||
- !isFeatureEnabled(FeatureFlag.ChartPluginsExperimental),
},
},
],
[
{
- name: 'column_config',
+ name: 'comparison_color_scheme',
config: {
- type: 'ColumnConfigControl',
- label: t('Customize columns'),
- description: t('Further customize how to display each column'),
- width: 400,
- height: 320,
+ type: 'SelectControl',
+ label: t('color type'),
+ default: ColorSchemeEnum.Green,
renderTrigger: true,
- shouldMapStateToProps() {
- return true;
- },
- mapStateToProps(explore, _, chart) {
- return {
- queryResponse: chart?.queriesResponse?.[0] as
- | ChartDataResponseResult
- | undefined,
- };
- },
+ choices: [
+ [ColorSchemeEnum.Green, 'Green for increase, red for
decrease'],
+ [ColorSchemeEnum.Red, 'Red for increase, green for decrease'],
+ ],
+ visibility: ({ controls }) =>
+ controls?.comparison_color_enabled?.value === true,
Review Comment:
Non blocking: Should we make this control also visible only if user has the
`Enable Time Comparison` set as true only? if false then ignore this setting
entirely? SO we don't affect existing vizs?
--
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
