From ab2b3d624844187daa561281915d20f999c3719d Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Wed, 6 Nov 2024 19:05:01 +0200 Subject: [PATCH 1/7] feat(ui5-color-picker): add HSL color selection --- packages/base/src/util/ColorConversion.ts | 47 +++--- packages/main/src/ColorPicker.hbs | 67 ++++---- packages/main/src/ColorPicker.ts | 152 ++++++++++++++++-- .../main/src/i18n/messagebundle.properties | 12 ++ packages/main/src/themes/ColorPicker.css | 14 +- packages/main/test/pages/ColorPicker.html | 7 + 6 files changed, 228 insertions(+), 71 deletions(-) diff --git a/packages/base/src/util/ColorConversion.ts b/packages/base/src/util/ColorConversion.ts index c3631b0a8a00..6d2e58c67b36 100644 --- a/packages/base/src/util/ColorConversion.ts +++ b/packages/base/src/util/ColorConversion.ts @@ -241,8 +241,8 @@ const RGBStringToRGBObject = (color: string): ColorRGB => { const HSLToRGB = (color: ColorHSL): ColorRGB => { // Formula taken from https://www.rapidtables.com/convert/color/hsl-to-rgb.html - let saturation = color.s * 100, - lightness = color.l * 100, + let saturation = color.s, + lightness = color.l, red, green, blue; @@ -263,7 +263,7 @@ const HSLToRGB = (color: ColorHSL): ColorRGB => { lightness /= 100; } - const hue = color.h, + const hue = ((color.h % 360) + 360) % 360, d = saturation * (1 - Math.abs(2 * lightness - 1)), m = 255 * (lightness - 0.5 * d), x = d * (1 - Math.abs(((hue / 60) % 2) - 1)), @@ -361,30 +361,35 @@ const RGBToHSL = (color: ColorRGB): ColorHSL => { min = Math.min(R, G, B), delta = max - min; - let h = 0, - s; + let h = (max + min) / 2; + let s = (max + min) / 2; + let l = (max + min) / 2; - // Hue calculation - if (delta === 0) { + if (max === min) { h = 0; - } else if (max === R) { - h = 60 * (((G - B) / delta) % 6); - } else if (max === G) { - h = 60 * (((B - R) / delta) + 2); - } else if (max === B) { - h = 60 * (((R - G) / delta) + 4); - } - - // Lightness calculation - const l = (max + min) / 2; - - // Saturation calculation - if (delta === 0) { s = 0; } else { - s = delta / (1 - Math.abs(2 * l - 1)); + s = l > 0.5 ? delta / (2 - max - min) : delta / (max + min); + + switch (max) { + case R: + h = (G - B) / delta + (G < B ? 6 : 0); + break; + case G: + h = (B - R) / delta + 2; + break; + case B: + h = (R - G) / delta + 4; + break; + } + + h /= 6; } + h = Math.round(h * 360); + s = Math.round(s * 100); + l = Math.round(l * 100); + return { h, s, diff --git a/packages/main/src/ColorPicker.hbs b/packages/main/src/ColorPicker.hbs index 73a256664477..ca45c81e868b 100644 --- a/packages/main/src/ColorPicker.hbs +++ b/packages/main/src/ColorPicker.hbs @@ -57,44 +57,33 @@
-
- - R -
-
- - G -
-
- - B -
-
+ {{#each colorChannelInputs}} +
+ + {{label}} + + {{#if showPercentSymbol}} + % + {{/if}} +
+ {{/each}} + +
A
+ +
+ +
diff --git a/packages/main/src/ColorPicker.ts b/packages/main/src/ColorPicker.ts index e2040b5fa32f..20c1476eba16 100644 --- a/packages/main/src/ColorPicker.ts +++ b/packages/main/src/ColorPicker.ts @@ -22,6 +22,7 @@ import ColorPickerTemplate from "./generated/templates/ColorPickerTemplate.lit.j import Input from "./Input.js"; import Slider from "./Slider.js"; import Label from "./Label.js"; +import Button from "./Button.js"; import { COLORPICKER_ALPHA_SLIDER, @@ -31,6 +32,10 @@ import { COLORPICKER_GREEN, COLORPICKER_BLUE, COLORPICKER_ALPHA, + COLORPICKER_SATURATION, + COLORPICKER_LIGHT, + COLORPICKER_HUE, + COLORPICKER_TOGGLE_MODE_TOOLTIP, } from "./generated/i18n/i18n-defaults.js"; // Styles @@ -43,6 +48,14 @@ type ColorCoordinates = { y: number, } +type ColorChannelInput = { + id: string, + value: number, + accessibleName: string + label: string, + showPercentSymbol?: boolean, +} + /** * @class * @@ -79,6 +92,7 @@ type ColorCoordinates = { Input, Slider, Label, + Button, ], shadowRootOptions: { delegatesFocus: true }, }) @@ -134,6 +148,13 @@ class ColorPicker extends UI5Element implements IFormInputElement { @property({ type: Object }) _value: ColorRGB = getRGBColor(this.value); + /** + * Defines the currenty selected color in HSL. + * @private + */ + @property({ type: Object }) + _valueHSL: ColorHSL = RGBToHSL(this._value); + /** * @private */ @@ -170,6 +191,12 @@ class ColorPicker extends UI5Element implements IFormInputElement { @property({ type: Boolean }) _wrongHEX = false; + /** + * @private + */ + @property({ type: Boolean }) + _displayHSL = false; + selectedHue: number; mouseDown: boolean; @@ -212,6 +239,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { onBeforeRendering() { // we have the color & ._mainValue properties here this._value = getRGBColor(this.value); + this._valueHSL = RGBToHSL(this._value); const tempColor = `rgba(${this._value.r},${this._value.g},${this._value.b},1)`; this._setHex(); this._setValues(); @@ -332,27 +360,50 @@ class ColorPicker extends UI5Element implements IFormInputElement { } } - _handleRGBInputsChange(e: CustomEvent) { + _togglePickerMode() { + this._displayHSL = !this._displayHSL; + } + + _handleColorInputChange(e: CustomEvent) { const target = e.target as Input; const targetValue = parseInt(target.value) || 0; - let tempColor; + const colorValue = this._displayHSL ? this._valueHSL : this._value; + let tempColor: ColorHSL | ColorRGB; + switch (target.id) { case "red": - tempColor = { ...this._value, r: targetValue }; + tempColor = { ...colorValue, r: targetValue }; break; case "green": - tempColor = { ...this._value, g: targetValue }; + tempColor = { ...colorValue, g: targetValue }; break; case "blue": - tempColor = { ...this._value, b: targetValue }; + tempColor = { ...colorValue, b: targetValue }; + break; + + case "hue": + tempColor = { ...colorValue, h: targetValue }; break; + + case "saturation": + tempColor = { ...colorValue, s: targetValue }; + break; + + case "light": + tempColor = { ...colorValue, l: targetValue }; + break; + default: - tempColor = { ...this._value }; + tempColor = { ...colorValue }; + } + + if (this._displayHSL) { + tempColor = HSLToRGB(tempColor as ColorHSL); } - this._setColor(tempColor); + this._setColor(tempColor as ColorRGB); } _setMainColor(hueValue: number) { @@ -440,9 +491,9 @@ class ColorPicker extends UI5Element implements IFormInputElement { } return { - h, - s, - l, + h: Math.round(h), + s: Math.round(s * 100), + l: Math.round(l * 100), }; } @@ -475,10 +526,10 @@ class ColorPicker extends UI5Element implements IFormInputElement { } _setValues() { - const hslColours: ColorHSL = RGBToHSL(this._value); + const hslColours: ColorHSL = this._valueHSL; this._selectedCoordinates = { - x: ((Math.round(hslColours.l * 100) * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the width of the circle - y: (256 - (Math.round(hslColours.s * 100) * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the height of the circle + x: ((hslColours.l * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the width of the circle + y: (256 - (hslColours.s * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the height of the circle }; if (this._isSelectedColorChanged) { // We shouldn't update the hue value when user presses over the main color section. @@ -517,10 +568,26 @@ class ColorPicker extends UI5Element implements IFormInputElement { return ColorPicker.i18nBundle.getText(COLORPICKER_BLUE); } + get hueInputLabel() { + return ColorPicker.i18nBundle.getText(COLORPICKER_HUE); + } + + get saturationInputLabel() { + return ColorPicker.i18nBundle.getText(COLORPICKER_SATURATION); + } + + get lightInputLabel() { + return ColorPicker.i18nBundle.getText(COLORPICKER_LIGHT); + } + get alphaInputLabel() { return ColorPicker.i18nBundle.getText(COLORPICKER_ALPHA); } + get toggleModeTooltip() { + return ColorPicker.i18nBundle.getText(COLORPICKER_TOGGLE_MODE_TOOLTIP); + } + get inputsDisabled() { return this._wrongHEX ? true : undefined; } @@ -529,6 +596,65 @@ class ColorPicker extends UI5Element implements IFormInputElement { return this._wrongHEX ? "Error" : undefined; } + get rgbInputs(): Array { + const redInput = { + id: "red", + value: this._value.r, + label: "R", + accessibleName: this.redInputLabel, + }; + + const greenInput = { + id: "green", + value: this._value.g, + label: "G", + accessibleName: this.blueInputLabel, + }; + + const blueInput = { + id: "blue", + value: this._value.b, + label: "B", + accessibleName: this.blueInputLabel, + }; + + return [redInput, greenInput, blueInput]; + } + + get hslInputs(): Array { + const hueInput = { + id: "hue", + value: this._valueHSL.h, + label: "H", + accessibleName: this.hueInputLabel, + }; + + const saturationInput = { + id: "saturation", + value: this._valueHSL.s, + label: "S", + accessibleName: this.saturationInputLabel, + showPercentSymbol: true, + }; + + const lightInput = { + id: "light", + value: this._valueHSL.l, + label: "L", + accessibleName: this.lightInputLabel, + showPercentSymbol: true, + }; + + return [hueInput, saturationInput, lightInput]; + } + + get colorChannelInputs() { + if (!this._displayHSL) { + return this.rgbInputs; + } + return this.hslInputs; + } + get styles() { return { mainColor: { diff --git a/packages/main/src/i18n/messagebundle.properties b/packages/main/src/i18n/messagebundle.properties index eadb967ade56..0b1f3943af93 100644 --- a/packages/main/src/i18n/messagebundle.properties +++ b/packages/main/src/i18n/messagebundle.properties @@ -139,6 +139,18 @@ COLORPICKER_GREEN=Green #XTOL: Blue color for the ColorPicker control COLORPICKER_BLUE=Blue +#XTOL: Hue value for the ColorPicker control +COLORPICKER_HUE=Hue + +#XTOL: Saturation value for the ColorPicker control +COLORPICKER_SATURATION=Saturation + +#XTOL: Light value for the ColorPicker control +COLORPICKER_LIGHT=Light + +#XTOL: Tooltip for color picker mode toggle button +COLORPICKER_TOGGLE_MODE_TOOLTIP=Change Color Mode + #XTOL: Alpha chanel transparency value for RGBA color mode COLORPICKER_ALPHA=Alpha diff --git a/packages/main/src/themes/ColorPicker.css b/packages/main/src/themes/ColorPicker.css index 5bc5a98ae79b..b7d2603b6cb3 100644 --- a/packages/main/src/themes/ColorPicker.css +++ b/packages/main/src/themes/ColorPicker.css @@ -156,20 +156,28 @@ margin-left: .2rem; } -.ui5-color-picker-rgb-wrapper { +.ui5-color-channel-inputs-wrapper { display: flex; justify-content: space-around; width: 100%; + align-items: center; } -.ui5-color-picker-rgb { +.ui5-color-channel { display: flex; flex-direction: column; align-items: center; margin-top: 1rem; + position: relative; +} + +.ui5-color-channel-percentage-label { + position: absolute; + top: 25%; + left: 100%; } -.ui5-color-picker-rgb-input { +.ui5-color-channel-input { width: 2.5rem; min-width: 2.5rem; text-align: center; diff --git a/packages/main/test/pages/ColorPicker.html b/packages/main/test/pages/ColorPicker.html index 3e2cac1f1e7d..bd36ca0495f5 100644 --- a/packages/main/test/pages/ColorPicker.html +++ b/packages/main/test/pages/ColorPicker.html @@ -45,10 +45,16 @@ +
+
+ + + From 25e94abc08f7d3530b27125adf4acac43ecf845d Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Thu, 7 Nov 2024 13:56:46 +0200 Subject: [PATCH 2/7] feat: add tests --- packages/main/src/ColorPicker.ts | 2 +- packages/main/test/specs/ColorPicker.spec.js | 66 ++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/packages/main/src/ColorPicker.ts b/packages/main/src/ColorPicker.ts index 20c1476eba16..2f55929d24cf 100644 --- a/packages/main/src/ColorPicker.ts +++ b/packages/main/src/ColorPicker.ts @@ -608,7 +608,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { id: "green", value: this._value.g, label: "G", - accessibleName: this.blueInputLabel, + accessibleName: this.greenInputLabel, }; const blueInput = { diff --git a/packages/main/test/specs/ColorPicker.spec.js b/packages/main/test/specs/ColorPicker.spec.js index 7b377031d15a..72ab3c06fd39 100644 --- a/packages/main/test/specs/ColorPicker.spec.js +++ b/packages/main/test/specs/ColorPicker.spec.js @@ -145,6 +145,11 @@ describe("Color Picker general interaction", () => { const blueInput = await colorPicker.shadow$("#blue"); const alphaInput = await colorPicker.shadow$("#alpha"); + const colorPickerHSL = await browser.$("#cp-hsl"); + const hueInput = await colorPickerHSL.shadow$("#hue"); + const saturationInput = await colorPickerHSL.shadow$("#saturation"); + const lightInput = await colorPickerHSL.shadow$("#light"); + assert.strictEqual(await hueSlider.getAttribute("accessible-name"), "Hue control", "Hue slider accessible-name attribute properly set"); assert.strictEqual(await alphaSlider.getAttribute("accessible-name"), "Alpha control", "Alpha slider accessible-name attribute properly set"); assert.strictEqual(await hexInput.getAttribute("accessible-name"), "Hexadecimal", "Hex input accessible-name attribute properly set"); @@ -152,5 +157,66 @@ describe("Color Picker general interaction", () => { assert.strictEqual(await greenInput.getAttribute("accessible-name"), "Green", "Green input accessible-name attribute properly set"); assert.strictEqual(await blueInput.getAttribute("accessible-name"), "Blue", "Blue input accessible-name attribute properly set"); assert.strictEqual(await alphaInput.getAttribute("accessible-name"), "Alpha", "Alpha input accessible-name attribute properly set"); + assert.strictEqual(await hueInput.getAttribute("accessible-name"), "Hue", "Hue input accessible-name attribute properly set"); + assert.strictEqual(await saturationInput.getAttribute("accessible-name"), "Saturation", "Saturation input accessible-name attribute properly set"); + assert.strictEqual(await lightInput.getAttribute("accessible-name"), "Light", "Light input accessible-name attribute properly set"); + }); + + it("Button toggles HSL and RGB inputs", async () => { + const colorPicker = await browser.$("#cp1"); + const toggleColorMode = await colorPicker.shadow$("#toggle-picker-mode"); + + await colorPicker.setAttribute("value", "rgba(112, 178, 225, 1)"); + + await toggleColorMode.click(); + + const hueInput = await colorPicker.shadow$("#hue"); + assert.strictEqual(await hueInput.getProperty("value"), "205", "Button toggled into HSL and Hue value is correctly converted from RGB"); + + const saturationInput = await colorPicker.shadow$("#saturation"); + assert.strictEqual(await saturationInput.getProperty("value"), "65", "Saturation is correctly converted from RGB"); + + const lightInput = await colorPicker.shadow$("#light"); + assert.strictEqual(await lightInput.getProperty("value"), "66", "Light is correctly converted from RGB"); + }); + + it("Hue value change via the input field", async () => { + const colorPicker = await browser.$("#cp-hsl"); + const hueInput = await colorPicker.shadow$("#hue"); + + await colorPicker.setAttribute("value", "rgba(112, 178, 225, 1)"); + + await hueInput.doubleClick(); + await browser.keys("130"); + await browser.keys("Enter"); + + assert.strictEqual(await colorPicker.getProperty("value"), "rgba(112, 225, 131, 1)", "Hue value is parsed correctly"); + }); + + it("Saturation value change via the input field", async () => { + const colorPicker = await browser.$("#cp-hsl"); + const saturationInput = await colorPicker.shadow$("#saturation"); + + await colorPicker.setAttribute("value", "rgba(112, 225, 131, 1)"); + + await saturationInput.doubleClick(); + await browser.keys("44"); + await browser.keys("Enter"); + + assert.strictEqual(await colorPicker.getProperty("value"), "rgba(130, 206, 143, 1)", "Saturation value is parsed correctly"); + }); + + it("Saturation value change via the input field", async () => { + const colorPicker = await browser.$("#cp-hsl"); + const lightInput = await colorPicker.shadow$("#light"); + + await colorPicker.setAttribute("value", "rgba(130, 206, 143, 1)"); + + await lightInput.doubleClick(); + await browser.keys("30"); + await browser.keys("Enter"); + + assert.strictEqual(await colorPicker.getProperty("value"), "rgba(43, 110, 54, 1)", "Light value is parsed correctly"); }); + }); From 29d247fda06c3a1199ae25185e9c529f9371352f Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Thu, 7 Nov 2024 14:51:01 +0200 Subject: [PATCH 3/7] fix: failing test --- packages/main/test/specs/ColorPalette.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/main/test/specs/ColorPalette.spec.js b/packages/main/test/specs/ColorPalette.spec.js index f71da7313b76..e4741c4f9e6b 100644 --- a/packages/main/test/specs/ColorPalette.spec.js +++ b/packages/main/test/specs/ColorPalette.spec.js @@ -87,6 +87,7 @@ describe("ColorPalette interactions", () => { await browser.keys("Tab"); // Green await browser.keys("Tab"); // Blue await browser.keys("Tab"); // Alpha + await browser.keys("Tab"); // Toggle Mode Button await browser.keys("Tab"); // Ok Button await browser.keys("Enter"); // Close the dialog & change the value of the color palette From 2e71d1952d10929051e1058a21e331f1604dc0d4 Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Mon, 11 Nov 2024 14:31:24 +0200 Subject: [PATCH 4/7] fix: align button and inputs better --- packages/main/src/ColorPicker.hbs | 8 +++---- packages/main/src/themes/ColorPicker.css | 22 +++++++++++++------ .../main/src/themes/base/sizes-parameters.css | 6 +++++ packages/main/test/specs/ColorPicker.spec.js | 1 - 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/main/src/ColorPicker.hbs b/packages/main/src/ColorPicker.hbs index ca45c81e868b..420f036b07ad 100644 --- a/packages/main/src/ColorPicker.hbs +++ b/packages/main/src/ColorPicker.hbs @@ -70,11 +70,10 @@ value="{{value}}" > {{label}} - +
+
{{#if showPercentSymbol}} - % + % {{/if}}
{{/each}} @@ -94,6 +93,7 @@
{ assert.strictEqual(await colorPicker.getProperty("value"), "rgba(43, 110, 54, 1)", "Light value is parsed correctly"); }); - }); From b4b1f1de26dc1b7e66cb0b42a1d9ae80b32b38f0 Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Mon, 11 Nov 2024 18:44:30 +0200 Subject: [PATCH 5/7] fix: add icon dependency --- packages/main/src/ColorPicker.hbs | 2 +- packages/main/src/ColorPicker.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/main/src/ColorPicker.hbs b/packages/main/src/ColorPicker.hbs index 420f036b07ad..ac243c237234 100644 --- a/packages/main/src/ColorPicker.hbs +++ b/packages/main/src/ColorPicker.hbs @@ -95,7 +95,7 @@ Date: Mon, 18 Nov 2024 07:54:37 +0200 Subject: [PATCH 6/7] test: convert tests to cypress --- packages/main/cypress/specs/ColorPicker.cy.ts | 98 +++++++++++++++++++ packages/main/cypress/support/commands.ts | 3 + .../support/commands/ColorPicker.commands.ts | 24 +++++ packages/main/test/specs/ColorPicker.spec.js | 65 ------------ 4 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 packages/main/cypress/support/commands/ColorPicker.commands.ts diff --git a/packages/main/cypress/specs/ColorPicker.cy.ts b/packages/main/cypress/specs/ColorPicker.cy.ts index 30805685506c..ca2230a90604 100644 --- a/packages/main/cypress/specs/ColorPicker.cy.ts +++ b/packages/main/cypress/specs/ColorPicker.cy.ts @@ -24,4 +24,102 @@ describe("Color Picker tests", () => { .find("#alpha") .should("not.exist"); }); + + it("should toggle display to RGB or HSL when button is selected", () => { + cy.mount(html``); + + cy.get("ui5-color-picker") + .as("colorPicker"); + + cy.get("@colorPicker") + .ui5ColorPickerToggleColorMode(); + + cy.get("@colorPicker") + .shadow() + .find("#hue") + .should("have.attr", "value", "205"); + + cy.get("@colorPicker") + .shadow() + .find("#saturation") + .should("have.attr", "value", "65"); + + cy.get("@colorPicker") + .shadow() + .find("#light") + .should("have.attr", "value", "66"); + }); + + it("should update value when hue is changed via the input field", () => { + cy.mount(html``); + + cy.get("ui5-color-picker") + .as("colorPicker"); + + cy.get("@colorPicker") + .ui5ColorPickerToggleColorMode(); + + cy.get("@colorPicker") + .ui5ColorPickerUpdateInput("#hue", "130"); + + cy.get("@colorPicker") + .should("have.attr", "value", "rgba(112, 225, 131, 1)"); + }); + + it("should update value when saturation is changed via the input field", () => { + cy.mount(html``); + + cy.get("ui5-color-picker") + .as("colorPicker"); + + cy.get("@colorPicker") + .ui5ColorPickerToggleColorMode(); + + cy.get("@colorPicker") + .ui5ColorPickerUpdateInput("#saturation", "44"); + + cy.get("@colorPicker") + .should("have.attr", "value", "rgba(130, 206, 143, 1)"); + }); + + it("should update value when light is changed via the input field", () => { + cy.mount(html``); + + cy.get("ui5-color-picker") + .as("colorPicker"); + + cy.get("@colorPicker") + .ui5ColorPickerToggleColorMode(); + + cy.get("@colorPicker") + .ui5ColorPickerUpdateInput("#light", "30"); + + cy.get("@colorPicker") + .should("have.attr", "value", "rgba(43, 110, 54, 1)"); + }); + + it("should show correct accessibility info for HSL inputs", () => { + cy.mount(html``); + + cy.get("ui5-color-picker") + .as("colorPicker"); + + cy.get("@colorPicker") + .ui5ColorPickerToggleColorMode(); + + cy.get("@colorPicker") + .shadow() + .find("#hue") + .should("have.attr", "accessible-name", "Hue"); + + cy.get("@colorPicker") + .shadow() + .find("#saturation") + .should("have.attr", "accessible-name", "Saturation"); + + cy.get("@colorPicker") + .shadow() + .find("#light") + .should("have.attr", "accessible-name", "Light"); + }); }); diff --git a/packages/main/cypress/support/commands.ts b/packages/main/cypress/support/commands.ts index 8dfff424cd18..1536e64d97d4 100644 --- a/packages/main/cypress/support/commands.ts +++ b/packages/main/cypress/support/commands.ts @@ -38,6 +38,7 @@ import { internals, isPhone } from "@ui5/webcomponents-base/dist/Device.js"; import "./commands/Menu.commands.js"; +import "./commands/ColorPicker.commands.js"; type SimulationDevices = "phone" @@ -50,6 +51,8 @@ declare global { ui5MenuItemClick(): Chainable ui5DOMRef(): Chainable ui5MenuItemPress(key: any): Chainable + ui5ColorPickerToggleColorMode(): Chainable + ui5ColorPickerUpdateInput(name: string, value: string): Chainable } } } diff --git a/packages/main/cypress/support/commands/ColorPicker.commands.ts b/packages/main/cypress/support/commands/ColorPicker.commands.ts new file mode 100644 index 000000000000..1aa1cab39f4a --- /dev/null +++ b/packages/main/cypress/support/commands/ColorPicker.commands.ts @@ -0,0 +1,24 @@ +Cypress.Commands.add("ui5ColorPickerToggleColorMode", { prevSubject: true }, subject => { + cy.wrap(subject) + .as("colorPicker") + .should("be.visible"); + + cy.get("@colorPicker") + .shadow() + .find("#toggle-picker-mode") + .realClick(); +}); + +Cypress.Commands.add("ui5ColorPickerUpdateInput", { prevSubject: true }, (subject, name, value) => { + cy.wrap(subject) + .as("colorPicker") + .should("be.visible"); + + cy.get("@colorPicker") + .shadow() + .find(name) + .realClick({ clickCount: 2 }) + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + .realType(value) + .realPress("Enter"); +}); diff --git a/packages/main/test/specs/ColorPicker.spec.js b/packages/main/test/specs/ColorPicker.spec.js index da6b6e8ecb35..7b377031d15a 100644 --- a/packages/main/test/specs/ColorPicker.spec.js +++ b/packages/main/test/specs/ColorPicker.spec.js @@ -145,11 +145,6 @@ describe("Color Picker general interaction", () => { const blueInput = await colorPicker.shadow$("#blue"); const alphaInput = await colorPicker.shadow$("#alpha"); - const colorPickerHSL = await browser.$("#cp-hsl"); - const hueInput = await colorPickerHSL.shadow$("#hue"); - const saturationInput = await colorPickerHSL.shadow$("#saturation"); - const lightInput = await colorPickerHSL.shadow$("#light"); - assert.strictEqual(await hueSlider.getAttribute("accessible-name"), "Hue control", "Hue slider accessible-name attribute properly set"); assert.strictEqual(await alphaSlider.getAttribute("accessible-name"), "Alpha control", "Alpha slider accessible-name attribute properly set"); assert.strictEqual(await hexInput.getAttribute("accessible-name"), "Hexadecimal", "Hex input accessible-name attribute properly set"); @@ -157,65 +152,5 @@ describe("Color Picker general interaction", () => { assert.strictEqual(await greenInput.getAttribute("accessible-name"), "Green", "Green input accessible-name attribute properly set"); assert.strictEqual(await blueInput.getAttribute("accessible-name"), "Blue", "Blue input accessible-name attribute properly set"); assert.strictEqual(await alphaInput.getAttribute("accessible-name"), "Alpha", "Alpha input accessible-name attribute properly set"); - assert.strictEqual(await hueInput.getAttribute("accessible-name"), "Hue", "Hue input accessible-name attribute properly set"); - assert.strictEqual(await saturationInput.getAttribute("accessible-name"), "Saturation", "Saturation input accessible-name attribute properly set"); - assert.strictEqual(await lightInput.getAttribute("accessible-name"), "Light", "Light input accessible-name attribute properly set"); - }); - - it("Button toggles HSL and RGB inputs", async () => { - const colorPicker = await browser.$("#cp1"); - const toggleColorMode = await colorPicker.shadow$("#toggle-picker-mode"); - - await colorPicker.setAttribute("value", "rgba(112, 178, 225, 1)"); - - await toggleColorMode.click(); - - const hueInput = await colorPicker.shadow$("#hue"); - assert.strictEqual(await hueInput.getProperty("value"), "205", "Button toggled into HSL and Hue value is correctly converted from RGB"); - - const saturationInput = await colorPicker.shadow$("#saturation"); - assert.strictEqual(await saturationInput.getProperty("value"), "65", "Saturation is correctly converted from RGB"); - - const lightInput = await colorPicker.shadow$("#light"); - assert.strictEqual(await lightInput.getProperty("value"), "66", "Light is correctly converted from RGB"); - }); - - it("Hue value change via the input field", async () => { - const colorPicker = await browser.$("#cp-hsl"); - const hueInput = await colorPicker.shadow$("#hue"); - - await colorPicker.setAttribute("value", "rgba(112, 178, 225, 1)"); - - await hueInput.doubleClick(); - await browser.keys("130"); - await browser.keys("Enter"); - - assert.strictEqual(await colorPicker.getProperty("value"), "rgba(112, 225, 131, 1)", "Hue value is parsed correctly"); - }); - - it("Saturation value change via the input field", async () => { - const colorPicker = await browser.$("#cp-hsl"); - const saturationInput = await colorPicker.shadow$("#saturation"); - - await colorPicker.setAttribute("value", "rgba(112, 225, 131, 1)"); - - await saturationInput.doubleClick(); - await browser.keys("44"); - await browser.keys("Enter"); - - assert.strictEqual(await colorPicker.getProperty("value"), "rgba(130, 206, 143, 1)", "Saturation value is parsed correctly"); - }); - - it("Saturation value change via the input field", async () => { - const colorPicker = await browser.$("#cp-hsl"); - const lightInput = await colorPicker.shadow$("#light"); - - await colorPicker.setAttribute("value", "rgba(130, 206, 143, 1)"); - - await lightInput.doubleClick(); - await browser.keys("30"); - await browser.keys("Enter"); - - assert.strictEqual(await colorPicker.getProperty("value"), "rgba(43, 110, 54, 1)", "Light value is parsed correctly"); }); }); From 59f361fc9d355fbc68300933d2e36fb6e667cd98 Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Mon, 25 Nov 2024 00:25:11 +0200 Subject: [PATCH 7/7] fix: introduce colorValue class to correctly handle color operations --- packages/base/src/util/ColorConversion.ts | 4 +- packages/main/src/ColorPicker.hbs | 4 +- packages/main/src/ColorPicker.ts | 155 ++++++++------------ packages/main/src/ColorValue.ts | 171 ++++++++++++++++++++++ 4 files changed, 234 insertions(+), 100 deletions(-) create mode 100644 packages/main/src/ColorValue.ts diff --git a/packages/base/src/util/ColorConversion.ts b/packages/base/src/util/ColorConversion.ts index 6d2e58c67b36..7964baeef091 100644 --- a/packages/base/src/util/ColorConversion.ts +++ b/packages/base/src/util/ColorConversion.ts @@ -332,8 +332,8 @@ const HEXToRGB = (hex: string): ColorRGB => { * @param {Object} color Receives an object with the properties for each of the main colors(r, g, b) */ const RGBtoHEX = (color: ColorRGB): string => { - const hexMap = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "A", "B", "C", "D", "E", "F"]; - let hexValue = "#"; + const hexMap = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "a", "b", "c", "d", "e", "f"]; + let hexValue = ""; let divisionNumber = color.r / 16; let remainder = color.r % 16; diff --git a/packages/main/src/ColorPicker.hbs b/packages/main/src/ColorPicker.hbs index e95d5af61d65..e2886a44c661 100644 --- a/packages/main/src/ColorPicker.hbs +++ b/packages/main/src/ColorPicker.hbs @@ -49,7 +49,7 @@ Hex diff --git a/packages/main/src/ColorPicker.ts b/packages/main/src/ColorPicker.ts index 7332aad8f462..38e14dda64db 100644 --- a/packages/main/src/ColorPicker.ts +++ b/packages/main/src/ColorPicker.ts @@ -10,9 +10,6 @@ import { getScopedVarName } from "@ui5/webcomponents-base/dist/CustomElementsSco import type { IFormInputElement } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js"; import { getRGBColor, - HSLToRGB, - HEXToRGB, - RGBToHSL, } from "@ui5/webcomponents-base/dist/util/ColorConversion.js"; import type { ColorHSL, @@ -26,6 +23,7 @@ import Label from "./Label.js"; import Button from "./Button.js"; import Icon from "./Icon.js"; import ColorPickerDisplayMode from "./types/ColorPickerDisplayMode.js"; +import ColorValue from "./ColorValue.js"; import { COLORPICKER_ALPHA_SLIDER, @@ -138,15 +136,6 @@ class ColorPicker extends UI5Element implements IFormInputElement { @property() displayMode: `${ColorPickerDisplayMode}` = "Default"; - /** - * Defines the HEX code of the currently selected color - * - * **Note**: If Alpha(transperancy) is set it is not included in this property. Use `color` property. - * @private - */ - @property({ noAttribute: true }) - hex = "ffffff"; - /** * Defines the current main color which is selected via the hue slider and is shown in the main color square. * @private @@ -155,18 +144,11 @@ class ColorPicker extends UI5Element implements IFormInputElement { _mainValue: ColorRGB; /** - * Defines the currenty selected color from the main color section. - * @private - */ - @property({ type: Object }) - _value: ColorRGB = getRGBColor(this.value); - - /** - * Defines the currenty selected color in HSL. + * Defines the currenty selected color. * @private */ - @property({ type: Object }) - _valueHSL: ColorHSL = RGBToHSL(this._value); + @property({ type: Boolean }) + _colorValue: ColorValue; /** * @private @@ -229,6 +211,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { constructor() { super(); + this._colorValue = new ColorValue(); // Bottom Right corner this._selectedCoordinates = { @@ -250,11 +233,11 @@ class ColorPicker extends UI5Element implements IFormInputElement { } onBeforeRendering() { - // we have the color & ._mainValue properties here - this._value = getRGBColor(this.value); - this._valueHSL = RGBToHSL(this._value); - const tempColor = `rgba(${this._value.r},${this._value.g},${this._value.b},1)`; - this._setHex(); + const valueAsRGB = getRGBColor(this.value); + if (!this._isColorValueEqual(valueAsRGB)) { + this._colorValue.RGB = valueAsRGB; + } + const tempColor = this._colorValue.toString(); this._setValues(); this.style.setProperty(getScopedVarName("--ui5_Color_Picker_Progress_Container_Color"), tempColor); } @@ -322,8 +305,9 @@ class ColorPicker extends UI5Element implements IFormInputElement { if (Number.isNaN(this._alpha)) { this._alpha = 1; } + this._colorValue.Alpha = this._alpha; this._isHueValueChanged = true; - this._setColor(this._value); + this._setColor(); } _handleHueInput(e: CustomEvent) { @@ -333,17 +317,14 @@ class ColorPicker extends UI5Element implements IFormInputElement { // Idication that changes to the hue value triggered as a result of user pressing over the hue slider. this._isHueValueChanged = true; - const x: number = this._selectedCoordinates.x + PICKER_POINTER_WIDTH; - const y: number = this._selectedCoordinates.y + PICKER_POINTER_WIDTH; - const tempColor = this._calculateColorFromCoordinates(x, y); + const hue = Math.round(this._hue / 4.251); + const normalizedHue = ((hue % 360) + 360) % 360; - if (tempColor) { - this._setColor(HSLToRGB(tempColor)); - } + this._colorValue.H = normalizedHue; + this._setColor(); } _handleHEXChange(e: CustomEvent | KeyboardEvent) { - const hexRegex = new RegExp("^[<0-9 abcdef]+$"); const input: Input = (e.target as Input); let inputValueLowerCase = input.value.toLowerCase(); @@ -352,24 +333,19 @@ class ColorPicker extends UI5Element implements IFormInputElement { inputValueLowerCase = `${inputValueLowerCase[0]}${inputValueLowerCase[0]}${inputValueLowerCase[1]}${inputValueLowerCase[1]}${inputValueLowerCase[2]}${inputValueLowerCase[2]}`; } - const isNewValueValid = inputValueLowerCase.length === 6 && hexRegex.test(inputValueLowerCase); + this._colorValue.HEX = inputValueLowerCase; + const isValidColor = this._colorValue.isColorValueValid(); - if (isNewValueValid && input.value !== inputValueLowerCase) { + if (isValidColor && input.value !== inputValueLowerCase) { this._wrongHEX = false; input.value = inputValueLowerCase; } - if (inputValueLowerCase === this.hex) { - return; - } - - this.hex = inputValueLowerCase; - - if (!isNewValueValid) { + if (!isValidColor) { this._wrongHEX = true; } else { this._wrongHEX = false; - this._setColor(HEXToRGB(this.hex)); + this._setColor(); } } @@ -380,43 +356,34 @@ class ColorPicker extends UI5Element implements IFormInputElement { _handleColorInputChange(e: CustomEvent) { const target = e.target as Input; const targetValue = parseInt(target.value) || 0; - const colorValue = this._displayHSL ? this._valueHSL : this._value; - let tempColor: ColorHSL | ColorRGB; switch (target.id) { case "red": - tempColor = { ...colorValue, r: targetValue }; + this._colorValue.R = targetValue; break; case "green": - tempColor = { ...colorValue, g: targetValue }; + this._colorValue.G = targetValue; break; case "blue": - tempColor = { ...colorValue, b: targetValue }; + this._colorValue.B = targetValue; break; case "hue": - tempColor = { ...colorValue, h: targetValue }; + this._colorValue.H = targetValue; break; case "saturation": - tempColor = { ...colorValue, s: targetValue }; + this._colorValue.S = targetValue; break; case "light": - tempColor = { ...colorValue, l: targetValue }; + this._colorValue.L = targetValue; break; - - default: - tempColor = { ...colorValue }; - } - - if (this._displayHSL) { - tempColor = HSLToRGB(tempColor as ColorHSL); } - this._setColor(tempColor as ColorRGB); + this._setColor(); } _setMainColor(hueValue: number) { @@ -462,6 +429,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { _handleAlphaChange() { this._alpha = this._alpha < 0 ? 0 : this._alpha; this._alpha = this._alpha > 1 ? 1 : this._alpha; + this._colorValue.Alpha = this._alpha; } _changeSelectedColor(x: number, y: number) { @@ -475,7 +443,8 @@ class ColorPicker extends UI5Element implements IFormInputElement { const tempColor = this._calculateColorFromCoordinates(x, y); if (tempColor) { - this._setColor(HSLToRGB(tempColor)); + this._colorValue.HSL = tempColor; + this._setColor(); } } @@ -498,7 +467,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { // 0 ≤ V ≤ 1 const l = +(Math.round(parseFloat((x / 256) + "e+2")) + "e-2"); // eslint-disable-line - if (!s || !l) { + if (Number.isNaN(s) || Number.isNaN(l)) { // The event is finished out of the main color section return; } @@ -510,36 +479,14 @@ class ColorPicker extends UI5Element implements IFormInputElement { }; } - _setColor(color: ColorRGB = { r: 0, g: 0, b: 0 }) { - this.value = `rgba(${color.r}, ${color.g}, ${color.b}, ${this._alpha})`; - this._wrongHEX = !this.isValidRGBColor(color); + _setColor() { + this.value = this._colorValue.toString(); + this._wrongHEX = !this._colorValue.isColorValueValid(); this.fireDecoratorEvent("change"); } - isValidRGBColor(color: ColorRGB) { - return color.r >= 0 && color.r <= 255 && color.g >= 0 && color.g <= 255 && color.b >= 0 && color.b <= 255; - } - - _setHex() { - let red = this._value.r.toString(16), - green = this._value.g.toString(16), - blue = this._value.b.toString(16); - - if (red.length === 1) { - red = `0${red}`; - } - if (green.length === 1) { - green = `0${green}`; - } - if (blue.length === 1) { - blue = `0${blue}`; - } - - this.hex = red + green + blue; - } - _setValues() { - const hslColours: ColorHSL = this._valueHSL; + const hslColours: ColorHSL = this._colorValue.HSL; this._selectedCoordinates = { x: ((hslColours.l * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the width of the circle y: (256 - (hslColours.s * 2.56)) - PICKER_POINTER_WIDTH, // Center the coordinates, because of the height of the circle @@ -557,6 +504,12 @@ class ColorPicker extends UI5Element implements IFormInputElement { this._setMainColor(this._hue); } + _isColorValueEqual(value: ColorRGB): boolean { + return this._colorValue.R === value.r + && this._colorValue.G === value.g + && this._colorValue.B === value.b; + } + get hueSliderLabel() { return ColorPicker.i18nBundle.getText(COLORPICKER_HUE_SLIDER); } @@ -612,21 +565,24 @@ class ColorPicker extends UI5Element implements IFormInputElement { get rgbInputs(): Array { const redInput = { id: "red", - value: this._value.r, + value: this._colorValue.R, + disabled: this.inputsDisabled, label: "R", accessibleName: this.redInputLabel, }; const greenInput = { id: "green", - value: this._value.g, + value: this._colorValue.G, + disabled: this.inputsDisabled, label: "G", accessibleName: this.greenInputLabel, }; const blueInput = { id: "blue", - value: this._value.b, + value: this._colorValue.B, + disabled: this.inputsDisabled, label: "B", accessibleName: this.blueInputLabel, }; @@ -637,14 +593,16 @@ class ColorPicker extends UI5Element implements IFormInputElement { get hslInputs(): Array { const hueInput = { id: "hue", - value: this._valueHSL.h, + value: this._colorValue.H, + disabled: this.inputsDisabled, label: "H", accessibleName: this.hueInputLabel, }; const saturationInput = { id: "saturation", - value: this._valueHSL.s, + value: this._colorValue.S, + disabled: this.inputsDisabled, label: "S", accessibleName: this.saturationInputLabel, showPercentSymbol: true, @@ -652,7 +610,8 @@ class ColorPicker extends UI5Element implements IFormInputElement { const lightInput = { id: "light", - value: this._valueHSL.l, + value: this._colorValue.L, + disabled: this.inputsDisabled, label: "L", accessibleName: this.lightInputLabel, showPercentSymbol: true, @@ -661,6 +620,10 @@ class ColorPicker extends UI5Element implements IFormInputElement { return [hueInput, saturationInput, lightInput]; } + get HEX() { + return this._colorValue.HEX; + } + get colorChannelInputs() { if (!this._displayHSL) { return this.rgbInputs; @@ -682,7 +645,7 @@ class ColorPicker extends UI5Element implements IFormInputElement { top: `${this._selectedCoordinates.y}px`, }, colorSpan: { - "background-color": `rgba(${this._value.r}, ${this._value.g}, ${this._value.b}, ${this._alpha})`, + "background-color": this._colorValue.toString(), }, }; } diff --git a/packages/main/src/ColorValue.ts b/packages/main/src/ColorValue.ts new file mode 100644 index 000000000000..39eca69dfb6b --- /dev/null +++ b/packages/main/src/ColorValue.ts @@ -0,0 +1,171 @@ +import { + getRGBColor, + HSLToRGB, + HEXToRGB, + RGBToHSL, + RGBtoHEX, +} from "@ui5/webcomponents-base/dist/util/ColorConversion.js"; +import type { + ColorHSL, + ColorRGB, +} from "@ui5/webcomponents-base/dist/util/ColorConversion.js"; + +/** + * @class + * + * ### Overview + * + * The `ui5-color` class represents the `value` used in `ui5-color-picker` + * + * A color can be represented using rgb, hsl, or hex values. A color also has an alpha value. + * @public + */ +class ColorValue { + _rgb: ColorRGB; + _hsl: ColorHSL; + _alpha: number; + _hex: string; + _valid: boolean; + + constructor(color: string = "rgba(255,255,255,1)") { + this._rgb = getRGBColor(color); + this._hsl = RGBToHSL(this._rgb); + this._hex = RGBtoHEX(this._rgb); + this._alpha = 1; + this._valid = true; + } + + get RGB() { + return this._rgb; + } + + get HSL() { + return this._hsl; + } + + get H() { + return this._hsl.h; + } + + get S() { + return this._hsl.s; + } + + get L() { + return this._hsl.l; + } + + get R() { + return this._rgb.r; + } + + get G() { + return this._rgb.g; + } + + get B() { + return this._rgb.b; + } + + get Alpha(): number { + return this._alpha; + } + + get HEX() { + return this._hex; + } + + set RGB(value: ColorRGB) { + this._rgb = value; + if (this._valid) { + this._hsl = RGBToHSL(value); + this._hex = RGBtoHEX(value); + } + } + + set HSL(value: ColorHSL) { + this._hsl = value; + if (this._valid) { + this._rgb = HSLToRGB(value); + this._hex = RGBtoHEX(this._rgb); + } + } + + set HEX(value: string) { + this._hex = value; + this.validateHEX(value); + if (this._valid) { + this._rgb = HEXToRGB(value); + this._hsl = RGBToHSL(this._rgb); + } + } + + set H(value: number) { + this.validateHSLValue(value, true); + this.HSL = { h: value, s: this.S, l: this.L }; + } + + set S(value: number) { + this.validateHSLValue(value); + this.HSL = { h: this.H, s: value, l: this.L }; + } + + set L(value: number) { + this.validateHSLValue(value); + this.HSL = { h: this.H, s: this.S, l: value }; + } + + set R(value: number) { + this.validateRGBValue(value); + this.RGB = { r: value, g: this.G, b: this.B }; + } + + set G(value: number) { + this.validateRGBValue(value); + this.RGB = { r: this.R, g: value, b: this.B }; + } + + set B(value: number) { + this.validateRGBValue(value); + this.RGB = { r: this.R, g: this.G, b: value }; + } + + set Alpha(value: number) { + this._alpha = value; + } + + isColorValueValid(): boolean { + return this._valid; + } + + validateRGBValue(value: number) { + this._valid = this._isValidRGBValue(value); + } + + validateHSLValue(value: number, validateHue: boolean = false) { + this._valid = validateHue ? this._isValidHValue(value) : this._isValidSLValue(value); + } + + validateHEX(value: string) { + const hexRegex = new RegExp("^[<0-9 abcdef]+$"); + this._valid = value.length === 6 && hexRegex.test(value); + } + + _isValidRGBValue(value: number): boolean { + return value >= 0 && value <= 255; + } + + _isValidHValue(value: number): boolean { + return value >= 0 && value <= 360; + } + + _isValidSLValue(value: number): boolean { + return value >= 0 && value <= 100; + } + + toString(): string { + return `rgba(${this._rgb.r}, ${this._rgb.g}, ${this._rgb.b}, ${this._alpha})`; + } +} + +export default ColorValue;