SBIN2010 commented on PR #42053: URL: https://github.com/apache/superset/pull/42053#issuecomment-5013105152
> Thanks @SBIN2010, the picker is a nice upgrade over the dropdown. A few things while this is in draft: > > The failing jest shards are real: the new early-return for string `colorScheme` in `getColorFunction` skips the `useGradient === false` branch, so existing saved rules (they're all strings) get gradient alpha applied to what should be solid colors, sometimes fully transparent. I think normalizing the string/rgb value to hex up front and keeping the original flow would sort it. > > I'm also wondering about theme tokens. `getColorFormatters` resolves saved `color*` token names at render time so rules adapt when the theme switches, but the picker resolves tokens to hex when building presets and stores the raw rgb on change. Could `handleChange` map preset hexes back to their token names, the same way Green/Red are special-cased? > > Also curious how a user tells the two "Trend colors" swatches apart now that the "Green for increase, red for decrease" labels are gone. Maybe tooltips on the presets? > > I wrote the unit tests Bito was asking for on `rgbaToHex`/`forceHexAlpha`, but I can't push to your branch, as it turns out. Dropping them here if you want to pull them in: > > additions to `packages/superset-ui-core/test/color/utils.test.ts` > Holler if you want a hand with any of this. Thanks for the tests—I’ve added them! I also incorporated some of the points Copilot raised. I’d appreciate any help with tokens and hints. -- 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]
