Skip to content

Commit

Permalink
[FIX] *: standardization of color representation
Browse files Browse the repository at this point in the history
We handle colors as 'css-valid' representations, as such, several
repesentations are equivalent but cannot be compared by strict equality,
they need to be converted to the same representation. This commit
aims to unify to the uppercase hex representation as it follows the
representation yielded by the helper `toHex` which (at this point in
time) seems to be the 'rallying' point when manipulating colors.

Task: 3193882
Part-of: #2300
  • Loading branch information
rrahir committed Apr 4, 2023
1 parent 4289c2a commit bec6ce8
Show file tree
Hide file tree
Showing 14 changed files with 200 additions and 162 deletions.
6 changes: 5 additions & 1 deletion src/components/color_picker/color_picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
MENU_SEPARATOR_BORDER_WIDTH,
MENU_SEPARATOR_PADDING,
} from "../../constants";
import { hslaToRGBA, isColorValid, rgbaToHex, toHex } from "../../helpers";
import { hslaToRGBA, isColorValid, isSameColor, rgbaToHex, toHex } from "../../helpers";
import { chartFontColor } from "../../helpers/figures/charts";
import { Color, Pixel } from "../../types";
import { SpreadsheetChildEnv } from "../../types/env";
Expand Down Expand Up @@ -281,6 +281,10 @@ export class ColorPicker extends Component<ColorPickerProps, SpreadsheetChildEnv
display === "block" ? `background-color:${background};left:${left};top:${top};` : ""
}`;
}

isSameColor(color1: Color, color2: Color): boolean {
return isSameColor(color1, color2);
}
}

ColorPicker.props = {
Expand Down
4 changes: 2 additions & 2 deletions src/components/color_picker/color_picker.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
t-on-click="() => this.onColorClick(color)"
t-attf-style="background-color:{{color}};">
<div
t-if="props.currentColor === color"
t-if="isSameColor(props.currentColor, color)"
align="center"
t-attf-style="color:{{getCheckMarkColor()}}">
&#10004;
Expand All @@ -42,7 +42,7 @@
t-attf-style="background-color:{{color}};"
t-on-click="() => this.onColorClick(color)">
<div
t-if="props.currentColor === color"
t-if="isSameColor(props.currentColor, color)"
align="center"
t-attf-style="color:{{getCheckMarkColor()}}">
&#10004;
Expand Down
146 changes: 73 additions & 73 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,88 +22,88 @@ export const FILTERS_COLOR = "#188038";
export const BACKGROUND_HEADER_FILTER_COLOR = "#E6F4EA";
export const BACKGROUND_HEADER_SELECTED_FILTER_COLOR = "#CEEAD6";

// Color picker
// Color picker defaults as upper case HEX to match `toHex`helper
export const COLOR_PICKER_DEFAULTS = [
"#000000",
"#434343",
"#666666",
"#999999",
"#b7b7b7",
"#cccccc",
"#d9d9d9",
"#efefef",
"#f3f3f3",
"#ffffff",
"#B7B7B7",
"#CCCCCC",
"#D9D9D9",
"#EFEFEF",
"#F3F3F3",
"#FFFFFF",
"#980000",
"#ff0000",
"#ff9900",
"#ffff00",
"#00ff00",
"#00ffff",
"#4a86e8",
"#0000ff",
"#9900ff",
"#ff00ff",
"#e6b8af",
"#f4cccc",
"#fce5cd",
"#fff2cc",
"#d9ead3",
"#d0e0e3",
"#c9daf8",
"#cfe2f3",
"#d9d2e9",
"#ead1dc",
"#dd7e6b",
"#ea9999",
"#f9cb9c",
"#ffe599",
"#b6d7a8",
"#a2c4c9",
"#a4c2f4",
"#9fc5e8",
"#b4a7d6",
"#d5a6bd",
"#cc4125",
"#e06666",
"#f6b26b",
"#ffd966",
"#93c47d",
"#76a5af",
"#6d9eeb",
"#6fa8dc",
"#8e7cc3",
"#c27ba0",
"#a61c00",
"#cc0000",
"#e69138",
"#f1c232",
"#6aa84f",
"#45818e",
"#3c78d8",
"#3d85c6",
"#674ea7",
"#a64d79",
"#85200c",
"#FF0000",
"#FF9900",
"#FFFF00",
"#00FF00",
"#00FFFF",
"#4A86E8",
"#0000FF",
"#9900FF",
"#FF00FF",
"#E6B8AF",
"#F4CCCC",
"#FCE5CD",
"#FFF2CC",
"#D9EAD3",
"#D0E0E3",
"#C9DAF8",
"#CFE2F3",
"#D9D2E9",
"#EAD1DC",
"#DD7E6B",
"#EA9999",
"#F9CB9C",
"#FFE599",
"#B6D7A8",
"#A2C4C9",
"#A4C2F4",
"#9FC5E8",
"#B4A7D6",
"#D5A6BD",
"#CC4125",
"#E06666",
"#F6B26B",
"#FFD966",
"#93C47D",
"#76A5AF",
"#6D9EEB",
"#6FA8DC",
"#8E7CC3",
"#C27BA0",
"#A61C00",
"#CC0000",
"#E69138",
"#F1C232",
"#6AA84F",
"#45818E",
"#3C78D8",
"#3D85C6",
"#674EA7",
"#A64D79",
"#85200C",
"#990000",
"#b45f06",
"#bf9000",
"#38761d",
"#134f5c",
"#1155cc",
"#0b5394",
"#351c75",
"#741b47",
"#5b0f00",
"#B45F06",
"#BF9000",
"#38761D",
"#134F5C",
"#1155CC",
"#0B5394",
"#351C75",
"#741B47",
"#5B0F00",
"#660000",
"#783f04",
"#7f6000",
"#274e13",
"#0c343d",
"#1c4587",
"#783F04",
"#7F6000",
"#274E13",
"#0C343D",
"#1C4587",
"#073763",
"#20124d",
"#4c1130",
"#20124D",
"#4C1130",
];

// Dimensions
Expand Down
78 changes: 49 additions & 29 deletions src/helpers/color.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { Color, HSLA, RGBA } from "../types";
import { concat } from "./misc";

const RBA_REGEX = /rgba?\(|\s+|\)/gi;
const HEX_MATCH = /^#([A-F\d]{2}){3,4}$/g;

export const colors = [
"#eb6d00",
"#0074d9",
Expand Down Expand Up @@ -28,7 +31,10 @@ export function colorNumberString(color: number): Color {

/**
* Converts any CSS color value to a standardized hex6 value.
* Accepts: hex3, hex6, hex8 and rgb (rgba is not supported)
* Accepts: hex3, hex6, hex8, rgb[1] and rgba[1].
*
* [1] under the form rgb(r, g, b, a?) or rgba(r, g, b, a?)
* with r,g,b ∈ [0, 255] and a ∈ [0, 1]
*
* toHex("#ABC")
* >> "#AABBCC"
Expand All @@ -39,30 +45,31 @@ export function colorNumberString(color: number): Color {
* toHex("rgb(30, 80, 16)")
* >> "#1E5010"
*
* * toHex("rgb(30, 80, 16, 0.5)")
* >> "#1E501080"
*
*/
export function toHex(color: Color): Color {
if (color.includes("rgba")) {
throw new Error(`rgba() conversion currently not supported: ${color}`);
}
if (color.includes("rgb")) {
return rgbToHex6(color);
}
color = color.replace("#", "").toUpperCase();
if (color.length === 3 || color.length === 4) {
color = color.split("").reduce((acc, h) => acc + h + h, "");
let hexColor = color;
if (color.startsWith("rgb")) {
hexColor = rgbaStringToHex(color);
} else {
hexColor = color.replace("#", "").toUpperCase();
if (hexColor.length === 3 || hexColor.length === 4) {
hexColor = hexColor.split("").reduce((acc, h) => acc + h + h, "");
}
hexColor = `#${hexColor}`;
}
if (color.replace(/[a-f0-9]/gi, "") !== "") {
throw new Error("invalid color");
if (!hexColor.match(HEX_MATCH)) {
throw new Error(`invalid color input: ${color}`);
}
return "#" + color;
return hexColor;
}

