diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index 68ae08f93e1..2fa14a7997e 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -36,6 +36,7 @@ import type { QueryResponse, TimeFormatter, } from '@superset-ui/core'; +import { type RGBColor } from '@superset-ui/core/components'; import { GenericDataType } from '@apache-superset/core/common'; import { sharedControls, sharedControlComponents } from './shared-controls'; @@ -494,7 +495,7 @@ export type ConditionalFormattingConfig = { targetValueLeft?: number; targetValueRight?: number; column?: string; - colorScheme?: string; + colorScheme?: RGBColor | string; toAllRow?: boolean; toTextColor?: boolean; useGradient?: boolean; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts b/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts index 730d679c894..0b1ff0e6ce6 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts @@ -19,7 +19,7 @@ import memoizeOne from 'memoize-one'; import { isString, isBoolean } from 'lodash-es'; import { isBlank } from '@apache-superset/core/utils'; -import { addAlpha, DataRecord } from '@superset-ui/core'; +import { addAlpha, DataRecord, rgbaToHex } from '@superset-ui/core'; import tinycolor from 'tinycolor2'; import { ColorFormatters, @@ -27,6 +27,7 @@ import { ConditionalFormattingConfig, MultipleValueComparators, ResolvedColorFormatterResult, + ColorSchemeEnum, } from '../types'; export const round = (num: number, precision = 0) => @@ -71,6 +72,9 @@ export const getOpacity = ( ); }; +const isSpecialColor = (value: unknown): value is ColorSchemeEnum => + Object.values(ColorSchemeEnum).includes(value as ColorSchemeEnum); + export const getColorFunction = ( { operator, @@ -270,19 +274,51 @@ export const getColorFunction = ( if (compareResult === false) return undefined; const { cutoffValue, extremeValue } = compareResult; - // If useGradient is explicitly false, return solid color - if (useGradient === false) { + if (typeof colorScheme === 'string') { + if (isSpecialColor(colorScheme)) { + return colorScheme; + } + + if ( + useGradient === false || + (useGradient === undefined && colorScheme.length === 9) + ) { + if (alpha === false) { + return colorScheme.length === 9 + ? colorScheme.slice(0, 7) + : colorScheme; + } + return colorScheme; + } + + const cleanHex = + colorScheme.length === 9 ? colorScheme.slice(0, 7) : colorScheme; + + if (alpha === undefined || alpha) { + return addAlpha( + cleanHex, + getOpacity(value, cutoffValue, extremeValue, minOpacity, maxOpacity), + ); + } return colorScheme; } + // If useGradient is explicitly false, return solid color + if (useGradient === false || useGradient === undefined) { + if (alpha === false) { + return rgbaToHex({ ...colorScheme, a: 1 }); + } + return rgbaToHex(colorScheme); + } + const baseHexColor = rgbaToHex({ ...colorScheme, a: 1 }); // Otherwise apply gradient (default behavior for backward compatibility) if (alpha === undefined || alpha) { return addAlpha( - colorScheme, + baseHexColor, getOpacity(value, cutoffValue, extremeValue, minOpacity, maxOpacity), ); } - return colorScheme; + return baseHexColor; }; }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts index 821a864a546..4e3ab26e4c4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts @@ -952,3 +952,167 @@ test('correct column boolean config', () => { expect(colorFormatters[3].getColorFromValue(true)).toEqual('#FF0000FF'); expect(colorFormatters[3].getColorFromValue(false)).toEqual('#FF0000FF'); }); + +test('should return hex color when colorScheme is an RGB object', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.None, + colorScheme: { r: 255, g: 128, b: 0, a: 1 }, + column: 'name', + }, + strValues, + ); + expect(colorFunction('Diana')).toEqual('#ff8000'); + expect(colorFunction('Carlos')).toEqual('#ff8000'); + expect(colorFunction('Brian')).toEqual('#ff8000'); +}); + +test('should return token name as-is when colorScheme is a string token', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.None, + colorScheme: 'Green', + column: 'name', + }, + strValues, + ); + expect(colorFunction('Diana')).toEqual('Green'); + expect(colorFunction('Carlos')).toEqual('Green'); + expect(colorFunction('Brian')).toEqual('Green'); +}); + +test('should return solid hex color when useGradient is false or true', () => { + const columnConfig = [ + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: { r: 0, g: 47, b: 255, a: 1 }, + column: 'count', + useGradient: false, + }, + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: { r: 255, g: 166, b: 0, a: 1 }, + column: 'count', + useGradient: true, + }, + ]; + const colorFormatters = getColorFormatters(columnConfig, mockData); + expect(colorFormatters.length).toEqual(2); + + // First formatter with useGradient: false should return solid color + expect(colorFormatters[0].column).toEqual('count'); + expect(colorFormatters[0].getColorFromValue(100)).toEqual('#002fff'); + + // Second formatter with useGradient: true should return gradient color + expect(colorFormatters[1].column).toEqual('count'); + expect(colorFormatters[1].getColorFromValue(100)).toEqual('#ffa600FF'); +}); + +test('should return hex color without alpha for GreaterThan operator with RGB colorScheme', () => { + const config = { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: { r: 255, g: 0, b: 0, a: 1 }, + useGradient: true, + }; + + const columnValues = [10, 50, 100]; + + const alpha = false; + const colorFunction = getColorFunction(config, columnValues, alpha); + + expect(colorFunction(100)).toEqual('#ff0000'); +}); + +test('should preserve alpha from colorScheme when useGradient is false', () => { + const config = { + operator: Comparator.None, + colorScheme: { r: 255, g: 0, b: 0, a: 0.5 }, + useGradient: false, + }; + + const colorFunction = getColorFunction(config, [10, 20, 30]); + const result = colorFunction(20); + + expect(result).not.toBe('#ff0000'); + expect(result).not.toBe('rgb(255, 0, 0)'); +}); + +test('should force opaque color when useGradient is false but alpha is explicitly false', () => { + const config = { + operator: Comparator.None, + colorScheme: { r: 255, g: 0, b: 0, a: 0.5 }, + useGradient: false, + }; + + const colorFunction = getColorFunction(config, [10, 20, 30], false); + const result = colorFunction(20); + + expect(result).toBe('#ff0000'); +}); + +test('should return colorScheme as-is when alpha is false and length is 7', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: '#FF0000', + useGradient: false, + column: 'count', + }, + countValues, + false, + ); + + expect(colorFunction(100)).toEqual('#FF0000'); +}); + +test('should preserve alpha when alpha is undefined and colorScheme has 9 chars', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: '#FF000080', + useGradient: false, + column: 'count', + }, + countValues, + ); + + expect(colorFunction(100)).toEqual('#FF000080'); +}); + +test('should preserve alpha when alpha is true and colorScheme has 9 chars', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: '#FF000080', + useGradient: false, + column: 'count', + }, + countValues, + true, + ); + + expect(colorFunction(100)).toEqual('#FF000080'); +}); + +test('should strip alpha channel when alpha is false and colorScheme has 9 chars', () => { + const colorFunction = getColorFunction( + { + operator: Comparator.GreaterThan, + targetValue: 50, + colorScheme: '#FF000080', + useGradient: false, + column: 'count', + }, + countValues, + false, + ); + + expect(colorFunction(100)).toEqual('#FF0000'); + expect(colorFunction(100)).toHaveLength(7); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/color/utils.ts b/superset-frontend/packages/superset-ui-core/src/color/utils.ts index c0f33f9f8c1..58f1aa70742 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/utils.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/utils.ts @@ -17,6 +17,7 @@ * under the License. */ import tinycolor from 'tinycolor2'; +import { type RGBColor } from '@superset-ui/core/components'; const rgbRegex = /^rgb\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)$/; export function getContrastingColor(color: string, thresholds = 186) { @@ -120,3 +121,45 @@ export function rgbToHex(red: number, green: number, blue: number) { return `#${r}${g}${b}`; } + +export function rgbaToHex(rgb: RGBColor): string { + const { r, g, b, a = 1 } = rgb; + const clampChannel = (value: number) => + Math.min(255, Math.max(0, Math.round(value))); + const clampAlpha = (value: number) => Math.min(1, Math.max(0, value)); + const toHex = (value: number) => { + const hex = value.toString(16); + return hex.length === 1 ? `0${hex}` : hex; + }; + const hexColor = `#${toHex(clampChannel(r))}${toHex(clampChannel(g))}${toHex(clampChannel(b))}`; + const clampedAlpha = clampAlpha(a); + if (clampedAlpha !== 1) { + return `${hexColor}${toHex(Math.round(clampedAlpha * 255))}`; + } + return hexColor; +} + +export const forceHexAlpha = (color: string | RGBColor): string => { + if (typeof color === 'object' && color !== null) { + return rgbaToHex({ ...color, a: 0.6 }); + } + + let hex = color.startsWith('#') ? color : `#${color}`; + + // Expand shorthand hex (#rgb, #rgba) to full length before appending or + // replacing the alpha channel, otherwise the result is not a valid 6- or + // 8-digit CSS hex color. + if (hex.length === 4 || hex.length === 5) { + hex = `#${hex + .slice(1) + .split('') + .map(char => char + char) + .join('')}`; + } + + if (hex.length === 9) { + return `${hex.slice(0, -2)}99`; + } + + return `${hex}99`; +}; diff --git a/superset-frontend/packages/superset-ui-core/test/color/utils.test.ts b/superset-frontend/packages/superset-ui-core/test/color/utils.test.ts index e481a1d7e80..f98357abf5a 100644 --- a/superset-frontend/packages/superset-ui-core/test/color/utils.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/color/utils.test.ts @@ -22,6 +22,8 @@ import { addAlpha, hexToRgb, rgbToHex, + rgbaToHex, + forceHexAlpha, } from '@superset-ui/core'; describe('color utils', () => { @@ -106,4 +108,51 @@ describe('color utils', () => { expect(rgbToHex(0, 0, 0)).toBe('#000000'); }); }); + describe('rgbaToHex', () => { + test('omits the alpha channel for opaque colors', () => { + expect(rgbaToHex({ r: 255, g: 0, b: 0 })).toBe('#ff0000'); + expect(rgbaToHex({ r: 255, g: 0, b: 0, a: 1 })).toBe('#ff0000'); + }); + test('appends the alpha channel for translucent colors', () => { + expect(rgbaToHex({ r: 0, g: 150, b: 0, a: 0.2 })).toBe('#00960033'); + expect(rgbaToHex({ r: 0, g: 0, b: 0, a: 0.5 })).toBe('#00000080'); + }); + test('fully transparent colors keep an explicit 00 alpha', () => { + expect(rgbaToHex({ r: 255, g: 255, b: 255, a: 0 })).toBe('#ffffff00'); + }); + test('zero-pads single-digit channels', () => { + expect(rgbaToHex({ r: 1, g: 2, b: 3 })).toBe('#010203'); + }); + test('rounds fractional channel values', () => { + expect(rgbaToHex({ r: 254.6, g: 0.4, b: 0 })).toBe('#ff0000'); + }); + test('clamps out-of-range channel and alpha values', () => { + expect(rgbaToHex({ r: 300, g: -10, b: 0 })).toBe('#ff0000'); + expect(rgbaToHex({ r: 0, g: 0, b: 0, a: 1.5 })).toBe('#000000'); + expect(rgbaToHex({ r: 0, g: 0, b: 0, a: -0.5 })).toBe('#00000000'); + }); + }); + describe('forceHexAlpha', () => { + test('appends 60% alpha to a 6-digit hex string', () => { + expect(forceHexAlpha('#ff0000')).toBe('#ff000099'); + }); + test('adds the # prefix when missing', () => { + expect(forceHexAlpha('ff0000')).toBe('#ff000099'); + }); + test('replaces the existing alpha on an 8-digit hex string', () => { + expect(forceHexAlpha('#ff000033')).toBe('#ff000099'); + }); + test('converts an RGBColor object using 60% alpha', () => { + expect(forceHexAlpha({ r: 255, g: 0, b: 0 })).toBe('#ff000099'); + }); + test('overrides the alpha of a translucent RGBColor object', () => { + expect(forceHexAlpha({ r: 0, g: 150, b: 0, a: 0.2 })).toBe('#00960099'); + }); + test('expands a shorthand 3-digit hex string before adding alpha', () => { + expect(forceHexAlpha('#fff')).toBe('#ffffff99'); + }); + test('expands a shorthand 4-digit hex string before replacing alpha', () => { + expect(forceHexAlpha('#ff03')).toBe('#ffff0099'); + }); + }); }); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx index 3ef4ee27a40..e95bb121242 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx @@ -707,12 +707,8 @@ const config: ControlPanelConfig = { const extraColorChoices = hasTimeComparison ? [ { - value: ColorSchemeEnum.Green, - label: t('Green for increase, red for decrease'), - }, - { - value: ColorSchemeEnum.Red, - label: t('Red for increase, green for decrease'), + label: t('Trend colors'), + colors: [ColorSchemeEnum.Green, ColorSchemeEnum.Red], }, ] : []; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx index 68c34b5f98e..9819281390c 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx @@ -154,12 +154,8 @@ test('extraColorChoices included when time comparison is enabled', () => { expect(result.extraColorChoices).toEqual([ { - value: ColorSchemeEnum.Green, - label: expect.stringContaining('Green for increase'), - }, - { - value: ColorSchemeEnum.Red, - label: expect.stringContaining('Red for increase'), + label: expect.stringContaining('Trend colors'), + colors: [ColorSchemeEnum.Green, ColorSchemeEnum.Red], }, ]); expect(result.columnOptions).not.toEqual( diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts index ab94f125e89..61ed7f7244c 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts @@ -30,7 +30,11 @@ import { TimeFormats, } from '@superset-ui/core'; import { GenericDataType } from '@apache-superset/core/common'; -import { getColorFormatters } from '@superset-ui/chart-controls'; +import { + ColorSchemeEnum, + ConditionalFormattingConfig, + getColorFormatters, +} from '@superset-ui/chart-controls'; import { DateFormatter, PivotTableQueryFormData, QueryData } from '../types'; import buildGroupbyCombinations, { additiveReducerFor, @@ -206,8 +210,17 @@ export default function transformProps(chartProps: ChartProps) { }, {}, ); + // The "Green"/"Red" trend-color tokens are resolved by the Table chart's + // own comparison-aware formatter, which this renderer does not implement. + // Filter them out so a stale config (e.g. carried over from switching viz + // types) doesn't leak the raw token name through as a literal CSS color. + const pivotConditionalFormatting = conditionalFormatting?.filter( + (config: ConditionalFormattingConfig) => + config.colorScheme !== ColorSchemeEnum.Green && + config.colorScheme !== ColorSchemeEnum.Red, + ); const metricColorFormatters = getColorFormatters( - conditionalFormatting, + pivotConditionalFormatting, mainQuery.data, theme, ); diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 65bbc5b3702..1f6d634e597 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -46,6 +46,7 @@ import { BinaryQueryObjectFilterClause, extractTextFromHTML, TimeGranularity, + forceHexAlpha, } from '@superset-ui/core'; import { styled, @@ -1094,7 +1095,7 @@ export default function TableChart( formatter.objectFormatting === ObjectFormattingEnum.CELL_BAR ) { if (generalShowCellBars) - backgroundColorCellBar = formatterResult.slice(0, -2); + backgroundColorCellBar = forceHexAlpha(formatterResult); } else { backgroundColor = formatterResult; valueRangeFlag = false; @@ -1182,7 +1183,7 @@ export default function TableChart( alignPositiveNegative, })}%`}; background-color: ${ - (backgroundColorCellBar && `${backgroundColorCellBar}99`) || + backgroundColorCellBar || cellBackground({ value: value as number, colorPositiveNegative, diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index 5a81e4ff526..50e468195b5 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -757,12 +757,8 @@ const config: ControlPanelConfig = { const extraColorChoices = hasTimeComparison ? [ { - value: ColorSchemeEnum.Green, - label: t('Green for increase, red for decrease'), - }, - { - value: ColorSchemeEnum.Red, - label: t('Red for increase, green for decrease'), + label: t('Trend colors'), + colors: [ColorSchemeEnum.Green, ColorSchemeEnum.Red], }, ] : []; @@ -774,6 +770,7 @@ const config: ControlPanelConfig = { (item: ConditionalFormattingConfig, index, array) => { if ( item.colorScheme && + typeof item.colorScheme === 'string' && !['Green', 'Red'].includes(item.colorScheme) ) { if (item.columnFormatting === undefined) { diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts index e8a15d4a67d..d71eec62135 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts @@ -752,8 +752,15 @@ const transformProps = ( const basicColorFormatters = comparisonColorEnabled && getBasicColorFormatter(baseQuery?.data, columns); const columnColorFormatters = - getColorFormatters(conditionalFormatting, passedData, theme) ?? - defaultColorFormatters; + getColorFormatters( + (conditionalFormatting || []).filter( + (config: ConditionalFormattingConfig) => + config.colorScheme !== ColorSchemeEnum.Green && + config.colorScheme !== ColorSchemeEnum.Red, + ), + passedData, + theme, + ) ?? defaultColorFormatters; const basicColorColumnFormatters = getBasicColorFormatterForColumn( baseQuery?.data, diff --git a/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx index 9c45037683f..dadeaa34fe3 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx +++ b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx @@ -156,12 +156,8 @@ test('extraColorChoices included when time comparison is enabled', () => { expect(result.extraColorChoices).toEqual([ { - value: ColorSchemeEnum.Green, - label: expect.stringContaining('Green for increase'), - }, - { - value: ColorSchemeEnum.Red, - label: expect.stringContaining('Red for increase'), + label: expect.stringContaining('Trend colors'), + colors: [ColorSchemeEnum.Green, ColorSchemeEnum.Red], }, ]); expect(result.columnOptions).not.toEqual( diff --git a/superset-frontend/src/explore/components/controls/ColorPickerControl.test.tsx b/superset-frontend/src/explore/components/controls/ColorPickerControl.test.tsx index d39200da33e..77e7630236c 100644 --- a/superset-frontend/src/explore/components/controls/ColorPickerControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/ColorPickerControl.test.tsx @@ -16,7 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { render, screen, userEvent } from 'spec/helpers/testing-library'; +import { + render, + screen, + userEvent, + waitFor, +} from 'spec/helpers/testing-library'; import { CategoricalScheme, getCategoricalSchemeRegistry, @@ -28,67 +33,208 @@ const defaultProps = { onChange: jest.fn(), }; -// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('ColorPickerControl', () => { - beforeAll(() => { - getCategoricalSchemeRegistry() - .registerValue( - 'test', - new CategoricalScheme({ - id: 'test', - colors: ['#ff0000', '#00ff00', '#0000ff'], - }), - ) - .setDefaultKey('test'); +beforeAll(() => { + getCategoricalSchemeRegistry() + .registerValue( + 'test', + new CategoricalScheme({ + id: 'test', + colors: ['#ff0000', '#00ff00', '#0000ff'], + }), + ) + .setDefaultKey('test'); +}); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('renders a ColorPicker component', () => { + render(); + + // AntD ColorPicker renders a trigger element with class + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); +}); + +test('displays the correct color value', () => { + render(); + + // The color should be displayed as hex #007A87 (uppercase in AntD) + expect(screen.getByText('#007A87')).toBeInTheDocument(); +}); + +test('calls onChange with RGB values when color changes', async () => { + const onChange = jest.fn(); + render(); + + // Open the color picker + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); + + if (colorPickerTrigger) { + await userEvent.click(colorPickerTrigger); + } + + // Note: Testing actual color selection in AntD ColorPicker would require more complex mocking + // as it uses complex internal components. The main functionality is covered by the component itself. +}); + +test('includes preset colors from the categorical scheme', () => { + render(); + + // The component should have access to the preset colors from the registry + // This is tested by ensuring the component renders without errors with the presets + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); +}); + +test('calls onChange with string key "Green" when resolveThemeTokens is true', async () => { + const onChange = jest.fn(); + + render( + , + ); + + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); + await userEvent.click(colorPickerTrigger!); + + await waitFor(() => { + expect( + document.querySelector('.ant-color-picker-presets-color'), + ).toBeInTheDocument(); }); + + const presets = document.querySelectorAll('.ant-color-picker-presets-color'); + const greenPreset = presets[0]; - beforeEach(() => { - jest.clearAllMocks(); + expect(greenPreset).toBeInTheDocument(); + await userEvent.click(greenPreset); + + expect(onChange).toHaveBeenCalledWith('Green'); +}); + +test('calls onChange with RGB object when resolveThemeTokens is false', async () => { + const onChange = jest.fn(); + + render( + , + ); + + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); + await userEvent.click(colorPickerTrigger!); + + await waitFor(() => { + expect( + document.querySelector('.ant-color-picker-presets-color'), + ).toBeInTheDocument(); }); + + const presets = document.querySelectorAll('.ant-color-picker-presets-color'); + const greenPreset = presets[0]; + + expect(greenPreset).toBeInTheDocument(); + await userEvent.click(greenPreset); + + expect(onChange).toHaveBeenCalledWith('#00960033'); +}); - test('renders a ColorPicker component', () => { - render(); +test('resolves colorSuccess theme token correctly when matching color is selected', async () => { + const onChange = jest.fn(); - // AntD ColorPicker renders a trigger element with class - const colorPickerTrigger = document.querySelector( - '.ant-color-picker-trigger', - ); - expect(colorPickerTrigger).toBeInTheDocument(); + jest + .spyOn(require('@apache-superset/core/theme'), 'useTheme') + .mockReturnValue({ + colors: { + colorSuccess: 'rgba(82, 196, 26, 1)', + }, + }); + + render( + , + ); + + const colorPickerTrigger = document.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); + await userEvent.click(colorPickerTrigger!); + + await waitFor(() => { + expect( + document.querySelector('.ant-color-picker-presets-items'), + ).toBeInTheDocument(); }); + + const successPreset = document.querySelector( + '.ant-color-picker-presets-color [style*="82, 196, 26"]', + ) as HTMLElement | null; - test('displays the correct color value', () => { - render(); + expect(successPreset).toBeInTheDocument(); - // The color should be displayed as hex #007A87 (uppercase in AntD) - expect(screen.getByText('#007A87')).toBeInTheDocument(); - }); + await userEvent.click(successPreset!); + + expect(onChange).toHaveBeenCalledWith('colorSuccess'); +}); - test('calls onChange with RGB values when color changes', async () => { - const onChange = jest.fn(); - render(); +test('handles theme with nested colors object', () => { + jest + .spyOn(require('@apache-superset/core/theme'), 'useTheme') + .mockReturnValue({ + colors: { primary: '#007bff' }, + }); - // Open the color picker - const colorPickerTrigger = document.querySelector( - '.ant-color-picker-trigger', - ); - expect(colorPickerTrigger).toBeInTheDocument(); + const { container } = render(); + expect( + container.querySelector('.ant-color-picker-trigger'), + ).toBeInTheDocument(); +}); - if (colorPickerTrigger) { - await userEvent.click(colorPickerTrigger); - } +test('handles theme without colors field', () => { + jest + .spyOn(require('@apache-superset/core/theme'), 'useTheme') + .mockReturnValue({ + primary: '#007bff', + }); - // Note: Testing actual color selection in AntD ColorPicker would require more complex mocking - // as it uses complex internal components. The main functionality is covered by the component itself. - }); + const { container } = render(); + expect( + container.querySelector('.ant-color-picker-trigger'), + ).toBeInTheDocument(); +}); - test('includes preset colors from the categorical scheme', () => { - render(); +test('handles undefined theme gracefully', () => { + jest + .spyOn(require('@apache-superset/core/theme'), 'useTheme') + .mockReturnValue(undefined); - // The component should have access to the preset colors from the registry - // This is tested by ensuring the component renders without errors with the presets - const colorPickerTrigger = document.querySelector( - '.ant-color-picker-trigger', - ); - expect(colorPickerTrigger).toBeInTheDocument(); - }); + expect(() => render()).not.toThrow(); }); diff --git a/superset-frontend/src/explore/components/controls/ColorPickerControl.tsx b/superset-frontend/src/explore/components/controls/ColorPickerControl.tsx index ea16158a5de..cdbb83aeb45 100644 --- a/superset-frontend/src/explore/components/controls/ColorPickerControl.tsx +++ b/superset-frontend/src/explore/components/controls/ColorPickerControl.tsx @@ -16,70 +16,215 @@ * specific language governing permissions and limitations * under the License. */ -import { getCategoricalSchemeRegistry } from '@superset-ui/core'; +import { useMemo } from 'react'; +import { getCategoricalSchemeRegistry, rgbaToHex } from '@superset-ui/core'; +import { t } from '@apache-superset/core/translation'; import { ColorPicker, type RGBColor, type ColorValue, } from '@superset-ui/core/components'; import ControlHeader from '../ControlHeader'; +import { useTheme, type SupersetTheme } from '@apache-superset/core/theme'; + +const SPECIAL_COLORS = { + Red: { r: 150, g: 0, b: 0, a: 0.2 }, + Green: { r: 0, g: 150, b: 0, a: 0.2 }, +} as const; + +type SpecialColorKey = keyof typeof SPECIAL_COLORS; +export type ColorPickerValue = RGBColor | SpecialColorKey | string; +export type ColorOutputFormat = 'hex' | 'rgb'; export interface ColorPickerControlProps { - onChange?: (color: RGBColor) => void; - value?: RGBColor; + onChange?: (color: ColorPickerValue) => void; + value?: ColorPickerValue; name?: string; label?: string; description?: string; renderTrigger?: boolean; hovered?: boolean; warning?: string; + presets?: { label: string; colors: string[] }[]; + ariaLabel?: string; + resolveThemeTokens?: boolean; + outputFormat?: ColorOutputFormat; } -function rgbToHex(rgb: RGBColor): string { - const { r, g, b, a = 1 } = rgb; - const toHex = (value: number) => { - const hex = Math.round(value).toString(16); - return hex.length === 1 ? `0${hex}` : hex; - }; +const normalizeColorToHex = (color: string): string => { + if (!color) return ''; - const hexColor = `#${toHex(r)}${toHex(g)}${toHex(b)}`; - - if (a !== undefined && a !== 1) { - return `${hexColor}${toHex(Math.round(a * 255))}`; + if (color.startsWith('#')) { + return color.toLowerCase(); } - return hexColor; + const div = document.createElement('div'); + div.style.color = color; + const normalized = div.style.color || ''; + + const match = /^rgba?\((\d+),\s+(\d+),\s+(\d+)(?:,\s*([\d.]+))?\)$/.exec( + normalized, + ); + if (match) { + return rgbaToHex({ + r: parseInt(match[1], 10), + g: parseInt(match[2], 10), + b: parseInt(match[3], 10), + a: match[4] !== undefined ? parseFloat(match[4]) : 1, + }).toLowerCase(); + } + + return color.toLowerCase(); +}; + +const getReverseThemeColorMap = ( + themeColors: Record, +): Map => { + const reverseMap = new Map(); + if (!themeColors) return reverseMap; + + Object.entries(themeColors).forEach(([name, value]) => { + if (typeof value === 'string') { + const hex = normalizeColorToHex(value); + if (!reverseMap.has(hex)) { + reverseMap.set(hex, name); + } + } + }); + + return reverseMap; +}; + +function toDisplayHex( + value: ColorPickerValue | undefined, + themeColors: Record, +): string | undefined { + if (!value) return undefined; + + if (typeof value === 'string') { + if (value in SPECIAL_COLORS) { + return rgbaToHex(SPECIAL_COLORS[value as SpecialColorKey]).toLowerCase(); + } + if ( + themeColors && + Object.prototype.hasOwnProperty.call(themeColors, value) + ) { + return themeColors[value as string].toLowerCase(); + } + return value.toLowerCase(); + } + + return rgbaToHex(value).toLowerCase(); } +const extractThemeColors = ( + theme: SupersetTheme | undefined | null, +): Record => { + if (!theme || typeof theme !== 'object') { + return {}; + } + + if ( + 'colors' in theme && + typeof theme.colors === 'object' && + theme.colors !== null + ) { + return theme.colors as Record; + } + + return theme as unknown as Record; +}; + export default function ColorPickerControl({ onChange, value, + presets: customPresets, + ariaLabel, + resolveThemeTokens = false, + outputFormat = 'rgb', ...headerProps }: ColorPickerControlProps) { const categoricalScheme = getCategoricalSchemeRegistry().get(); - const presetColors = categoricalScheme?.colors.slice(0, 9) || []; + const defaultPresets = categoricalScheme?.colors.slice(0, 9) || []; + const theme = useTheme(); + + const themeColors = useMemo>( + () => extractThemeColors(theme), + [theme], + ); + + const reverseMap = useMemo( + () => getReverseThemeColorMap(themeColors), + [themeColors], + ); + + const presets = useMemo(() => { + if (customPresets) { + return customPresets.map(item => ({ + label: item.label, + colors: item.colors.map(color => { + if (color in SPECIAL_COLORS) { + return rgbaToHex( + SPECIAL_COLORS[color as SpecialColorKey], + ).toLowerCase(); + } + if ( + themeColors && + Object.prototype.hasOwnProperty.call(themeColors, color as string) + ) { + return themeColors[color as string].toLowerCase(); + } + return String(color).toLowerCase(); + }), + })); + } + + return [ + { + label: t('Theme colors'), + colors: defaultPresets.map(c => String(c).toLowerCase()), + }, + ]; + }, [customPresets, themeColors, defaultPresets]); const handleChange = (color: ColorValue) => { - if (onChange) { - const rgb = color.toRgb(); - onChange({ - r: rgb.r, - g: rgb.g, - b: rgb.b, - a: rgb.a, - }); + if (!onChange) return; + + const rgb = color.toRgb(); + const hex = rgbaToHex(rgb).toLowerCase(); + + const specialEntry = resolveThemeTokens + ? Object.entries(SPECIAL_COLORS).find( + ([, rgba]) => rgbaToHex(rgba).toLowerCase() === hex, + ) + : undefined; + + if (specialEntry) { + onChange(specialEntry[0] as SpecialColorKey); + return; } + + if (resolveThemeTokens && reverseMap.has(hex)) { + const tokenName = reverseMap.get(hex); + if (tokenName) { + onChange(tokenName); + return; + } + } + if (outputFormat === 'rgb') onChange(rgb); + else onChange(hex); }; - const hexValue = value ? rgbToHex(value) : undefined; + const hexValue = toDisplayHex(value, themeColors); return (
diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.test.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.test.tsx index 7a64b1537d1..774d89e7085 100644 --- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.test.tsx @@ -21,6 +21,7 @@ import { screen, fireEvent, waitFor, + userEvent, } from 'spec/helpers/testing-library'; import { Comparator, ColorSchemeEnum } from '@superset-ui/chart-controls'; import { GenericDataType } from '@apache-superset/core/common'; @@ -51,12 +52,8 @@ const mixColumns = [ const extraColorChoices = [ { - value: ColorSchemeEnum.Green, - label: 'Green for increase, red for decrease', - }, - { - value: ColorSchemeEnum.Red, - label: 'Red for increase, green for decrease', + label: 'Colors', + colors: [ColorSchemeEnum.Green, ColorSchemeEnum.Red], }, ]; @@ -117,7 +114,7 @@ test('renders the correct input fields based on the selected operator', async () }); test('renders None for operator when Green for increase is selected', async () => { - render( + const { container } = render( , ); - // Select the 'Green for increase' color scheme - fireEvent.change(screen.getAllByLabelText(/color scheme/i)[0], { - target: { value: ColorSchemeEnum.Green }, + const colorPickerTrigger = container.querySelector( + '.ant-color-picker-trigger', + ); + expect(colorPickerTrigger).toBeInTheDocument(); + await userEvent.click(colorPickerTrigger!); + + await waitFor(() => { + expect( + document.querySelector('.ant-color-picker-presets-items'), + ).toBeInTheDocument(); }); - fireEvent.click(await screen.findByTitle(/green for increase/i)); + const presets = document.querySelectorAll('.ant-color-picker-presets-color'); + const greenPreset = Array.from(presets).find(preset => { + const inner = preset.querySelector('.ant-color-picker-color-block-inner'); + return ( + inner && inner.getAttribute('style')?.includes('rgba(0, 150, 0, 0.2)') + ); + }); - // Assert that the operator is set to 'None' - expect(screen.getByText(/none/i)).toBeInTheDocument(); + expect(greenPreset).toBeDefined(); + expect(greenPreset).toBeInTheDocument(); + const safeGreenPreset = greenPreset as HTMLElement; + + const innerColorBlock = safeGreenPreset.querySelector( + '.ant-color-picker-color-block-inner', + ); + expect(innerColorBlock).toHaveStyle({ background: 'rgba(0, 150, 0, 0.2)' }); + + expect(safeGreenPreset).toBeInTheDocument(); + await userEvent.click(safeGreenPreset); + + const operatorInput = screen.getByLabelText('Operator'); + expect(operatorInput).toBeInTheDocument(); + + const operatorSelect = operatorInput.closest('.ant-select-content'); + expect(operatorSelect).toBeInTheDocument(); + expect(operatorSelect).toHaveTextContent(/none/i); }); test('displays the correct input fields based on the selected string type operator', async () => { @@ -295,7 +321,7 @@ test('should hide formatting fields when allColumns is empty', async () => { test('should hide formatting fields when color scheme is Green', async () => { render( { expect(screen.queryByText('Formatting object')).not.toBeInTheDocument(); }); }); + +test('should not display tooltip when extraColorChoices is not provided', async () => { + const { container } = render( + , + ); + + const tooltipIcon = container.querySelector('.ant-form-item-tooltip'); + expect(tooltipIcon).not.toBeInTheDocument(); +}); + +test('should display tooltip icon when extraColorChoices is provided', () => { + const { container } = render( + , + ); + + const tooltipIcon = container.querySelector('.ant-form-item-tooltip'); + expect(tooltipIcon).toBeInTheDocument(); + + const questionIcon = tooltipIcon?.querySelector( + '[aria-label="question-circle"]', + ); + expect(questionIcon).toBeInTheDocument(); +}); + +test('should not display tooltip icon when extraColorChoices is empty', () => { + const { container } = render( + , + ); + + const tooltipIcon = container.querySelector('.ant-form-item-tooltip'); + expect(tooltipIcon).not.toBeInTheDocument(); +}); diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx index 5bda7ba898a..a9bab84e31c 100644 --- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx +++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx @@ -44,8 +44,9 @@ import { stringOperatorOptions, booleanOperatorOptions, formattingOptions, - colorSchemeOptions, + colorScheme, } from './constants'; +import ColorPickerControl from '../ColorPickerControl'; const FullWidthInputNumber = styled(InputNumber)` width: 100%; @@ -236,11 +237,11 @@ export const FormattingPopoverContent = ({ config?: ConditionalFormattingConfig; onChange: (config: ConditionalFormattingConfig) => void; columns: { label: string; value: string; dataType: GenericDataType }[]; - extraColorChoices?: { label: string; value: string }[]; + extraColorChoices?: { label: string; colors: string[] }[]; allColumns?: ColumnOption[]; }) => { const [form] = Form.useForm(); - const colorScheme = colorSchemeOptions(); + const colors = colorScheme(); const [showOperatorFields, setShowOperatorFields] = useState( config === undefined || (config?.colorScheme !== ColorSchemeEnum.Green && @@ -319,6 +320,7 @@ export const FormattingPopoverContent = ({ () => allColumns.filter(col => col.dataType === GenericDataType.Numeric), [allColumns], ); + const defaultColorToken = colors[0]?.colors?.[0]; const visibleUseGradient = useMemo( () => @@ -365,6 +367,14 @@ export const FormattingPopoverContent = ({ } }, [column, columns, previousColumnType]); + const trendColorsTooltip = ( +
+
{t('Trend colors are added (for time-based comparison):')}
+
{t('green — increase / red — decrease')}
+
{t('red — increase / green — decrease')}
+
+ ); + return (
0 ? trendColorsTooltip : ''} > -