rusackas commented on code in PR #42053:
URL: https://github.com/apache/superset/pull/42053#discussion_r3732204246
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -16,70 +16,151 @@
* 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 {
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;
}
-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, any>,
+): 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;
+};
- const hexColor = `#${toHex(r)}${toHex(g)}${toHex(b)}`;
+function toDisplayHex(
+ value: ColorPickerValue | undefined,
+ themeColors: Record<string, string>,
+): string | undefined {
+ if (!value) return undefined;
- if (a !== undefined && a !== 1) {
- return `${hexColor}${toHex(Math.round(a * 255))}`;
+ if (typeof value === 'string') {
+ if (value in SPECIAL_COLORS) {
+ return rgbaToHex(SPECIAL_COLORS[value as SpecialColorKey]).toLowerCase();
+ }
+ if (themeColors && value in themeColors) {
+ return themeColors[value].toLowerCase();
+ }
+ return value.toLowerCase();
Review Comment:
Fair edge case (a saved token no longer in the active theme), but antd's
picker won't crash on an unresolved string, it just won't render a swatch. Not
sure there's a clean fallback without more context on the desired UX here,
might be worth a follow-up if it actually bites someone.
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts:
##########
@@ -270,19 +274,35 @@ export const getColorFunction = (
if (compareResult === false) return undefined;
const { cutoffValue, extremeValue } = compareResult;
+ if (typeof colorScheme === 'string') {
+ if (isSpecialColor(colorScheme)) {
+ return colorScheme;
+ }
Review Comment:
Valid gap, pivot table's transformProps passed conditional_formatting
straight through unfiltered, so a stray Green/Red token (say, carried over from
switching viz types) would render as a literal CSS color. Added the same filter
Table chart already uses before calling getColorFormatters.
##########
superset-frontend/plugins/plugin-chart-table/src/transformProps.ts:
##########
@@ -737,8 +737,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;
Review Comment:
The column picker for conditional formatting already only offers the
prefixed comparison columns (Main/#/△/%) once time comparison is on, see
processComparisonColumns in controlPanel.tsx, so config.column matches the
transformed keys in passedData. I don't think this is actually reachable.
--
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]
