Skip to content

Commit

Permalink
[FIX] ColorPicker: Convert custom string to valid HEX color
Browse files Browse the repository at this point in the history
When defining a custom color, a user can input pretty much any string,
the validity of the said string will only be checked when he will try to
add the custom value.
More specifically, a user can input a hex value without its prefix `#`
(e.g. `eeeaaa` instead of `#eeeaaa`). Such value is interpreted in css
but not by the canvas context. Trying to set the `fillColor` to a hex
value without the prefix will simply fail.

```javascript
const canvas = document.querySelector('canvas')
const ctx = canvas.getContext('2d')

ctx.fillColor = "#FFF"
console.log(ctx.fillColor) // "#FFF"

ctx.fillColor = "eeeaaa"
console.log(ctx.fillColor) // "#FFF"

ctx.fillColor = "#eeeaaa"
console.log(ctx.fillColor) // "#eeeaaa"
```

This commit ensures that we convert the user input to a proper HEX
representation before invoking the `onColorPicked` callback.

closes #2214

Task: 3224839
X-original-commit: 0c4f643
Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
rrahir committed Mar 13, 2023
1 parent 8b1620b commit 5da3ae4
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 17 deletions.
8 changes: 5 additions & 3 deletions 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_PADDING,
SEPARATOR_COLOR,
} from "../../constants";
import { hslaToRGBA, isColorValid, rgbaToHex } from "../../helpers";
import { hslaToRGBA, isColorValid, rgbaToHex, toHex } from "../../helpers";
import { chartFontColor } from "../../helpers/figures/charts";
import { Color, Pixel, Rect } from "../../types";
import { SpreadsheetChildEnv } from "../../types/env";
Expand Down Expand Up @@ -222,7 +222,7 @@ export class ColorPicker extends Component<ColorPickerProps, SpreadsheetChildEnv
}
onColorClick(color: Color) {
if (color) {
this.props.onColorPicked(color);
this.props.onColorPicked(toHex(color));
}
}

Expand All @@ -240,8 +240,10 @@ export class ColorPicker extends Component<ColorPickerProps, SpreadsheetChildEnv
this.state.isCurrentColorInvalid = true;
return;
}
const color = toHex(this.state.currentColor);
this.state.isCurrentColorInvalid = false;
this.props.onColorPicked(this.state.currentColor);
this.props.onColorPicked(color);
this.state.currentColor = color;
}

toggleColorPicker() {
Expand Down
4 changes: 2 additions & 2 deletions tests/components/charts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ describe("figures", () => {
sheetId,
definition: {
...model.getters.getChartDefinition(chartId),
baselineColorUp: "#0000ff",
baselineColorUp: "#0000FF",
},
});

Expand All @@ -883,7 +883,7 @@ describe("figures", () => {
sheetId,
definition: {
...model.getters.getChartDefinition(chartId),
baselineColorDown: "#ff0000",
baselineColorDown: "#FF0000",
},
});
});
Expand Down
32 changes: 20 additions & 12 deletions tests/components/color_picker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe("Color Picker buttons", () => {
const onColorPicked = jest.fn();
await mountColorPicker({ onColorPicked });
await simulateClick("div[data-color='#ff9900']");
expect(onColorPicked).toHaveBeenCalledWith("#ff9900");
expect(onColorPicked).toHaveBeenCalledWith("#FF9900");
});

test("Can pick a custom color in the gradient", async () => {
Expand All @@ -72,31 +72,39 @@ describe("Color Picker buttons", () => {
await simulateClick(".o-color-picker-toggler");
await simulateClick(".o-gradient");
const inputCodeEl = fixture.querySelector(".o-custom-input-preview input") as HTMLInputElement;
const previewColor = toHex(
getElComputedStyle(".o-color-preview", "backgroundColor")
).toLowerCase();
const inputColorCode = inputCodeEl.value;
expect(previewColor).toEqual(inputColorCode.toLowerCase());
const previewColor = toHex(getElComputedStyle(".o-color-preview", "backgroundColor"));
const inputColorCode = toHex(inputCodeEl.value);
expect(previewColor).toEqual(inputColorCode);
await simulateClick(".o-add-button");
expect(onColorPicked).toHaveBeenCalledWith(previewColor);
expect(onColorPicked).toHaveBeenCalledWith(inputColorCode);
});

test("Can choose a custom color with the input", async () => {
const onColorPicked = jest.fn();
await mountColorPicker({ onColorPicked });
await simulateClick(".o-color-picker-toggler");
await simulateClick(".o-gradient");
const color = "#12ef78";
setInputValueAndTrigger(".o-custom-input-preview input", "#12ef78", "input");
const color = "#12EF78";
setInputValueAndTrigger(".o-custom-input-preview input", color, "input");
await nextTick();
const previewColor = toHex(
getElComputedStyle(".o-color-preview", "backgroundColor")
).toLowerCase();
const previewColor = toHex(getElComputedStyle(".o-color-preview", "backgroundColor"));
expect(previewColor).toEqual(color);
await simulateClick(".o-add-button");
expect(onColorPicked).toHaveBeenCalledWith(color);
});

test("Color from the input is sanitized", async () => {
const onColorPicked = jest.fn();
await mountColorPicker({ onColorPicked });
await simulateClick(".o-color-picker-toggler");
await simulateClick(".o-gradient");
const color = "12ef78";
setInputValueAndTrigger(".o-custom-input-preview input", color, "input");
await nextTick();
await simulateClick(".o-add-button");
expect(onColorPicked).toHaveBeenCalledWith(toHex(color));
});

test("Cannot input an invalid color code", async () => {
const onColorPicked = jest.fn();
await mountColorPicker({ onColorPicked });
Expand Down

0 comments on commit 5da3ae4

Please sign in to comment.