export function isColorValid(color: Color): boolean {
try {
const { r, g, b, a } = colorToRGBA(color);
return (
isColorValueValid(r) && isColorValueValid(g) && isColorValueValid(b) && isColorValueValid(a)
);
toHex(color);
return true;
} catch (error) {
return false;
}
Expand Down Expand Up @@ -99,19 +106,28 @@ export function relativeLuminance(color: Color): number {
/**
* Convert a CSS rgb color string to a standardized hex6 color value.
*
* rgbToHex6("rgb(30, 80, 16)")
* rgbaStringToHex("rgb(30, 80, 16)")
* >> "#1E5010"
*
* rgbaStringToHex("rgba(30, 80, 16, 0.5)")
* >> "#1E501080"
*
* DOES NOT SUPPORT NON INTEGER RGB VALUES
*/
function rgbToHex6(color: Color): Color {
return (
"#" +
concat(
color
.slice(4, -1)
.split(",")
.map((valueString) => parseInt(valueString, 10).toString(16).padStart(2, "0"))
).toUpperCase()
);
function rgbaStringToHex(color: Color): Color {
const stringVals = color.replace(RBA_REGEX, "").split(",");
let alphaHex: number = 255;
if (stringVals.length !== 3 && stringVals.length !== 4) {
throw new Error("invalid color");
} else if (stringVals.length === 4) {
const alpha = parseFloat(stringVals.pop() || "1");
alphaHex = Math.round((alpha || 1) * 255);
}
const vals = stringVals.map((val) => parseInt(val, 10));
if (alphaHex !== 255) {
vals.push(alphaHex);
}
return "#" + concat(vals.map((value) => value.toString(16).padStart(2, "0"))).toUpperCase();
}

/**
Expand All @@ -131,7 +147,7 @@ export function rgbaToHex(rgba: RGBA): Color {
if (a.length == 1) a = "0" + a;
if (a === "ff") a = "";

return "#" + r + g + b + a;
return ("#" + r + g + b + a).toUpperCase();
}

/**
Expand Down Expand Up @@ -257,3 +273,7 @@ export function rgbaToHSLA(rgba: RGBA): HSLA {

return { a: rgba.a, h, s, l };
}

export function isSameColor(color1: Color, color2: Color): boolean {
return isColorValid(color1) && isColorValid(color2) && toHex(color1) === toHex(color2);
}
6 changes: 2 additions & 4 deletions src/plugins/ui_core_views/custom_colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ export class CustomColorsPlugin extends UIPlugin {
// remove duplicates first to check validity on a reduced
// set of colors, then normalize to HEX and remove duplicates
// again
[...new Set([...usedColors, ...this.customColors])]
.filter(isColorValid)
.map((c) => toHex(c).toLowerCase())
[...new Set([...usedColors, ...this.customColors])].filter(isColorValid).map(toHex)
),
]).filter((color) => !COLOR_PICKER_DEFAULTS.includes(color));
}
Expand Down Expand Up @@ -168,7 +166,7 @@ export class CustomColorsPlugin extends UIPlugin {
}

private tryToAddColor(color: Color) {
const formattedColor = toHex(color).toLowerCase();
const formattedColor = toHex(color);
if (color && !COLOR_PICKER_DEFAULTS.includes(formattedColor)) {
this.customColors.add(formattedColor);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/components/color_picker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("Color Picker buttons", () => {
test("Can pick a standard color", async () => {
const onColorPicked = jest.fn();
await mountColorPicker({ onColorPicked });
await simulateClick("div[data-color='#ff9900']");
await simulateClick("div[data-color='#FF9900']");
expect(onColorPicked).toHaveBeenCalledWith("#FF9900");
});

Expand All @@ -81,8 +81,8 @@ describe("Color Picker buttons", () => {
await simulateClick(".o-gradient");
const inputCodeEl = fixture.querySelector(".o-custom-input-preview input") as HTMLInputElement;
const previewColor = toHex(getElComputedStyle(".o-color-preview", "backgroundColor"));
const inputColorCode = toHex(inputCodeEl.value);
expect(previewColor).toEqual(inputColorCode);
const inputColorCode = inputCodeEl.value;
expect(previewColor).toBeSameColorAs(inputColorCode);
await simulateClick(".o-add-button");
expect(onColorPicked).toHaveBeenCalledWith(inputColorCode);
});
Expand Down Expand Up @@ -141,7 +141,7 @@ describe("Color Picker buttons", () => {

test("initial standard color", async () => {
await mountColorPicker({ currentColor: "#45818e" });
const color = fixture.querySelector("div[data-color='#45818e']") as HTMLElement;
const color = fixture.querySelector("div[data-color='#45818E']") as HTMLElement;
expect(color?.textContent).toBe(" ✔ ");
});

Expand Down
6 changes: 3 additions & 3 deletions tests/components/composer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
tokenColor,
} from "../../src/components/composer/composer/composer";
import { fontSizes } from "../../src/fonts";
import { colors, toHex, toZone } from "../../src/helpers/index";
import { colors, toZone } from "../../src/helpers/index";
import { Model } from "../../src/model";
import { Highlight } from "../../src/types";
import { getClipboardEvent, MockClipboardData } from "../test_helpers/clipboard";
Expand Down Expand Up @@ -1122,8 +1122,8 @@ describe("composer", () => {
const gridComposer = fixture.querySelector(".o-grid-composer")! as HTMLElement;
expect(gridComposer.style.textDecoration).toBe("line-through underline");
expect(gridComposer.style.fontWeight).toBe("bold");
expect(toHex(gridComposer.style.background)).toBe("#0000FF");
expect(toHex(gridComposer.style.color)).toBe("#FF0000");
expect(gridComposer.style.background).toBeSameColorAs("#0000FF");
expect(gridComposer.style.color).toBeSameColorAs("#FF0000");
});
});

Expand Down
Loading

0 comments on commit bec6ce8

Please sign in to comment.