diff --git a/packages/jaeger-ui-components/src/TracePageHeader/SpanGraph/CanvasSpanGraph.tsx b/packages/jaeger-ui-components/src/TracePageHeader/SpanGraph/CanvasSpanGraph.tsx index e3574b156d7..a0454c80009 100644 --- a/packages/jaeger-ui-components/src/TracePageHeader/SpanGraph/CanvasSpanGraph.tsx +++ b/packages/jaeger-ui-components/src/TracePageHeader/SpanGraph/CanvasSpanGraph.tsx @@ -50,7 +50,7 @@ export class UnthemedCanvasSpanGraph extends React.PureComponent getRgbColorByKey(key); + getColor = (key: string) => getRgbColorByKey(key, this.props.theme); componentDidMount() { this._draw(); diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx index 1f026127e96..8fadcba3532 100644 --- a/packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx +++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx @@ -395,12 +395,13 @@ export class UnthemedVirtualizedTraceView extends React.Component diff --git a/packages/jaeger-ui-components/src/utils/color-generator.test.js b/packages/jaeger-ui-components/src/utils/color-generator.test.js index 65b087cba88..b71eed2644c 100644 --- a/packages/jaeger-ui-components/src/utils/color-generator.test.js +++ b/packages/jaeger-ui-components/src/utils/color-generator.test.js @@ -12,26 +12,45 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { getColorByKey, clear } from './color-generator'; +import { createTheme } from '@grafana/data'; +import { colors } from '@grafana/ui'; + +import { getColorByKey, getFilteredColors, clear } from './color-generator'; + +const colorsToFilter = [...colors]; it('gives the same color for the same key', () => { clear(); - const colorOne = getColorByKey('serviceA'); - const colorTwo = getColorByKey('serviceA'); + const colorOne = getColorByKey('serviceA', createTheme()); + const colorTwo = getColorByKey('serviceA', createTheme()); expect(colorOne).toBe(colorTwo); }); it('gives different colors for each for each key', () => { clear(); - const colorOne = getColorByKey('serviceA'); - const colorTwo = getColorByKey('serviceB'); + const colorOne = getColorByKey('serviceA', createTheme()); + const colorTwo = getColorByKey('serviceB', createTheme()); expect(colorOne).not.toBe(colorTwo); }); it('should not allow red', () => { - clear(); - // when aPAKNMeFcF is hashed it's index is 4 - // which is red, which we disallow because it looks like an error - const colorOne = getColorByKey('aPAKNMeFcF'); - expect(colorOne).not.toBe('#E24D42'); + expect(colorsToFilter.indexOf('#E24D42')).toBe(4); + const filteredColors = getFilteredColors(colorsToFilter, createTheme()); + expect(filteredColors.indexOf('#E24D42')).toBe(-1); +}); + +it('should not allow colors with a contrast ratio < 3 in light mode', () => { + expect(colorsToFilter.indexOf('#7EB26D')).toBe(0); + expect(colorsToFilter.indexOf('#EAB839')).toBe(1); + const filteredColors = getFilteredColors(colorsToFilter, createTheme({ colors: { mode: 'light' } })); + expect(filteredColors.indexOf('#7EB26D')).toBe(-1); + expect(filteredColors.indexOf('#EAB839')).toBe(-1); +}); + +it('should not allow colors with a contrast ratio < 3 in dark mode', () => { + expect(colorsToFilter.indexOf('#890F02')).toBe(11); + expect(colorsToFilter.indexOf('#0A437C')).toBe(12); + const filteredColors = getFilteredColors(colorsToFilter, createTheme({ colors: { mode: 'dark' } })); + expect(filteredColors.indexOf('#890F02')).toBe(-1); + expect(filteredColors.indexOf('#0A437C')).toBe(-1); }); diff --git a/packages/jaeger-ui-components/src/utils/color-generator.tsx b/packages/jaeger-ui-components/src/utils/color-generator.tsx index ab28d407238..2f12451d940 100644 --- a/packages/jaeger-ui-components/src/utils/color-generator.tsx +++ b/packages/jaeger-ui-components/src/utils/color-generator.tsx @@ -13,7 +13,9 @@ // limitations under the License. import memoizeOne from 'memoize-one'; +import tinycolor from 'tinycolor2'; +import { GrafanaTheme2 } from '@grafana/data'; import { colors } from '@grafana/ui'; // TS needs the precise return type @@ -32,9 +34,10 @@ class ColorGenerator { colorsRgb: Array<[number, number, number]>; cache: Map; - constructor(colorsHex: string[]) { - this.colorsHex = colorsHex; - this.colorsRgb = colorsHex.map(strToRgb); + constructor(colorsHex: string[], theme: GrafanaTheme2) { + const filteredColors = getFilteredColors(colorsHex, theme); + this.colorsHex = filteredColors; + this.colorsRgb = filteredColors.map(strToRgb); this.cache = new Map(); } @@ -43,7 +46,6 @@ class ColorGenerator { if (i == null) { const hash = this.hashCode(key.toLowerCase()); const hashIndex = Math.abs(hash % this.colorsHex.length); - // colors[4] is red (which we want to disallow as a span color because it looks like an error) i = hashIndex === 4 ? hashIndex + 1 : hashIndex; this.cache.set(key, i); } @@ -86,18 +88,36 @@ class ColorGenerator { } } -const getGenerator = memoizeOne((colors: string[]) => { - return new ColorGenerator(colors); +const getGenerator = memoizeOne((colors: string[], theme: GrafanaTheme2) => { + return new ColorGenerator(colors, theme); }); -export function clear() { - getGenerator([]); +export function clear(theme: GrafanaTheme2) { + getGenerator([], theme); } -export function getColorByKey(key: string) { - return getGenerator(colors).getColorByKey(key); +export function getColorByKey(key: string, theme: GrafanaTheme2) { + return getGenerator(colors, theme).getColorByKey(key); } -export function getRgbColorByKey(key: string): [number, number, number] { - return getGenerator(colors).getRgbColorByKey(key); +export function getRgbColorByKey(key: string, theme: GrafanaTheme2): [number, number, number] { + return getGenerator(colors, theme).getRgbColorByKey(key); +} + +export function getFilteredColors(colorsHex: string[], theme: GrafanaTheme2) { + // Remove red as a span color because it looks like an error + const redIndex = colorsHex.indexOf('#E24D42'); + if (redIndex > -1) { + colorsHex.splice(redIndex, 1); + } + + // Only add colors that have a contrast ratio >= 3 for the current theme + let filteredColors = []; + for (const color of colorsHex) { + if (tinycolor.readability(theme.colors.background.primary, color) >= 3) { + filteredColors.push(color); + } + } + + return filteredColors; }