sadpandajoe commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4384348414
##########
superset-frontend/plugins/plugin-chart-echarts/test/Gauge/transformProps.test.ts:
##########
@@ -687,41 +764,121 @@ describe('Min/Max calculation and axis labels', () => {
});
describe('getIntervalBoundsAndColors', () => {
- test('should generate correct interval bounds and colors', () => {
+ test('should generate correct interval bounds and colors from legacy
indices', () => {
const colorFn = CategoricalColorNamespace.getScale(
'supersetColors' as string,
);
- expect(getIntervalBoundsAndColors('', '', colorFn, 0, 10)).toEqual([]);
- expect(getIntervalBoundsAndColors('4, 10', '1, 2', colorFn, 0,
10)).toEqual(
- [
- [0.4, '#1f77b4'],
- [1, '#ff7f0e'],
- ],
- );
expect(
- getIntervalBoundsAndColors('4, 8, 10', '9, 8, 7', colorFn, 0, 10),
+ getIntervalBoundsAndColors('', '', undefined, colorFn, 0, 10),
+ ).toEqual([]);
+ expect(
+ getIntervalBoundsAndColors('4, 10', '1, 2', undefined, colorFn, 0, 10),
+ ).toEqual([
+ [0.4, '#1f77b4'],
+ [1, '#ff7f0e'],
+ ]);
+ expect(
+ getIntervalBoundsAndColors(
+ '4, 8, 10',
+ '9, 8, 7',
+ undefined,
+ colorFn,
+ 0,
+ 10,
+ ),
).toEqual([
[0.4, '#bcbd22'],
[0.8, '#7f7f7f'],
[1, '#e377c2'],
]);
- expect(getIntervalBoundsAndColors('4, 10', '1, 2', colorFn, 2,
10)).toEqual(
- [
- [0.25, '#1f77b4'],
- [1, '#ff7f0e'],
- ],
- );
expect(
- getIntervalBoundsAndColors('-4, 0', '1, 2', colorFn, -10, 0),
+ getIntervalBoundsAndColors('4, 10', '1, 2', undefined, colorFn, 2, 10),
+ ).toEqual([
+ [0.25, '#1f77b4'],
+ [1, '#ff7f0e'],
+ ]);
+ expect(
+ getIntervalBoundsAndColors('-4, 0', '1, 2', undefined, colorFn, -10, 0),
).toEqual([
[0.6, '#1f77b4'],
[1, '#ff7f0e'],
]);
expect(
- getIntervalBoundsAndColors('-4, -2', '1, 2', colorFn, -10, -2),
+ getIntervalBoundsAndColors('-4, -2', '1, 2', undefined, colorFn, -10,
-2),
).toEqual([
[0.75, '#1f77b4'],
[1, '#ff7f0e'],
]);
});
+
+ test('should prefer interval_colors (real hex values) over legacy indices
when present', () => {
+ const colorFn = CategoricalColorNamespace.getScale(
+ 'supersetColors' as string,
+ );
+ expect(
+ getIntervalBoundsAndColors(
+ '4, 10',
+ '1, 2',
+ ['#ff0000', '#00ff00'],
+ colorFn,
+ 0,
+ 10,
+ ),
+ ).toEqual([
+ [0.4, '#ff0000'],
+ [1, '#00ff00'],
+ ]);
+ });
+
+ test('should fall back to the color scheme for bounds missing an explicit
interval_colors entry', () => {
+ const colorFn = CategoricalColorNamespace.getScale(
+ 'supersetColors' as string,
+ );
+ expect(
+ getIntervalBoundsAndColors('4, 8, 10', '', ['#ff0000'], colorFn, 0, 10),
+ ).toEqual([
+ [0.4, '#ff0000'],
+ [0.8, colorFn.colors[1]],
+ [1, colorFn.colors[2]],
+ ]);
+ });
+
+ test('wraps the interval_colors fallback around the scheme like the editor
does', () => {
+ // With more bounds than colors in the scheme, a bound past the end of
+ // `intervalColors` must wrap around the scheme the same way
+ // `IntervalColorsControl`'s own display does (`index %
schemeColors.length`),
+ // rather than indexing straight off the end into `undefined`.
+ const colorFn = new CategoricalColorScale([
+ '#1f77b4',
+ '#ff7f0e',
+ '#2ca02c',
+ ]);
+ expect(
+ getIntervalBoundsAndColors(
+ '4, 6, 8, 10',
+ '',
+ ['#ff0000'],
+ colorFn,
+ 0,
+ 10,
+ ),
+ ).toEqual([
+ [0.4, '#ff0000'],
+ [0.6, '#ff7f0e'],
+ [0.8, '#2ca02c'],
+ [1, '#1f77b4'],
+ ]);
+ });
+
+ test('an empty interval_colors array falls back to legacy indices', () => {
+ const colorFn = CategoricalColorNamespace.getScale(
+ 'supersetColors' as string,
+ );
+ expect(
+ getIntervalBoundsAndColors('4, 10', '1, 2', [], colorFn, 0, 10),
Review Comment:
This test would still pass if the renderer's `intervalColors.length > 0`
guard were removed. Legacy indices `'1, 2'` resolve to `colorFn.colors[0]` and
`colorFn.colors[1]`, which is exactly the positional wrap-around that an empty
`interval_colors` array falls into without the guard. So a saved Gauge whose
indices differ from the positional order (say `'2,1'`) could regress to the
default `interval_colors: []` overriding its saved colors without any test
failing. Could this use non-positional indices such as `'2, 1'` and assert
`[0.4, '#ff7f0e'], [1, '#1f77b4']`, so the legacy path is what actually gets
pinned?
--
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]
