sadpandajoe commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4366863021
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -193,10 +234,12 @@ export default function ColorPickerControl({
const rgb = color.toRgb();
const hex = rgbaToHex(rgb).toLowerCase();
+ // Match against the same hex the preset chip and trigger display, so a
+ // special color picked from a theme-resolved chip maps back to its key.
const specialEntry = resolveThemeTokens
- ? Object.entries(SPECIAL_COLORS).find(
- ([, rgba]) => rgbaToHex(rgba).toLowerCase() === hex,
- )
+ ? Object.keys(SPECIAL_COLORS)
+ .map(key => [key, toDisplayHex(key, themeColors, true)] as const)
+ .find(([, displayHex]) => displayHex === hex)
Review Comment:
The preset restriction covers callers that offer no trend presets, but Table
with time comparison passes both the ordinary `colorSuccess`/`colorError`
swatches and `Green`/`Red` (`presets={[...colors, ...extraColorChoices]}`).
Those resolve to the same hex, so picking the ordinary `colorSuccess` swatch
still emits `Green` (the `.find` matches it first) and the operator fields
disappear. Also, with a theme value such as `colorSuccess: 'rgb(82, 196, 26)'`,
`toDisplayHex('Green', ...)` returns the raw rgb string while `hex` is the
normalized `#52c41a`, so choosing the Green preset falls through to
`colorSuccess` instead of `Green`. Could the picker tell which swatch was
chosen (rather than comparing resolved colors) and normalize both sides when it
does compare?
--
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]
