sadpandajoe commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4366134065


##########
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:
   Picking the ordinary success swatch now stores `Green` instead of 
`colorSuccess`. With `theme.colorSuccess = '#52c41a'`, `toDisplayHex('Green', 
themeColors, true)` returns that same hex, so this match fires for any control 
with `resolveThemeTokens`, whether or not Green/Red presets were supplied. In 
Table conditional formatting (no `extraColorChoices`), choosing the 
`colorSuccess` (or `colorError`) swatch emits `Green`/`Red`, which hides the 
operator and target fields (`showOperatorFields` in `FormattingPopoverContent`) 
and turns an ordinary color rule into a trend rule. Could the special-key match 
be limited to the Green/Red values actually present in the supplied presets, or 
checked after the theme-token reverse lookup? A component test with 
`colorSuccess` set to the SPECIAL_COLORS-resolved hex, clicking the ordinary 
`colorSuccess` swatch with no trend presets and asserting 
`onChange('colorSuccess')`, would pin it.



-- 
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]

Reply via email to