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.

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

feat(color-picker, color-picker-hex-input): Add input auto commit, blur and auto select enhancements. #9701

Merged
merged 22 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6591424
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 11, 2024
5043c9b
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jun 11, 2024
54cd63e
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 12, 2024
69b82eb
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 12, 2024
3336d89
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 12, 2024
d53e0b2
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 12, 2024
2ceb323
fix(color-picker-hex-input): fix color not auto updating
aPreciado88 Jun 17, 2024
20d7630
feat(color-picker,color-picker-hex-input) pull dev
aPreciado88 Jun 21, 2024
d88d363
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jun 21, 2024
c7e73dc
feat(color-picker,color-picker-hex-input): add component enhancements
aPreciado88 Jun 26, 2024
1335003
feat(color-picker,color-picker-hex-input): fix merge conflicts
aPreciado88 Jun 26, 2024
6b16caa
feat(color-picker,color-picker-hex-input): update click handler logic
aPreciado88 Jun 26, 2024
23e4bb7
feat(color-picker,color-picker-hex-input): update input blur logic
aPreciado88 Jun 29, 2024
a425167
feat(color-picker,color-picker-hex-input): update input blur function…
aPreciado88 Jun 29, 2024
70a3c0c
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jul 1, 2024
3019101
feat(color-picker,color-picker-hex-input): update end to end tests
aPreciado88 Jul 2, 2024
eb646aa
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jul 2, 2024
fcf027c
fix(tile): update input blur logic and code clean up
aPreciado88 Jul 5, 2024
a5a6a2a
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jul 8, 2024
f21e7fa
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Jul 15, 2024
15f4822
feat(color-picker, color-picker-hex-input): code clean up
aPreciado88 Jul 19, 2024
0a9afce
Merge branch 'dev' into aPreciado88/9624-color-picker-component-enhan…
aPreciado88 Jul 19, 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
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,66 @@ describe("calcite-color-picker-hex-input", () => {
expect(await input.getProperty("value")).toBe("#fafafafa");
});

it("commits shorthand hex on blur", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Not critical for this PR, but can you look into moving hex and hexa-specific tests under their respective describe blocks?

This test suite needs to be revisited, so we could also create a follow-up issue to restructure/simplify the suite and tackle this there.

const defaultHex = "#b33f33";
const editedHex = "#aabbcc";
const page = await newE2EPage();
await page.setContent(`<calcite-color-picker-hex-input value='${defaultHex}'></calcite-color-picker-hex-input>`);

const input = await page.find(`calcite-color-picker-hex-input`);
await selectText(input);
await page.keyboard.type("ab");
await page.keyboard.press("Tab");
await page.waitForChanges();

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

await selectText(input);
await page.keyboard.type("abc");
await page.keyboard.press("Tab");
await page.waitForChanges();

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

await selectText(input);
await page.keyboard.type("abcd");
await page.keyboard.press("Tab");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(editedHex);
});

it("commits shorthand hexa on blur", async () => {
const defaultHexa = "#b33f33ff";
const editedHexa = "#aabbccdd";
const page = await newE2EPage();
await page.setContent(
`<calcite-color-picker-hex-input alpha-channel value='${defaultHexa}'></calcite-color-picker-hex-input>`,
);

const input = await page.find(`calcite-color-picker-hex-input`);
await selectText(input);
await page.keyboard.type("abc");
await page.keyboard.press("Tab");
await page.waitForChanges();

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

await selectText(input);
await page.keyboard.type("abcd");
await page.keyboard.press("Tab");
await page.waitForChanges();

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

await selectText(input);
await page.keyboard.type("abcde");
await page.keyboard.press("Tab");
await page.waitForChanges();

expect(await input.getProperty("value")).toBe(editedHexa);
});

