Skip to content

Commit

Permalink
fix(color-picker): maintains correct numbering system when entering i…
Browse files Browse the repository at this point in the history 
…nvalid RGB value (#7327)

**Related Issue:** #5978

## Summary

Entering an RGB channel value greater than 255 or less than 0 clamps the
value so it doesn't become invalid. This caused the numbering system to
revert to the default `latn`.

It is a timing issue with `calcite-input`'s lifecycle methods and it
reproduces outside of `calcite-color-picker`. Here is a demo showing
that using a timeout or requesting an animation frame (kind of) fixes
the issue:

https://codepen.io/benesri/pen/vYQjYJK

The issue isn't reproducible in `calcite-input-number`.
  • Loading branch information
@benelan
benelan authored Jul 19, 2023
1 parent f291719 commit 8d2a3a5
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 101 deletions.
60 changes: 30 additions & 30 deletions ...es/calcite-components/src/components/color-picker-hex-input/color-picker-hex-input.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent("<calcite-color-picker-hex-input allow-empty></calcite-color-picker-hex-input>");

const input = await page.find(`calcite-color-picker-hex-input`);
await input.setProperty("value", null);
input.setProperty("value", null);
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(null);
Expand All @@ -69,7 +69,7 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent("<calcite-color-picker-hex-input></calcite-color-picker-hex-input>");

const input = await page.find(`calcite-color-picker-hex-input`);
await input.setProperty("value", "#abc");
input.setProperty("value", "#abc");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe("#aabbcc");
Expand All @@ -80,7 +80,7 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent("<calcite-color-picker-hex-input alpha-channel></calcite-color-picker-hex-input>");

const input = await page.find(`calcite-color-picker-hex-input`);
await input.setProperty("value", "#abcd");
input.setProperty("value", "#abcd");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe("#aabbccdd");
Expand All @@ -91,7 +91,7 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent("<calcite-color-picker-hex-input></calcite-color-picker-hex-input>");

const input = await page.find(`calcite-color-picker-hex-input`);
await input.setProperty("value", "#fafafa");
input.setProperty("value", "#fafafa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe("#fafafa");
Expand All @@ -102,7 +102,7 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent("<calcite-color-picker-hex-input alpha-channel></calcite-color-picker-hex-input>");

const input = await page.find(`calcite-color-picker-hex-input`);
await input.setProperty("value", "#fafafafa");
input.setProperty("value", "#fafafafa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe("#fafafafa");
Expand Down Expand Up @@ -132,37 +132,37 @@ describe("calcite-color-picker-hex-input", () => {
await page.setContent(`<calcite-color-picker-hex-input value='${hex}'></calcite-color-picker-hex-input>`);
const input = await page.find(`calcite-color-picker-hex-input`);

await input.setProperty("value", null);
input.setProperty("value", null);
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "wrong");
input.setProperty("value", "wrong");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#");
input.setProperty("value", "#");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#a");
input.setProperty("value", "#a");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aa");
input.setProperty("value", "#aa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aaaa");
input.setProperty("value", "#aaaa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aaaaa");
input.setProperty("value", "#aaaaa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);
Expand All @@ -176,37 +176,37 @@ describe("calcite-color-picker-hex-input", () => {
);
const input = await page.find(`calcite-color-picker-hex-input`);

await input.setProperty("value", null);
input.setProperty("value", null);
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "wrong");
input.setProperty("value", "wrong");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#");
input.setProperty("value", "#");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#a");
input.setProperty("value", "#a");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aa");
input.setProperty("value", "#aa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aaaaa");
input.setProperty("value", "#aaaaa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);

await input.setProperty("value", "#aaaaaaa");
input.setProperty("value", "#aaaaaaa");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(hex);
Expand All @@ -219,7 +219,7 @@ describe("calcite-color-picker-hex-input", () => {
const input = await page.find("calcite-color-picker-hex-input");
const spy = await input.spyOnEvent("calciteColorPickerHexInputChange");

await input.setProperty("value", "#abcdef");
input.setProperty("value", "#abcdef");
await page.waitForChanges();

expect(spy).toHaveReceivedEventTimes(0);
Expand Down Expand Up @@ -360,7 +360,7 @@ describe("calcite-color-picker-hex-input", () => {
const initialHex = "#000000";

await input.callMethod("setFocus");
await input.setProperty("value", initialHex);
input.setProperty("value", initialHex);
await page.waitForChanges();

await page.keyboard.press("ArrowUp");
Expand Down Expand Up @@ -404,7 +404,7 @@ describe("calcite-color-picker-hex-input", () => {

it("restores previous value when a nudge key is pressed and no-color is allowed and set", async () => {
const noColorValue = null;
await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();
await input.callMethod("setFocus");
await page.waitForChanges();
Expand All @@ -413,22 +413,22 @@ describe("calcite-color-picker-hex-input", () => {
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(startingHex);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.press("ArrowDown");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(startingHex);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.down("Shift");
await page.keyboard.press("ArrowUp");
await page.keyboard.up("Shift");
expect(await input.getProperty("value")).toBe(startingHex);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.down("Shift");
Expand Down Expand Up @@ -474,7 +474,7 @@ describe("calcite-color-picker-hex-input", () => {
const initialHex = "#000000ff";

await input.callMethod("setFocus");
await input.setProperty("value", initialHex);
input.setProperty("value", initialHex);
await page.waitForChanges();

await page.keyboard.press("ArrowUp");
Expand Down Expand Up @@ -523,7 +523,7 @@ describe("calcite-color-picker-hex-input", () => {

it("restores previous value when a nudge key is pressed and no-color is allowed and set", async () => {
const noColorValue = null;
await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();
await input.callMethod("setFocus");
await page.waitForChanges();
Expand All @@ -532,22 +532,22 @@ describe("calcite-color-picker-hex-input", () => {
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(startingHexa);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.press("ArrowDown");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(startingHexa);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.down("Shift");
await page.keyboard.press("ArrowUp");
await page.keyboard.up("Shift");
expect(await input.getProperty("value")).toBe(startingHexa);

await input.setProperty("value", noColorValue);
input.setProperty("value", noColorValue);
await page.waitForChanges();

await page.keyboard.down("Shift");
Expand Down
16 changes: 7 additions & 9 deletions packages/calcite-components/src/components/color-picker-hex-input/color-picker-hex-input.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ export class ColorPickerHexInput implements LoadableComponent {
if (isValidHex(hex)) {
event.preventDefault();
this.hexInputNode.value = hex.slice(1);
this.hexInputNode.internalSyncChildElValue();
}
};

Expand All @@ -292,14 +291,14 @@ export class ColorPickerHexInput implements LoadableComponent {
//
//--------------------------------------------------------------------------

private hexInputNode: HTMLCalciteInputElement;
private hexInputNode: HTMLCalciteInputTextElement;

/**
* The last valid/selected color. Used as a fallback if an invalid hex code is entered.
*/
@State() internalColor: Color | null = DEFAULT_COLOR;

private opacityInputNode: HTMLCalciteInputElement;
private opacityInputNode: HTMLCalciteInputNumberElement;

private previousNonNullValue: string = this.value;

Expand All @@ -317,13 +316,12 @@ export class ColorPickerHexInput implements LoadableComponent {

return (
<div class={CSS.container}>
<calcite-input
<calcite-input-text
class={CSS.hexInput}
label={messages?.hex || hexLabel}
maxLength={6}
numberingSystem={this.numberingSystem}
onCalciteInputChange={this.onHexInputChange}
onCalciteInternalInputBlur={this.onHexInputBlur}
onCalciteInputTextChange={this.onHexInputChange}
onCalciteInternalInputTextBlur={this.onHexInputBlur}
onKeyDown={this.onInputKeyDown}
onPaste={this.onHexInputPaste}
prefixText="#"
Expand Down Expand Up @@ -414,11 +412,11 @@ export class ColorPickerHexInput implements LoadableComponent {
this.value = oldValue;
}

private storeHexInputRef = (node: HTMLCalciteInputElement): void => {
private storeHexInputRef = (node: HTMLCalciteInputTextElement): void => {
this.hexInputNode = node;
};

private storeOpacityInputRef = (node: HTMLCalciteInputElement): void => {
private storeOpacityInputRef = (node: HTMLCalciteInputNumberElement): void => {
this.opacityInputNode = node;
};

Expand Down
Loading

0 comments on commit 8d2a3a5

Please sign in to comment.