rusackas commented on PR #42053:
URL: https://github.com/apache/superset/pull/42053#issuecomment-5008684611
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:
<details>
<summary>additions to
<code>packages/superset-ui-core/test/color/utils.test.ts</code></summary>
```ts
// add to the imports
import { rgbaToHex, forceHexAlpha } from '@superset-ui/core';
// add inside describe('color utils', ...)
describe('rgbaToHex', () => {
test('omits the alpha channel for opaque colors', () => {
expect(rgbaToHex({ r: 255, g: 0, b: 0 })).toBe('#ff0000');
expect(rgbaToHex({ r: 255, g: 0, b: 0, a: 1 })).toBe('#ff0000');
});
test('appends the alpha channel for translucent colors', () => {
expect(rgbaToHex({ r: 0, g: 150, b: 0, a: 0.2 })).toBe('#00960033');
expect(rgbaToHex({ r: 0, g: 0, b: 0, a: 0.5 })).toBe('#00000080');
});
test('fully transparent colors keep an explicit 00 alpha', () => {
expect(rgbaToHex({ r: 255, g: 255, b: 255, a: 0 })).toBe('#ffffff00');
});
test('zero-pads single-digit channels', () => {
expect(rgbaToHex({ r: 1, g: 2, b: 3 })).toBe('#010203');
});
test('rounds fractional channel values', () => {
expect(rgbaToHex({ r: 254.6, g: 0.4, b: 0 })).toBe('#ff0000');
});
});
describe('forceHexAlpha', () => {
test('appends 60% alpha to a 6-digit hex string', () => {
expect(forceHexAlpha('#ff0000')).toBe('#ff000099');
});
test('adds the # prefix when missing', () => {
expect(forceHexAlpha('ff0000')).toBe('#ff000099');
});
test('replaces the existing alpha on an 8-digit hex string', () => {
expect(forceHexAlpha('#ff000033')).toBe('#ff000099');
});
test('converts an RGBColor object using 60% alpha', () => {
expect(forceHexAlpha({ r: 255, g: 0, b: 0 })).toBe('#ff000099');
});
test('overrides the alpha of a translucent RGBColor object', () => {
expect(forceHexAlpha({ r: 0, g: 150, b: 0, a: 0.2 })).toBe('#00960099');
});
});
```
</details>
Holler if you want a hand with any of this.
--
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]
