Skip to content

Commit

Permalink
fix(ui5-color-picker): adjust hue value update when user presses over…
Browse files Browse the repository at this point in the history
… the main color section (#4601)

 - Hue value remains unchanged when users press over the
 main color section, in order to change the color.

Fixes: #4540
  • Loading branch information
unazko committed Feb 8, 2022
1 parent 29967f0 commit 4b03374
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
31 changes: 29 additions & 2 deletions packages/main/src/ColorPicker.js
Expand Up @@ -87,6 +87,20 @@ const metadata = {
defaultValue: 0,
},

/**
* @private
*/
_isSelectedColorChanged: {
type: Boolean,
},

/**
* @private
*/
_isHueValueChanged: {
type: Boolean,
},

/**
* @private
*/
Expand Down Expand Up @@ -326,6 +340,8 @@ class ColorPicker extends UI5Element {
this.selectedHue = event.target.value;
this._hue = this.selectedHue;
this._setMainColor(this._hue);
// Idication that changes to the hue value triggered as a result of user pressing over the hue slider.
this._isHueValueChanged = true;

const tempColor = this._calculateColorFromCoordinates(this._selectedCoordinates.x + 6.5, this._selectedCoordinates.y + 6.5);

Expand Down Expand Up @@ -425,6 +441,9 @@ class ColorPicker extends UI5Element {
y: y - 6.5, // Center the coordinates, because of the height of the circle
};

// Idication that changes to the color settings are triggered as a result of user pressing over the main color section.
this._isSelectedColorChanged = true;

const tempColor = this._calculateColorFromCoordinates(x, y);
if (tempColor) {
this._setColor(HSLToRGB(tempColor));
Expand All @@ -435,7 +454,7 @@ class ColorPicker extends UI5Element {
// By using the selected coordinates(x = Lightness, y = Saturation) and hue(selected from the hue slider)
// and HSL format, the color will be parsed to RGB

const h = Math.round(this._hue / 4.25), // 0 ≤ H < 360
const h = this._hue / 4.25, // 0 ≤ H < 360
// 0 ≤ S ≤ 1
s = 1 - +(Math.round((y / 256) + "e+2") + "e-2"), // eslint-disable-line
// 0 ≤ V ≤ 1
Expand Down Expand Up @@ -488,7 +507,15 @@ class ColorPicker extends UI5Element {
y: (256 - (Math.round(hslColours.s * 100) * 2.56)) - 6.5, // Center the coordinates, because of the height of the circle
};

this._hue = this.selectedHue ? this.selectedHue : Math.round(hslColours.h * 4.25);
if (this._isSelectedColorChanged) { // We shouldn't update the hue value when user presses over the main color section.
this._isSelectedColorChanged = false;
} else if (this._isHueValueChanged) { // We shouldn't recalculate the hue value when user changes the hue slider.
this._isHueValueChanged = false;
this._hue = this.selectedHue ? this.selectedHue : this._hue;
} else {
this._hue = Math.round(hslColours.h * 4.25);
}

this._setMainColor(this._hue);
}

Expand Down
18 changes: 17 additions & 1 deletion packages/main/test/specs/ColorPicker.spec.js
Expand Up @@ -78,7 +78,7 @@ describe("Color Picker general interaction", () => {

await hueSliderHandle.dragAndDrop({ x: 200, y: 0 });

assert.strictEqual(await colorPicker.getAttribute("color"), "rgba(183, 61, 183, 0.83)", "Color properly changed");
assert.strictEqual(await colorPicker.getAttribute("color"), "rgba(183, 61, 182, 0.83)", "Color properly changed");
assert.strictEqual(await stepInput.getAttribute("value"), "2", "Change event gets fired on hue slider change");
});

Expand All @@ -102,4 +102,20 @@ describe("Color Picker general interaction", () => {
assert.strictEqual(await hexInput.getProperty("value"), "808080", "CSS values are parsed correctly");
});

it("Hue value remains unchanged when user presses over the main color section", async () => {
const colorPicker = await browser.$("#change-event");
const hexInput = await colorPicker.shadow$(".ui5-color-picker-hex-input");
const mainColorSection = await colorPicker.shadow$(".ui5-color-picker-main-color");

await hexInput.doubleClick();
await browser.keys("0a6ed1");
await browser.keys("Enter");

const hueValue = await colorPicker.getAttribute("_hue");

await mainColorSection.click();

assert.strictEqual(await colorPicker.getAttribute("_hue"), hueValue, "Hue value remained unchanged");
});

});

0 comments on commit 4b03374

Please sign in to comment.