rusackas commented on code in PR #42053:
URL: https://github.com/apache/superset/pull/42053#discussion_r3732205126
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -16,70 +16,156 @@
* 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 } 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 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;
}
-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 getReverseThemeColorMap = (
+ themeColors: Record<string, string>,
+): Record<string, string> => {
+ const reverseMap: Record<string, string> = {};
+ if (!themeColors) return reverseMap;
+
+ Object.entries(themeColors).forEach(([name, value]) => {
+ if (typeof value === 'string') {
+ reverseMap[value.toLowerCase()] = name;
+ }
+ });
+
+ return reverseMap;
Review Comment:
This looks already resolved, getReverseThemeColorMap normalizes theme colors
through normalizeColorToHex before building the map, and handleChange
normalizes the picked color the same way before the lookup.
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -16,70 +16,209 @@
* 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 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;
}
-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 '';
+
+ if (color.startsWith('#')) {
+ return color.toLowerCase();
+ }
- const hexColor = `#${toHex(r)}${toHex(g)}${toHex(b)}`;
+ const div = document.createElement('div');
+ div.style.color = color;
+ const normalized = div.style.color || '';
- if (a !== undefined && a !== 1) {
- return `${hexColor}${toHex(Math.round(a * 255))}`;
+ 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 hexColor;
+ return color.toLowerCase();
+};
+
+const getReverseThemeColorMap = (
+ themeColors: Record<string, string>,
+): Record<string, string> => {
+ const reverseMap: Record<string, string> = {};
+ if (!themeColors) return reverseMap;
+
+ Object.entries(themeColors).forEach(([name, value]) => {
+ if (typeof value === 'string') {
+ reverseMap[normalizeColorToHex(value)] = name;
+ }
+ });
+
+ return reverseMap;
+};
+
+function toDisplayHex(
+ value: ColorPickerValue | undefined,
+ themeColors: Record<string, string>,
+): 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<string, string> => {
+ if (!theme || typeof theme !== 'object') {
+ return {};
+ }
+
+ if (
+ 'colors' in theme &&
+ typeof theme.colors === 'object' &&
+ theme.colors !== null
+ ) {
+ return theme.colors as Record<string, string>;
+ }
+
+ return theme as unknown as Record<string, string>;
+};
+
export default function ColorPickerControl({
onChange,
value,
+ presets: customPresets,
+ ariaLabel,
+ resolveThemeTokens = false,
...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<Record<string, string>>(
+ () => 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 &&
+ Object.prototype.hasOwnProperty.call(reverseMap, hex)
+ ) {
+ onChange(reverseMap[hex]);
+ return;
}
+
+ onChange(rgb);
Review Comment:
Confirmed, ConditionalFormattingControl passes outputFormat="hex" now, so
this persists a hex string instead of an RGBColor object. Rollback
compatibility should be fine.
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -16,70 +16,209 @@
* 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 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;
}
-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 '';
+
+ if (color.startsWith('#')) {
+ return color.toLowerCase();
+ }
- const hexColor = `#${toHex(r)}${toHex(g)}${toHex(b)}`;
+ const div = document.createElement('div');
+ div.style.color = color;
+ const normalized = div.style.color || '';
- if (a !== undefined && a !== 1) {
- return `${hexColor}${toHex(Math.round(a * 255))}`;
+ 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 hexColor;
+ return color.toLowerCase();
+};
+
+const getReverseThemeColorMap = (
+ themeColors: Record<string, string>,
+): Record<string, string> => {
+ const reverseMap: Record<string, string> = {};
+ if (!themeColors) return reverseMap;
+
+ Object.entries(themeColors).forEach(([name, value]) => {
+ if (typeof value === 'string') {
+ reverseMap[normalizeColorToHex(value)] = name;
Review Comment:
The dedup in getReverseThemeColorMap picks a first-wins token when two
tokens share the same hex, so identity is deterministic now but still not fully
preserved for colliding tokens. Probably fine for now, flagging in case it
matters more in practice than I think.
--
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]