it("normalizes value when initialized", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker-hex-input value='#f0f'></calcite-color-picker-hex-input>");
Expand Down Expand Up @@ -272,11 +332,10 @@ describe("calcite-color-picker-hex-input", () => {
await selectText(input);
const longhandHexWithExtraChars = "bbbbbbbbc";
await page.keyboard.type(longhandHexWithExtraChars);
await page.keyboard.press("Enter");
driskull marked this conversation as resolved.
Show resolved Hide resolved
await page.waitForChanges();

const hexWithPreviousAlphaCharsPreserved = "#bbbbbbdd";
expect(await input.getProperty("value")).toBe(hexWithPreviousAlphaCharsPreserved);
const hexWithAlphaCharsPreserved = "#bbbbbbbb";
expect(await input.getProperty("value")).toBe(hexWithAlphaCharsPreserved);
});

describe("keyboard interaction", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
hexChar,
hexify,
isLonghandHex,
isShorthandHex,
isValidHex,
normalizeHex,
opacityToAlpha,
Expand Down Expand Up @@ -146,8 +147,10 @@ export class ColorPickerHexInput implements LoadableComponent {
const willClearValue = allowEmpty && !inputValue;
const isLonghand = isLonghandHex(hex);

// ensure modified pasted hex values are committed since we prevent default to remove the # char.
this.onHexInputChange();
if (isShorthandHex(hex, this.alphaChannel)) {
// ensure modified pasted hex values are committed since we prevent default to remove the # char.
this.onHexInputChange();
}

if (willClearValue || (isValidHex(hex) && isLonghand)) {
return;
Expand Down Expand Up @@ -180,6 +183,14 @@ export class ColorPickerHexInput implements LoadableComponent {
allowEmpty && !internalColor ? "" : this.formatOpacityForInternalInput(internalColor);
};

private onOpacityInputFocus = (): void => {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: selection text handlers could be merged into one and reused.

this.opacityInputNode.selectText();
};

private onOpacityInputInput = (): void => {
this.onOpacityInputChange();
};

private onHexInputChange = (): void => {
const nodeValue = this.hexInputNode.value;
let value = nodeValue;
Expand Down Expand Up @@ -210,7 +221,11 @@ export class ColorPickerHexInput implements LoadableComponent {
this.internalSetValue(value, this.value);
};

private onHexInput = (): void => {
private onHexInputFocus = (): void => {
this.hexInputNode.selectText();
};

private onHexInputInput = (): void => {
const hexInputValue = `#${this.hexInputNode.value}`;
const oldValue = this.value;

Expand All @@ -228,7 +243,7 @@ export class ColorPickerHexInput implements LoadableComponent {
const { key } = event;
const composedPath = event.composedPath();

if (key === "Tab" || key === "Enter") {
if ((key === "Tab" && isShorthandHex(value, this.alphaChannel)) || key === "Enter") {
if (composedPath.includes(hexInputNode)) {
this.onHexInputChange();
} else {
Expand Down Expand Up @@ -326,10 +341,11 @@ export class ColorPickerHexInput implements LoadableComponent {
<calcite-input-text
class={CSS.hexInput}
label={messages?.hex || hexLabel}
maxLength={6}
maxLength={this.alphaChannel ? 8 : 6}
onCalciteInputTextChange={this.onHexInputChange}
onCalciteInputTextInput={this.onHexInput}
onCalciteInputTextInput={this.onHexInputInput}
onCalciteInternalInputTextBlur={this.onHexInputBlur}
onCalciteInternalInputTextFocus={this.onHexInputFocus}
onKeyDown={this.onInputKeyDown}
onPaste={this.onHexInputPaste}
prefixText="#"
Expand All @@ -347,8 +363,9 @@ export class ColorPickerHexInput implements LoadableComponent {
min={OPACITY_LIMITS.min}
numberButtonType="none"
numberingSystem={this.numberingSystem}
onCalciteInputNumberChange={this.onOpacityInputChange}
onCalciteInputNumberInput={this.onOpacityInputInput}
onCalciteInternalInputNumberBlur={this.onOpacityInputBlur}
onCalciteInternalInputNumberFocus={this.onOpacityInputFocus}
onKeyDown={this.onInputKeyDown}
ref={this.storeOpacityInputRef}
scale={inputScale}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,14 @@ describe("calcite-color-picker", () => {
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
await selectText(channelInput);
await channelInput.type("254");
await channelInput.press("Enter");
await page.waitForChanges();
expect(changeSpy).toHaveReceivedEventTimes(4);
expect(inputSpy).toHaveReceivedEventTimes(4);
expect(changeSpy).toHaveReceivedEventTimes(6);
expect(inputSpy).toHaveReceivedEventTimes(6);

// change by clicking stored color
await (await page.find(`calcite-color-picker >>> .${CSS.savedColor}`)).click();
expect(changeSpy).toHaveReceivedEventTimes(5);
expect(inputSpy).toHaveReceivedEventTimes(5);
expect(changeSpy).toHaveReceivedEventTimes(7);
expect(inputSpy).toHaveReceivedEventTimes(7);

// change by dragging color field thumb
const mouseDragSteps = 10;
Expand All @@ -222,8 +221,8 @@ describe("calcite-color-picker", () => {
await page.mouse.up();
await page.waitForChanges();

expect(changeSpy).toHaveReceivedEventTimes(6);
expect(inputSpy.length).toBeGreaterThan(6); // input event fires more than once
expect(changeSpy).toHaveReceivedEventTimes(8);
expect(inputSpy.length).toBeGreaterThan(8); // input event fires more than once

// change by dragging hue slider thumb
[hueScopeX, hueScopeY] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueScope}`);
Expand All @@ -235,7 +234,7 @@ describe("calcite-color-picker", () => {
await page.mouse.up();
await page.waitForChanges();

expect(changeSpy).toHaveReceivedEventTimes(7);
expect(changeSpy).toHaveReceivedEventTimes(9);
expect(inputSpy.length).toBeGreaterThan(previousInputEventLength + 1); // input event fires more than once

previousInputEventLength = inputSpy.length;
Expand All @@ -246,10 +245,112 @@ describe("calcite-color-picker", () => {
picker.setProperty("value", "#fff");
await page.waitForChanges();

expect(changeSpy).toHaveReceivedEventTimes(7);
expect(changeSpy).toHaveReceivedEventTimes(9);
expect(inputSpy.length).toBe(previousInputEventLength);
});

it("increments channel's value by 1 when clearing input and pressing ArrowUp. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker></calcite-color-picker>");
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
const currentValue = await channelInput.getProperty("value");

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.keyboard.press("ArrowUp");
await page.waitForChanges();

expect(await channelInput.getProperty("value")).toBe(`${Number(currentValue) + 1}`);
});

it("decrements channel's value by 1 when clearing input and pressing ArrowDown. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker value='#b33f33'></calcite-color-picker>");
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
const currentValue = await channelInput.getProperty("value");

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.keyboard.press("ArrowDown");
await page.waitForChanges();

expect(await channelInput.getProperty("value")).toBe(`${Number(currentValue) - 1}`);
});

it("prevents channel's value from going over its limit when clearing input and pressing ArrowUp. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker value='#ffffff'></calcite-color-picker>");
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.keyboard.press("ArrowUp");
await page.waitForChanges();

expect(await channelInput.getProperty("value")).toBe("255");
});

it("prevents channel's value from being less than 0 when clearing input and pressing ArrowDown. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker></calcite-color-picker>");
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.keyboard.press("ArrowDown");
await page.waitForChanges();

expect(await channelInput.getProperty("value")).toBe("0");
});

it("restores original channel value when input is cleared and blur is triggered. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker></calcite-color-picker>");
const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
const currentValue = await channelInput.getProperty("value");

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.keyboard.press("Tab");
await page.waitForChanges();

expect(await channelInput.getProperty("value")).toBe(currentValue);
});

it("auto commits channel value when typing. Same should apply to other channel inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker></calcite-color-picker>");

const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
const picker = await page.find("calcite-color-picker");
const changeSpy = await picker.spyOnEvent("calciteColorPickerChange");

await selectText(channelInput);
await page.keyboard.type("123");
await page.waitForChanges();

expect(changeSpy).toHaveReceivedEventTimes(3);
expect(await channelInput.getProperty("value")).toBe("123");
});

it("blurs focused input when clicking anywhere within the component. It should apply to all inputs", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-color-picker></calcite-color-picker>");

const channelInput = await page.find(`calcite-color-picker >>> .${CSS.channel}`);
const currentValue = await channelInput.getProperty("value");
const picker = await page.find("calcite-color-picker");
const blurSpy = await picker.spyOnEvent("calciteInternalInputNumberBlur");

await selectText(channelInput);
await page.keyboard.press("Backspace");
await page.mouse.click(0, 0);
await page.waitForChanges();

expect(blurSpy).toHaveReceivedEventTimes(1);
expect(await channelInput.getProperty("value")).toBe(currentValue);
});

it("does not emit on initialization", async () => {
const page = await newProgrammaticE2EPage();

Expand Down
Loading
Loading