Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: use external color handling #1611

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

refactor: use external color handling #1611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a73c1f2
refactor: use external color handling
haxxmaxx Sep 16, 2024
fed9229
fix: some tests
haxxmaxx Sep 16, 2024
f30b21a
fix: stuff
haxxmaxx Sep 17, 2024
771f819
fix: last test
haxxmaxx Sep 17, 2024
99e6ace
fix: lint
haxxmaxx Sep 18, 2024
49b8f77
fix: remove comments
haxxmaxx Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apis/nucleus/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"@testing-library/react": "^14.3.1",
"extend": "3.0.2",
"node-event-emitter": "0.0.1",
"qlik-chart-modules": "0.77.0",
"react": "18.3.1",
"react-dom": "18.3.1",
"react-test-renderer": "18.3.1",
Expand Down
76 changes: 5 additions & 71 deletions apis/nucleus/src/components/listbox/assets/__tests__/styling.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import getStyling, {
CONTRAST_THRESHOLD,
DEFAULT_SELECTION_COLORS,
getContrast,
getContrastingColor,
getOverridesAsObject,
} from '../styling';
import getStyling, { DEFAULT_SELECTION_COLORS, getOverridesAsObject } from '../styling';

describe('styling', () => {
let theme = {};
Expand Down Expand Up @@ -116,14 +110,14 @@ describe('styling', () => {
expect(styles.search.color).toEqual('object.listBox,content,color');
});
it('search - should get desired color if contrasting enough', () => {
themeApi.getStyle = () => '#999';
themeApi.getStyle = () => '#888888';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the contrast for this was actually pretty bad (< 3:1) which isn't acceptable in any case. Probably due to the incorrect contrast calculations, and the arbitrary threshold, this was considered good enough

const styles = getStyling({ app, themeApi, theme, components: [] });
expect(styles.search.color).toEqual('#999');
expect(styles.search.color).toEqual('#888888');
});
it('search - should get a better contrasting color if not good contrast against white', () => {
themeApi.getStyle = () => '#aaa';
const styles = getStyling({ app, themeApi, theme, components: [] });
expect(styles.search.color).toEqual('#000');
expect(styles.search.color).toEqual('#000000');
});
it('header', () => {
components = [
Expand Down Expand Up @@ -165,7 +159,7 @@ describe('styling', () => {
},
];
const POSSIBLE_COLOR = 'rgb(255, 255, 255)';
const CONTRASTING_TO_POSSIBLE = '#000';
const CONTRASTING_TO_POSSIBLE = '#000000';

themeApi.getStyle = (a, b, c) => (c === 'backgroundColor' ? POSSIBLE_COLOR : `${a},${b},${c}`);
components[0].content.fontColor.color = '#FFFFFF';
Expand Down Expand Up @@ -200,66 +194,6 @@ describe('styling', () => {
});
});

const hasEnoughContrast = (a, b) => getContrast(a, b) > CONTRAST_THRESHOLD;

describe('contrast', () => {
it('should return undefined for unsupported or invalid color(s)', () => {
expect(getContrast('rgb(0,0,0)', 'transparent')).toEqual(undefined);
expect(getContrast('rgb(0,0,0)', 'asdasd')).toEqual(undefined);
expect(getContrast('dsadasd', 'rgb(0,0,0)')).toEqual(undefined);
});

it('should fallback to false when contrast is undefined for unsupported or invalid color(s)', () => {
expect(hasEnoughContrast('#ddd', 'white')).toEqual(false);
expect(hasEnoughContrast('#ccc', 'white')).toEqual(false);
expect(hasEnoughContrast('rgb(0,0,0)', 'transparent')).toEqual(false);
expect(hasEnoughContrast('transparent', 'transparent')).toEqual(false);
expect(hasEnoughContrast('red', 'blue')).toEqual(true);
expect(hasEnoughContrast('misspelled', 'hey hey')).toEqual(false);
expect(hasEnoughContrast('misspelled', 'hey hey')).toEqual(false);
expect(hasEnoughContrast('transparent', 'transparent')).toEqual(false);
expect(hasEnoughContrast(' hsl (0, 0 , 0 , 1.0 )', '#FFF')).toEqual(false);
expect(hasEnoughContrast('hsl(0, 0, 0, 1)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('hsl(0,0,0,1.0)', '#FFF')).toEqual(false);
expect(hasEnoughContrast(undefined, '#FFF')).toEqual(false);
expect(hasEnoughContrast('', '#FFF')).toEqual(false);
});
it('should detect transparent colors and then always return false since we do not know color is behind the transparent color', () => {
expect(hasEnoughContrast(' rgba (0, 0 , 0 , 0.2 )', '#FFF')).toEqual(false);
expect(hasEnoughContrast(' hsla (0, 0 , 0 , 0.2 )', '#FFF')).toEqual(false);
expect(hasEnoughContrast(' rgba (0, 0 , 0 , .2 )', '#FFF')).toEqual(false);
expect(hasEnoughContrast(' hsla (0, 0 , 0 , .2 )', '#FFF')).toEqual(false);
expect(hasEnoughContrast('rgba(0, 0, 0, 0)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('rgba(0,0,0,0)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('hsla(0, 0, 0, 0)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('hsla(0,0,0,0)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('transparent', '#FFF')).toEqual(false);
});
it('should fallback to false for unsupported formats', () => {
expect(hasEnoughContrast('rgb(0 0 0 / 0%', '#FFF')).toEqual(false);
expect(hasEnoughContrast('rgb(0 0 0 / 100%)', '#FFF')).toEqual(false);
expect(hasEnoughContrast('rgb(255 255 255 / 0%)', '#000')).toEqual(false);
expect(hasEnoughContrast('rgb(255 255 255 / 100%)', '#000')).toEqual(false);
});

it('should not detect as transparent colors', () => {
expect(hasEnoughContrast('rgba(0, 0, 0, 1)', '#FFF')).toEqual(true);
expect(hasEnoughContrast('hsla(0,0,0,1.0)', '#FFF')).toEqual(true);
expect(hasEnoughContrast('rgba(0,0,0,1.0)', '#FFF')).toEqual(true);
expect(hasEnoughContrast('red', '#FFF')).toEqual(true);
});
});

describe('get contrasting color', () => {
it('should prefer light color even though contrast is higher for desired color', () => {
const bg = '#474747';
const c1 = getContrast('#000', bg);
const c2 = getContrast('#fff', bg);
expect(c1 > c2).toBeTruthy(); // although it does not make sense when comparing with the eye
expect(getContrastingColor(bg, '#000')).toEqual('#FFF');
});
});

describe('getSelectionColors', () => {
let themeSelectionColorsEnabled;

Expand Down
52 changes: 3 additions & 49 deletions apis/nucleus/src/components/listbox/assets/styling.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import { Color } from '@nebula.js/theme';
import { createColor, getContrastingColor } from 'qlik-chart-modules';
import { resolveBgColor, resolveBgImage } from '../../../utils/style/styling-props';

const LIGHT = '#FFF';
const DARK = '#000';

export const CONTRAST_THRESHOLD = 1.5;
const LIGHT_PREFERRED_THRESHOLD = 3;

export const DEFAULT_SELECTION_COLORS = {
selected: '#00873D',
alternative: '#E4E4E4',
Expand All @@ -15,46 +9,6 @@ export const DEFAULT_SELECTION_COLORS = {
possible: '#FFFFFF',
};

export const getContrast = (desired, background) => {
let contrast = false;

const des = new Color(desired);
const bg = new Color(background);

if (bg.isInvalid() || des.isInvalid() || bg.getAlpha() < 1 || des.getAlpha() < 1) {
return undefined;
}

try {
contrast = Color.getContrast(des, bg);
} catch (err) {
contrast = undefined;
}
return contrast;
};

export function getContrastingColor(backgroundColor, desiredTextColor = undefined, dark = DARK, light = LIGHT) {
const lightColor = new Color(light);
const bg = new Color(backgroundColor);
const des = new Color(desiredTextColor);
if (bg.isInvalid() || des.isInvalid() || bg.getAlpha() < 1 || des.getAlpha() < 1) {
return desiredTextColor;
}

// Always prioritise light color if it gives better contrast than desired color's.
const lightColorContrast = getContrast(lightColor, bg);
const desiredColorContrast = getContrast(des, bg);
const useLightColor = lightColorContrast > desiredColorContrast || lightColorContrast > LIGHT_PREFERRED_THRESHOLD;

let contrastingColor;
if (desiredTextColor && desiredColorContrast > CONTRAST_THRESHOLD && !useLightColor) {
contrastingColor = desiredTextColor;
} else {
contrastingColor = useLightColor ? light : dark;
}
return contrastingColor;
}

const SUPPORTED_COMPONENTS = ['theme', 'selections'];

export function getOverridesAsObject(components = []) {
Expand Down Expand Up @@ -141,9 +95,9 @@ function getSearchColor(getListboxStyle) {
}

function getSearchBGColor(bgCol, getListboxStyle) {
const searchBgColorObj = new Color(getListboxStyle('', 'backgroundColor'));
const searchBgColorObj = createColor(getListboxStyle('', 'backgroundColor'));
searchBgColorObj.setAlpha(0.7);
return searchBgColorObj.isInvalid() ? bgCol : searchBgColorObj.toRGBA();
return searchBgColorObj.isInvalid() ? bgCol : searchBgColorObj.getRGBA();
}

export default function getStyles({
Expand Down
4 changes: 2 additions & 2 deletions apis/nucleus/src/utils/style/__tests__/styling-props.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('Styling property resolver', () => {
jest.spyOn(resolveColor, 'default').mockReturnValue('resolvedColor');
const color = resolveBgColor(bgCompLayout, t);
expect(resolveColor.default).toHaveBeenCalledTimes(0);
expect(color).toBe('rgb(255, 0, 0)');
expect(color).toBe('rgb(255,0,0)');
});

test('should resolve background color by picker', () => {
Expand All @@ -87,7 +87,7 @@ describe('Styling property resolver', () => {

const color = resolveBgColor(bgCompLayout, t);
expect(resolveColor.default).toHaveBeenCalledTimes(0);
expect(color).toBe('rgb(255, 0, 0)');
expect(color).toBe('rgb(255,0,0)');
});

test('should resolve background color by theme', () => {
Expand Down
4 changes: 2 additions & 2 deletions apis/theme/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"private": true,
"main": "src/index.js",
"devDependencies": {
"d3-color": "3.1.0",
"extend": "3.0.2",
"node-event-emitter": "0.0.1"
"node-event-emitter": "0.0.1",
"qlik-chart-modules": "0.77.0"
}
}
7 changes: 3 additions & 4 deletions ...c/contraster/__tests__/contraster.test.js → apis/theme/src/__tests__/contraster.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as chartModules from 'qlik-chart-modules';
import create from '../contraster';
import * as luminanceModule from '../luminance';
import * as contrastModule from '../contrast';

describe('contraster', () => {
let luminanceMock;
Expand All @@ -10,8 +9,8 @@ describe('contraster', () => {
luminanceMock = jest.fn();
contrastMock = jest.fn();

jest.spyOn(luminanceModule, 'default').mockImplementation(luminanceMock);
jest.spyOn(contrastModule, 'default').mockImplementation(contrastMock);
jest.spyOn(chartModules, 'getLuminance').mockImplementation(luminanceMock);
jest.spyOn(chartModules, 'getContrastRatio').mockImplementation(contrastMock);
});

afterEach(() => {
Expand Down
6 changes: 3 additions & 3 deletions apis/theme/src/__tests__/theme.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as EventEmitter from 'node-event-emitter';
import * as chartModules from 'qlik-chart-modules';
import * as setThemeModule from '../set-theme';
import * as paletterResolverFnModule from '../palette-resolver';
import * as styleResolverFnModule from '../style-resolver';
import * as contrasterFnModule from '../contraster/contraster';
import * as luminanceFnModule from '../contraster/luminance';
import * as contrasterFnModule from '../contraster';
import create from '../index';

jest.mock('node-event-emitter');
Expand Down Expand Up @@ -32,7 +32,7 @@ describe('theme', () => {
jest.spyOn(styleResolverFnModule, 'default').mockImplementation(styleResolverFnMock);
styleResolverFnMock.resolveRawTheme = 'raw';
jest.spyOn(contrasterFnModule, 'default').mockImplementation(contrasterFnMock);
jest.spyOn(luminanceFnModule, 'default').mockImplementation(luminanceFnMock);
jest.spyOn(chartModules, 'getLuminance').mockImplementation(luminanceFnMock);
emitter = {
prototype: {
emit: emitterEmitMock,
Expand Down
18 changes: 5 additions & 13 deletions apis/theme/src/color-scale.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { color, rgb } from 'd3-color';
import { createColor, getBlendedColor } from 'qlik-chart-modules';

/**
* Gets this mapping between the scaled value and the color parts
Expand All @@ -10,7 +10,7 @@ function limitFunction(scaledValue, numParts) {
/*
* Color-Scale doesn't calculate exact color blends based of the scaled value. It instead shifts the value inwards to achieve
* a better color representation at the edges. Primarily this is done to allow setting custom limits to where each color begins
* and ends. If a color begins and ends at 1, it should not be visible. The simplest way to achive this is to remove 1 and 0
* and ends. If a color begins and ends at 1, it should not be visible. The simplest way to achieve this is to remove 1 and 0
* from the possible numbers that can be used. Colors that are not equal to 1 or 0 should not be affected.
*/

Expand All @@ -24,21 +24,13 @@ function getLevel(scale, level) {
return Math.min(level || scale.startLevel, scale.colorParts.length - 1);
}

function blend(c1, c2, t) {
const r = Math.floor(c1.r + (c2.r - c1.r) * t);
const g = Math.floor(c1.g + (c2.g - c1.g) * t);
const b = Math.floor(c1.b + (c2.b - c1.b) * t);
const a = Math.floor(c1.opacity + (c2.opacity - c1.opacity) * t);
return rgb(r, g, b, a);
}

class ColorScale {
constructor(nanColor) {
this.colorParts = [];
this.startLevel = 0;
this.max = 1;
this.min = 0;
this.nanColor = color(nanColor);
this.nanColor = createColor(nanColor);
}

/**
Expand All @@ -55,7 +47,7 @@ class ColorScale {
if (!this.colorParts[level]) {
this.colorParts[level] = [];
}
this.colorParts[level].push([color(color1), color(color2)]);
this.colorParts[level].push([createColor(color1), createColor(color2)]);
}

/**
Expand Down Expand Up @@ -86,7 +78,7 @@ class ColorScale {
}

const t = k - f;
const uc = blend(c1, c2, t);
const uc = getBlendedColor(c1, c2, t);
return uc;
}
}
Expand Down
9 changes: 4 additions & 5 deletions apis/theme/src/contraster/contraster.js → apis/theme/src/contraster.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
/* eslint no-cond-assign: 0 */
import luminance from './luminance';
import contrast from './contrast';
import { getContrastRatio, getLuminance } from 'qlik-chart-modules';

const MAX_SIZE = 1000;

export default function colorFn(colors = ['#333333', '#ffffff']) {
let cache = {};
let n = 0;

const luminances = colors.map(luminance);
const luminances = colors.map(getLuminance);

return {
getBestContrastColor(colorString) {
Expand All @@ -17,9 +16,9 @@ export default function colorFn(colors = ['#333333', '#ffffff']) {
cache = {};
n = 0;
}
const L = luminance(colorString);
const L = getLuminance(colorString);

const contrasts = luminances.map((lum) => contrast(L, lum));
const contrasts = luminances.map((lum) => getContrastRatio(L, lum));
const c = colors[contrasts.indexOf(Math.max(...contrasts))];

cache[colorString] = c;
Expand Down
22 changes: 0 additions & 22 deletions apis/theme/src/contraster/__tests__/contrast.test.js
View file
Open in desktop

This file was deleted.

31 changes: 0 additions & 31 deletions apis/theme/src/contraster/__tests__/luminance.test.js
View file
Open in desktop

This file was deleted.

Loading